Skip to content

[iOS] Fixes strange gradient appears when using borderTopLeftRadius (or similar) not uniform #49531

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

Closed
wants to merge 2 commits into from

Conversation

zhongwuzw
Copy link
Contributor

Summary:

Fixes #49442. We should enable Rasterize when we draw the contents by ourselves. Otherwise, it may produce strange gradient.

Changelog:

[IOS] [FIXED] - Fixes strange gradient appears when using borderTopLeftRadius (or similar) not uniform

Test Plan:

Repro please see #49442.

@facebook-github-bot facebook-github-bot added CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team. labels Feb 19, 2025
@joevilches
Copy link
Contributor

Hmm I am not the most familiar with the implications of shouldRasterize on CALayer, but it seems like it could have quite a large impact and is not something we would want to naively turn on for every border/outline layer we make, especially if the issue we are seeing is rather specific (scaling transform on parent with custom border drawing on child).

I would assume that there is some fix to the logic in the border drawing we have that would let us fix this problem without having to set this value, which would be ideal. If you want to tackle this issue can you go down that avenue as a potential solution? The code would live in RCTBorderDrawing.m

@zhongwuzw
Copy link
Contributor Author

zhongwuzw commented Feb 24, 2025

@joevilches Thanks for the review, the border drawing seems to have no issue, the root cause is CALayer rendering custom content has some bugs. How about using shouldRasterizeIOS prop reactnative.dev/docs/view#shouldrasterizeios-ios? User can enable specific view's shouldRasterizeIOS to true if it hits the issue. It's more controllable.

PS. shouldRasterizeIOS is not work in Fabric, #49615 tries to fix it.

@joevilches
Copy link
Contributor

Nice yeah given the prop already exists I think that makes more sense as a solution, especially if this is deeper problem within CALayer itself. Nice!

Comment on lines +914 to +919
if (_props->shouldRasterize) {
_borderLayer.shouldRasterize = YES;
_borderLayer.rasterizationScale = self.traitCollection.displayScale;
} else {
_borderLayer.shouldRasterize = NO;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

It might be a bit unintuitive that this magic border layer receives the effects of this prop, yet nothing else does. Does rasterizing the view-backing layer fix this issue too?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Does rasterizing the view-backing layer fix this issue too?

@joevilches Yes, the paper arch uses a view-backing layer for border rendering https://github.com/facebook/react-native/blob/main/packages/react-native/React/Views/RCTView.m#L859-L862, also if user set shouldRasterizeIOS to enable rasterizing, it works.

Copy link
Contributor

Choose a reason for hiding this comment

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

My question was more around, can we fix this issue with the weird gradient on the border if we JUST set shouldRasterize on the view-backing layer, and not on the border layer.

Originally I thought we were only using shouldRasterize on the border layer here (which I found confusing), but I see that the code does indeed already exist at https://github.com/facebook/react-native/blob/main/packages/react-native/React/Fabric/Mounting/ComponentViews/View/RCTViewComponentView.mm#L285 so we set it on the view-backing layer too.

So now I am wondering is this fixable by just setting shouldRasterizeIOS which maps to the view backing layer currently and wondering if you tested that beforehand (in a case where this border layer is present). If it does not then I think we are good to merge this!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@joevilches Yes, you're right, #49615 also fixes this issue, we don't need to set it on the border layer. I'm going to close this. Thank you.

@zhongwuzw zhongwuzw requested a review from joevilches February 26, 2025 15:07
@zhongwuzw
Copy link
Contributor Author

Close because #49615 already fixes this issue.

@zhongwuzw zhongwuzw closed this Feb 27, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[iOS] Strange gradient appears when using borderTopLeftRadius (or similar) inside a scaled View
3 participants