From 7f8583abf3c4ff9ca88eff81ecc2d12a80533b6d Mon Sep 17 00:00:00 2001 From: Ryan Linton Date: Thu, 25 Apr 2024 05:39:48 -0700 Subject: [PATCH] Update ColorPropConverterto support color function values (#44237) Summary: Pull Request resolved: https://github.com/facebook/react-native/pull/44237 This adds support for color function values to ColorPropConverter per the wide gamut color [RFC](https://github.com/react-native-community/discussions-and-proposals/pull/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: https://github.com/facebook/react-native/pull/43031 Test Plan: Colors should work exactly the same as before. Follow test steps from https://github.com/facebook/react-native/pull/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 --- .../ReactAndroid/api/ReactAndroid.api | 1 + .../react/bridge/ColorPropConverter.java | 90 ++++++++++++++++++- 2 files changed, 90 insertions(+), 1 deletion(-) diff --git a/packages/react-native/ReactAndroid/api/ReactAndroid.api b/packages/react-native/ReactAndroid/api/ReactAndroid.api index 5040a7e010f7ca..7e59978d255dfa 100644 --- a/packages/react-native/ReactAndroid/api/ReactAndroid.api +++ b/packages/react-native/ReactAndroid/api/ReactAndroid.api @@ -649,6 +649,7 @@ public class com/facebook/react/bridge/ColorPropConverter { public fun ()V public static fun getColor (Ljava/lang/Object;Landroid/content/Context;)Ljava/lang/Integer; public static fun getColor (Ljava/lang/Object;Landroid/content/Context;I)Ljava/lang/Integer; + public static fun getColorInstance (Ljava/lang/Object;Landroid/content/Context;)Landroid/graphics/Color; public static fun resolveResourcePath (Landroid/content/Context;Ljava/lang/String;)Ljava/lang/Integer; } diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/bridge/ColorPropConverter.java b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/bridge/ColorPropConverter.java index afc690dedaabbf..e176305c61195a 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/bridge/ColorPropConverter.java +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/bridge/ColorPropConverter.java @@ -9,13 +9,22 @@ import android.content.Context; import android.content.res.Resources; +import android.graphics.Color; +import android.graphics.ColorSpace; +import android.os.Build; import android.util.TypedValue; +import androidx.annotation.ColorLong; import androidx.annotation.Nullable; import androidx.core.content.res.ResourcesCompat; import com.facebook.common.logging.FLog; import com.facebook.react.common.ReactConstants; public class ColorPropConverter { + + private static boolean supportWideGamut() { + return Build.VERSION.SDK_INT >= Build.VERSION_CODES.O; + } + private static final String JSON_KEY = "resource_paths"; private static final String PREFIX_RESOURCE = "@"; private static final String PREFIX_ATTR = "?"; @@ -24,7 +33,8 @@ public class ColorPropConverter { private static final String ATTR = "attr"; private static final String ATTR_SEGMENT = "attr/"; - public static Integer getColor(Object value, Context context) { + @Nullable + private static Integer getColorInteger(Object value, Context context) { if (value == null) { return null; } @@ -39,6 +49,16 @@ public static Integer getColor(Object value, Context context) { if (value instanceof ReadableMap) { ReadableMap map = (ReadableMap) value; + + if (map.hasKey("space")) { + int r = (int) ((float) map.getDouble("r") * 255); + int g = (int) ((float) map.getDouble("g") * 255); + int b = (int) ((float) map.getDouble("b") * 255); + int a = (int) ((float) map.getDouble("a") * 255); + + return Color.argb(a, r, g, b); + } + ReadableArray resourcePaths = map.getArray(JSON_KEY); if (resourcePaths == null) { @@ -63,6 +83,74 @@ public static Integer getColor(Object value, Context context) { "ColorValue: the value must be a number or Object."); } + @Nullable + public static Color getColorInstance(Object value, Context context) { + if (value == null) { + return null; + } + + if (supportWideGamut() && value instanceof Double) { + return Color.valueOf(((Double) value).intValue()); + } + + if (context == null) { + throw new RuntimeException("Context may not be null."); + } + + if (value instanceof ReadableMap) { + ReadableMap map = (ReadableMap) value; + + if (supportWideGamut() && map.hasKey("space")) { + String rawColorSpace = map.getString("space"); + boolean isDisplayP3 = rawColorSpace != null && rawColorSpace.equals("display-p3"); + ColorSpace space = + ColorSpace.get(isDisplayP3 ? ColorSpace.Named.DISPLAY_P3 : ColorSpace.Named.SRGB); + float r = (float) map.getDouble("r"); + float g = (float) map.getDouble("g"); + float b = (float) map.getDouble("b"); + float a = (float) map.getDouble("a"); + + @ColorLong long color = Color.pack(r, g, b, a, space); + return Color.valueOf(color); + } + + ReadableArray resourcePaths = map.getArray(JSON_KEY); + if (resourcePaths == null) { + throw new JSApplicationCausedNativeException( + "ColorValue: The `" + JSON_KEY + "` must be an array of color resource path strings."); + } + + for (int i = 0; i < resourcePaths.size(); i++) { + Integer result = resolveResourcePath(context, resourcePaths.getString(i)); + if (supportWideGamut() && result != null) { + return Color.valueOf(result); + } + } + + throw new JSApplicationCausedNativeException( + "ColorValue: None of the paths in the `" + + JSON_KEY + + "` array resolved to a color resource."); + } + throw new JSApplicationCausedNativeException( + "ColorValue: the value must be a number or Object."); + } + + @Nullable + public static Integer getColor(Object value, Context context) { + try { + if (supportWideGamut()) { + Color color = getColorInstance(value, context); + if (color != null) { + return color.toArgb(); + } + } + } catch (JSApplicationCausedNativeException ex) { + FLog.w(ReactConstants.TAG, ex, "Error extracting color from WideGamut"); + } + return getColorInteger(value, context); + } + public static Integer getColor(Object value, Context context, int defaultInt) { try { return getColor(value, context);