From 1d5566e9a77a968fb82916305e38783cb026fca8 Mon Sep 17 00:00:00 2001 From: Ramanpreet Nara Date: Mon, 31 Mar 2025 13:20:19 -0700 Subject: [PATCH 1/3] 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 3caaf7bdf10..86f34ec72e7 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 b430e7a5b9a..17022f79828 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 b7f85f83fc3..81682fa031a 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 1b18a7989243e96ead9e9c87e863f6f7218c6ceb Mon Sep 17 00:00:00 2001 From: Ramanpreet Nara Date: Mon, 31 Mar 2025 13:20:19 -0700 Subject: [PATCH 2/3] 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 3906df778bc..c7e7837e7cd 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 468967f2e191c7c509a806ff9e414df9833cbb66 Mon Sep 17 00:00:00 2001 From: Ramanpreet Nara Date: Mon, 31 Mar 2025 13:20:19 -0700 Subject: [PATCH 3/3] 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 --- .../textlayoutmanager/RCTTextLayoutManager.h | 7 +++++-- .../textlayoutmanager/RCTTextLayoutManager.mm | 12 +++++++++--- .../renderer/textlayoutmanager/TextLayoutManager.mm | 6 ++++-- 3 files changed, 18 insertions(+), 7 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 7ef23cc308c..d6550b1106b 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,11 +29,13 @@ using RCTTextLayoutFragmentEnumerationBlock = - (facebook::react::TextMeasurement)measureAttributedString:(facebook::react::AttributedString)attributedString paragraphAttributes:(facebook::react::ParagraphAttributes)paragraphAttributes - layoutConstraints:(facebook::react::LayoutConstraints)layoutConstraints; + layoutConstraints:(facebook::react::LayoutConstraints)layoutConstraints + layoutContext:(facebook::react::TextLayoutContext)layoutContext; - (facebook::react::TextMeasurement)measureNSAttributedString:(NSAttributedString *)attributedString paragraphAttributes:(facebook::react::ParagraphAttributes)paragraphAttributes - layoutConstraints:(facebook::react::LayoutConstraints)layoutConstraints; + layoutConstraints:(facebook::react::LayoutConstraints)layoutConstraints + layoutContext:(facebook::react::TextLayoutContext)layoutContext; - (void)drawAttributedString:(facebook::react::AttributedString)attributedString paragraphAttributes:(facebook::react::ParagraphAttributes)paragraphAttributes 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 781143a384a..319091430a0 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 @@ -38,6 +38,7 @@ static NSLineBreakMode RCTNSLineBreakModeFromEllipsizeMode(EllipsizeMode ellipsi - (TextMeasurement)measureNSAttributedString:(NSAttributedString *)attributedString paragraphAttributes:(ParagraphAttributes)paragraphAttributes layoutConstraints:(LayoutConstraints)layoutConstraints + layoutContext:(TextLayoutContext)layoutContext { if (attributedString.length == 0) { // This is not really an optimization because that should be checked much earlier on the call stack. @@ -51,16 +52,18 @@ - (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 layoutConstraints:(LayoutConstraints)layoutConstraints + layoutContext:(TextLayoutContext)layoutContext { return [self measureNSAttributedString:[self _nsAttributedStringFromAttributedString:attributedString] paragraphAttributes:paragraphAttributes - layoutConstraints:layoutConstraints]; + layoutConstraints:layoutConstraints + layoutContext:layoutContext]; } - (void)drawAttributedString:(AttributedString)attributedString @@ -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 cc0edbccfbc..745f61e3802 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,7 +47,8 @@ auto measurement = [textLayoutManager measureAttributedString:attributedString paragraphAttributes:paragraphAttributes - layoutConstraints:layoutConstraints]; + layoutConstraints:layoutConstraints + layoutContext:layoutContext]; if (telemetry) { telemetry->didMeasureText(); @@ -69,7 +70,8 @@ measurement = [textLayoutManager measureNSAttributedString:nsAttributedString paragraphAttributes:paragraphAttributes - layoutConstraints:layoutConstraints]; + layoutConstraints:layoutConstraints + layoutContext:layoutContext]; if (telemetry) { telemetry->didMeasureText();