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

Fixes UI glitch when drawing selective corners with borderRadius #19451

Closed
wants to merge 1 commit into from
Closed

Fixes UI glitch when drawing selective corners with borderRadius #19451

wants to merge 1 commit into from

Conversation

mattijsf
Copy link
Contributor

@mattijsf mattijsf commented May 25, 2018

Fixes #11897

On devices (not simulator) running iOS 11 there seems to be a bug when drawing selective borders with a corner radius. A "bleeding" line appears on corners that don't have a radius while an adjacent corner has.

It’s most likely a bug with iOS 11 itself as the original code is according to spec and it works fine in an iOS 11 simulator. However it turns out that this patch is a workaround for this problem.

This patch increases the stretchable area to 2pt instead of 1pt after which the glitch disappears. I assume shouldn’t have a negative performance impact as it also enforces resizingMode UIImageResizingModeStretch.

In all fairness, though this workaround seems to have a positive impact on the bug, the exact reason why some cap inset areas are partially stretched across the resizable area remains unknown to me.

Screenshots:

The screenshots below are created using an iPhone 5s Device (e.g. in simulator the bug doesn't manifest):

From left to right:

  • Raw resizable image that React Native uses to render the borders. Where I added:
    • Blue lines for debugging to indicate the corners (cap insets) of the image that are supposed to be unchanged when resized
    • Green pixels that indicate stretchable content area
  • How the image is resized in the actual rendered view (with the blue lines for debugging)
  • How the image is resized in the actual rendered view (without the blue lines for debugging)

Before fix:
before_fix

After fix:
after_fix

As you can see, there is still some bleeding happening but not in the final result.

Test Plan

See #11897 (comment)

Release Notes

[IOS] [BUGFIX] [View] - Fixes bleeding corner borders on iOS 11 devices

@mattijsf mattijsf requested a review from shergin as a code owner May 25, 2018 21:02
@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 May 25, 2018
@mattijsf
Copy link
Contributor Author

mattijsf commented Jun 11, 2018

@sophiebits Can you help reviewing this? Or do you know someone who can?

@hramos
Copy link
Contributor

hramos commented Jun 11, 2018

Which devices have you tested this on? Might want to ensure it works as expected on @1x, @2x, and @3x devices.

@mattijsf
Copy link
Contributor Author

mattijsf commented Jun 13, 2018

I already tested it on a iPhone 5 and 5s, I just also tested on a iPhone 7 plus and iPhone X and it works. However, on these 3x devices the glitch never manifested but at least I can confirm that everything still works as expected on these devices.

I'm not sure how to test 1x though... I might have to get my hands on an 1st gen iPad mini which runs iOS 9 and has a screen scale of 1x.

Screenshots iPhone 7+:

iphone 7plus_after

iPhone X:

iphone x_after

Copy link
Contributor

@hramos hramos left a comment

Choose a reason for hiding this comment

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

Approved. Please rebase in order to take care of the test failures. They're unrelated to your diff, but rebasing should bring them back to green on this PR.

On devices (not simulator) running iOS 11 there seems to be a bug when drawing selective borders with a corner radius. A "bleeding" line appears on corners that don't have a radius while an adjacent corner has.

It’s most likely a bug with iOS 11 itself as the original code is according to spec and it works fine in an iOS 11 simulator. However it turns out that this patch is a workaround for this problem.

This patch increases the stretchable area to 2pt instead of 1pt after witch the glitch dissappears. I assume shouldn’t have a negative performance impact as it also enforces resizingMode UIImageResizingModeStretch.
@mattijsf
Copy link
Contributor Author

Done. Tests are passing again.

gbhasha added a commit to gbhasha/react-native-segmented-control-ui that referenced this pull request Aug 6, 2018
@jamesreggio
Copy link
Contributor

Hey @mattijsf@hramos would like us to coordinate to determine which of our fixes to accept. (Mine is over in #21208.)

Are there are test cases you're aware of where your approach does not properly address the issue?

@mattijsf
Copy link
Contributor Author

mattijsf commented Oct 2, 2018

Sorry @jamesreggio I didn't have the test readily available for re-creating the "intermediate"-state images that I used for my PR. But I think your PR has a proper solution compared to the workaround I used in my PR.

Nice work! 👍 Good to see that it landed in master now!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Platform: iOS iOS applications.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Artifacts on View with borderRadius on only 2 corners on iOS
6 participants