Skip to content

Commit

Permalink
Back out "fix(iOS): Replace uses of CGColorRef with UIColor to avoi…
Browse files Browse the repository at this point in the history
…d manual memory management" (facebook#46967)

Summary:
Pull Request resolved: facebook#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
  • Loading branch information
joevilches authored and facebook-github-bot committed Oct 10, 2024
1 parent f25abe5 commit b0e2cb3
Show file tree
Hide file tree
Showing 5 changed files with 73 additions and 49 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand All @@ -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) {
Expand All @@ -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;
Expand All @@ -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 {
Expand All @@ -928,6 +938,8 @@ - (void)invalidateLayer
borderColors,
RCTUIEdgeInsetsFromEdgeInsets(borderMetrics.borderWidths),
RCTBorderStyleFromBorderStyle(borderMetrics.borderStyles.left));

RCTReleaseRCTBorderColors(borderColors);
}

// outline
Expand All @@ -946,18 +958,21 @@ - (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,
RCTCreateOutlineCornerRadiiFromBorderRadii(borderMetrics.borderRadii, _props->outlineWidth),
RCTBorderColors{outlineColor, outlineColor, outlineColor, outlineColor},
UIEdgeInsets{_props->outlineWidth, _props->outlineWidth, _props->outlineWidth, _props->outlineWidth},
RCTBorderStyleFromOutlineStyle(_props->outlineStyle));

CGColorRelease(outlineColor);
}
}

Expand Down
6 changes: 6 additions & 0 deletions packages/react-native/React/Fabric/RCTConversions.h
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down
10 changes: 5 additions & 5 deletions packages/react-native/React/Views/RCTBorderDrawing.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;

/**
Expand Down Expand Up @@ -67,5 +67,5 @@ RCT_EXTERN UIImage *RCTGetBorderImage(
RCTCornerRadii cornerRadii,
UIEdgeInsets borderInsets,
RCTBorderColors borderColors,
UIColor *backgroundColor,
CGColorRef backgroundColor,
BOOL drawToEdge);
42 changes: 22 additions & 20 deletions packages/react-native/React/Views/RCTBorderDrawing.m
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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;
Expand All @@ -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);
Expand Down Expand Up @@ -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);
Expand All @@ -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);
Expand Down Expand Up @@ -319,7 +321,7 @@ static CGPathRef RCTPathCreateOuterOutline(BOOL drawToEdge, CGRect rect, RCTCorn
}
}

UIColor *currentColor = nil;
CGColorRef currentColor = NULL;

// RIGHT
if (borderInsets.right > 0) {
Expand All @@ -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;
}
Expand All @@ -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;
}
Expand All @@ -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);
}

Expand Down Expand Up @@ -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);
Expand All @@ -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);
}

Expand All @@ -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);
Expand All @@ -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) {
Expand Down
19 changes: 10 additions & 9 deletions packages/react-native/React/Views/RCTView.m
Original file line number Diff line number Diff line change
Expand Up @@ -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,
};
}

Expand Down Expand Up @@ -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;
Expand Down

0 comments on commit b0e2cb3

Please sign in to comment.