Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix artifacting on RN-drawn borders with asymmetric radii #21208

Closed
wants to merge 1 commit into from

Conversation

jamesreggio
Copy link
Contributor

This PR fixes an obscure rendering bug on iOS for borders with asymmetric radii. It appears to be a problem with the custom drawing that React Native performs when it cannot use native UIKit/CoreAnimation border drawing.

Test Plan:

We've been using this patch in our production app without issue since July 2018. I've seen this issue on the latest version of iOS 10, as well as iOS 11.2 (though it seems to not be an issue on iOS 11.4 any longer).

I've included before/after screenshots below, which also show the CSS needed to cause the artifacting.

react-native-border-before

react-native-border-after

Release Notes:

[IOS] [BUGFIX] [BorderDrawing] - Fixed visual artifacts on borders with asymmetric radii

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Sep 19, 2018
@jamesreggio
Copy link
Contributor Author

jamesreggio commented Sep 19, 2018

Ah, it looks like this PR addresses the broadly-felt issue in #11897.

There's another PR from @mattijsf (#19451) that also attempts to fix this.

@mattijsf, you mention that there's still some bleeding happening, but not in the final result. Could you try my patch and see if it completely resolves the issue? I'd be happy to try any other test cases you've used, too.

@hramos
Copy link
Contributor

hramos commented Sep 20, 2018

Thanks for the PR! Please ping me once you've resolved this with @mattijsf.

@jamesreggio
Copy link
Contributor Author

Hi @hramos — I haven't heard anything from @mattijsf on this or the other PR. Given the age of his original PR, I think he may have moved on.

My suggestion would be to evaluate and merge my fix on its merits, and if the chorus of users in Issue #11897 continue to experience problems after merging my fix, we can revive the other PR.

@facebook-github-bot facebook-github-bot added the Import Started This pull request has been imported. This does not imply the PR has been approved. label Oct 1, 2018
Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hramos is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@react-native-bot
Copy link
Collaborator

@jamesreggio merged commit 9e65223 into facebook:master.

@facebook facebook locked as resolved and limited conversation to collaborators Oct 1, 2018
@react-native-bot react-native-bot added the Merged This PR has been merged. label Oct 1, 2018
grabbou pushed a commit that referenced this pull request Oct 11, 2018
Summary:
This PR fixes an obscure rendering bug on iOS for borders with asymmetric radii. It appears to be a problem with the custom drawing that React Native performs when it cannot use native UIKit/CoreAnimation border drawing.
Pull Request resolved: #21208

Differential Revision: D10130120

Pulled By: hramos

fbshipit-source-id: d9fbc5c622c060db15658d038a068216b47bb26d
@hramos hramos removed Import Started This pull request has been imported. This does not imply the PR has been approved. labels Feb 6, 2019
t-nanava pushed a commit to microsoft/react-native-macos that referenced this pull request Jun 17, 2019
…1208)

Summary:
This PR fixes an obscure rendering bug on iOS for borders with asymmetric radii. It appears to be a problem with the custom drawing that React Native performs when it cannot use native UIKit/CoreAnimation border drawing.
Pull Request resolved: facebook#21208

Differential Revision: D10130120

Pulled By: hramos

fbshipit-source-id: d9fbc5c622c060db15658d038a068216b47bb26d
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Merged This PR has been merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants