Add Support for Wide Gamut (DisplayP3) Colors to React Native #738
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Co-authored-by: Riccardo Cipolleschi <riccardo.cipolleschi@gmail.com>
Co-authored-by: Riccardo Cipolleschi <riccardo.cipolleschi@gmail.com>
ryanlntn
changed the title
NickGerleman
reviewed
Dec 2, 2023
|
|
||
| 1. Parse the color() function in [normalizeColor](https://github.com/facebook/react-native/blob/63213712125795ac082597dad2716258b90cdcd5/packages/normalize-color/index.js#L235) | ||
|
|
||
| ```js |
There was a problem hiding this comment.
The motivation and API here make sense. How close is this towards a PR?
There was a problem hiding this comment.
I haven't done any implementation work yet. Riccardo has been helping me with this so we were planning on leaving this open for feedback for a couple weeks before starting on implementation when he gets back.
|
Also FYI @vincentriemer as someone who has been interested in extended color space before. |
|
cc @andreialecu who is also interested in this topic. my understanding is that, unlike flutter to draw everything by their own. by leveraging the UIImage, we have the display p3 support already. the only missing piece is our coloring system as this pr pointed out. alternatively, i was thinking to change from native code only without touching js code. if the screen supports display p3, we should just use the display p3 UIColor methods. having a small poc here: https://github.com/Kudo/displayp3/blob/main/modules/p3color/ios/p3colorView.swift |
okwasniewski
reviewed
Dec 7, 2023
|
|
||
| 2. **Utilizing Swizzling on iOS:** <br/> As an alternative to the proposed iOS changes, swizzling could be considered similar to [RN #41517](https://github.com/facebook/react-native/issues/41517). This has a major downside of being an all or nothing breaking change forcing all React Native users into the DisplayP3 color space as well as being a more confusing and less maintainable solution. | ||
|
|
||
| 3. **Full switch to DisplayP3:** <br/> In a similar vein we could just update the constructors directly without preserving existing behavior. While this would be a simpler implementation it would be a breaking change. Any existing visual regression tests would likely break and React Native users would now be required to specify all colors in a wider gamut. |
There was a problem hiding this comment.
What do you think about a way to optionally fully opt-in to use DisplayP3?
The current approach would take a lot of extra steps to ensure every color has ["display-p3"]: true
There was a problem hiding this comment.
This seems like a good idea.
Many libraries have predefined colors in them; I'd like to see how everything looks with display-p3 across the board. Usually, the differences should be slight and mainly positive, with colors being more vibrant.
There was a problem hiding this comment.
What do you think about a way to optionally fully opt-in to use DisplayP3?
I can definitely see the utility of this. I'm not really sure what the API for that should look like though. AFAIK the APIs on Android, iOS, and web all only allow specifying color space per instance. Since color properties already mostly implement CSS Color Module Level 4 we were mainly planning on extending that further with this proposed API.
There was a problem hiding this comment.
I agree that it could be useful.
The way I think this should be implemented is by factoring out the constructor functions in a single factory method so we can decide which initializer we want to use as default. At that point, we can specify a static feature flag that controls the default behavior.
For Android things are a little bit more convoluted because Android uses int to represent sRGB colors and long to represent WideGamut colors. So, not only we need to update how a color is created but we also have to overload several methods to achieve what we need.
But it is probably doable.
@ryanlntn could you update the proposal to add a feature flag that will control the default init behavior?
This was referenced Feb 9, 2024
Closed
This was referenced Feb 22, 2024
facebook-github-bot
pushed a commit
to facebook/react-native
that referenced
this pull request
Feb 27, 2024
Summary: This adds initial support for wide gamut (DisplayP3) colors to React Native iOS per the [RFC](react-native-community/discussions-and-proposals#738). It provides the ability to set the default color space to sRGB or DisplayP3 and provides the native code necessary to support `color()` function syntax per the [W3C CSS Color Module Level 4](https://www.w3.org/TR/css-color-4/#color-function) spec. It does _not_ yet support animations and requires additional JS code before fully supporting the `color()` function syntax. bypass-github-export-checks ## Changelog: [IOS] [ADDED] - Add basic DisplayP3 color support Pull Request resolved: #42830 Test Plan:  Follow test steps from #42831 to test support for `color()` function syntax. To globally change the default color space to DisplayP3 make the following changes to RNTester AppDelegate.mm: ```diff + #import <React/RCTConvert.h> - (BOOL)application:(UIApplication *)application didFinishLaunchingWithOptions:(NSDictionary *)launchOptions { // ... + RCTSetDefaultColorSpace(RCTColorSpaceDisplayP3); return [super application:application didFinishLaunchingWithOptions:launchOptions]; } ``` Reviewed By: javache Differential Revision: D53380407 Pulled By: cipolleschi fbshipit-source-id: 938523958f9021e8d98bdb1d4e254047e3ecdad7
facebook-github-bot
pushed a commit
to facebook/react-native
that referenced
this pull request
Feb 28, 2024
Summary: This adds support for 64 bit integer (long) values to MapBuffer. Per the wide gamut color [RFC](react-native-community/discussions-and-proposals#738) Android encodes wide gamut colors as long values so we need to update MapBuffer to support 64 bit integers as well. ## Changelog: [ANDROID] [ADDED] - Add 64 bit integer (long) value support to MapBuffer Pull Request resolved: #43030 Test Plan: I've added a test to the MapBuffer test suite. This new API is otherwise currently unused but will be used in subsequent PRs as part of wide gamut color support changes. Reviewed By: mdvacca Differential Revision: D53881809 Pulled By: NickGerleman fbshipit-source-id: 39c20b93493a2609db9f66426640ef5e97d6e1a8
facebook-github-bot
pushed a commit
to facebook/react-native
that referenced
this pull request
Mar 5, 2024
Summary: This adds support for 64 bit integer (long) values to the Android bridge. Per the wide gamut color [RFC](react-native-community/discussions-and-proposals#738) Android encodes wide gamut colors as long values so we need to update the bridge to support 64 bit integers as well since these classes will soon receive those values from native. ## Changelog: [ANDROID] [ADDED] - Update bridge to handle long values Pull Request resolved: #43158 Test Plan: I added tests where I could for long types and truncation. I would like to add tests for ReadableNativeArray and ReadableNativeMap but I'm not sure how to go about mocking HybridData. Reviewed By: cipolleschi Differential Revision: D54276496 Pulled By: NickGerleman fbshipit-source-id: 1e71b5283f662748beef1bdb34d9c86099baecb0
cipolleschi
pushed a commit
to cipolleschi/react-native
that referenced
this pull request
Apr 23, 2024
…3031) Summary: This adds support for color function values to ColorPropConverter per the wide gamut color [RFC](react-native-community/discussions-and-proposals#738). It updates the color conversion code so that it returns a Color instance before ultimately being converted to an Integer in preparation for returning long values as needed. bypass-github-export-checks ## Changelog: [ANDROID] [ADDED] - Update ColorPropConverter to support color function values Pull Request resolved: facebook#43031 Test Plan: Colors should work exactly the same as before. Follow test steps from facebook#42831 to test support for color() function syntax. While colors specified with color() function syntax will not yet render in DisplayP3 color space they will not be misrecognized as resource path colors but will instead fallback to their sRGB color space values. Differential Revision: D55749058
cipolleschi
pushed a commit
to cipolleschi/react-native
that referenced
this pull request
Apr 23, 2024
Summary: This adds support for enabling wide color gamut mode for ReactActivity per the wide gamut color [RFC](react-native-community/discussions-and-proposals#738). ## Changelog: [ANDROID] [ADDED] - Add isWideColorGamutEnabled to ReactActivityDelegate Pull Request resolved: facebook#43036 Test Plan: Update RNTesterActivity.kt to enable wide color gamut: ```diff class RNTesterActivity : ReactActivity() { class RNTesterActivityDelegate(val activity: ReactActivity, mainComponentName: String) : // ... override fun getLaunchOptions() = if (this::initialProps.isInitialized) initialProps else Bundle() + override fun isWideColorGamutEnabled() = true } ``` Differential Revision: D55749124 Reviewed By: cortinico Pulled By: cortinico
cipolleschi
pushed a commit
to cipolleschi/react-native
that referenced
this pull request
Apr 23, 2024
…3031) Summary: This adds support for color function values to ColorPropConverter per the wide gamut color [RFC](react-native-community/discussions-and-proposals#738). It updates the color conversion code so that it returns a Color instance before ultimately being converted to an Integer in preparation for returning long values as needed. bypass-github-export-checks ## Changelog: [ANDROID] [ADDED] - Update ColorPropConverter to support color function values Pull Request resolved: facebook#43031 Test Plan: Colors should work exactly the same as before. Follow test steps from facebook#42831 to test support for color() function syntax. While colors specified with color() function syntax will not yet render in DisplayP3 color space they will not be misrecognized as resource path colors but will instead fallback to their sRGB color space values. Differential Revision: D55749058
cipolleschi
pushed a commit
to cipolleschi/react-native
that referenced
this pull request
Apr 23, 2024
Summary: This adds support for enabling wide color gamut mode for ReactActivity per the wide gamut color [RFC](react-native-community/discussions-and-proposals#738). ## Changelog: [ANDROID] [ADDED] - Add isWideColorGamutEnabled to ReactActivityDelegate Pull Request resolved: facebook#43036 Test Plan: Update RNTesterActivity.kt to enable wide color gamut: ```diff class RNTesterActivity : ReactActivity() { class RNTesterActivityDelegate(val activity: ReactActivity, mainComponentName: String) : // ... override fun getLaunchOptions() = if (this::initialProps.isInitialized) initialProps else Bundle() + override fun isWideColorGamutEnabled() = true } ``` Differential Revision: D55749124 Reviewed By: cortinico Pulled By: cortinico
facebook-github-bot
pushed a commit
to facebook/react-native
that referenced
this pull request
Apr 23, 2024
Summary: This adds support for color function values to ColorPropConverter per the wide gamut color [RFC](react-native-community/discussions-and-proposals#738). It updates the color conversion code so that it returns a Color instance before ultimately being converted to an Integer in preparation for returning long values as needed. bypass-github-export-checks ## Changelog: [ANDROID] [ADDED] - Update ColorPropConverter to support color function values Pull Request resolved: #43031 Test Plan: Colors should work exactly the same as before. Follow test steps from #42831 to test support for color() function syntax. While colors specified with color() function syntax will not yet render in DisplayP3 color space they will not be misrecognized as resource path colors but will instead fallback to their sRGB color space values. Reviewed By: cortinico Differential Revision: D55749058 Pulled By: cipolleschi fbshipit-source-id: 37659d22c1db4b1a27a9a4f88c9beb703517b01f
facebook-github-bot
pushed a commit
to facebook/react-native
that referenced
this pull request
Apr 23, 2024
Summary: This adds support for enabling wide color gamut mode for ReactActivity per the wide gamut color [RFC](react-native-community/discussions-and-proposals#738). ## Changelog: [ANDROID] [ADDED] - Add isWideColorGamutEnabled to ReactActivityDelegate Pull Request resolved: #43036 Test Plan: Update RNTesterActivity.kt to enable wide color gamut: ```diff class RNTesterActivity : ReactActivity() { class RNTesterActivityDelegate(val activity: ReactActivity, mainComponentName: String) : // ... override fun getLaunchOptions() = if (this::initialProps.isInitialized) initialProps else Bundle() + override fun isWideColorGamutEnabled() = true } ``` Reviewed By: cortinico Differential Revision: D55749124 Pulled By: cipolleschi fbshipit-source-id: 44dd5631e1a2e429c86c01ed8747bbebbc8bdb3b
cipolleschi
pushed a commit
to cipolleschi/react-native
that referenced
this pull request
Apr 24, 2024
…3031) Summary: This adds support for color function values to ColorPropConverter per the wide gamut color [RFC](react-native-community/discussions-and-proposals#738). It updates the color conversion code so that it returns a Color instance before ultimately being converted to an Integer in preparation for returning long values as needed. ## Changelog: [ANDROID] [ADDED] - Update ColorPropConverter to support color function values Test Plan: Colors should work exactly the same as before. Follow test steps from facebook#42831 to test support for color() function syntax. While colors specified with color() function syntax will not yet render in DisplayP3 color space they will not be misrecognized as resource path colors but will instead fallback to their sRGB color space values. --- After the failure with the tests, I reapplied the changes and test some Jest e2e tests that were failing yesterday: {F1495277376} Differential Revision: D56517579 Pulled By: cipolleschi
cipolleschi
pushed a commit
to cipolleschi/react-native
that referenced
this pull request
Apr 24, 2024
…4237) Summary: This adds support for color function values to ColorPropConverter per the wide gamut color [RFC](react-native-community/discussions-and-proposals#738). It updates the color conversion code so that it returns a Color instance before ultimately being converted to an Integer in preparation for returning long values as needed. ## Changelog: [ANDROID] [ADDED] - Update ColorPropConverter to support color function values Test Plan: Colors should work exactly the same as before. Follow test steps from facebook#42831 to test support for color() function syntax. While colors specified with color() function syntax will not yet render in DisplayP3 color space they will not be misrecognized as resource path colors but will instead fallback to their sRGB color space values. --- After the failure with the tests, I reapplied the changes and test some Jest e2e tests that were failing yesterday: {F1495277376} Differential Revision: D56517579 Pulled By: cipolleschi
cipolleschi
pushed a commit
to cipolleschi/react-native
that referenced
this pull request
Apr 25, 2024
…4237) Summary: Pull Request resolved: facebook#44237 This adds support for color function values to ColorPropConverter per the wide gamut color [RFC](react-native-community/discussions-and-proposals#738). It updates the color conversion code so that it returns a Color instance before ultimately being converted to an Integer in preparation for returning long values as needed. ## Changelog: [ANDROID] [ADDED] - Update ColorPropConverter to support color function values Pull Request resolved: facebook#43031 Test Plan: Colors should work exactly the same as before. Follow test steps from facebook#42831 to test support for color() function syntax. While colors specified with color() function syntax will not yet render in DisplayP3 color space they will not be misrecognized as resource path colors but will instead fallback to their sRGB color space values. --- After the failure with the tests, I reapplied the changes and test some Jest e2e tests that were failing yesterday: {F1495277376} Differential Revision: D56517579 Pulled By: cipolleschi
cipolleschi
pushed a commit
to cipolleschi/react-native
that referenced
this pull request
Apr 25, 2024
…4237) Summary: Pull Request resolved: facebook#44237 This adds support for color function values to ColorPropConverter per the wide gamut color [RFC](react-native-community/discussions-and-proposals#738). It updates the color conversion code so that it returns a Color instance before ultimately being converted to an Integer in preparation for returning long values as needed. ## Changelog: [ANDROID] [ADDED] - Update ColorPropConverter to support color function values Pull Request resolved: facebook#43031 Test Plan: Colors should work exactly the same as before. Follow test steps from facebook#42831 to test support for color() function syntax. While colors specified with color() function syntax will not yet render in DisplayP3 color space they will not be misrecognized as resource path colors but will instead fallback to their sRGB color space values. --- After the failure with the tests, I reapplied the changes and test some Jest e2e tests that were failing yesterday: {F1495277376} Differential Revision: D56517579 Pulled By: cipolleschi
cipolleschi
pushed a commit
to cipolleschi/react-native
that referenced
this pull request
Apr 29, 2024
…4237) Summary: Pull Request resolved: facebook#44237 This adds support for color function values to ColorPropConverter per the wide gamut color [RFC](react-native-community/discussions-and-proposals#738). It updates the color conversion code so that it returns a Color instance before ultimately being converted to an Integer in preparation for returning long values as needed. ## Changelog: [ANDROID] [ADDED] - Update ColorPropConverter to support color function values Pull Request resolved: facebook#43031 Test Plan: Colors should work exactly the same as before. Follow test steps from facebook#42831 to test support for color() function syntax. While colors specified with color() function syntax will not yet render in DisplayP3 color space they will not be misrecognized as resource path colors but will instead fallback to their sRGB color space values. --- After the failure with the tests, I reapplied the changes and test some Jest e2e tests that were failing yesterday: {F1495277376} Differential Revision: D56517579 Pulled By: cipolleschi
cipolleschi
pushed a commit
to cipolleschi/react-native
that referenced
this pull request
Apr 30, 2024
…4237) Summary: Pull Request resolved: facebook#44237 This adds support for color function values to ColorPropConverter per the wide gamut color [RFC](react-native-community/discussions-and-proposals#738). It updates the color conversion code so that it returns a Color instance before ultimately being converted to an Integer in preparation for returning long values as needed. ## Changelog: [ANDROID] [ADDED] - Update ColorPropConverter to support color function values Pull Request resolved: facebook#43031 Test Plan: Colors should work exactly the same as before. Follow test steps from facebook#42831 to test support for color() function syntax. While colors specified with color() function syntax will not yet render in DisplayP3 color space they will not be misrecognized as resource path colors but will instead fallback to their sRGB color space values. --- After the failure with the tests, I reapplied the changes and test some Jest e2e tests that were failing yesterday: {F1495277376} Differential Revision: D56517579 Pulled By: cipolleschi
kosmydel
pushed a commit
to kosmydel/react-native
that referenced
this pull request
May 6, 2024
…3031) Summary: This adds support for color function values to ColorPropConverter per the wide gamut color [RFC](react-native-community/discussions-and-proposals#738). It updates the color conversion code so that it returns a Color instance before ultimately being converted to an Integer in preparation for returning long values as needed. bypass-github-export-checks ## Changelog: [ANDROID] [ADDED] - Update ColorPropConverter to support color function values Pull Request resolved: facebook#43031 Test Plan: Colors should work exactly the same as before. Follow test steps from facebook#42831 to test support for color() function syntax. While colors specified with color() function syntax will not yet render in DisplayP3 color space they will not be misrecognized as resource path colors but will instead fallback to their sRGB color space values. Reviewed By: cortinico Differential Revision: D55749058 Pulled By: cipolleschi fbshipit-source-id: 37659d22c1db4b1a27a9a4f88c9beb703517b01f
kosmydel
pushed a commit
to kosmydel/react-native
that referenced
this pull request
May 6, 2024
Summary: This adds support for enabling wide color gamut mode for ReactActivity per the wide gamut color [RFC](react-native-community/discussions-and-proposals#738). ## Changelog: [ANDROID] [ADDED] - Add isWideColorGamutEnabled to ReactActivityDelegate Pull Request resolved: facebook#43036 Test Plan: Update RNTesterActivity.kt to enable wide color gamut: ```diff class RNTesterActivity : ReactActivity() { class RNTesterActivityDelegate(val activity: ReactActivity, mainComponentName: String) : // ... override fun getLaunchOptions() = if (this::initialProps.isInitialized) initialProps else Bundle() + override fun isWideColorGamutEnabled() = true } ``` Reviewed By: cortinico Differential Revision: D55749124 Pulled By: cipolleschi fbshipit-source-id: 44dd5631e1a2e429c86c01ed8747bbebbc8bdb3b
cipolleschi
pushed a commit
to cipolleschi/react-native
that referenced
this pull request
May 20, 2024
…4237) Summary: This adds support for color function values to ColorPropConverter per the wide gamut color [RFC](react-native-community/discussions-and-proposals#738). It updates the color conversion code so that it returns a Color instance before ultimately being converted to an Integer in preparation for returning long values as needed. ## Changelog: [ANDROID] [ADDED] - Update ColorPropConverter to support color function values Test Plan: Colors should work exactly the same as before. Follow test steps from facebook#42831 to test support for color() function syntax. While colors specified with color() function syntax will not yet render in DisplayP3 color space they will not be misrecognized as resource path colors but will instead fallback to their sRGB color space values. --- After the failure with the tests, I reapplied the changes and test some Jest e2e tests that were failing yesterday: {F1495277376} Reviewed By: cortinico Differential Revision: D56517579 Pulled By: cipolleschi
facebook-github-bot
pushed a commit
to facebook/react-native
that referenced
this pull request
May 20, 2024
Summary: Pull Request resolved: #44237 This adds support for color function values to ColorPropConverter per the wide gamut color [RFC](react-native-community/discussions-and-proposals#738). It updates the color conversion code so that it returns a Color instance before ultimately being converted to an Integer in preparation for returning long values as needed. ## Changelog: [ANDROID] [ADDED] - Update ColorPropConverter to support color function values Pull Request resolved: #43031 Test Plan: Colors should work exactly the same as before. Follow test steps from #42831 to test support for color() function syntax. While colors specified with color() function syntax will not yet render in DisplayP3 color space they will not be misrecognized as resource path colors but will instead fallback to their sRGB color space values. --- After the failure with the tests, I reapplied the changes and test some Jest e2e tests that were failing yesterday: {F1495277376} Reviewed By: cortinico Differential Revision: D56517579 Pulled By: cipolleschi fbshipit-source-id: ae9b5bc2afe9eb9760dd91afb090385daf7102b8
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
React Native does not currently support wide gamut color spaces (i.e. DisplayP3). This proposal discusses adding support for this color space in React Native.
Thanks to @cipolleschi for his support in drafting this proposal!