[iOS] Fabric: Fixes crash of dynamic color when light/dark mode changed #48496

zhongwuzw · 2025-01-05T07:48:51Z

Summary:

The reason is when light/dark mode changed, the hash value also changed because we used color.getColor(). leads to size balanced break.

Assertion failed: (index_.size() == lru_.size()), function size, file EvictingCacheMap.h, line 439.


(lldb) bt
* thread #1, queue = 'com.apple.main-thread', stop reason = signal SIGABRT
    frame #0: 0x00000001089a9108 libsystem_kernel.dylib`__pthread_kill + 8
    frame #1: 0x0000000105de3408 libsystem_pthread.dylib`pthread_kill + 256
    frame #2: 0x000000018016c4ec libsystem_c.dylib`abort + 104
    frame #3: 0x000000018016b934 libsystem_c.dylib`__assert_rtn + 268
  * frame #4: 0x00000001073e386c React_FabricComponents`folly::EvictingCacheMap<facebook::react::AttributedString, std::__1::shared_ptr<void>, folly::HeterogeneousAccessHash<facebook::react::AttributedString, void>, folly::HeterogeneousAccessEqualTo<facebook::react::AttributedString, void>>::size(this=0x0000600003900348) const at EvictingCacheMap.h:439:5
    frame #5: 0x00000001073e34f4 React_FabricComponents`void folly::EvictingCacheMap<facebook::react::AttributedString, std::__1::shared_ptr<void>, folly::HeterogeneousAccessHash<facebook::react::AttributedString, void>, folly::HeterogeneousAccessEqualTo<facebook::react::AttributedString, void>>::setImpl<facebook::react::AttributedString>(this=0x0000600003900348, key=0x000000016b9f20a8, value=nullptr, promote=true, pruneHook=folly::EvictingCacheMap<facebook::react::AttributedString, std::__1::shared_ptr<void>, folly::HeterogeneousAccessHash<facebook::react::AttributedString, void>, folly::HeterogeneousAccessEqualTo<facebook::react::AttributedString, void> >::PruneHookCall @ 0x000000016b9f1cc8) at EvictingCacheMap.h:674:27
    frame #6: 0x00000001073deb88 React_FabricComponents`folly::EvictingCacheMap<facebook::react::AttributedString, std::__1::shared_ptr<void>, folly::HeterogeneousAccessHash<facebook::react::AttributedString, void>, folly::HeterogeneousAccessEqualTo<facebook::react::AttributedString, void>>::set(this=0x0000600003900348, key=0x000000016b9f20a8, value=ptr = 0x60000024ae20 strong=2 weak=1, promote=true, pruneHook=folly::EvictingCacheMap<facebook::react::AttributedString, std::__1::shared_ptr<void>, folly::HeterogeneousAccessHash<facebook::react::AttributedString, void>, folly::HeterogeneousAccessEqualTo<facebook::react::AttributedString, void> >::PruneHookCall @ 0x000000016b9f1d98) at EvictingCacheMap.h:346:5
    frame #7: 0x00000001073d91dc React_FabricComponents`facebook::react::SimpleThreadSafeCache<facebook::react::AttributedString, std::__1::shared_ptr<void>, 256>::get(this=0x0000600003900348, key=0x000000016b9f20a8, generator= Lambda in File RCTTextLayoutManager.mm at Line 337) const at SimpleThreadSafeCache.h:40:12
    frame #8: 0x00000001073d9058 React_FabricComponents`-[RCTTextLayoutManager _nsAttributedStringFromAttributedString:](self=0x0000600003900340, _cmd="_nsAttributedStringFromAttributedString:", attributedString=AttributedString @ 0x000000016b9f20a8) at RCTTextLayoutManager.mm:337:42
    frame #9: 0x00000001073d6378 React_FabricComponents`-[RCTTextLayoutManager drawAttributedString:paragraphAttributes:frame:drawHighlightPath:](self=0x0000600003900340, _cmd="drawAttributedString:paragraphAttributes:frame:drawHighlightPath:", attributedString=AttributedString @ 0x000000016b9f23a8, paragraphAttributes=ParagraphAttributes @ 0x000000016b9f2378, frame=(origin = (x = 0, y = 0), size = (width = 92, height = 21.666748046875)), block=0x00000001061602d0) at RCTTextLayoutManager.mm:73:56
    frame #10: 0x000000010616020c RCTFabric`-[RCTParagraphTextView drawRect:](self=0x000000012beb9dc0, _cmd="drawRect:", rect=(origin = (x = 0, y = 0.000081380208335701809), size = (width = 92, height = 21.666666666666664))) at RCTParagraphComponentView.mm:346:3
    frame #11: 0x0000000186043e60 UIKitCore`-[UIView(CALayerDelegate) drawLayer:inContext:] + 584
    frame #12: 0x000000018af40080 QuartzCore`CABackingStoreUpdate_ + 244
    frame #13: 0x000000018b0bec88 QuartzCore`invocation function for block in CA::Layer::display_() + 108
    frame #14: 0x000000018b0b5524 QuartzCore`-[CALayer _display] + 1596
    frame #15: 0x000000018b0c7e74 QuartzCore`CA::Layer::layout_and_display_if_needed(CA::Transaction*) + 392
    frame #16: 0x000000018affca50 QuartzCore`CA::Context::commit_transaction(CA::Transaction*, double, double*) + 464
    frame #17: 0x000000018b02b260 QuartzCore`CA::Transaction::commit() + 652
    frame #18: 0x000000018b02c7b4 QuartzCore`CA::Transaction::flush_as_runloop_observer(bool) + 68
    frame #19: 0x0000000185ad6c1c UIKitCore`_UIApplicationFlushCATransaction + 48
    frame #20: 0x0000000185a07ccc UIKitCore`__setupUpdateSequence_block_invoke_2 + 352
    frame #21: 0x000000018505d28c UIKitCore`_UIUpdateSequenceRun + 76
    frame #22: 0x0000000185a07670 UIKitCore`schedulerStepScheduledMainSection + 168
    frame #23: 0x0000000185a06aa8 UIKitCore`runloopSourceCallback + 80
    frame #24: 0x000000018041b7c4 CoreFoundation`__CFRUNLOOP_IS_CALLING_OUT_TO_A_SOURCE0_PERFORM_FUNCTION__ + 24
    frame #25: 0x000000018041b70c CoreFoundation`__CFRunLoopDoSource0 + 172
    frame #26: 0x000000018041ae70 CoreFoundation`__CFRunLoopDoSources0 + 232
    frame #27: 0x00000001804153b4 CoreFoundation`__CFRunLoopRun + 788
    frame #28: 0x0000000180414c24 CoreFoundation`CFRunLoopRunSpecific + 552
    frame #29: 0x000000019020ab10 GraphicsServices`GSEventRunModal + 160
    frame #30: 0x0000000185ad82fc UIKitCore`-[UIApplication _run] + 796
    frame #31: 0x0000000185adc4f4 UIKitCore`UIApplicationMain + 124
    frame #32: 0x0000000104521f68 RNTester.debug.dylib`main(argc=1, argv=0x000000016b9f5af8) at main.m:15:12
    frame #33: 0x00000001045b9410 dyld_sim`start_sim + 20
    frame #34: 0x0000000104796274 dyld`start + 2840

Changelog:

[IOS] [FIXED] - Fabric: Fixes crash of dynamic color when light/dark mode changed

Test Plan:

RNTester -> PlatformColor example -> changed the dark/light mode in the system settings -> go back to App and pop and push the PlatformColor example, it would crash:

…/fix_dynamic_crash

zhongwuzw · 2025-01-06T09:04:36Z

Also fixed #48493

cipolleschi

Thanks for fixing it! The code looks good, but I think we missed a bit.
I think we have to invalidate the cache uiColorHashValue_ when the uiColor_ changes.

Otherwise we risk to set a color, compute the hash, change the color, but the hash remains the same.

WDYT?

zhongwuzw · 2025-01-06T14:50:39Z

Thanks for fixing it! The code looks good, but I think we missed a bit. I think we have to invalidate the cache uiColorHashValue_ when the uiColor_ changes.

Otherwise we risk to set a color, compute the hash, change the color, but the hash remains the same.

WDYT?

@cipolleschi Thanks for the review, seems we don't have any interface which can change the uiColor_ after Color initialized.

cipolleschi

You are right... C++ contructors confused me...

facebook-github-bot · 2025-01-14T16:07:49Z

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

javache · 2025-01-14T16:54:28Z

...eactCommon/react/renderer/graphics/platform/ios/react/renderer/graphics/HostPlatformColor.mm

@@ -142,6 +148,40 @@ int32_t ColorFromUIColor(const std::shared_ptr<void> &uiColor)
  return static_cast<float>(rgba[channelId]);
 }

+int32_t Color::getUIColorHash() const
+{
+  if (!uiColorHashValue_) {


Would this work?

UIColor *color = (UIColor *)unwrapManagedObject(uiColor_); return [color hashValue];

The point here is: the UIColor does not change when the UITraitCollection changes.
It changes only when we resolve the uicolor against the trait collection.

The uiColor is the actual invariant here, we don't need to compute the hash resolving the uicolor against all the trait collections.

@javache @cipolleschi I don't know details about how UIColor wether implements hash correctly. But seems the hash value is different even two dynamic colors have the same color in all trait collections.

For example: Two methods like below, [self dynamicColor].hash is not equal to [self dynamicColor1].hash. Actually they are the same color. So it may have potential issues if we use the hash value to check equality?

+ (UIColor *)dynamicColor { return [UIColor colorWithDynamicProvider:^UIColor * _Nonnull(UITraitCollection * _Nonnull traitCollection) { if (traitCollection.userInterfaceStyle == UIUserInterfaceStyleDark) { return [UIColor whiteColor]; } else { return [UIColor blackColor]; } }]; } + (UIColor *)dynamicColor1 { return [UIColor colorWithDynamicProvider:^UIColor * _Nonnull(UITraitCollection * _Nonnull traitCollection) { if (traitCollection.userInterfaceStyle == UIUserInterfaceStyleDark) { return [UIColor whiteColor]; } else { return [UIColor blackColor]; } }]; }

PS. The crash disappeared if we return color.hash.

So UIColor hashValue is probably just falling back to NSObject, which means the value won't change over the objects lifetime, but also can't be reliably used to compare objects that are equal :(

My concern is that hashing is supposed to be cheap, and we use it implicitly in a number of different places. This makes the common case of a non-platform color significantly more expensive.

The hash is computed only once, when needed the first time, and then cached in the uiColorHashValue_. Isn't that enough? The tradeoff is that we recompute the same hash if we have two instances of the same color, but that should be ok, no?

javache · 2025-01-14T22:11:48Z

Alternative suggestion: let's pre-calculate the hash and store it as an associated object on the UIColor

One more issue potentially here (which would lead to cache misses!): Color::operator== is always going to be false for dynamic colors.

…IColor

zhongwuzw · 2025-01-15T09:54:26Z

Alternative suggestion: let's pre-calculate the hash and store it as an associated object on the UIColor

One more issue potentially here (which would lead to cache misses!): Color::operator== is always going to be false for dynamic colors.

@javache @cipolleschi I updated the code to pre-calculate the hash in the constructor. Please review that 👀 !

cipolleschi

Left a question. It looks good to me. @javache, wdyt?

...eactCommon/react/renderer/graphics/platform/ios/react/renderer/graphics/HostPlatformColor.mm

facebook-github-bot · 2025-01-16T10:29:33Z

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

cipolleschi · 2025-01-16T18:30:19Z

...ReactCommon/react/renderer/graphics/platform/ios/react/renderer/graphics/HostPlatformColor.h

    auto seed = size_t{0};
-    facebook::react::hash_combine(seed, color.getColor());
+    facebook::react::hash_combine(seed, color.getUIColorHash());
    return seed;


@javache No value in calling hash_combine again with seed 0.

Suggested change

auto seed = size_t{0};

facebook::react::hash_combine(seed, color.getColor());

facebook::react::hash_combine(seed, color.getUIColorHash());

return seed;

return color.getUIColorHash();

cipolleschi · 2025-01-16T18:31:12Z

...eactCommon/react/renderer/graphics/platform/ios/react/renderer/graphics/HostPlatformColor.mm

+  auto colorHash = facebook::react::hash_combine(intColor, 0);
+  color.reactHash = colorHash;


@javache

Suggested change

auto colorHash = facebook::react::hash_combine(intColor, 0);

color.reactHash = colorHash;

color.reactHash = [colorHash](facebook::react::hash_combine(intColor, 0));

cipolleschi · 2025-01-16T18:31:31Z

...eactCommon/react/renderer/graphics/platform/ios/react/renderer/graphics/HostPlatformColor.mm

+    auto colorHash = facebook::react::hash_combine(dark, light, highContrastDark, highContrastLight, 0);
+    color.reactHash = colorHash;


@javache

Suggested change

auto colorHash = facebook::react::hash_combine(dark, light, highContrastDark, highContrastLight, 0);

color.reactHash = colorHash;

color.reactHash = [colorHash;](acebook::react::hash_combine(dark, light, highContrastDark, highContrastLight, 0);)

cipolleschi · 2025-01-16T18:33:03Z

...eactCommon/react/renderer/graphics/platform/ios/react/renderer/graphics/HostPlatformColor.mm

+  auto colorHash = facebook::react::hash_combine(color, components.colorSpace == ColorSpace::DisplayP3);
+  uiColor.reactHash = colorHash;


@javache

Suggested change

auto colorHash = facebook::react::hash_combine(color, components.colorSpace == ColorSpace::DisplayP3);

uiColor.reactHash = colorHash;

uiColor.reactHash = facebook::react::hash_combine(color, components.colorSpace == ColorSpace::DisplayP3);

cipolleschi · 2025-01-16T18:34:20Z

...eactCommon/react/renderer/graphics/platform/ios/react/renderer/graphics/HostPlatformColor.mm

@@ -114,14 +195,20 @@ int32_t ColorFromUIColor(const std::shared_ptr<void> &uiColor)

 Color::Color(std::shared_ptr<void> uiColor)


@javache

When would this still get called? Can we make this constructor private so we can guarantee all UIColor objects are created with the hash in place?

Seems UndefinedColor and semantic color used it, I mark it private and add a static method createSemanticColor to create semantic color.

zhongwuzw · 2025-01-23T09:16:49Z

@javache Hi, please help to review again :)

facebook-github-bot · 2025-01-27T17:36:25Z

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

facebook-github-bot · 2025-01-28T19:24:54Z

@cipolleschi merged this pull request in cbaff1c.

react-native-bot · 2025-01-28T19:24:55Z

This pull request was successfully merged by @zhongwuzw in cbaff1c

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

facebook-github-bot · 2025-01-30T00:01:47Z

This pull request has been reverted by 04afbd9.

zhongwuzw · 2025-01-31T16:38:22Z

Is there anything I can do because of this commit was reverted? :)

cipolleschi · 2025-02-03T13:52:09Z

Yeah, you can. Thanks for the offer.
The problem was with the UIColor+graphics files. We have a very weird setup internally and sometimes it doesn't like when we add extra files. We had crashes only with the release build because those files were not linked to the binary. Funnily, the debug build was working well... That's why we didn't catch the crash before it landed.

A solution would be to remove the two files we added and use two plain C functions instead, getUIColorReactHash and setUIColorReactHash in the HostColorPlatform.mm file. We store the hash as a member of the Color struct.

zhongwuzw · 2025-02-03T16:43:19Z

@cipolleschi Thanks for your information. Maybe it's because not added -Objc C flag? I moved the category to HostPlatformColor.mm, because I think it's the easiest way to fix this issue, WDYT?

cipolleschi · 2025-02-03T17:39:59Z

@javache was suggesting to drop the category altogether, in favor of the simpler c functions... HostPlatformColor is already an Objective-C++ file, so even if we keep the category it should be good. Let's wait what he thinks about the category approach.

javache · 2025-02-03T18:05:38Z

Let's use a static function here. Categories have overhead in Objective-C and we have various compiler transforms that may cause issues here.

zhongwuzw · 2025-02-04T14:44:48Z

@cipolleschi @javache I store the hash as the member of color and calculate it in the constructor.

cipolleschi

looks good to me! :D
Thanks for the patience and for going back to work on this.

cipolleschi

looks good to me! :D
Thanks for the patience and for going back to work on this.

cipolleschi · 2025-02-07T15:29:25Z

Our system is not very happy with me trying to import a PR that result merged internally. I'm talking with our OSS team that manages the GitHub integration to see if we can find a solution.

zhongwuzw · 2025-02-07T15:53:43Z

New PR please see #49265.

…ode changed (#49265) Summary: Reland #48496 . ## Changelog: [IOS] [FIXED] - Fabric: Fixes crash of dynamic color when light/dark mode changed Pull Request resolved: #49265 Test Plan: RNTester -> PlatformColor example -> changed the dark/light mode in the system settings -> go back to App and pop and push the PlatformColor example, it would crash: Reviewed By: javache Differential Revision: D69309825 Pulled By: cipolleschi fbshipit-source-id: 7a533a73ef343b071000388b653b2d1d0c54ae88

…cebook#48496) Summary: The reason is when light/dark mode changed, the `hash` value also changed because we used `color.getColor()`. leads to size balanced break. ``` Assertion failed: (index_.size() == lru_.size()), function size, file EvictingCacheMap.h, line 439. (lldb) bt * thread facebook#1, queue = 'com.apple.main-thread', stop reason = signal SIGABRT frame #0: 0x00000001089a9108 libsystem_kernel.dylib`__pthread_kill + 8 frame facebook#1: 0x0000000105de3408 libsystem_pthread.dylib`pthread_kill + 256 frame facebook#2: 0x000000018016c4ec libsystem_c.dylib`abort + 104 frame facebook#3: 0x000000018016b934 libsystem_c.dylib`__assert_rtn + 268 * frame facebook#4: 0x00000001073e386c React_FabricComponents`folly::EvictingCacheMap<facebook::react::AttributedString, std::__1::shared_ptr<void>, folly::HeterogeneousAccessHash<facebook::react::AttributedString, void>, folly::HeterogeneousAccessEqualTo<facebook::react::AttributedString, void>>::size(this=0x0000600003900348) const at EvictingCacheMap.h:439:5 frame facebook#5: 0x00000001073e34f4 React_FabricComponents`void folly::EvictingCacheMap<facebook::react::AttributedString, std::__1::shared_ptr<void>, folly::HeterogeneousAccessHash<facebook::react::AttributedString, void>, folly::HeterogeneousAccessEqualTo<facebook::react::AttributedString, void>>::setImpl<facebook::react::AttributedString>(this=0x0000600003900348, key=0x000000016b9f20a8, value=nullptr, promote=true, pruneHook=folly::EvictingCacheMap<facebook::react::AttributedString, std::__1::shared_ptr<void>, folly::HeterogeneousAccessHash<facebook::react::AttributedString, void>, folly::HeterogeneousAccessEqualTo<facebook::react::AttributedString, void> >::PruneHookCall @ 0x000000016b9f1cc8) at EvictingCacheMap.h:674:27 frame facebook#6: 0x00000001073deb88 React_FabricComponents`folly::EvictingCacheMap<facebook::react::AttributedString, std::__1::shared_ptr<void>, folly::HeterogeneousAccessHash<facebook::react::AttributedString, void>, folly::HeterogeneousAccessEqualTo<facebook::react::AttributedString, void>>::set(this=0x0000600003900348, key=0x000000016b9f20a8, value=ptr = 0x60000024ae20 strong=2 weak=1, promote=true, pruneHook=folly::EvictingCacheMap<facebook::react::AttributedString, std::__1::shared_ptr<void>, folly::HeterogeneousAccessHash<facebook::react::AttributedString, void>, folly::HeterogeneousAccessEqualTo<facebook::react::AttributedString, void> >::PruneHookCall @ 0x000000016b9f1d98) at EvictingCacheMap.h:346:5 frame facebook#7: 0x00000001073d91dc React_FabricComponents`facebook::react::SimpleThreadSafeCache<facebook::react::AttributedString, std::__1::shared_ptr<void>, 256>::get(this=0x0000600003900348, key=0x000000016b9f20a8, generator= Lambda in File RCTTextLayoutManager.mm at Line 337) const at SimpleThreadSafeCache.h:40:12 frame facebook#8: 0x00000001073d9058 React_FabricComponents`-[RCTTextLayoutManager _nsAttributedStringFromAttributedString:](self=0x0000600003900340, _cmd="_nsAttributedStringFromAttributedString:", attributedString=AttributedString @ 0x000000016b9f20a8) at RCTTextLayoutManager.mm:337:42 frame facebook#9: 0x00000001073d6378 React_FabricComponents`-[RCTTextLayoutManager drawAttributedString:paragraphAttributes:frame:drawHighlightPath:](self=0x0000600003900340, _cmd="drawAttributedString:paragraphAttributes:frame:drawHighlightPath:", attributedString=AttributedString @ 0x000000016b9f23a8, paragraphAttributes=ParagraphAttributes @ 0x000000016b9f2378, frame=(origin = (x = 0, y = 0), size = (width = 92, height = 21.666748046875)), block=0x00000001061602d0) at RCTTextLayoutManager.mm:73:56 frame facebook#10: 0x000000010616020c RCTFabric`-[RCTParagraphTextView drawRect:](self=0x000000012beb9dc0, _cmd="drawRect:", rect=(origin = (x = 0, y = 0.000081380208335701809), size = (width = 92, height = 21.666666666666664))) at RCTParagraphComponentView.mm:346:3 frame facebook#11: 0x0000000186043e60 UIKitCore`-[UIView(CALayerDelegate) drawLayer:inContext:] + 584 frame facebook#12: 0x000000018af40080 QuartzCore`CABackingStoreUpdate_ + 244 frame facebook#13: 0x000000018b0bec88 QuartzCore`invocation function for block in CA::Layer::display_() + 108 frame facebook#14: 0x000000018b0b5524 QuartzCore`-[CALayer _display] + 1596 frame facebook#15: 0x000000018b0c7e74 QuartzCore`CA::Layer::layout_and_display_if_needed(CA::Transaction*) + 392 frame facebook#16: 0x000000018affca50 QuartzCore`CA::Context::commit_transaction(CA::Transaction*, double, double*) + 464 frame facebook#17: 0x000000018b02b260 QuartzCore`CA::Transaction::commit() + 652 frame facebook#18: 0x000000018b02c7b4 QuartzCore`CA::Transaction::flush_as_runloop_observer(bool) + 68 frame facebook#19: 0x0000000185ad6c1c UIKitCore`_UIApplicationFlushCATransaction + 48 frame facebook#20: 0x0000000185a07ccc UIKitCore`__setupUpdateSequence_block_invoke_2 + 352 frame facebook#21: 0x000000018505d28c UIKitCore`_UIUpdateSequenceRun + 76 frame facebook#22: 0x0000000185a07670 UIKitCore`schedulerStepScheduledMainSection + 168 frame facebook#23: 0x0000000185a06aa8 UIKitCore`runloopSourceCallback + 80 frame facebook#24: 0x000000018041b7c4 CoreFoundation`__CFRUNLOOP_IS_CALLING_OUT_TO_A_SOURCE0_PERFORM_FUNCTION__ + 24 frame facebook#25: 0x000000018041b70c CoreFoundation`__CFRunLoopDoSource0 + 172 frame facebook#26: 0x000000018041ae70 CoreFoundation`__CFRunLoopDoSources0 + 232 frame facebook#27: 0x00000001804153b4 CoreFoundation`__CFRunLoopRun + 788 frame facebook#28: 0x0000000180414c24 CoreFoundation`CFRunLoopRunSpecific + 552 frame facebook#29: 0x000000019020ab10 GraphicsServices`GSEventRunModal + 160 frame facebook#30: 0x0000000185ad82fc UIKitCore`-[UIApplication _run] + 796 frame facebook#31: 0x0000000185adc4f4 UIKitCore`UIApplicationMain + 124 frame facebook#32: 0x0000000104521f68 RNTester.debug.dylib`main(argc=1, argv=0x000000016b9f5af8) at main.m:15:12 frame facebook#33: 0x00000001045b9410 dyld_sim`start_sim + 20 frame facebook#34: 0x0000000104796274 dyld`start + 2840 ``` ## Changelog: [IOS] [FIXED] - Fabric: Fixes crash of dynamic color when light/dark mode changed Pull Request resolved: facebook#48496 Test Plan: RNTester -> PlatformColor example -> changed the dark/light mode in the system settings -> go back to App and pop and push the PlatformColor example, it would crash: ![Simulator Screen Recording - iPhone 16 - 2025-01-05 at 15 46 08](https://github.com/user-attachments/assets/f6faaf80-ad03-49c6-9a56-b1117bdc2659) Reviewed By: sammy-SC Differential Revision: D68157559 Pulled By: cipolleschi fbshipit-source-id: 01959845b742ce748186d3877b2792f0f9132ff5

…ode changed (facebook#49265) Summary: Reland facebook#48496 . ## Changelog: [IOS] [FIXED] - Fabric: Fixes crash of dynamic color when light/dark mode changed Pull Request resolved: facebook#49265 Test Plan: RNTester -> PlatformColor example -> changed the dark/light mode in the system settings -> go back to App and pop and push the PlatformColor example, it would crash: Reviewed By: javache Differential Revision: D69309825 Pulled By: cipolleschi fbshipit-source-id: 7a533a73ef343b071000388b653b2d1d0c54ae88

zhongwuzw added 3 commits November 8, 2024 17:14

fix crash

795e684

Merge commit 'b867c01fa624a3140ac1b8415b34b6db33fae1b4' into features…

adeafa9

…/fix_dynamic_crash

format

a720b3d

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. Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team. labels Jan 5, 2025

zhongwuzw mentioned this pull request Jan 5, 2025

Dev app crashes when switching color scheme while Text element with DynamicColorIOS is used #48493

Open

cipolleschi requested changes Jan 6, 2025

View reviewed changes

zhongwuzw requested a review from cipolleschi January 8, 2025 14:32

cipolleschi approved these changes Jan 14, 2025

View reviewed changes

javache reviewed Jan 14, 2025

View reviewed changes

pre-calculate the hash and store it as an associated object on the U…

8949170

…IColor

zhongwuzw force-pushed the features/fix_dynamic_crash branch from 42b5cd4 to 8949170 Compare January 15, 2025 09:46

cipolleschi approved these changes Jan 15, 2025

View reviewed changes

...eactCommon/react/renderer/graphics/platform/ios/react/renderer/graphics/HostPlatformColor.mm Outdated Show resolved Hide resolved

update

3e72b4d

cipolleschi reviewed Jan 20, 2025

View reviewed changes

Apply review back

dad55b5

zhongwuzw requested review from cipolleschi and javache January 21, 2025 07:13

facebook-github-bot closed this in cbaff1c Jan 28, 2025

facebook-github-bot added the Merged This PR has been merged. label Jan 28, 2025

facebook-github-bot added the Reverted label Jan 30, 2025

cipolleschi reopened this Feb 3, 2025

Move category to HostPlatformColor

fabf86f

Put hash value to constructor

6329f2e

cipolleschi approved these changes Feb 7, 2025

View reviewed changes

zhongwuzw mentioned this pull request Feb 7, 2025

Reland: [iOS] [Fabric] Fixes crash of dynamic color when light/dark mode changed #49265

Closed

zhongwuzw closed this Feb 7, 2025

		auto colorHash = facebook::react::hash_combine(intColor, 0);
		color.reactHash = colorHash;

	auto colorHash = facebook::react::hash_combine(intColor, 0);
	color.reactHash = colorHash;
	color.reactHash = [colorHash](facebook::react::hash_combine(intColor, 0));

	auto colorHash = facebook::react::hash_combine(dark, light, highContrastDark, highContrastLight, 0);
	color.reactHash = colorHash;
	color.reactHash = [colorHash;](acebook::react::hash_combine(dark, light, highContrastDark, highContrastLight, 0);)

		auto colorHash = facebook::react::hash_combine(color, components.colorSpace == ColorSpace::DisplayP3);
		uiColor.reactHash = colorHash;

	auto colorHash = facebook::react::hash_combine(color, components.colorSpace == ColorSpace::DisplayP3);
	uiColor.reactHash = colorHash;
	uiColor.reactHash = facebook::react::hash_combine(color, components.colorSpace == ColorSpace::DisplayP3);

		@@ -114,14 +195,20 @@ int32_t ColorFromUIColor(const std::shared_ptr<void> &uiColor)

		Color::Color(std::shared_ptr<void> uiColor)

[iOS] Fabric: Fixes crash of dynamic color when light/dark mode changed #48496

[iOS] Fabric: Fixes crash of dynamic color when light/dark mode changed #48496

Uh oh!

Conversation

zhongwuzw commented Jan 5, 2025

Summary:

Changelog:

Test Plan:

Uh oh!

zhongwuzw commented Jan 6, 2025

Uh oh!

cipolleschi left a comment

Choose a reason for hiding this comment

Uh oh!

zhongwuzw commented Jan 6, 2025

Uh oh!

cipolleschi left a comment

Choose a reason for hiding this comment

Uh oh!

facebook-github-bot commented Jan 14, 2025

Uh oh!

javache Jan 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cipolleschi Jan 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zhongwuzw Jan 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

javache commented Jan 14, 2025

Uh oh!

zhongwuzw commented Jan 15, 2025

Uh oh!

cipolleschi left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

facebook-github-bot commented Jan 16, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zhongwuzw commented Jan 23, 2025

Uh oh!

facebook-github-bot commented Jan 27, 2025

Uh oh!

facebook-github-bot commented Jan 28, 2025

Uh oh!

react-native-bot commented Jan 28, 2025

Uh oh!

facebook-github-bot commented Jan 30, 2025

Uh oh!

zhongwuzw commented Jan 31, 2025

Uh oh!

cipolleschi commented Feb 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

zhongwuzw commented Feb 3, 2025

Uh oh!

cipolleschi commented Feb 3, 2025

Uh oh!

javache Jan 14, 2025 •

edited

Loading

cipolleschi Jan 14, 2025 •

edited

Loading

zhongwuzw Jan 14, 2025 •

edited

Loading

cipolleschi commented Feb 3, 2025 •

edited

Loading