Skip to content

Distinguish transparent colors from undefined props - #58093

Open
ngocdevv wants to merge 2 commits into
react:mainfrom
ngocdevv:fix/transparent-color-prop-diff
Open

Distinguish transparent colors from undefined props#58093
ngocdevv wants to merge 2 commits into
react:mainfrom
ngocdevv:fix/transparent-color-prop-diff

Conversation

@ngocdevv

Copy link
Copy Markdown

Summary:

Fixes #58085.

Android represents both an undefined color and explicit transparent black as ARGB 0. SharedColor previously compared only that raw value, so Props 2.0 considered an explicitly supplied transparent color equal to an absent prop and omitted it from the mount diff.

This change tracks color presence separately from the platform value and includes it in equality, boolean conversion, and hashing. Platform parsers and the few call sites that intentionally produce an undefined color now preserve that state explicitly.

Changelog:

[ANDROID] [FIXED] - Preserve explicitly transparent colors during Props 2.0 reconciliation.

Test Plan:

  • Added ColorTest.testTransparentColorIsDistinctFromUndefined.
  • Compiled and ran a standalone regression harness against Android Color.cpp; it failed before this change and passes afterward.
  • Built ReactAndroid CMake Debug for arm64-v8a successfully.
  • Validated all 9 ReactCommon, ReactAndroid, and ReactApple C++ API snapshots against Doxygen 1.16.1.
  • Ran targeted clang-format and git diff --check.

@meta-cla meta-cla Bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Aug 24, 2026
@facebook-github-tools facebook-github-tools Bot added the Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team. label Aug 24, 2026
Comment thread packages/react-native/ReactCommon/react/renderer/graphics/Color.h Outdated
Abbondanzo

This comment was marked as duplicate.

@Abbondanzo Abbondanzo left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Sorry for the spam, this looks great! Importing it now

@meta-codesync

meta-codesync Bot commented Aug 25, 2026

Copy link
Copy Markdown

@Abbondanzo has imported this pull request. If you are a Meta employee, you can view this in D117292692.

@ngocdevv

Copy link
Copy Markdown
Author

Sorry for the spam, this looks great! Importing it now

It's fine, thank you.

@expo-bot

Copy link
Copy Markdown

Outcome: pass

@brentvatne — here is the verification. The pull request is by ngocdevv. It fixes #58085, filed by t0maboro.

The cause is confirmed. On Android Color was int32_t, so an absent color prop and an explicit transparent were both 0. SharedColor::operator== compared only that number. I compiled the real Color.h and the Android HostPlatformColor.h from both revisions and ran 19 assertions. The base revision fails 6. Head 9b91a09 passes all 19. Every "must not change" assertion holds in both arms.

People hitting this can work around it today. Use a transparent color with non-zero RGB, for example rgba(255,255,255,0). Its integer form is not 0, so the diff keeps it. The reported defect also needs enablePropsUpdateReconciliationAndroid, which is off by default.

A maintainer must decide three things. First, this change is wider than its title. It also makes SharedColor::operator bool() true for transparent. Five call sites test that predicate. Two of them, boxShadow and CSS gradients, discard the whole prop today. Those two are not behind the feature flag. Second, toDynamic(uint32_t) has no call site I can find, and it adds a symbol to three API snapshots. Third, sizeof(SharedColor) grows from 4 to 8 bytes.

Environment, and what I could not run

Linux sandbox, Debian 12, g++ 12.2.0. No simulator and no emulator ran. There are no screenshots, because nothing decisive in this run happened on a device.

What is measured and what is read. The measured unit is SharedColor equality, truthiness and hashing, compiled from the real headers of both revisions. The drop from the mount diff follows from reading getDiffProps. I did not construct a props object, a mount diff, or a view.

No Android build. This sandbox has no Android SDK or NDK, so I did not confirm that the whole Android C++ tree still compiles. That is the largest residual risk, because the change replaces a scalar type with a struct across every props struct on Android. I considered a GitHub Actions ubuntu-24.04 runner, whose image does ship the SDK and NDK. I did not use it. A ReactAndroid CMake build has to run Gradle, build hermesc, prepare boost, folly, glog and double-conversion, then compile the NDK targets. That routinely exceeds the ~30 minute limit on a hosted runner, and it would consume this run's last sandbox. So the only evidence that the full build compiles is the author's statement in the test plan.

No host test job covers this code. react_native_android_selector picks platform/cxx/react/renderer/graphics/HostPlatformColor.h off Android, and RN_SERIALIZABLE_STATE is defined only if(ANDROID) in react-native-flags.cmake. Fantom therefore builds the cxx color type, not the Android one.

The new test does not run in this repository. ColorTest.testTransparentColorIsDistinctFromUndefined is guarded by #ifdef ANDROID, and ReactCommon/react/renderer/graphics/tests/ has no CMake or GitHub Actions wiring here. It runs only inside Meta's build.

Not covered. I did not run the reporter's Switch repro on a device. That needs an Android build of this branch with the feature flag on. I did not test iOS, which this change does not touch. I ran one JavaScript test, quoted below, and not yarn lint or yarn flow-check. I did not read Meta's internal diff D117292692. The sizeof numbers come from g++ on x86-64 Linux, not from the NDK toolchain.

What the reporter asked for, and whether this answers it

I read #58085 in the sandbox. The reporter states the symptom this way: with Props 2.0 enabled, 'transparent' set at mount is silently dropped, and the component falls back to the theme default. Their repro sets a transparent thumbColor on Switch. They also state the workaround: any color with alpha=0 and non-zero RGB does update, because its integer form is not 0.

Their own diagnosis names SharedColor::operator== comparing the raw integer. That is the same mechanism this pull request changes, and the same one I measured. So the change addresses the reported cause.

The reporter also links a third-party case, software-mansion/react-native-screens#4524.

I did not run their repro, so I did not observe the Switch thumb turn transparent before and after.

Procedure and result

I cloned the repository in the sandbox and checked out two revisions: base c057b1fa0164458dea49333d5a521f79156b394d and head 9b91a09126664720fec5024b51fa8cab12cce7ff. Both come from the pull request metadata, and I confirmed the head against the GitHub API.

One harness compiles against the real headers of each revision. It includes react/renderer/graphics/Color.h and links the real Color.cpp and ColorComponents.cpp. The include path points at platform/android, so the Android Color type is the one under test.

g++ -std=c++20 -DANDROID -Wall -I$RC -I$RC/react/renderer/graphics/platform/android \
    harness.cpp $RC/react/renderer/graphics/Color.cpp $RC/react/renderer/graphics/ColorComponents.cpp

The base arm needs one extra flag to compile at all: -include cstdint. The base Android header uses uint8_t without including <cstdint>. The pull request adds that include.

This is the whole harness:

// Standalone harness: exercises the REAL Color.h / android HostPlatformColor.h
// from the react-native tree under test. Mirrors the SharedColor comparison
// that Props 2.0 getDiffProps() performs:  if (color != oldProps->color) {...}
#include <react/renderer/graphics/Color.h>
#include <cstdio>
#include <unordered_set>

using namespace facebook::react;

static int failures = 0;
static void check(bool cond, const char* what) {
  std::printf("%-62s %s\n", what, cond ? "PASS" : "FAIL");
  if (!cond) failures++;
}

// Same conversion fromRawValueShared.h performs for a JS integer color.
static SharedColor fromArgb(int64_t argb) {
  ColorComponents c{};
  float ratio = 255.f;
  c.alpha = ((argb >> 24) & 0xFF) / ratio;
  c.red = ((argb >> 16) & 0xFF) / ratio;
  c.green = ((argb >> 8) & 0xFF) / ratio;
  c.blue = (argb & 0xFF) / ratio;
  return colorFromComponents(c);
}

int main() {
  SharedColor absent;                      // prop not supplied
  SharedColor transparent = fromArgb(0);   // JS: 'transparent' / 'rgba(0,0,0,0)'
  SharedColor opaqueRed = fromArgb(0xFFFF0000);

  check(absent != transparent,
        "absent prop != explicit transparent (mount diff emits it)");
  check(!(absent == transparent), "operator== agrees");
  check(static_cast<bool>(transparent),
        "explicit transparent is truthy (SharedColor::operator bool)");
  check(!static_cast<bool>(absent), "absent prop is falsy");
  check(transparent == clearColor(), "clearColor() == explicit transparent");
  check(absent != clearColor(), "clearColor() != absent prop");

  // Values that must NOT change.
  check(static_cast<int32_t>(*transparent) == 0, "transparent serializes to 0");
  check(static_cast<int32_t>(*absent) == 0, "absent serializes to 0");
  check(static_cast<int32_t>(*opaqueRed) == static_cast<int32_t>(0xFFFF0000),
        "opaque red round-trips to 0xFFFF0000");
  check(opaqueRed != transparent, "opaque red != transparent");
  check(opaqueRed == fromArgb(0xFFFF0000), "opaque red == same opaque red");
  check(alphaFromColor(opaqueRed) == 255 && redFromColor(opaqueRed) == 255 &&
        greenFromColor(opaqueRed) == 0 && blueFromColor(opaqueRed) == 0,
        "channel accessors unchanged for opaque red");
  check(!isColorMeaningful(transparent), "isColorMeaningful(transparent)==false");
  check(isColorMeaningful(opaqueRed), "isColorMeaningful(opaqueRed)==true");
  check(*blackColor() == *fromArgb(0xFF000000), "blackColor() unchanged");
  check(*whiteColor() == *fromArgb(0xFFFFFFFF), "whiteColor() unchanged");

  // BoxShadow / gradient / drop-shadow path: coerceColor("transparent") ends in
  // fromCSSColor(CSSColor{0,0,0,0}) -> hostPlatformColorFromRGBA(0,0,0,0).
  // Those parsers do `if (!color) { return {}; }` and drop the WHOLE prop.
  SharedColor cssTransparent = hostPlatformColorFromRGBA(0, 0, 0, 0);
  check(static_cast<bool>(cssTransparent),
        "fromCSSColor(transparent) truthy: boxShadow/gradient keep it");
  SharedColor cssRed = hostPlatformColorFromRGBA(255, 0, 0, 255);
  check(static_cast<bool>(cssRed), "fromCSSColor(red) truthy (unchanged)");

  // std::hash must still work and must separate the two states.
  std::unordered_set<SharedColor> set;
  set.insert(absent);
  set.insert(transparent);
  check(set.size() == 2, "std::hash<SharedColor>: absent and transparent differ");

  std::printf("\nsizeof(Color)=%zu sizeof(SharedColor)=%zu\n", sizeof(Color),
              sizeof(SharedColor));
  std::printf("%s\n", failures == 0 ? "ALL PASS" : "SOME FAILED");
  return failures == 0 ? 0 : 1;
}

Raw output, base c057b1f:

compile exit=0
absent prop != explicit transparent (mount diff emits it)      FAIL
operator== agrees                                              FAIL
explicit transparent is truthy (SharedColor::operator bool)    FAIL
absent prop is falsy                                           PASS
clearColor() == explicit transparent                           PASS
clearColor() != absent prop                                    FAIL
transparent serializes to 0                                    PASS
absent serializes to 0                                         PASS
opaque red round-trips to 0xFFFF0000                           PASS
opaque red != transparent                                      PASS
opaque red == same opaque red                                  PASS
channel accessors unchanged for opaque red                     PASS
isColorMeaningful(transparent)==false                          PASS
isColorMeaningful(opaqueRed)==true                             PASS
blackColor() unchanged                                         PASS
whiteColor() unchanged                                         PASS
fromCSSColor(transparent) truthy: boxShadow/gradient keep it   FAIL
fromCSSColor(red) truthy (unchanged)                           PASS
std::hash<SharedColor>: absent and transparent differ          FAIL

sizeof(Color)=4 sizeof(SharedColor)=4
SOME FAILED
run exit=1

Raw output, head 9b91a09:

compile exit=0
absent prop != explicit transparent (mount diff emits it)      PASS
operator== agrees                                              PASS
explicit transparent is truthy (SharedColor::operator bool)    PASS
absent prop is falsy                                           PASS
clearColor() == explicit transparent                           PASS
clearColor() != absent prop                                    PASS
transparent serializes to 0                                    PASS
absent serializes to 0                                         PASS
opaque red round-trips to 0xFFFF0000                           PASS
opaque red != transparent                                      PASS
opaque red == same opaque red                                  PASS
channel accessors unchanged for opaque red                     PASS
isColorMeaningful(transparent)==false                          PASS
isColorMeaningful(opaqueRed)==true                             PASS
blackColor() unchanged                                         PASS
whiteColor() unchanged                                         PASS
fromCSSColor(transparent) truthy: boxShadow/gradient keep it   PASS
fromCSSColor(red) truthy (unchanged)                           PASS
std::hash<SharedColor>: absent and transparent differ          PASS

sizeof(Color)=8 sizeof(SharedColor)=8
ALL PASS
run exit=0

I also ran the repository's own generator test for the changed codegen template, after yarn install --frozen-lockfile at the head revision:

$ yarn jest packages/react-native-codegen/src/generators/components/__tests__/GeneratePropsCpp-test.js
Test Suites: 1 passed, 1 total
Tests:       38 passed, 38 total
Snapshots:   38 passed, 38 total
Time:        0.551 s, estimated 1 s
Done in 1.19s.
JEST_EXIT=0

So the committed snapshot matches the changed generator.

Cause

HostPlatformColor.h declares using Color = int32_t and UndefinedColor = 0. ARGB 0 is also zero-alpha black, that is transparent.

SharedColor::operator== compares only that raw value, and operator bool() tests it against UndefinedColor. So on Android the two states were one state.

Props 2.0 emits a prop only when the values differ. See HostPlatformViewProps::getDiffProps. Two situations hit the defect.

  • First mount. When prevProps is null, the function compares against a static default-constructed props object, whose colors are undefined. See lines 456-464. A view mounted with a transparent color compared equal to that default, so the prop was never sent.
  • Update. A prop that moves from absent to transparent, or back, also compared equal. So the update was dropped.

iOS does not have this defect. Color::operator== treats "no UIColor" as a state that is separate from any real UIColor. This pull request gives Android the same distinction with an isDefined flag.

Behavior that changes beyond the description

The description says the change affects Props 2.0 reconciliation. That path is behind enablePropsUpdateReconciliationAndroid, which has defaultValue: false in the feature flag config.

The change also flips SharedColor::operator bool(). Five other Android call sites test that predicate, and none of them is behind that flag. All five move Android to the behavior iOS already has.

  1. boxShadow. BoxShadowPropsConversions.h rejects a shadow whose color is falsy. One rejected entry empties the whole list. So boxShadow: '0 0 10px transparent' drops every shadow on the view today.
  2. Gradients. BackgroundImagePropsConversions.cpp discards the whole backgroundImage value when a color stop is falsy. So linear-gradient(transparent, red) renders nothing today.
  3. filter: drop-shadow. FilterPropsConversions.h assigns the parsed color only when it is truthy. A transparent drop shadow falls back to the default color today.
  4. Text color inheritance. TextAttributes::apply keeps the parent color when the child color is falsy. A nested <Text style={{color: 'transparent'}}> inherits the parent color today.
  5. The text MapBuffer path. conversions.h writes a color key only when the color is truthy. Transparent text colors now reach the Android text layout manager.

Cases 1, 2 and 3 reach coerceColor, which ends in fromCSSColor and then hostPlatformColorFromRGBA(0,0,0,0). The harness runs that exact call. It is falsy on base and truthy on head, and that is the predicate those parsers test.

I did not measure any of the five on a device. I read the code and measured the predicate. A maintainer should decide whether these belong in this pull request, and whether they want test coverage for them.

Review notes

toDynamic(uint32_t) has no call site I could find. The pull request adds this overload to propsConversions.h and records it in three .api snapshots. I searched every toDynamic( call in ReactCommon at the head revision, and I read the two generators that emit toDynamic calls into generated code. GeneratePropsCpp.js emits toDynamic only for object, array, enum, ImageSource, Point, EdgeInsets and Dimension props. GeneratePropsH.js writes scalar struct members directly, and routes only reserved and object types through toDynamic. Colors reach toDynamic(const SharedColor&) in Color.h. Please confirm the overload is needed. If it is not, removing it also removes three snapshot lines.

ColorAnimatedNode keeps a magic 0. ColorAnimatedNode.h at head still declares Color color_{0}. The pull request converts the other two such sites to {}. So this initial value is now a defined transparent color instead of an undefined one. Every consumer casts it with static_cast<int32_t>, so the result does not change. It is still inconsistent with the convention the pull request introduces.

Size. Color grows from 4 to 8 bytes on Android, and so does SharedColor. ViewProps and its Android subclass hold about a dozen color fields. Every shadow node's props grow by that much. This is a cost to accept knowingly.

The cxx platform keeps the defect. platform/cxx/HostPlatformColor.h keeps using Color = int32_t. That matches the [ANDROID] changelog tag. Out-of-tree C++ platforms keep the old behavior.

Implicit conversions. The new Android Color has a non-explicit constructor from int32_t and a non-explicit operator int32_t(). That pair can make an expression like color == 0 ambiguous. I searched the tree and found no such comparison. The iOS Color already has the same pair, so the shape is not new.

Duplicates. The supplied related list is empty. I did not run my own search for duplicate issues or competing pull requests.

@Abbondanzo

Copy link
Copy Markdown
Contributor

Just a heads up, landing this today! Sorry for the radio silence--wanted to hold off until the breaking change window opened yesterday to get this in

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.

[Android][Fabric] enablePropsUpdateReconciliationAndroid conflates transparent color with undefined value

3 participants