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 GH-41226 by suppressing path adjustment when not actually drawing a border #46239

Closed
wants to merge 1 commit into from

Conversation

knappam
Copy link
Contributor

@knappam knappam commented Aug 28, 2024

When not drawing a border, the mGapBetweenPaths adjustment can create noticable pixelation when drawing curves through a low number of pixels. This is noticable mostly on buttons and such on low-dpi devices. This fix only applies the fix if clipping for the border radius is done.

Summary:

When drawing small radius rounded backgrounds (e.g. to draw a circle or button) we see visible pixelation (see GH-41226) This is particularly noticable on low DPI devices.

Changelog:

[ANDROID] [FIXED] - Don't use mGapBetweenPaths if not drawing a border

Test Plan:

Built an android app that directly uses CSSBackgroundDrawable to draw a background and verified repro of this issue.
pre-fix
Then modified the code according to this PR and verified that anti-aliasing is appropriately applied
fix

@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. amazon Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team. labels Aug 28, 2024
@jorge-cab
Copy link
Contributor

jorge-cab commented Aug 28, 2024

@knappam Can you link the issue and make the title be descriptive of what your change is?

@knappam knappam changed the title Fix GH-41226 Fix GH-41226 by suppressing path adjustment when not actually drawing a border Aug 28, 2024
@knappam
Copy link
Contributor Author

knappam commented Aug 28, 2024

@knappam Can you link the issue and make the title be descriptive of what your change is?

done, thanks!

@facebook-github-bot
Copy link
Contributor

@jorge-cab has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

Comment on lines 716 to 719
mInnerClipTempRectForBorderRadius.left - pathAdjustment;
mInnerClipTempRectForBorderRadius.top - pathAdjustment;
mInnerClipTempRectForBorderRadius.right + pathAdjustment;
mInnerClipTempRectForBorderRadius.bottom + pathAdjustment;
Copy link
Contributor

Choose a reason for hiding this comment

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

, instead of ;

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

When not drawing a border, the mGapBetweenPaths adjustment
can create noticable pixelation when drawing curves through a low
number of pixels.  This is noticable mostly on buttons and such
on low-dpi devices.  This fix only applies the fix if clipping
for the border radius is done.
@facebook-github-bot
Copy link
Contributor

@jorge-cab has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@facebook-github-bot facebook-github-bot added the Merged This PR has been merged. label Aug 29, 2024
@facebook-github-bot
Copy link
Contributor

@jorge-cab merged this pull request in 8501b63.

@react-native-bot
Copy link
Collaborator

This pull request was successfully merged by @knappam in 8501b63

When will my fix make it into a release? | How to file a pick request?

cipolleschi pushed a commit that referenced this pull request Sep 11, 2024
… a border (#46239)

Summary:
When not drawing a border, the mGapBetweenPaths adjustment can create noticable pixelation when drawing curves through a low number of pixels.  This is noticable mostly on buttons and such on low-dpi devices.  This fix only applies the fix if clipping for the border radius is done.

When drawing small radius rounded backgrounds (e.g. to draw a circle or button) we see visible pixelation (see [GH-41226](#41226)) This is particularly noticable on low DPI devices.

## Changelog:

[ANDROID] [FIXED] - Don't use mGapBetweenPaths if not drawing a border

Pull Request resolved: #46239

Test Plan:
Built an android app that directly uses CSSBackgroundDrawable to draw a background and verified repro of this issue.
![pre-fix](https://github.com/user-attachments/assets/e56a41b1-60f6-4953-9e91-b95a3380f2d7)
Then modified the code according to this PR and verified that anti-aliasing is appropriately applied
![fix](https://github.com/user-attachments/assets/b6b1aecf-a713-4e0a-9759-82c2dd862991)

Reviewed By: NickGerleman

Differential Revision: D61925281

Pulled By: jorge-cab

fbshipit-source-id: 93014629d031bd0d716cd3bb11e2c294dedad639
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. Merged This PR has been merged. 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.

4 participants