Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(iOS): Replace uses of CGColorRef with UIColor to avoid manual memory management #46847

Closed
wants to merge 4 commits into from
Closed

fix(iOS): Replace uses of CGColorRef with UIColor to avoid manual memory management #46847

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
18c646a
Avoid uses of CGColorRef in border drawing
Saadnajmi Oct 4, 2024
2e5ccd9
Remove RCTCreateCGColorRefFromSharedColor altogether
Saadnajmi Oct 4, 2024
809054e
Update RCTView.m
Saadnajmi Oct 5, 2024
625b5d5
Fix warnings in RCTView
Saadnajmi Oct 8, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 15 additions & 30 deletions packages/react-native/React/Fabric/Mounting/ComponentViews/View/RCTViewComponentView.mm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -242,9 +242,8 @@ - (void)updateProps:(const Props::Shared &)props oldProps:(const Props::Shared &

// `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;
}

Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -870,7 +861,7 @@ - (void)invalidateLayer
(*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
Expand All @@ -880,7 +871,7 @@ - (void)invalidateLayer
if (useCoreAnimationBorderRendering) {
[_backgroundColorLayer removeFromSuperlayer];
_backgroundColorLayer = nil;
layer.backgroundColor = backgroundColor;
layer.backgroundColor = backgroundColor.CGColor;
} else {
layer.backgroundColor = nil;
if (!_backgroundColorLayer) {
Expand All @@ -890,7 +881,7 @@ - (void)invalidateLayer
[self.layer addSublayer:_backgroundColorLayer];
}

_backgroundColorLayer.backgroundColor = backgroundColor;
_backgroundColorLayer.backgroundColor = backgroundColor.CGColor;
if (borderMetrics.borderRadii.isUniform()) {
_backgroundColorLayer.mask = nil;
_backgroundColorLayer.cornerRadius = borderMetrics.borderRadii.topLeft.horizontal;
Expand All @@ -911,9 +902,8 @@ - (void)invalidateLayer
_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 {
Expand All @@ -938,8 +928,6 @@ - (void)invalidateLayer
borderColors,
RCTUIEdgeInsetsFromEdgeInsets(borderMetrics.borderWidths),
RCTBorderStyleFromBorderStyle(borderMetrics.borderStyles.left));

RCTReleaseRCTBorderColors(borderColors);
}

// outline
Expand All @@ -958,21 +946,18 @@ - (void)invalidateLayer
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,
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: 0 additions & 6 deletions packages/react-native/React/Fabric/RCTConversions.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down
10 changes: 5 additions & 5 deletions packages/react-native/React/Views/RCTBorderDrawing.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;

/**
Expand Down Expand Up @@ -67,5 +67,5 @@ RCT_EXTERN UIImage *RCTGetBorderImage(
RCTCornerRadii cornerRadii,
UIEdgeInsets borderInsets,
RCTBorderColors borderColors,
CGColorRef backgroundColor,
UIColor *backgroundColor,
BOOL drawToEdge);
42 changes: 20 additions & 22 deletions packages/react-native/React/Views/RCTBorderDrawing.m
View file
Open in desktop
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, 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)
Expand Down Expand Up @@ -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;
Expand All @@ -197,7 +197,7 @@ static CGPathRef RCTPathCreateOuterOutline(BOOL drawToEdge, CGRect rect, RCTCorn
CGSize viewSize,
UIEdgeInsets borderInsets,
RCTBorderColors borderColors,
CGColorRef backgroundColor,
UIColor *backgroundColor,
BOOL drawToEdge)
{
const BOOL hasCornerRadii = RCTCornerRadiiAreAboveThreshold(cornerRadii);
Expand Down Expand Up @@ -233,18 +233,16 @@ 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);
CGContextSetFillColorWithColor(context, backgroundColor.CGColor);
CGContextAddPath(context, path);
CGContextFillPath(context);
}
CGColorRelease(backgroundColor);

CGContextAddPath(context, path);
CGPathRelease(path);
Expand All @@ -256,7 +254,7 @@ static CGPathRef RCTPathCreateOuterOutline(BOOL drawToEdge, CGRect rect, RCTCorn

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);
Expand Down Expand Up @@ -321,7 +319,7 @@ static CGPathRef RCTPathCreateOuterOutline(BOOL drawToEdge, CGRect rect, RCTCorn
}
}

CGColorRef currentColor = NULL;
UIColor *currentColor = nil;

// RIGHT
if (borderInsets.right > 0) {
Expand All @@ -345,8 +343,8 @@ static CGPathRef RCTPathCreateOuterOutline(BOOL drawToEdge, CGRect rect, RCTCorn
(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;
}
Expand All @@ -362,8 +360,8 @@ static CGPathRef RCTPathCreateOuterOutline(BOOL drawToEdge, CGRect rect, RCTCorn
(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;
}
Expand All @@ -379,15 +377,15 @@ static CGPathRef RCTPathCreateOuterOutline(BOOL drawToEdge, CGRect rect, RCTCorn
(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);
}

Expand Down Expand Up @@ -467,7 +465,7 @@ static CGPathRef RCTPathCreateOuterOutline(BOOL drawToEdge, CGRect rect, RCTCorn
CGSize viewSize,
UIEdgeInsets borderInsets,
RCTBorderColors borderColors,
CGColorRef backgroundColor,
UIColor *backgroundColor,
BOOL drawToEdge)
{
NSCParameterAssert(borderStyle == RCTBorderStyleDashed || borderStyle == RCTBorderStyleDotted);
Expand All @@ -494,7 +492,7 @@ static CGPathRef RCTPathCreateOuterOutline(BOOL drawToEdge, CGRect rect, RCTCorn
CGContextAddPath(context, outerPath);
CGPathRelease(outerPath);

CGContextSetFillColorWithColor(context, backgroundColor);
CGContextSetFillColorWithColor(context, backgroundColor.CGColor);
CGContextFillPath(context);
}

Expand All @@ -513,7 +511,7 @@ static CGPathRef RCTPathCreateOuterOutline(BOOL drawToEdge, CGRect rect, RCTCorn
CGContextSetStrokeColorWithColor(context, [UIColor yellowColor].CGColor);

CGContextAddPath(context, path);
CGContextSetStrokeColorWithColor(context, borderColors.top);
CGContextSetStrokeColorWithColor(context, borderColors.top.CGColor);
CGContextStrokePath(context);

CGPathRelease(path);
Expand All @@ -526,7 +524,7 @@ static CGPathRef RCTPathCreateOuterOutline(BOOL drawToEdge, CGRect rect, RCTCorn
RCTCornerRadii cornerRadii,
UIEdgeInsets borderInsets,
RCTBorderColors borderColors,
CGColorRef backgroundColor,
UIColor *backgroundColor,
BOOL drawToEdge)
{
switch (borderStyle) {
Expand Down
32 changes: 17 additions & 15 deletions packages/react-native/React/Views/RCTView.m
View file
Open in desktop
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).CGColor,
(directionAwareBorderLeftColor ?: borderColor).CGColor,
(borderBottomColor ?: borderColor).CGColor,
(directionAwareBorderRightColor ?: borderColor).CGColor,
(borderTopColor ?: borderColor),
(directionAwareBorderLeftColor ?: borderColor),
(borderBottomColor ?: borderColor),
(directionAwareBorderRightColor ?: borderColor),
};
}

Expand Down Expand Up @@ -807,28 +807,30 @@ - (void)displayLayer:(CALayer *)layer
const UIEdgeInsets borderInsets = [self bordersAsInsets];
const RCTBorderColors borderColors = [self borderColorsWithTraitCollection:self.traitCollection];

BOOL useIOSBorderRendering = RCTCornerRadiiAreEqualAndSymmetrical(cornerRadii) &&
RCTBorderInsetsAreEqual(borderInsets) && RCTBorderColorsAreEqual(borderColors) &&
BOOL useIOSBorderRendering =
RCTCornerRadiiAreEqualAndSymmetrical(cornerRadii) &&
RCTBorderInsetsAreEqual(borderInsets) &&
RCTBorderColorsAreEqual(borderColors) &&

// iOS draws borders in front of the content whereas CSS draws them behind
// the content. For this reason, only use iOS border drawing when clipping
// or when the border is hidden.
// iOS draws borders in front of the content whereas CSS draws them behind
// 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;
Expand Down
Loading