Skip to content

[Android] Update HostPlatformColor to support color longs - #43163

Closed
ryanlntn wants to merge 2 commits into
react:mainfrom
ryanlntn:feat/android-host-platform-color-longs
Closed

[Android] Update HostPlatformColor to support color longs#43163
ryanlntn wants to merge 2 commits into
react:mainfrom
ryanlntn:feat/android-host-platform-color-longs

Conversation

@ryanlntn

Copy link
Copy Markdown
Contributor

Summary:

This adds support for color longs to HostPlatformColor per the wide gamut color RFC.

Changelog:

[ANDROID] [ADDED] - Update HostPlatformColor to support color longs

Test Plan:

This requires changes from #43030 to build and will only break text color until those components are updated to support color long values as well. Will update with a link to that PR soon.

@facebook-github-botfacebook-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 22, 2024
@ryanlntn
ryanlntnforce-pushed the feat/android-host-platform-color-longs branch 2 times, most recently from 8a65961 to e0d99c3CompareMarch 15, 2024 10:34
@facebook-github-bot

Copy link
Copy Markdown
Contributor

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

@NickGerleman

Copy link
Copy Markdown
Contributor

Could we document what the magic numbers are doing? Maybe pull them out into constants?

@ryanlntn
ryanlntnforce-pushed the feat/android-host-platform-color-longs branch from e0d99c3 to 61ea51dCompareApril 17, 2024 20:43
@ryanlntn

Copy link
Copy Markdown
ContributorAuthor

Could we document what the magic numbers are doing? Maybe pull them out into constants?

@NickGerleman I pulled out the color space and component ratios definitions into constants. Should I do similar with the bit masks/shifts? I can't decide what's more readable.

@analysis-bot

Copy link
Copy Markdown
PlatformEngineArchSize (bytes)Diff
androidhermesarm64-v8a19,394,073+32
androidhermesarmeabi-v7an/a--
androidhermesx86n/a--
androidhermesx86_64n/a--
androidjscarm64-v8a22,767,695+13
androidjscarmeabi-v7an/a--
androidjscx86n/a--
androidjscx86_64n/a--

Base commit: fe9942a
Branch: main

@NickGerleman

Copy link
Copy Markdown
Contributor

The shifts seem reasonably readable to me

@facebook-github-bot

Copy link
Copy Markdown
Contributor

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

@react-native-bot

Copy link
Copy Markdown
Collaborator

This PR is stale because it has been open 180 days with no activity. Remove stale label or comment or this will be closed in 7 days.

@react-native-botreact-native-bot added the Stale There has been a lack of activity on this issue and it may be closed soon. label Oct 20, 2024
@react-native-bot

Copy link
Copy Markdown
Collaborator

This PR was closed because it has been stalled for 7 days with no activity.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA SignedThis label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.Shared with MetaApplied via automation to indicate that an Issue or Pull Request has been shared with the team.StaleThere has been a lack of activity on this issue and it may be closed soon.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@ryanlntn@facebook-github-bot@NickGerleman@analysis-bot@react-native-bot