Skip to content

Commit

Permalink
Fixed border smearing issue
Browse files Browse the repository at this point in the history
Summary:
public
The iOS border rendering code did not follow the CSS spec in cases where the sum of adjacent border radii was greater than the width of the view, resulting in drawing glitches such as pixel smear and borders appearing stretched or squashed.

This diff brings our implementation closer to spec-compliance in these cases. I also fixed a longstanding issue with ghostly diagonal lines appearing at the corners due to antialiasing rounding errors!

Fixes

#1572
#2089
#4604

Reviewed By: tadeuzagallo

Differential Revision: D2811249

fb-gh-sync-id: c3dd2721e0a01a432fa4dc78daa05680595edd08
  • Loading branch information
nicklockwood authored and facebook-github-bot-3 committed Jan 7, 2016
1 parent abb81eb commit b115277
Show file tree
Hide file tree
Showing 6 changed files with 78 additions and 23 deletions.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
72 changes: 56 additions & 16 deletions React/Views/RCTBorderDrawing.m
Original file line number Diff line number Diff line change
Expand Up @@ -96,10 +96,22 @@ CGPathRef RCTPathCreateWithRoundedRect(CGRect bounds,
const CGFloat maxX = CGRectGetMaxX(bounds);
const CGFloat maxY = CGRectGetMaxY(bounds);

const CGSize topLeft = cornerInsets.topLeft;
const CGSize topRight = cornerInsets.topRight;
const CGSize bottomLeft = cornerInsets.bottomLeft;
const CGSize bottomRight = cornerInsets.bottomRight;
const CGSize topLeft = {
MAX(0, MIN(cornerInsets.topLeft.width, bounds.size.width - cornerInsets.topRight.width)),
MAX(0, MIN(cornerInsets.topLeft.height, bounds.size.height - cornerInsets.bottomLeft.height)),
};
const CGSize topRight = {
MAX(0, MIN(cornerInsets.topRight.width, bounds.size.width - cornerInsets.topLeft.width)),
MAX(0, MIN(cornerInsets.topRight.height, bounds.size.height - cornerInsets.bottomRight.height)),
};
const CGSize bottomLeft = {
MAX(0, MIN(cornerInsets.bottomLeft.width, bounds.size.width - cornerInsets.bottomRight.width)),
MAX(0, MIN(cornerInsets.bottomLeft.height, bounds.size.height - cornerInsets.topLeft.height)),
};
const CGSize bottomRight = {
MAX(0, MIN(cornerInsets.bottomRight.width, bounds.size.width - cornerInsets.bottomLeft.width)),
MAX(0, MIN(cornerInsets.bottomRight.height, bounds.size.height - cornerInsets.topRight.height)),
};

CGMutablePathRef path = CGPathCreateMutable();
RCTPathAddEllipticArc(path, transform, (CGPoint){
Expand Down Expand Up @@ -174,6 +186,7 @@ static CGContextRef RCTUIGraphicsBeginImageContext(CGSize size, CGColorRef backg
}

static UIImage *RCTGetSolidBorderImage(RCTCornerRadii cornerRadii,
CGSize viewSize,
UIEdgeInsets borderInsets,
RCTBorderColors borderColors,
CGColorRef backgroundColor,
Expand All @@ -182,18 +195,28 @@ static CGContextRef RCTUIGraphicsBeginImageContext(CGSize size, CGColorRef backg
const BOOL hasCornerRadii = RCTCornerRadiiAreAboveThreshold(cornerRadii);
const RCTCornerInsets cornerInsets = RCTGetCornerInsets(cornerRadii, borderInsets);

const BOOL makeStretchable =
(borderInsets.left + cornerInsets.topLeft.width +
borderInsets.right + cornerInsets.bottomRight.width <= viewSize.width) &&
(borderInsets.left + cornerInsets.bottomLeft.width +
borderInsets.right + cornerInsets.topRight.width <= viewSize.width) &&
(borderInsets.top + cornerInsets.topLeft.height +
borderInsets.bottom + cornerInsets.bottomRight.height <= viewSize.height) &&
(borderInsets.top + cornerInsets.topRight.height +
borderInsets.bottom + cornerInsets.bottomLeft.height <= viewSize.height);

const UIEdgeInsets edgeInsets = (UIEdgeInsets){
borderInsets.top + MAX(cornerInsets.topLeft.height, cornerInsets.topRight.height),
borderInsets.left + MAX(cornerInsets.topLeft.width, cornerInsets.bottomLeft.width),
borderInsets.bottom + MAX(cornerInsets.bottomLeft.height, cornerInsets.bottomRight.height),
borderInsets.right + MAX(cornerInsets.bottomRight.width, cornerInsets.topRight.width)
};

const CGSize size = (CGSize){
const CGSize size = makeStretchable ? (CGSize){
// 1pt for the middle stretchable area along each axis
edgeInsets.left + 1 + edgeInsets.right,
edgeInsets.top + 1 + edgeInsets.bottom
};
} : viewSize;

CGContextRef ctx = RCTUIGraphicsBeginImageContext(size, backgroundColor, hasCornerRadii, drawToEdge);
const CGRect rect = {.size = size};
Expand Down Expand Up @@ -270,6 +293,8 @@ static CGContextRef RCTUIGraphicsBeginImageContext(CGSize size, CGColorRef backg
}
}

CGColorRef currentColor = NULL;

// RIGHT
if (borderInsets.right > 0) {

Expand All @@ -280,9 +305,8 @@ static CGContextRef RCTUIGraphicsBeginImageContext(CGSize size, CGColorRef backg
(CGPoint){size.width, size.height},
};

CGContextSetFillColorWithColor(ctx, borderColors.right);
currentColor = borderColors.right;
CGContextAddLines(ctx, points, sizeof(points)/sizeof(*points));
CGContextFillPath(ctx);
}

// BOTTOM
Expand All @@ -295,9 +319,12 @@ static CGContextRef RCTUIGraphicsBeginImageContext(CGSize size, CGColorRef backg
(CGPoint){size.width, size.height},
};

CGContextSetFillColorWithColor(ctx, borderColors.bottom);
if (!CGColorEqualToColor(currentColor, borderColors.bottom)) {
CGContextSetFillColorWithColor(ctx, currentColor);
CGContextFillPath(ctx);
currentColor = borderColors.bottom;
}
CGContextAddLines(ctx, points, sizeof(points)/sizeof(*points));
CGContextFillPath(ctx);
}

// LEFT
Expand All @@ -310,9 +337,12 @@ static CGContextRef RCTUIGraphicsBeginImageContext(CGSize size, CGColorRef backg
(CGPoint){0, size.height},
};

CGContextSetFillColorWithColor(ctx, borderColors.left);
if (!CGColorEqualToColor(currentColor, borderColors.left)) {
CGContextSetFillColorWithColor(ctx, currentColor);
CGContextFillPath(ctx);
currentColor = borderColors.left;
}
CGContextAddLines(ctx, points, sizeof(points)/sizeof(*points));
CGContextFillPath(ctx);
}

// TOP
Expand All @@ -325,18 +355,28 @@ static CGContextRef RCTUIGraphicsBeginImageContext(CGSize size, CGColorRef backg
(CGPoint){size.width, 0},
};

CGContextSetFillColorWithColor(ctx, borderColors.top);
if (!CGColorEqualToColor(currentColor, borderColors.top)) {
CGContextSetFillColorWithColor(ctx, currentColor);
CGContextFillPath(ctx);
currentColor = borderColors.top;
}
CGContextAddLines(ctx, points, sizeof(points)/sizeof(*points));
CGContextFillPath(ctx);
}

CGContextSetFillColorWithColor(ctx, currentColor);
CGContextFillPath(ctx);
}

CGPathRelease(insetPath);

UIImage *image = UIGraphicsGetImageFromCurrentImageContext();
UIGraphicsEndImageContext();

return [image resizableImageWithCapInsets:edgeInsets];
if (makeStretchable) {
image = [image resizableImageWithCapInsets:edgeInsets];
}

return image;
}

// Currently, the dashed / dotted implementation only supports a single colour +
Expand Down Expand Up @@ -469,7 +509,7 @@ static CGContextRef RCTUIGraphicsBeginImageContext(CGSize size, CGColorRef backg

switch (borderStyle) {
case RCTBorderStyleSolid:
return RCTGetSolidBorderImage(cornerRadii, borderInsets, borderColors, backgroundColor, drawToEdge);
return RCTGetSolidBorderImage(cornerRadii, viewSize, borderInsets, borderColors, backgroundColor, drawToEdge);
case RCTBorderStyleDashed:
case RCTBorderStyleDotted:
return RCTGetDashedOrDottedBorderImage(borderStyle, cornerRadii, viewSize, borderInsets, borderColors, backgroundColor, drawToEdge);
Expand Down
29 changes: 22 additions & 7 deletions React/Views/RCTView.m
Original file line number Diff line number Diff line change
Expand Up @@ -477,15 +477,26 @@ - (UIEdgeInsets)bordersAsInsets

- (RCTCornerRadii)cornerRadii
{
const CGRect bounds = self.bounds;
const CGFloat maxRadius = MIN(bounds.size.height, bounds.size.width);
// Get corner radii
const CGFloat radius = MAX(0, _borderRadius);

const CGFloat topLeftRadius = _borderTopLeftRadius >= 0 ? _borderTopLeftRadius : radius;
const CGFloat topRightRadius = _borderTopRightRadius >= 0 ? _borderTopRightRadius : radius;
const CGFloat bottomLeftRadius = _borderBottomLeftRadius >= 0 ? _borderBottomLeftRadius : radius;
const CGFloat bottomRightRadius = _borderBottomRightRadius >= 0 ? _borderBottomRightRadius : radius;

// Get scale factors required to prevent radii from overlapping
const CGSize size = self.bounds.size;
const CGFloat topScaleFactor = MIN(1, size.width / (topLeftRadius + topRightRadius));
const CGFloat bottomScaleFactor = MIN(1, size.width / (bottomLeftRadius + bottomRightRadius));
const CGFloat rightScaleFactor = MIN(1, size.height / (topRightRadius + bottomRightRadius));
const CGFloat leftScaleFactor = MIN(1, size.height / (topLeftRadius + bottomLeftRadius));

// Return scaled radii
return (RCTCornerRadii){
MIN(_borderTopLeftRadius >= 0 ? _borderTopLeftRadius : radius, maxRadius),
MIN(_borderTopRightRadius >= 0 ? _borderTopRightRadius : radius, maxRadius),
MIN(_borderBottomLeftRadius >= 0 ? _borderBottomLeftRadius : radius, maxRadius),
MIN(_borderBottomRightRadius >= 0 ? _borderBottomRightRadius : radius, maxRadius),
topLeftRadius * MIN(topScaleFactor, leftScaleFactor),
topRightRadius * MIN(topScaleFactor, rightScaleFactor),
bottomLeftRadius * MIN(bottomScaleFactor, leftScaleFactor),
bottomRightRadius * MIN(bottomScaleFactor, rightScaleFactor),
};
}

Expand All @@ -501,6 +512,10 @@ - (RCTBorderColors)borderColors

- (void)displayLayer:(CALayer *)layer
{
if (CGSizeEqualToSize(layer.bounds.size, CGSizeZero)) {
return;
}

const RCTCornerRadii cornerRadii = [self cornerRadii];
const UIEdgeInsets borderInsets = [self bordersAsInsets];
const RCTBorderColors borderColors = [self borderColors];
Expand Down

0 comments on commit b115277

Please sign in to comment.