Android: Adjust border radius to maximum of half the smallest side

[yedidyak]

Motivation

Currently, if a View has a border radius set to larger than the size of the View, and there s a borderWidth set, there is a very serious performance problem on Android.

Test Plan

This can be verified easily by making any view with a thin border, and watching the Android GPU performance or memory as it is rendered. Native memory allocation will immediately max out, and the GPU render profiler will look like this:

Memory:

With the fix, these problems disappear.

Release Notes

[ANDROID] [BUGFIX][View] - Treat larger than possible border radius as meaning 50% of the smallest side

[wodin]

These test failures are unrelated to this pull request.
The test-android one seems to be because ReactCommon/yoga/BUCK is missing the following:

include_defs("//ReactCommon/DEFS")

[facebook-github-bot]

@yedidyak I tried to find reviewers for this pull request and wanted to ping them to take another look. However, based on the blame information for the files in this pull request I couldn't find any reviewers. This sometimes happens when the files in the pull request are new or don't exist on master anymore. Is this pull request still relevant? If yes could you please rebase? In case you know who has context on this code feel free to mention them in a comment (one person is fine). Thanks for reading and hope you will continue contributing to the project.

[yedidyak]

@hramos @grabbou ^

This is being used in production as a patch in Wix.

[grabbou]

Looks good to me, @janicduplessis and @emilsjolander would you mind looking at this?

[grabbou]

For the test failures, please rebase your branch as the master is passing :)

[janicduplessis]

Does it also work when setting a specific corner border radius like bottomLeftBorderRadius?

I find the bug kind of weird though, would prefer to find what is actually causing the high memory usage. Have you tried looking at what the path we are drawing looks like to make sure that is ok?

[yedidyak]

I rebased, thanks.

@janicduplessis The end result looked correct, but the bounds of the ReactViewBackgroundDrawable expanded to huge numbers in all directions. I tried to analyse what was happening to cause that, I don't remember exactly what I found but the algorithm simply doesn't expect larger than possible border radii.

[janicduplessis]

@RSNara Any idea about what the best fix would be here since you are the last one to have worked on this? :)

[yedidyak]

@janicduplessis Whatever is the issue with the underlying algorithm, limiting the borderRadius to the maximum possible size is still worthwhile, no?

[yedidyak]

@janicduplessis @grabbou bump

[sercand]

After upgrading to React-Native 0.54 our app becomes so slow on because of this issue.

[facebook-github-bot]

@yedidyak I tried to find reviewers for this pull request and wanted to ping them to take another look. However, based on the blame information for the files in this pull request I couldn't find any reviewers. This sometimes happens when the files in the pull request are new or don't exist on master anymore. Is this pull request still relevant? If yes could you please rebase? In case you know who has context on this code feel free to mention them in a comment (one person is fine). Thanks for reading and hope you will continue contributing to the project.

[yedidyak]

@janicduplessis @grabbou bump?

[grabbou]

I'll leave the final word to @janicduplessis as he seemed to be more active than me in this thread.

[almostintuitive]

I assume this would fix:
#17267
#17311
#17144
#17224

And also help us greatly!

Is there any chance it can be approved/merged?
@janicduplessis @grabbou

@yedidyak says they have been using it in production for a while.

[atlanteh]

@janicduplessis can you please have a look?

[mezod]

+1

[atlanteh]

I'm not sure, but I think this PR can be closed, as the issue #17267 has been fixed which might be the root cause.