From b0e2cb3371d303301b77fcab2cc7f5002d209268 Mon Sep 17 00:00:00 2001 From: Joe Vilches Date: Thu, 10 Oct 2024 13:41:13 -0700 Subject: [PATCH] Back out "fix(iOS): Replace uses of `CGColorRef` with UIColor to avoid manual memory management" (#46967) Summary: Pull Request resolved: https://github.com/facebook/react-native/pull/46967 Original commit changeset: 5e85e17567e3 Original Phabricator Diff: D63989547 This is causing some crashes internally Changelog: [Internal] Reviewed By: shwanton Differential Revision: D64194385 fbshipit-source-id: 835469e09971ae39d2db6e82f6c05970b1e6238c --- .../View/RCTViewComponentView.mm | 45 ++++++++++++------- .../React/Fabric/RCTConversions.h | 6 +++ .../React/Views/RCTBorderDrawing.h | 10 ++--- .../React/Views/RCTBorderDrawing.m | 42 ++++++++--------- packages/react-native/React/Views/RCTView.m | 19 ++++---- 5 files changed, 73 insertions(+), 49 deletions(-) diff --git a/packages/react-native/React/Fabric/Mounting/ComponentViews/View/RCTViewComponentView.mm b/packages/react-native/React/Fabric/Mounting/ComponentViews/View/RCTViewComponentView.mm index 87983b0dbc1315..04b2ede4f96c96 100644 --- a/packages/react-native/React/Fabric/Mounting/ComponentViews/View/RCTViewComponentView.mm +++ b/packages/react-native/React/Fabric/Mounting/ComponentViews/View/RCTViewComponentView.mm @@ -242,8 +242,9 @@ - (void)updateProps:(const Props::Shared &)props oldProps:(const Props::Shared & // `shadowColor` if (oldViewProps.shadowColor != newViewProps.shadowColor) { - UIColor *shadowColor = RCTUIColorFromSharedColor(newViewProps.shadowColor); - self.layer.shadowColor = shadowColor.CGColor; + CGColorRef shadowColor = RCTCreateCGColorRefFromSharedColor(newViewProps.shadowColor); + self.layer.shadowColor = shadowColor; + CGColorRelease(shadowColor); needsInvalidateLayer = YES; } @@ -681,7 +682,7 @@ static void RCTAddContourEffectToLayer( const RCTBorderStyle &contourStyle) { UIImage *image = RCTGetBorderImage( - contourStyle, layer.bounds.size, cornerRadii, contourInsets, contourColors, [UIColor clearColor], NO); + contourStyle, layer.bounds.size, cornerRadii, contourInsets, contourColors, [UIColor clearColor].CGColor, NO); if (image == nil) { layer.contents = nil; @@ -710,10 +711,18 @@ static void RCTAddContourEffectToLayer( static RCTBorderColors RCTCreateRCTBorderColorsFromBorderColors(BorderColors borderColors) { return RCTBorderColors{ - .top = RCTUIColorFromSharedColor(borderColors.top), - .left = RCTUIColorFromSharedColor(borderColors.left), - .bottom = RCTUIColorFromSharedColor(borderColors.bottom), - .right = RCTUIColorFromSharedColor(borderColors.right)}; + .top = RCTCreateCGColorRefFromSharedColor(borderColors.top), + .left = RCTCreateCGColorRefFromSharedColor(borderColors.left), + .bottom = RCTCreateCGColorRefFromSharedColor(borderColors.bottom), + .right = RCTCreateCGColorRefFromSharedColor(borderColors.right)}; +} + +static void RCTReleaseRCTBorderColors(RCTBorderColors borderColors) +{ + CGColorRelease(borderColors.top); + CGColorRelease(borderColors.left); + CGColorRelease(borderColors.bottom); + CGColorRelease(borderColors.right); } static CALayerCornerCurve CornerCurveFromBorderCurve(BorderCurve borderCurve) @@ -861,7 +870,7 @@ - (void)invalidateLayer (*borderMetrics.borderColors.left).getUIColor() != nullptr)); // background color - UIColor *backgroundColor = [_backgroundColor resolvedColorWithTraitCollection:self.traitCollection]; + CGColorRef backgroundColor = [_backgroundColor resolvedColorWithTraitCollection:self.traitCollection].CGColor; // The reason we sometimes do not set self.layer's backgroundColor is because // we want to support non-uniform border radii, which apple does not natively // support. To get this behavior we need to create a CGPath in the shape that @@ -871,7 +880,7 @@ - (void)invalidateLayer if (useCoreAnimationBorderRendering) { [_backgroundColorLayer removeFromSuperlayer]; _backgroundColorLayer = nil; - layer.backgroundColor = backgroundColor.CGColor; + layer.backgroundColor = backgroundColor; } else { layer.backgroundColor = nil; if (!_backgroundColorLayer) { @@ -881,7 +890,7 @@ - (void)invalidateLayer [self.layer addSublayer:_backgroundColorLayer]; } - _backgroundColorLayer.backgroundColor = backgroundColor.CGColor; + _backgroundColorLayer.backgroundColor = backgroundColor; if (borderMetrics.borderRadii.isUniform()) { _backgroundColorLayer.mask = nil; _backgroundColorLayer.cornerRadius = borderMetrics.borderRadii.topLeft.horizontal; @@ -902,8 +911,9 @@ - (void)invalidateLayer _borderLayer = nil; layer.borderWidth = (CGFloat)borderMetrics.borderWidths.left; - UIColor *borderColor = RCTUIColorFromSharedColor(borderMetrics.borderColors.left); - layer.borderColor = borderColor.CGColor; + CGColorRef borderColor = RCTCreateCGColorRefFromSharedColor(borderMetrics.borderColors.left); + layer.borderColor = borderColor; + CGColorRelease(borderColor); layer.cornerRadius = (CGFloat)borderMetrics.borderRadii.topLeft.horizontal; layer.cornerCurve = CornerCurveFromBorderCurve(borderMetrics.borderCurves.topLeft); } else { @@ -928,6 +938,8 @@ - (void)invalidateLayer borderColors, RCTUIEdgeInsetsFromEdgeInsets(borderMetrics.borderWidths), RCTBorderStyleFromBorderStyle(borderMetrics.borderStyles.left)); + + RCTReleaseRCTBorderColors(borderColors); } // outline @@ -946,11 +958,12 @@ - (void)invalidateLayer layer.bounds, -_props->outlineOffset - _props->outlineWidth, -_props->outlineOffset - _props->outlineWidth); if (borderMetrics.borderRadii.isUniform() && borderMetrics.borderRadii.topLeft.horizontal == 0) { - UIColor *outlineColor = RCTUIColorFromSharedColor(_props->outlineColor); + CGColorRef outlineColor = RCTCreateCGColorRefFromSharedColor(_props->outlineColor); _outlineLayer.borderWidth = _props->outlineWidth; - _outlineLayer.borderColor = outlineColor.CGColor; + _outlineLayer.borderColor = outlineColor; + CGColorRelease(outlineColor); } else { - UIColor *outlineColor = RCTUIColorFromSharedColor(_props->outlineColor); + CGColorRef outlineColor = RCTCreateCGColorRefFromSharedColor(_props->outlineColor); RCTAddContourEffectToLayer( _outlineLayer, @@ -958,6 +971,8 @@ - (void)invalidateLayer RCTBorderColors{outlineColor, outlineColor, outlineColor, outlineColor}, UIEdgeInsets{_props->outlineWidth, _props->outlineWidth, _props->outlineWidth, _props->outlineWidth}, RCTBorderStyleFromOutlineStyle(_props->outlineStyle)); + + CGColorRelease(outlineColor); } } diff --git a/packages/react-native/React/Fabric/RCTConversions.h b/packages/react-native/React/Fabric/RCTConversions.h index f93d8937ea0255..204c54686322ba 100644 --- a/packages/react-native/React/Fabric/RCTConversions.h +++ b/packages/react-native/React/Fabric/RCTConversions.h @@ -40,6 +40,12 @@ inline UIColor *_Nullable RCTUIColorFromSharedColor(const facebook::react::Share return RCTPlatformColorFromColor(*sharedColor); } +inline CF_RETURNS_RETAINED CGColorRef _Nullable RCTCreateCGColorRefFromSharedColor( + const facebook::react::SharedColor &sharedColor) +{ + return CGColorRetain(RCTUIColorFromSharedColor(sharedColor).CGColor); +} + inline CGPoint RCTCGPointFromPoint(const facebook::react::Point &point) { return {point.x, point.y}; diff --git a/packages/react-native/React/Views/RCTBorderDrawing.h b/packages/react-native/React/Views/RCTBorderDrawing.h index b13ffdf6fbadeb..39105666f63daf 100644 --- a/packages/react-native/React/Views/RCTBorderDrawing.h +++ b/packages/react-native/React/Views/RCTBorderDrawing.h @@ -29,10 +29,10 @@ typedef struct { } RCTCornerInsets; typedef struct { - UIColor *top; - UIColor *left; - UIColor *bottom; - UIColor *right; + CGColorRef top; + CGColorRef left; + CGColorRef bottom; + CGColorRef right; } RCTBorderColors; /** @@ -67,5 +67,5 @@ RCT_EXTERN UIImage *RCTGetBorderImage( RCTCornerRadii cornerRadii, UIEdgeInsets borderInsets, RCTBorderColors borderColors, - UIColor *backgroundColor, + CGColorRef backgroundColor, BOOL drawToEdge); diff --git a/packages/react-native/React/Views/RCTBorderDrawing.m b/packages/react-native/React/Views/RCTBorderDrawing.m index b51499af33d093..67bf10cba935b0 100644 --- a/packages/react-native/React/Views/RCTBorderDrawing.m +++ b/packages/react-native/React/Views/RCTBorderDrawing.m @@ -30,9 +30,9 @@ BOOL RCTCornerRadiiAreEqualAndSymmetrical(RCTCornerRadii cornerRadii) BOOL RCTBorderColorsAreEqual(RCTBorderColors borderColors) { - return CGColorEqualToColor(borderColors.left.CGColor, borderColors.right.CGColor) && - CGColorEqualToColor(borderColors.left.CGColor, borderColors.top.CGColor) && - CGColorEqualToColor(borderColors.left.CGColor, borderColors.bottom.CGColor); + return CGColorEqualToColor(borderColors.left, borderColors.right) && + CGColorEqualToColor(borderColors.left, borderColors.top) && + CGColorEqualToColor(borderColors.left, borderColors.bottom); } RCTCornerInsets RCTGetCornerInsets(RCTCornerRadii cornerRadii, UIEdgeInsets edgeInsets) @@ -182,9 +182,9 @@ static CGPathRef RCTPathCreateOuterOutline(BOOL drawToEdge, CGRect rect, RCTCorn } static UIGraphicsImageRenderer * -RCTMakeUIGraphicsImageRenderer(CGSize size, UIColor *backgroundColor, BOOL hasCornerRadii, BOOL drawToEdge) +RCTMakeUIGraphicsImageRenderer(CGSize size, CGColorRef backgroundColor, BOOL hasCornerRadii, BOOL drawToEdge) { - const CGFloat alpha = CGColorGetAlpha(backgroundColor.CGColor); + const CGFloat alpha = CGColorGetAlpha(backgroundColor); const BOOL opaque = (drawToEdge || !hasCornerRadii) && alpha == 1.0; UIGraphicsImageRendererFormat *const rendererFormat = [UIGraphicsImageRendererFormat defaultFormat]; rendererFormat.opaque = opaque; @@ -197,7 +197,7 @@ static CGPathRef RCTPathCreateOuterOutline(BOOL drawToEdge, CGRect rect, RCTCorn CGSize viewSize, UIEdgeInsets borderInsets, RCTBorderColors borderColors, - UIColor *backgroundColor, + CGColorRef backgroundColor, BOOL drawToEdge) { const BOOL hasCornerRadii = RCTCornerRadiiAreAboveThreshold(cornerRadii); @@ -233,16 +233,18 @@ static CGPathRef RCTPathCreateOuterOutline(BOOL drawToEdge, CGRect rect, RCTCorn UIGraphicsImageRenderer *const imageRenderer = RCTMakeUIGraphicsImageRenderer(size, backgroundColor, hasCornerRadii, drawToEdge); + CGColorRetain(backgroundColor); UIImage *image = [imageRenderer imageWithActions:^(UIGraphicsImageRendererContext *_Nonnull rendererContext) { const CGContextRef context = rendererContext.CGContext; const CGRect rect = {.size = size}; CGPathRef path = RCTPathCreateOuterOutline(drawToEdge, rect, cornerRadii); if (backgroundColor) { - CGContextSetFillColorWithColor(context, backgroundColor.CGColor); + CGContextSetFillColorWithColor(context, backgroundColor); CGContextAddPath(context, path); CGContextFillPath(context); } + CGColorRelease(backgroundColor); CGContextAddPath(context, path); CGPathRelease(path); @@ -254,7 +256,7 @@ static CGPathRef RCTPathCreateOuterOutline(BOOL drawToEdge, CGRect rect, RCTCorn BOOL hasEqualColors = RCTBorderColorsAreEqual(borderColors); if ((drawToEdge || !hasCornerRadii) && hasEqualColors) { - CGContextSetFillColorWithColor(context, borderColors.left.CGColor); + CGContextSetFillColorWithColor(context, borderColors.left); CGContextAddRect(context, rect); CGContextAddPath(context, insetPath); CGContextEOFillPath(context); @@ -319,7 +321,7 @@ static CGPathRef RCTPathCreateOuterOutline(BOOL drawToEdge, CGRect rect, RCTCorn } } - UIColor *currentColor = nil; + CGColorRef currentColor = NULL; // RIGHT if (borderInsets.right > 0) { @@ -343,8 +345,8 @@ static CGPathRef RCTPathCreateOuterOutline(BOOL drawToEdge, CGRect rect, RCTCorn (CGPoint){size.width, size.height}, }; - if (!CGColorEqualToColor(currentColor.CGColor, borderColors.bottom.CGColor)) { - CGContextSetFillColorWithColor(context, currentColor.CGColor); + if (!CGColorEqualToColor(currentColor, borderColors.bottom)) { + CGContextSetFillColorWithColor(context, currentColor); CGContextFillPath(context); currentColor = borderColors.bottom; } @@ -360,8 +362,8 @@ static CGPathRef RCTPathCreateOuterOutline(BOOL drawToEdge, CGRect rect, RCTCorn (CGPoint){0, size.height}, }; - if (!CGColorEqualToColor(currentColor.CGColor, borderColors.left.CGColor)) { - CGContextSetFillColorWithColor(context, currentColor.CGColor); + if (!CGColorEqualToColor(currentColor, borderColors.left)) { + CGContextSetFillColorWithColor(context, currentColor); CGContextFillPath(context); currentColor = borderColors.left; } @@ -377,15 +379,15 @@ static CGPathRef RCTPathCreateOuterOutline(BOOL drawToEdge, CGRect rect, RCTCorn (CGPoint){size.width, 0}, }; - if (!CGColorEqualToColor(currentColor.CGColor, borderColors.top.CGColor)) { - CGContextSetFillColorWithColor(context, currentColor.CGColor); + if (!CGColorEqualToColor(currentColor, borderColors.top)) { + CGContextSetFillColorWithColor(context, currentColor); CGContextFillPath(context); currentColor = borderColors.top; } CGContextAddLines(context, points, sizeof(points) / sizeof(*points)); } - CGContextSetFillColorWithColor(context, currentColor.CGColor); + CGContextSetFillColorWithColor(context, currentColor); CGContextFillPath(context); } @@ -465,7 +467,7 @@ static CGPathRef RCTPathCreateOuterOutline(BOOL drawToEdge, CGRect rect, RCTCorn CGSize viewSize, UIEdgeInsets borderInsets, RCTBorderColors borderColors, - UIColor *backgroundColor, + CGColorRef backgroundColor, BOOL drawToEdge) { NSCParameterAssert(borderStyle == RCTBorderStyleDashed || borderStyle == RCTBorderStyleDotted); @@ -492,7 +494,7 @@ static CGPathRef RCTPathCreateOuterOutline(BOOL drawToEdge, CGRect rect, RCTCorn CGContextAddPath(context, outerPath); CGPathRelease(outerPath); - CGContextSetFillColorWithColor(context, backgroundColor.CGColor); + CGContextSetFillColorWithColor(context, backgroundColor); CGContextFillPath(context); } @@ -511,7 +513,7 @@ static CGPathRef RCTPathCreateOuterOutline(BOOL drawToEdge, CGRect rect, RCTCorn CGContextSetStrokeColorWithColor(context, [UIColor yellowColor].CGColor); CGContextAddPath(context, path); - CGContextSetStrokeColorWithColor(context, borderColors.top.CGColor); + CGContextSetStrokeColorWithColor(context, borderColors.top); CGContextStrokePath(context); CGPathRelease(path); @@ -524,7 +526,7 @@ static CGPathRef RCTPathCreateOuterOutline(BOOL drawToEdge, CGRect rect, RCTCorn RCTCornerRadii cornerRadii, UIEdgeInsets borderInsets, RCTBorderColors borderColors, - UIColor *backgroundColor, + CGColorRef backgroundColor, BOOL drawToEdge) { switch (borderStyle) { diff --git a/packages/react-native/React/Views/RCTView.m b/packages/react-native/React/Views/RCTView.m index b07e7ef9e54b92..8602dedc50dfc2 100644 --- a/packages/react-native/React/Views/RCTView.m +++ b/packages/react-native/React/Views/RCTView.m @@ -774,10 +774,10 @@ - (RCTBorderColors)borderColorsWithTraitCollection:(UITraitCollection *)traitCol [directionAwareBorderRightColor resolvedColorWithTraitCollection:self.traitCollection]; return (RCTBorderColors){ - (borderTopColor ?: borderColor), - (directionAwareBorderLeftColor ?: borderColor), - (borderBottomColor ?: borderColor), - (directionAwareBorderRightColor ?: borderColor), + (borderTopColor ?: borderColor).CGColor, + (directionAwareBorderLeftColor ?: borderColor).CGColor, + (borderBottomColor ?: borderColor).CGColor, + (directionAwareBorderRightColor ?: borderColor).CGColor, }; } @@ -814,20 +814,21 @@ - (void)displayLayer:(CALayer *)layer // the content. For this reason, only use iOS border drawing when clipping // or when the border is hidden. - (borderInsets.top == 0 || (borderColors.top && CGColorGetAlpha(borderColors.top.CGColor) == 0) || - self.clipsToBounds); + (borderInsets.top == 0 || (borderColors.top && CGColorGetAlpha(borderColors.top) == 0) || self.clipsToBounds); // iOS clips to the outside of the border, but CSS clips to the inside. To // solve this, we'll need to add a container view inside the main view to // correctly clip the subviews. - UIColor *backgroundColor = [_backgroundColor resolvedColorWithTraitCollection:self.traitCollection]; + CGColorRef backgroundColor; + + backgroundColor = [_backgroundColor resolvedColorWithTraitCollection:self.traitCollection].CGColor; if (useIOSBorderRendering) { layer.cornerRadius = cornerRadii.topLeftHorizontal; - layer.borderColor = borderColors.left.CGColor; + layer.borderColor = borderColors.left; layer.borderWidth = borderInsets.left; - layer.backgroundColor = backgroundColor.CGColor; + layer.backgroundColor = backgroundColor; layer.contents = nil; layer.needsDisplayOnBoundsChange = NO; layer.mask = nil;