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.

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

#1673 - Re-render Clipping Corners when User Interface Style Changes #1674

Merged
merged 2 commits into from
Sep 16, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
27 changes: 20 additions & 7 deletions Source/ASDisplayNode.mm
Original file line number Diff line number Diff line change
Expand Up @@ -442,16 +442,29 @@ - (void)asyncTraitCollectionDidChangeWithPreviousTraitCollection:(ASPrimitiveTra
if (self.primitiveTraitCollection.userInterfaceStyle != previousTraitCollection.userInterfaceStyle) {
// When changing between light and dark mode, often the entire node needs to re-render.
// This change doesn't happen frequently so it's fairly safe to render nodes again
if (_loaded(self) && self.isLayerBacked) {
// Background colors do not dynamically update for layer backed nodes since they utilize CGColorRef
// instead of UIColor. We utilize the _backgroundColor instance variable to track the full dynamic color
// and apply any changes here when trait collection updates occur.
CGColorRef cgBackgroundColor = _backgroundColor.CGColor;
if (!CGColorEqualToColor(_layer.backgroundColor, cgBackgroundColor)) {
_layer.backgroundColor = cgBackgroundColor;
BOOL needsClippingCornerUpdate = NO;
CGFloat cornerRadius = _cornerRadius;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Capturing these values up here so they're behind the lock, but in scope when we call _updateClipCornerLayerContentsWithRadius on line 466

ASCornerRoundingType cornerRoundingType = _cornerRoundingType;
UIColor *backgroundColor = self.backgroundColor;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

any reason we're using self.backgroundColor vs _backgroundColor

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

self.backgroundColor has a getter that pulls from the layer or the view. _backgroundColor should only be used if layer backed.

if (_loaded(self)) {
if (self.isLayerBacked) {
// Background colors do not dynamically update for layer backed nodes since they utilize CGColorRef
// instead of UIColor. We utilize the _backgroundColor instance variable to track the full dynamic color
// and apply any changes here when trait collection updates occur.
CGColorRef cgBackgroundColor = backgroundColor.CGColor;
if (!CGColorEqualToColor(_layer.backgroundColor, cgBackgroundColor)) {
_layer.backgroundColor = cgBackgroundColor;
}
}
// If we have clipping corners, re-render the clipping corner layer upon user interface style change
if (cornerRoundingType == ASCornerRoundingTypeClipping && cornerRadius > 0.0f) {
needsClippingCornerUpdate = YES;
}
}
__instanceLock__.unlock();
if (needsClippingCornerUpdate) {
[self _updateClipCornerLayerContentsWithRadius:cornerRadius backgroundColor:backgroundColor];
}
[self setNeedsDisplay];
return;
}
Expand Down
31 changes: 31 additions & 0 deletions Tests/ASDisplayNodeSnapshotTests.mm
Original file line number Diff line number Diff line change
Expand Up @@ -61,18 +61,49 @@ - (void)testPrecompositedCornerRounding

- (void)testClippingCornerRounding
{
#if AS_AT_LEAST_IOS13
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking for feedback on this inline approach. Should I delete the reference images for older Xcode/iOS versions?

Copy link
Contributor

@bolsinga bolsinga Sep 16, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I personally think this is fine for now. I'm assuming you tested both with & without iOS13?

if (@available(iOS 13.0, *)) {
ASConfiguration *config = [ASConfiguration new];
config.experimentalFeatures = ASExperimentalTraitCollectionDidChangeWithPreviousCollection;
[ASConfigurationManager test_resetWithConfiguration:config];
}
#endif

for (CACornerMask c = 1; c <= kASCACornerAllCorners; c |= (c << 1)) {
auto node = [[ASImageNode alloc] init];
auto bounds = CGRectMake(0, 0, 100, 100);
node.image = BlueImageMake(bounds);
node.frame = bounds;
node.cornerRoundingType = ASCornerRoundingTypeClipping;
#if AS_AT_LEAST_IOS13
if (@available(iOS 13.0, *)) {
node.backgroundColor = UIColor.systemBackgroundColor;
} else {
node.backgroundColor = UIColor.greenColor;
}
#else
node.backgroundColor = UIColor.greenColor;
#endif
node.maskedCorners = c;
node.cornerRadius = 15;
// A layout pass is required, because that's where we lay out the clip layers.
[node.layer layoutIfNeeded];

#if AS_AT_LEAST_IOS13
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should probably write a macro for light/dark testing if we're doing this a lot

if (@available(iOS 13.0, *)) {
[[UITraitCollection traitCollectionWithUserInterfaceStyle:UIUserInterfaceStyleLight] performAsCurrentTraitCollection:^{
ASSnapshotVerifyNode(node, ([NSString stringWithFormat:@"%d-light", (int)c]));
}];

[[UITraitCollection traitCollectionWithUserInterfaceStyle:UIUserInterfaceStyleDark] performAsCurrentTraitCollection:^{
ASSnapshotVerifyNode(node, ([NSString stringWithFormat:@"%d-dark", (int)c]));
}];
} else {
ASSnapshotVerifyNode(node, ([NSString stringWithFormat:@"%d", (int)c]));
}
#else
ASSnapshotVerifyNode(node, ([NSString stringWithFormat:@"%d", (int)c]));
#endif
}
}

Expand Down
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.
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.