mirror of
https://github.com/facebook/react-native.git
synced 2025-11-01 09:14:26 +00:00
fix(iOS): Replace uses of CGColorRef with UIColor to avoid manual memory management (#46847)
Summary: Update React Native on iOS to do less manual memory management, by replacing uses of `CGColorRef` with `UIColor`, and only calling `UIColor.CGColor` when needed. This results in much less manual memory management, which is probably a good thing in 2024 :D. The downside is there is a breaking change: the signature of a method in `RCTBorderDrawing` changes. This is a followup to https://github.com/facebook/react-native/issues/46797 . After that PR merged and I tested merging React Native macOS to the `0.76-stable` branch cut commit again, I saw even more places I needed to manually call `CGColorRetain` / `CGColorRelease`. The reason is due to React Native macOS specifics (explained below) and I could update React Native macOS to not need these changes, but I thought I would at least throw up a PR to propose the changes, as it may be good to move away from using Core Graphics' C base API as much as possible. ## Longer Explanation With https://github.com/microsoft/react-native-macos/pull/2209 , I wrote a shim of [UIGraphicsImageRenderer](https://developer.apple.com/documentation/uikit/uigraphicsimagerenderer) for macOS. The main difference is my shim calls the NSImage API [imageWithSize:flipped:drawingHandler:](https://developer.apple.com/documentation/appkit/nsimage/1519860-imagewithsize?language=objc) to shim the UIGraphicsImageRenderer API [imageWithData:](https://developer.apple.com/documentation/uikit/uiimage/1624137-imagewithdata). The difference between the two is that the macOS API copies the block, and executes it when Appkit is about to draw the image, while the iOS API executes the block right away. Because of this, I think I am hitting way more places where `CGColorRef` variables needed to be retained / released. Additionally, I hit more of them when I merge to the 0.76 branch cut commit (this is why I didn't catch it with https://github.com/facebook/react-native/issues/46797). Given this constraint, I have a couple of options: 1. Refactor my macOS shim to use the deprecated API [[NSImage lockFocus]](https://developer.apple.com/documentation/appkit/nsimage/1519891-lockfocus) - I am not a fan of this because `lockFocus` was deprecated for `imageWithSize:flipped:drawingHandler:` 2. Refactor my macOS shim to do what we used to do: Create a CGContext manually and write it to an image - This is probably OK. Relies on less Appkit specifics, and potentially gives more control to RN on the rendering layer, which I recall lenaic told me is better for Fabric) 3. Refactor React Native to avoid CGColorRef altogether, and use `UIColor` (which ARC will memory manage for us) as much as possible. - This is the approach of this PR and my preferred approach. The downside is this changes the signature of some of the methods in `RCTBorderDrawing` which is a breaking change. I've seen other PRs do this, so maybe its OK? ## Changelog: [IOS] [BREAKING] - Replace uses of `CGColorRef` with UIColor to avoid manual memory management Pull Request resolved: https://github.com/facebook/react-native/pull/46847 Test Plan: Launching RNTester's View example (which tests a lot of the border / outline / shadow rendering) does not crash for me on both iOS and macOS. Reviewed By: NickGerleman Differential Revision: D63989547 Pulled By: joevilches fbshipit-source-id: 5e85e17567e3dd8b4b0388452398954ad2e02464
This commit is contained in:
Saad Najmi
committed by
Facebook GitHub Bot
Facebook GitHub Bot
parent
0794fa909b
commit
b70709dbc2
+15
-30
@@ -242,9 +242,8 @@ const CGFloat BACKGROUND_COLOR_ZPOSITION = -1024.0f;
|
||||
|
||||
// `shadowColor`
|
||||
if (oldViewProps.shadowColor != newViewProps.shadowColor) {
|
||||
CGColorRef shadowColor = RCTCreateCGColorRefFromSharedColor(newViewProps.shadowColor);
|
||||
self.layer.shadowColor = shadowColor;
|
||||
CGColorRelease(shadowColor);
|
||||
UIColor *shadowColor = RCTUIColorFromSharedColor(newViewProps.shadowColor);
|
||||
self.layer.shadowColor = shadowColor.CGColor;
|
||||
needsInvalidateLayer = YES;
|
||||
}
|
||||
|
||||
@@ -682,7 +681,7 @@ static void RCTAddContourEffectToLayer(
|
||||
const RCTBorderStyle &contourStyle)
|
||||
{
|
||||
UIImage *image = RCTGetBorderImage(
|
||||
contourStyle, layer.bounds.size, cornerRadii, contourInsets, contourColors, [UIColor clearColor].CGColor, NO);
|
||||
contourStyle, layer.bounds.size, cornerRadii, contourInsets, contourColors, [UIColor clearColor], NO);
|
||||
|
||||
if (image == nil) {
|
||||
layer.contents = nil;
|
||||
@@ -711,18 +710,10 @@ static void RCTAddContourEffectToLayer(
|
||||
static RCTBorderColors RCTCreateRCTBorderColorsFromBorderColors(BorderColors borderColors)
|
||||
{
|
||||
return RCTBorderColors{
|
||||
.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);
|
||||
.top = RCTUIColorFromSharedColor(borderColors.top),
|
||||
.left = RCTUIColorFromSharedColor(borderColors.left),
|
||||
.bottom = RCTUIColorFromSharedColor(borderColors.bottom),
|
||||
.right = RCTUIColorFromSharedColor(borderColors.right)};
|
||||
}
|
||||
|
||||
static CALayerCornerCurve CornerCurveFromBorderCurve(BorderCurve borderCurve)
|
||||
@@ -870,7 +861,7 @@ static RCTBorderStyle RCTBorderStyleFromOutlineStyle(OutlineStyle outlineStyle)
|
||||
(*borderMetrics.borderColors.left).getUIColor() != nullptr));
|
||||
|
||||
// background color
|
||||
CGColorRef backgroundColor = [_backgroundColor resolvedColorWithTraitCollection:self.traitCollection].CGColor;
|
||||
UIColor *backgroundColor = [_backgroundColor resolvedColorWithTraitCollection:self.traitCollection];
|
||||
// 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
|
||||
@@ -880,7 +871,7 @@ static RCTBorderStyle RCTBorderStyleFromOutlineStyle(OutlineStyle outlineStyle)
|
||||
if (useCoreAnimationBorderRendering) {
|
||||
[_backgroundColorLayer removeFromSuperlayer];
|
||||
_backgroundColorLayer = nil;
|
||||
layer.backgroundColor = backgroundColor;
|
||||
layer.backgroundColor = backgroundColor.CGColor;
|
||||
} else {
|
||||
layer.backgroundColor = nil;
|
||||
if (!_backgroundColorLayer) {
|
||||
@@ -890,7 +881,7 @@ static RCTBorderStyle RCTBorderStyleFromOutlineStyle(OutlineStyle outlineStyle)
|
||||
[self.layer addSublayer:_backgroundColorLayer];
|
||||
}
|
||||
|
||||
_backgroundColorLayer.backgroundColor = backgroundColor;
|
||||
_backgroundColorLayer.backgroundColor = backgroundColor.CGColor;
|
||||
if (borderMetrics.borderRadii.isUniform()) {
|
||||
_backgroundColorLayer.mask = nil;
|
||||
_backgroundColorLayer.cornerRadius = borderMetrics.borderRadii.topLeft.horizontal;
|
||||
@@ -911,9 +902,8 @@ static RCTBorderStyle RCTBorderStyleFromOutlineStyle(OutlineStyle outlineStyle)
|
||||
_borderLayer = nil;
|
||||
|
||||
layer.borderWidth = (CGFloat)borderMetrics.borderWidths.left;
|
||||
CGColorRef borderColor = RCTCreateCGColorRefFromSharedColor(borderMetrics.borderColors.left);
|
||||
layer.borderColor = borderColor;
|
||||
CGColorRelease(borderColor);
|
||||
UIColor *borderColor = RCTUIColorFromSharedColor(borderMetrics.borderColors.left);
|
||||
layer.borderColor = borderColor.CGColor;
|
||||
layer.cornerRadius = (CGFloat)borderMetrics.borderRadii.topLeft.horizontal;
|
||||
layer.cornerCurve = CornerCurveFromBorderCurve(borderMetrics.borderCurves.topLeft);
|
||||
} else {
|
||||
@@ -938,8 +928,6 @@ static RCTBorderStyle RCTBorderStyleFromOutlineStyle(OutlineStyle outlineStyle)
|
||||
borderColors,
|
||||
RCTUIEdgeInsetsFromEdgeInsets(borderMetrics.borderWidths),
|
||||
RCTBorderStyleFromBorderStyle(borderMetrics.borderStyles.left));
|
||||
|
||||
RCTReleaseRCTBorderColors(borderColors);
|
||||
}
|
||||
|
||||
// outline
|
||||
@@ -958,12 +946,11 @@ static RCTBorderStyle RCTBorderStyleFromOutlineStyle(OutlineStyle outlineStyle)
|
||||
layer.bounds, -_props->outlineOffset - _props->outlineWidth, -_props->outlineOffset - _props->outlineWidth);
|
||||
|
||||
if (borderMetrics.borderRadii.isUniform() && borderMetrics.borderRadii.topLeft.horizontal == 0) {
|
||||
CGColorRef outlineColor = RCTCreateCGColorRefFromSharedColor(_props->outlineColor);
|
||||
UIColor *outlineColor = RCTUIColorFromSharedColor(_props->outlineColor);
|
||||
_outlineLayer.borderWidth = _props->outlineWidth;
|
||||
_outlineLayer.borderColor = outlineColor;
|
||||
CGColorRelease(outlineColor);
|
||||
_outlineLayer.borderColor = outlineColor.CGColor;
|
||||
} else {
|
||||
CGColorRef outlineColor = RCTCreateCGColorRefFromSharedColor(_props->outlineColor);
|
||||
UIColor *outlineColor = RCTUIColorFromSharedColor(_props->outlineColor);
|
||||
|
||||
RCTAddContourEffectToLayer(
|
||||
_outlineLayer,
|
||||
@@ -971,8 +958,6 @@ static RCTBorderStyle RCTBorderStyleFromOutlineStyle(OutlineStyle outlineStyle)
|
||||
RCTBorderColors{outlineColor, outlineColor, outlineColor, outlineColor},
|
||||
UIEdgeInsets{_props->outlineWidth, _props->outlineWidth, _props->outlineWidth, _props->outlineWidth},
|
||||
RCTBorderStyleFromOutlineStyle(_props->outlineStyle));
|
||||
|
||||
CGColorRelease(outlineColor);
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -40,12 +40,6 @@ 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};
|
||||
|
||||
@@ -29,10 +29,10 @@ typedef struct {
|
||||
} RCTCornerInsets;
|
||||
|
||||
typedef struct {
|
||||
CGColorRef top;
|
||||
CGColorRef left;
|
||||
CGColorRef bottom;
|
||||
CGColorRef right;
|
||||
UIColor *top;
|
||||
UIColor *left;
|
||||
UIColor *bottom;
|
||||
UIColor *right;
|
||||
} RCTBorderColors;
|
||||
|
||||
/**
|
||||
@@ -67,5 +67,5 @@ RCT_EXTERN UIImage *RCTGetBorderImage(
|
||||
RCTCornerRadii cornerRadii,
|
||||
UIEdgeInsets borderInsets,
|
||||
RCTBorderColors borderColors,
|
||||
CGColorRef backgroundColor,
|
||||
UIColor *backgroundColor,
|
||||
BOOL drawToEdge);
|
||||
|
||||
@@ -30,9 +30,9 @@ BOOL RCTCornerRadiiAreEqualAndSymmetrical(RCTCornerRadii cornerRadii)
|
||||
|
||||
BOOL RCTBorderColorsAreEqual(RCTBorderColors borderColors)
|
||||
{
|
||||
return CGColorEqualToColor(borderColors.left, borderColors.right) &&
|
||||
CGColorEqualToColor(borderColors.left, borderColors.top) &&
|
||||
CGColorEqualToColor(borderColors.left, borderColors.bottom);
|
||||
return CGColorEqualToColor(borderColors.left.CGColor, borderColors.right.CGColor) &&
|
||||
CGColorEqualToColor(borderColors.left.CGColor, borderColors.top.CGColor) &&
|
||||
CGColorEqualToColor(borderColors.left.CGColor, borderColors.bottom.CGColor);
|
||||
}
|
||||
|
||||
RCTCornerInsets RCTGetCornerInsets(RCTCornerRadii cornerRadii, UIEdgeInsets edgeInsets)
|
||||
@@ -182,9 +182,9 @@ static CGPathRef RCTPathCreateOuterOutline(BOOL drawToEdge, CGRect rect, RCTCorn
|
||||
}
|
||||
|
||||
static UIGraphicsImageRenderer *
|
||||
RCTMakeUIGraphicsImageRenderer(CGSize size, CGColorRef backgroundColor, BOOL hasCornerRadii, BOOL drawToEdge)
|
||||
RCTMakeUIGraphicsImageRenderer(CGSize size, UIColor *backgroundColor, BOOL hasCornerRadii, BOOL drawToEdge)
|
||||
{
|
||||
const CGFloat alpha = CGColorGetAlpha(backgroundColor);
|
||||
const CGFloat alpha = CGColorGetAlpha(backgroundColor.CGColor);
|
||||
const BOOL opaque = (drawToEdge || !hasCornerRadii) && alpha == 1.0;
|
||||
UIGraphicsImageRendererFormat *const rendererFormat = [UIGraphicsImageRendererFormat defaultFormat];
|
||||
rendererFormat.opaque = opaque;
|
||||
@@ -197,7 +197,7 @@ static UIImage *RCTGetSolidBorderImage(
|
||||
CGSize viewSize,
|
||||
UIEdgeInsets borderInsets,
|
||||
RCTBorderColors borderColors,
|
||||
CGColorRef backgroundColor,
|
||||
UIColor *backgroundColor,
|
||||
BOOL drawToEdge)
|
||||
{
|
||||
const BOOL hasCornerRadii = RCTCornerRadiiAreAboveThreshold(cornerRadii);
|
||||
@@ -233,18 +233,16 @@ static UIImage *RCTGetSolidBorderImage(
|
||||
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);
|
||||
CGContextSetFillColorWithColor(context, backgroundColor.CGColor);
|
||||
CGContextAddPath(context, path);
|
||||
CGContextFillPath(context);
|
||||
}
|
||||
CGColorRelease(backgroundColor);
|
||||
|
||||
CGContextAddPath(context, path);
|
||||
CGPathRelease(path);
|
||||
@@ -256,7 +254,7 @@ static UIImage *RCTGetSolidBorderImage(
|
||||
|
||||
BOOL hasEqualColors = RCTBorderColorsAreEqual(borderColors);
|
||||
if ((drawToEdge || !hasCornerRadii) && hasEqualColors) {
|
||||
CGContextSetFillColorWithColor(context, borderColors.left);
|
||||
CGContextSetFillColorWithColor(context, borderColors.left.CGColor);
|
||||
CGContextAddRect(context, rect);
|
||||
CGContextAddPath(context, insetPath);
|
||||
CGContextEOFillPath(context);
|
||||
@@ -321,7 +319,7 @@ static UIImage *RCTGetSolidBorderImage(
|
||||
}
|
||||
}
|
||||
|
||||
CGColorRef currentColor = NULL;
|
||||
UIColor *currentColor = nil;
|
||||
|
||||
// RIGHT
|
||||
if (borderInsets.right > 0) {
|
||||
@@ -345,8 +343,8 @@ static UIImage *RCTGetSolidBorderImage(
|
||||
(CGPoint){size.width, size.height},
|
||||
};
|
||||
|
||||
if (!CGColorEqualToColor(currentColor, borderColors.bottom)) {
|
||||
CGContextSetFillColorWithColor(context, currentColor);
|
||||
if (!CGColorEqualToColor(currentColor.CGColor, borderColors.bottom.CGColor)) {
|
||||
CGContextSetFillColorWithColor(context, currentColor.CGColor);
|
||||
CGContextFillPath(context);
|
||||
currentColor = borderColors.bottom;
|
||||
}
|
||||
@@ -362,8 +360,8 @@ static UIImage *RCTGetSolidBorderImage(
|
||||
(CGPoint){0, size.height},
|
||||
};
|
||||
|
||||
if (!CGColorEqualToColor(currentColor, borderColors.left)) {
|
||||
CGContextSetFillColorWithColor(context, currentColor);
|
||||
if (!CGColorEqualToColor(currentColor.CGColor, borderColors.left.CGColor)) {
|
||||
CGContextSetFillColorWithColor(context, currentColor.CGColor);
|
||||
CGContextFillPath(context);
|
||||
currentColor = borderColors.left;
|
||||
}
|
||||
@@ -379,15 +377,15 @@ static UIImage *RCTGetSolidBorderImage(
|
||||
(CGPoint){size.width, 0},
|
||||
};
|
||||
|
||||
if (!CGColorEqualToColor(currentColor, borderColors.top)) {
|
||||
CGContextSetFillColorWithColor(context, currentColor);
|
||||
if (!CGColorEqualToColor(currentColor.CGColor, borderColors.top.CGColor)) {
|
||||
CGContextSetFillColorWithColor(context, currentColor.CGColor);
|
||||
CGContextFillPath(context);
|
||||
currentColor = borderColors.top;
|
||||
}
|
||||
CGContextAddLines(context, points, sizeof(points) / sizeof(*points));
|
||||
}
|
||||
|
||||
CGContextSetFillColorWithColor(context, currentColor);
|
||||
CGContextSetFillColorWithColor(context, currentColor.CGColor);
|
||||
CGContextFillPath(context);
|
||||
}
|
||||
|
||||
@@ -467,7 +465,7 @@ static UIImage *RCTGetDashedOrDottedBorderImage(
|
||||
CGSize viewSize,
|
||||
UIEdgeInsets borderInsets,
|
||||
RCTBorderColors borderColors,
|
||||
CGColorRef backgroundColor,
|
||||
UIColor *backgroundColor,
|
||||
BOOL drawToEdge)
|
||||
{
|
||||
NSCParameterAssert(borderStyle == RCTBorderStyleDashed || borderStyle == RCTBorderStyleDotted);
|
||||
@@ -494,7 +492,7 @@ static UIImage *RCTGetDashedOrDottedBorderImage(
|
||||
CGContextAddPath(context, outerPath);
|
||||
CGPathRelease(outerPath);
|
||||
|
||||
CGContextSetFillColorWithColor(context, backgroundColor);
|
||||
CGContextSetFillColorWithColor(context, backgroundColor.CGColor);
|
||||
CGContextFillPath(context);
|
||||
}
|
||||
|
||||
@@ -513,7 +511,7 @@ static UIImage *RCTGetDashedOrDottedBorderImage(
|
||||
CGContextSetStrokeColorWithColor(context, [UIColor yellowColor].CGColor);
|
||||
|
||||
CGContextAddPath(context, path);
|
||||
CGContextSetStrokeColorWithColor(context, borderColors.top);
|
||||
CGContextSetStrokeColorWithColor(context, borderColors.top.CGColor);
|
||||
CGContextStrokePath(context);
|
||||
|
||||
CGPathRelease(path);
|
||||
@@ -526,7 +524,7 @@ UIImage *RCTGetBorderImage(
|
||||
RCTCornerRadii cornerRadii,
|
||||
UIEdgeInsets borderInsets,
|
||||
RCTBorderColors borderColors,
|
||||
CGColorRef backgroundColor,
|
||||
UIColor *backgroundColor,
|
||||
BOOL drawToEdge)
|
||||
{
|
||||
switch (borderStyle) {
|
||||
|
||||
@@ -774,10 +774,10 @@ static CGFloat RCTDefaultIfNegativeTo(CGFloat defaultValue, CGFloat x)
|
||||
[directionAwareBorderRightColor resolvedColorWithTraitCollection:self.traitCollection];
|
||||
|
||||
return (RCTBorderColors){
|
||||
(borderTopColor ?: borderColor).CGColor,
|
||||
(directionAwareBorderLeftColor ?: borderColor).CGColor,
|
||||
(borderBottomColor ?: borderColor).CGColor,
|
||||
(directionAwareBorderRightColor ?: borderColor).CGColor,
|
||||
(borderTopColor ?: borderColor),
|
||||
(directionAwareBorderLeftColor ?: borderColor),
|
||||
(borderBottomColor ?: borderColor),
|
||||
(directionAwareBorderRightColor ?: borderColor),
|
||||
};
|
||||
}
|
||||
|
||||
@@ -814,21 +814,20 @@ static CGFloat RCTDefaultIfNegativeTo(CGFloat defaultValue, CGFloat x)
|
||||
// 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) == 0) || self.clipsToBounds);
|
||||
(borderInsets.top == 0 || (borderColors.top && CGColorGetAlpha(borderColors.top.CGColor) == 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.
|
||||
|
||||
CGColorRef backgroundColor;
|
||||
|
||||
backgroundColor = [_backgroundColor resolvedColorWithTraitCollection:self.traitCollection].CGColor;
|
||||
UIColor *backgroundColor = [_backgroundColor resolvedColorWithTraitCollection:self.traitCollection];
|
||||
|
||||
if (useIOSBorderRendering) {
|
||||
layer.cornerRadius = cornerRadii.topLeftHorizontal;
|
||||
layer.borderColor = borderColors.left;
|
||||
layer.borderColor = borderColors.left.CGColor;
|
||||
layer.borderWidth = borderInsets.left;
|
||||
layer.backgroundColor = backgroundColor;
|
||||
layer.backgroundColor = backgroundColor.CGColor;
|
||||
layer.contents = nil;
|
||||
layer.needsDisplayOnBoundsChange = NO;
|
||||
layer.mask = nil;
|
||||
|
||||
Reference in New Issue
Block a user