From 660f3b7d1f60901dd66ae07cb5e28e92c95d072b Mon Sep 17 00:00:00 2001 From: Ramanpreet Nara Date: Mon, 31 Mar 2025 16:37:03 -0700 Subject: [PATCH 1/4] RCTImageLoader: Move screen size/scale access to ui thread Summary: Let's move the RCTScreenSize and RCTScreenScale calls to the ui thread. That way, these methods don't do an unsafe sync dispatch to the ui thread, which could deadlock react native. In the future, once all call-sites are migrated to the ui thread, we will just make these methods assert that they're being called from the ui thread. Changelog: [Internal] Differential Revision: D72177191 --- .../Libraries/Image/RCTImageLoader.mm | 37 ++++++++++++++++++- .../Libraries/Image/RCTImageUtils.h | 2 +- .../Libraries/Image/RCTImageUtils.mm | 5 ++- 3 files changed, 39 insertions(+), 5 deletions(-) diff --git a/packages/react-native/Libraries/Image/RCTImageLoader.mm b/packages/react-native/Libraries/Image/RCTImageLoader.mm index 3caaf7bdf10a..86f34ec72e73 100644 --- a/packages/react-native/Libraries/Image/RCTImageLoader.mm +++ b/packages/react-native/Libraries/Image/RCTImageLoader.mm @@ -8,6 +8,7 @@ #import #import #import +#import #import @@ -18,6 +19,7 @@ #import #import #import +#import #import #import #import @@ -45,6 +47,29 @@ static NSInteger RCTImageBytesForImage(UIImage *image) return error; } +template +using Block = T (^)(void); + +template +std::shared_future runOnMainQueue(Block block) +{ + __block std::promise promise; + RCTExecuteOnMainQueue(^{ + @try { + try { + T result = block(); + promise.set_value(result); + } catch (...) { + promise.set_exception(std::current_exception()); + } + } @catch (NSException *exception) { + auto cppException = std::runtime_error([exception.description UTF8String]); + promise.set_exception(std::make_exception_ptr(cppException)); + } + }); + return promise.get_future().share(); +} + @interface RCTImageLoader () @end @@ -84,6 +109,7 @@ @implementation RCTImageLoader { NSUInteger _activeBytes; std::mutex _loadersMutex; __weak id _redirectDelegate; + std::shared_future _screenSize; } @synthesize bridge = _bridge; @@ -107,6 +133,9 @@ + (BOOL)requiresMainQueueSetup - (instancetype)initWithRedirectDelegate:(id)redirectDelegate { if (self = [super init]) { + _screenSize = runOnMainQueue(^{ + return RCTScreenSize(); + }); _redirectDelegate = redirectDelegate; _isLoaderSetup = NO; } @@ -956,15 +985,19 @@ - (RCTImageLoaderCancellationBlock)decodeImageData:(NSData *)data // Mark these bytes as in-use self->_activeBytes += decodedImageBytes; + __block std::shared_future screenScale = runOnMainQueue(^{ + return RCTScreenScale(); + }); + // Do actual decompression on a concurrent background queue dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ^{ if (!std::atomic_load(cancelled.get())) { // Decompress the image data (this may be CPU and memory intensive) - UIImage *image = RCTDecodeImageWithData(data, size, scale, resizeMode); + UIImage *image = RCTDecodeImageWithData(data, size, scale, resizeMode, screenScale.get()); #if RCT_DEV CGSize imagePixelSize = RCTSizeInPixels(image.size, image.scale); - CGSize screenPixelSize = RCTSizeInPixels(RCTScreenSize(), RCTScreenScale()); + CGSize screenPixelSize = RCTSizeInPixels(self->_screenSize.get(), screenScale.get()); if (imagePixelSize.width * imagePixelSize.height > screenPixelSize.width * screenPixelSize.height) { RCTLogInfo( @"[PERF ASSETS] Loading image at size %@, which is larger " diff --git a/packages/react-native/Libraries/Image/RCTImageUtils.h b/packages/react-native/Libraries/Image/RCTImageUtils.h index b430e7a5b9a7..17022f798282 100644 --- a/packages/react-native/Libraries/Image/RCTImageUtils.h +++ b/packages/react-native/Libraries/Image/RCTImageUtils.h @@ -61,7 +61,7 @@ RCT_EXTERN BOOL RCTUpscalingRequired( * Pass a destSize of CGSizeZero to decode the image at its original size. */ RCT_EXTERN UIImage *__nullable -RCTDecodeImageWithData(NSData *data, CGSize destSize, CGFloat destScale, RCTResizeMode resizeMode); +RCTDecodeImageWithData(NSData *data, CGSize destSize, CGFloat destScale, RCTResizeMode resizeMode, CGFloat screenScale); /** * This function takes the source data for an image and decodes just the diff --git a/packages/react-native/Libraries/Image/RCTImageUtils.mm b/packages/react-native/Libraries/Image/RCTImageUtils.mm index b7f85f83fc33..81682fa031a9 100644 --- a/packages/react-native/Libraries/Image/RCTImageUtils.mm +++ b/packages/react-native/Libraries/Image/RCTImageUtils.mm @@ -256,7 +256,8 @@ BOOL RCTUpscalingRequired( } } -UIImage *__nullable RCTDecodeImageWithData(NSData *data, CGSize destSize, CGFloat destScale, RCTResizeMode resizeMode) +UIImage *__nullable +RCTDecodeImageWithData(NSData *data, CGSize destSize, CGFloat destScale, RCTResizeMode resizeMode, CGFloat screenScale) { CGImageSourceRef sourceRef = CGImageSourceCreateWithData((__bridge CFDataRef)data, NULL); if (!sourceRef) { @@ -280,7 +281,7 @@ BOOL RCTUpscalingRequired( destScale = 1; } } else if (!destScale) { - destScale = RCTScreenScale(); + destScale = screenScale; } if (resizeMode == RCTResizeModeStretch) { From bcdc95622dbd359bf176fb74b560110b8ecd1360 Mon Sep 17 00:00:00 2001 From: Ramanpreet Nara Date: Mon, 31 Mar 2025 16:37:03 -0700 Subject: [PATCH 2/4] RCTFabricSurface: Move screen size/scale access to ui thread Summary: Let's move the RCTScreenSize and RCTScreenScale calls to the ui thread. That way, these methods don't do an unsafe sync dispatch to the ui thread, which could deadlock react native. In the future, once all call-sites are migrated to the ui thread, we will just make these methods assert that they're being called from the ui thread. Changelog: [Internal] Differential Revision: D72177194 --- .../React/Fabric/Surface/RCTFabricSurface.mm | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/packages/react-native/React/Fabric/Surface/RCTFabricSurface.mm b/packages/react-native/React/Fabric/Surface/RCTFabricSurface.mm index 3906df778bcd..c7e7837e7cd0 100644 --- a/packages/react-native/React/Fabric/Surface/RCTFabricSurface.mm +++ b/packages/react-native/React/Fabric/Surface/RCTFabricSurface.mm @@ -62,9 +62,14 @@ - (instancetype)initWithSurfacePresenter:(RCTSurfacePresenter *)surfacePresenter [_surfacePresenter registerSurface:self]; - [self setMinimumSize:CGSizeZero maximumSize:RCTViewportSize()]; - - [self _updateLayoutContext]; + /** + * Viewport size, and screen scale are only available on the main thread. + * Therefore, we just set the constraints on the main thread. + */ + RCTExecuteOnMainQueue(^{ + [self setMinimumSize:CGSizeZero maximumSize:RCTViewportSize()]; + [self _updateLayoutContextWithScreenScale:RCTScreenScale()]; + }); [[NSNotificationCenter defaultCenter] addObserver:self selector:@selector(handleContentSizeCategoryDidChangeNotification:) @@ -143,7 +148,7 @@ - (RCTSurfaceView *)view if (!_view) { _view = [[RCTSurfaceView alloc] initWithSurface:(RCTSurface *)self]; - [self _updateLayoutContext]; + [self _updateLayoutContextWithScreenScale:RCTScreenScale()]; _touchHandler = [RCTSurfaceTouchHandler new]; [_touchHandler attachToView:_view]; } @@ -170,14 +175,14 @@ - (void)_propagateStageChange } } -- (void)_updateLayoutContext +- (void)_updateLayoutContextWithScreenScale:(CGFloat)screenScale { auto layoutConstraints = _surfaceHandler->getLayoutConstraints(); layoutConstraints.layoutDirection = RCTLayoutDirection([[RCTI18nUtil sharedInstance] isRTL]); auto layoutContext = _surfaceHandler->getLayoutContext(); - layoutContext.pointScaleFactor = RCTScreenScale(); + layoutContext.pointScaleFactor = screenScale; layoutContext.swapLeftAndRightInRTL = [[RCTI18nUtil sharedInstance] isRTL] && [[RCTI18nUtil sharedInstance] doLeftAndRightSwapInRTL]; layoutContext.fontSizeMultiplier = RCTFontSizeMultiplier(); @@ -271,7 +276,7 @@ - (BOOL)synchronouslyWaitFor:(NSTimeInterval)timeout - (void)handleContentSizeCategoryDidChangeNotification:(NSNotification *)notification { - [self _updateLayoutContext]; + [self _updateLayoutContextWithScreenScale:RCTScreenScale()]; } #pragma mark - Private From cd3582a203dc41a37c2412596fa0b8887116a983 Mon Sep 17 00:00:00 2001 From: Ramanpreet Nara Date: Mon, 31 Mar 2025 16:37:03 -0700 Subject: [PATCH 3/4] RCTTextLayoutManager: Migrate off screen scale access Summary: Let's move the RCTScreenSize and RCTScreenScale calls to the ui thread. That way, these methods don't do an unsafe sync dispatch to the ui thread, which could deadlock react native. In the future, once all call-sites are migrated to the ui thread, we will just make these methods assert that they're being called from the ui thread. Changelog: [Internal] Differential Revision: D72177192 --- .../renderer/textlayoutmanager/RCTTextLayoutManager.h | 3 +++ .../renderer/textlayoutmanager/RCTTextLayoutManager.mm | 10 ++++++++-- .../renderer/textlayoutmanager/TextLayoutManager.mm | 2 ++ 3 files changed, 13 insertions(+), 2 deletions(-) diff --git a/packages/react-native/ReactCommon/react/renderer/textlayoutmanager/platform/ios/react/renderer/textlayoutmanager/RCTTextLayoutManager.h b/packages/react-native/ReactCommon/react/renderer/textlayoutmanager/platform/ios/react/renderer/textlayoutmanager/RCTTextLayoutManager.h index 7ef23cc308c3..8e9273ecd7d6 100644 --- a/packages/react-native/ReactCommon/react/renderer/textlayoutmanager/platform/ios/react/renderer/textlayoutmanager/RCTTextLayoutManager.h +++ b/packages/react-native/ReactCommon/react/renderer/textlayoutmanager/platform/ios/react/renderer/textlayoutmanager/RCTTextLayoutManager.h @@ -10,6 +10,7 @@ #import #import #import +#import #import NS_ASSUME_NONNULL_BEGIN @@ -28,10 +29,12 @@ using RCTTextLayoutFragmentEnumerationBlock = - (facebook::react::TextMeasurement)measureAttributedString:(facebook::react::AttributedString)attributedString paragraphAttributes:(facebook::react::ParagraphAttributes)paragraphAttributes + layoutContext:(facebook::react::TextLayoutContext)layoutContext layoutConstraints:(facebook::react::LayoutConstraints)layoutConstraints; - (facebook::react::TextMeasurement)measureNSAttributedString:(NSAttributedString *)attributedString paragraphAttributes:(facebook::react::ParagraphAttributes)paragraphAttributes + layoutContext:(facebook::react::TextLayoutContext)layoutContext layoutConstraints:(facebook::react::LayoutConstraints)layoutConstraints; - (void)drawAttributedString:(facebook::react::AttributedString)attributedString diff --git a/packages/react-native/ReactCommon/react/renderer/textlayoutmanager/platform/ios/react/renderer/textlayoutmanager/RCTTextLayoutManager.mm b/packages/react-native/ReactCommon/react/renderer/textlayoutmanager/platform/ios/react/renderer/textlayoutmanager/RCTTextLayoutManager.mm index 781143a384af..77e1f06a695e 100644 --- a/packages/react-native/ReactCommon/react/renderer/textlayoutmanager/platform/ios/react/renderer/textlayoutmanager/RCTTextLayoutManager.mm +++ b/packages/react-native/ReactCommon/react/renderer/textlayoutmanager/platform/ios/react/renderer/textlayoutmanager/RCTTextLayoutManager.mm @@ -37,6 +37,7 @@ static NSLineBreakMode RCTNSLineBreakModeFromEllipsizeMode(EllipsizeMode ellipsi - (TextMeasurement)measureNSAttributedString:(NSAttributedString *)attributedString paragraphAttributes:(ParagraphAttributes)paragraphAttributes + layoutContext:(TextLayoutContext)layoutContext layoutConstraints:(LayoutConstraints)layoutConstraints { if (attributedString.length == 0) { @@ -51,15 +52,17 @@ - (TextMeasurement)measureNSAttributedString:(NSAttributedString *)attributedStr paragraphAttributes:paragraphAttributes size:maximumSize]; - return [self _measureTextStorage:textStorage paragraphAttributes:paragraphAttributes]; + return [self _measureTextStorage:textStorage paragraphAttributes:paragraphAttributes layoutContext:layoutContext]; } - (TextMeasurement)measureAttributedString:(AttributedString)attributedString paragraphAttributes:(ParagraphAttributes)paragraphAttributes + layoutContext:(TextLayoutContext)layoutContext layoutConstraints:(LayoutConstraints)layoutConstraints { return [self measureNSAttributedString:[self _nsAttributedStringFromAttributedString:attributedString] paragraphAttributes:paragraphAttributes + layoutContext:layoutContext layoutConstraints:layoutConstraints]; } @@ -329,6 +332,7 @@ - (NSAttributedString *)_nsAttributedStringFromAttributedString:(AttributedStrin - (TextMeasurement)_measureTextStorage:(NSTextStorage *)textStorage paragraphAttributes:(ParagraphAttributes)paragraphAttributes + layoutContext:(TextLayoutContext)layoutContext { NSLayoutManager *layoutManager = textStorage.layoutManagers.firstObject; NSTextContainer *textContainer = layoutManager.textContainers.firstObject; @@ -382,7 +386,9 @@ - (TextMeasurement)_measureTextStorage:(NSTextStorage *)textStorage size.height = enumeratedLinesHeight; } - size = (CGSize){RCTCeilPixelValue(size.width), RCTCeilPixelValue(size.height)}; + size = (CGSize){ + ceil(size.width * layoutContext.pointScaleFactor) / layoutContext.pointScaleFactor, + ceil(size.height * layoutContext.pointScaleFactor) / layoutContext.pointScaleFactor}; __block auto attachments = TextMeasurement::Attachments{}; diff --git a/packages/react-native/ReactCommon/react/renderer/textlayoutmanager/platform/ios/react/renderer/textlayoutmanager/TextLayoutManager.mm b/packages/react-native/ReactCommon/react/renderer/textlayoutmanager/platform/ios/react/renderer/textlayoutmanager/TextLayoutManager.mm index cc0edbccfbca..3220adcb9083 100644 --- a/packages/react-native/ReactCommon/react/renderer/textlayoutmanager/platform/ios/react/renderer/textlayoutmanager/TextLayoutManager.mm +++ b/packages/react-native/ReactCommon/react/renderer/textlayoutmanager/platform/ios/react/renderer/textlayoutmanager/TextLayoutManager.mm @@ -47,6 +47,7 @@ auto measurement = [textLayoutManager measureAttributedString:attributedString paragraphAttributes:paragraphAttributes + layoutContext:layoutContext layoutConstraints:layoutConstraints]; if (telemetry) { @@ -69,6 +70,7 @@ measurement = [textLayoutManager measureNSAttributedString:nsAttributedString paragraphAttributes:paragraphAttributes + layoutContext:layoutContext layoutConstraints:layoutConstraints]; if (telemetry) { From 823bb531096aa1feb7baa65d78e9c71a2b356277 Mon Sep 17 00:00:00 2001 From: Ramanpreet Nara Date: Mon, 31 Mar 2025 16:37:03 -0700 Subject: [PATCH 4/4] ModalHostView: Move screen size access to ui thread Summary: Let's move the RCTScreenSize and RCTScreenScale calls to the ui thread. That way, these methods don't do an unsafe sync dispatch to the ui thread, which could deadlock react native. In the future, once all call-sites are migrated to the ui thread, we will just make these methods assert that they're being called from the ui thread. Changelog: [Internal] Differential Revision: D72177190 --- .../modal/ModalHostViewShadowNode.h | 17 ++++++++++++++++ .../components/modal/ModalHostViewState.h | 13 ++---------- .../components/modal/ModalHostViewUtils.h | 16 --------------- .../components/modal/ModalHostViewUtils.mm | 20 ------------------- .../platform/ios/ReactCommon/RCTInstance.mm | 13 +++++++++++- 5 files changed, 31 insertions(+), 48 deletions(-) delete mode 100644 packages/react-native/ReactCommon/react/renderer/components/modal/ModalHostViewUtils.h delete mode 100644 packages/react-native/ReactCommon/react/renderer/components/modal/ModalHostViewUtils.mm diff --git a/packages/react-native/ReactCommon/react/renderer/components/modal/ModalHostViewShadowNode.h b/packages/react-native/ReactCommon/react/renderer/components/modal/ModalHostViewShadowNode.h index 7c832dc0ca51..8f2919ea2376 100644 --- a/packages/react-native/ReactCommon/react/renderer/components/modal/ModalHostViewShadowNode.h +++ b/packages/react-native/ReactCommon/react/renderer/components/modal/ModalHostViewShadowNode.h @@ -12,6 +12,11 @@ #include #include +#if defined(__APPLE__) && TARGET_OS_IOS +#include +#include +#endif + namespace facebook::react { extern const char ModalHostViewComponentName[]; @@ -32,6 +37,18 @@ class ModalHostViewShadowNode final : public ConcreteViewShadowNode< traits.set(ShadowNodeTraits::Trait::RootNodeKind); return traits; } + +#if defined(__APPLE__) && TARGET_OS_IOS + static ConcreteStateData initialStateData( + const Props::Shared& /*props*/, + const ShadowNodeFamily::Shared& /*family*/, + const ComponentDescriptor& componentDescriptor) { + const std::shared_ptr& contextContainer = + componentDescriptor.getContextContainer(); + Size screenSize = contextContainer->at("RCTScreenSize"); + return screenSize; + } +#endif }; } // namespace facebook::react diff --git a/packages/react-native/ReactCommon/react/renderer/components/modal/ModalHostViewState.h b/packages/react-native/ReactCommon/react/renderer/components/modal/ModalHostViewState.h index 2b713926dd5e..e5d00c14bf4a 100644 --- a/packages/react-native/ReactCommon/react/renderer/components/modal/ModalHostViewState.h +++ b/packages/react-native/ReactCommon/react/renderer/components/modal/ModalHostViewState.h @@ -14,10 +14,6 @@ #include #endif -#if defined(__APPLE__) && TARGET_OS_IOS -#include "ModalHostViewUtils.h" -#endif - namespace facebook::react { /* @@ -26,13 +22,8 @@ namespace facebook::react { class ModalHostViewState final { public: using Shared = std::shared_ptr; - -#if defined(__APPLE__) && TARGET_OS_IOS - ModalHostViewState() : screenSize(RCTModalHostViewScreenSize()) { -#else - ModalHostViewState(){ -#endif - }; + ModalHostViewState() = default; + ; ModalHostViewState(Size screenSize_) : screenSize(screenSize_){}; #ifdef ANDROID diff --git a/packages/react-native/ReactCommon/react/renderer/components/modal/ModalHostViewUtils.h b/packages/react-native/ReactCommon/react/renderer/components/modal/ModalHostViewUtils.h deleted file mode 100644 index fc038ad33de5..000000000000 --- a/packages/react-native/ReactCommon/react/renderer/components/modal/ModalHostViewUtils.h +++ /dev/null @@ -1,16 +0,0 @@ -/* - * Copyright (c) Meta Platforms, Inc. and affiliates. - * - * This source code is licensed under the MIT license found in the - * LICENSE file in the root directory of this source tree. - */ - -#pragma once - -#include - -namespace facebook::react { - -Size RCTModalHostViewScreenSize(void); - -} // namespace facebook::react diff --git a/packages/react-native/ReactCommon/react/renderer/components/modal/ModalHostViewUtils.mm b/packages/react-native/ReactCommon/react/renderer/components/modal/ModalHostViewUtils.mm deleted file mode 100644 index d2c3560dca8f..000000000000 --- a/packages/react-native/ReactCommon/react/renderer/components/modal/ModalHostViewUtils.mm +++ /dev/null @@ -1,20 +0,0 @@ -/* - * Copyright (c) Meta Platforms, Inc. and affiliates. - * - * This source code is licensed under the MIT license found in the - * LICENSE file in the root directory of this source tree. - */ - -#import "ModalHostViewUtils.h" -#import -#import - -namespace facebook::react { - -Size RCTModalHostViewScreenSize(void) -{ - CGSize screenSize = RCTScreenSize(); - return {screenSize.width, screenSize.height}; -} - -} // namespace facebook::react diff --git a/packages/react-native/ReactCommon/react/runtime/platform/ios/ReactCommon/RCTInstance.mm b/packages/react-native/ReactCommon/react/runtime/platform/ios/ReactCommon/RCTInstance.mm index 4778ec2a2689..30b5d64ca08d 100644 --- a/packages/react-native/ReactCommon/react/runtime/platform/ios/ReactCommon/RCTInstance.mm +++ b/packages/react-native/ReactCommon/react/runtime/platform/ios/ReactCommon/RCTInstance.mm @@ -39,6 +39,7 @@ #import #import #import +#import #import #import #import @@ -315,6 +316,17 @@ - (void)_start // Initialize RCTModuleRegistry so that TurboModules can require other TurboModules. [_bridgeModuleDecorator.moduleRegistry setTurboModuleRegistry:_turboModuleManager]; + auto contextContainer = std::make_shared(); + RCTExecuteOnMainQueue(^{ + CGSize screenSize = RCTScreenSize(); + contextContainer->insert( + "RCTScreenSize", + { + .width = screenSize.width, + .height = screenSize.height, + }); + }); + if (ReactNativeFeatureFlags::enableMainQueueModulesOnIOS()) { /** * Some native modules need to capture uikit objects on the main thread. @@ -345,7 +357,6 @@ - (void)_start RCTLogSetBridgelessModuleRegistry(_bridgeModuleDecorator.moduleRegistry); RCTLogSetBridgelessCallableJSModules(_bridgeModuleDecorator.callableJSModules); - auto contextContainer = std::make_shared(); [_delegate didCreateContextContainer:contextContainer]; contextContainer->insert(