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

Fix tint color dead lock #1731 #1732

Merged
merged 3 commits into from
Nov 25, 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
66 changes: 35 additions & 31 deletions Source/ASImageNode.mm
Original file line number Diff line number Diff line change
Expand Up @@ -302,44 +302,48 @@ - (void)setPlaceholderColor:(UIColor *)placeholderColor

- (NSObject *)drawParametersForAsyncLayer:(_ASDisplayLayer *)layer
{
ASLockScopeSelf();

UIImage *drawImage = _image;
if (AS_AVAILABLE_IOS_TVOS(13, 10)) {
if (_imageNodeFlags.regenerateFromImageAsset && drawImage != nil) {
_imageNodeFlags.regenerateFromImageAsset = NO;
UITraitCollection *tc = [UITraitCollection traitCollectionWithUserInterfaceStyle:_primitiveTraitCollection.userInterfaceStyle];
UIImage *generatedImage = [drawImage.imageAsset imageWithTraitCollection:tc];
if ( generatedImage != nil ) {
drawImage = generatedImage;
ASImageNodeDrawParameters *drawParameters = [[ASImageNodeDrawParameters alloc] init];

{
ASLockScopeSelf();
UIImage *drawImage = _image;
if (AS_AVAILABLE_IOS_TVOS(13, 10)) {
if (_imageNodeFlags.regenerateFromImageAsset && drawImage != nil) {
_imageNodeFlags.regenerateFromImageAsset = NO;
UITraitCollection *tc = [UITraitCollection traitCollectionWithUserInterfaceStyle:_primitiveTraitCollection.userInterfaceStyle];
UIImage *generatedImage = [drawImage.imageAsset imageWithTraitCollection:tc];
if ( generatedImage != nil ) {
drawImage = generatedImage;
}
}
}
}

ASImageNodeDrawParameters *drawParameters = [[ASImageNodeDrawParameters alloc] init];
drawParameters->_image = drawImage;
drawParameters->_image = drawImage;
drawParameters->_contentsScale = _contentsScaleForDisplay;
drawParameters->_cropEnabled = _imageNodeFlags.cropEnabled;
drawParameters->_forceUpscaling = _imageNodeFlags.forceUpscaling;
drawParameters->_forcedSize = _forcedSize;
drawParameters->_cropRect = _cropRect;
drawParameters->_cropDisplayBounds = _cropDisplayBounds;
drawParameters->_imageModificationBlock = _imageModificationBlock;
drawParameters->_willDisplayNodeContentWithRenderingContext = _willDisplayNodeContentWithRenderingContext;
drawParameters->_didDisplayNodeContentWithRenderingContext = _didDisplayNodeContentWithRenderingContext;
drawParameters->_traitCollection = _primitiveTraitCollection;

// Hack for now to retain the weak entry that was created while this drawing happened
drawParameters->_didDrawBlock = ^(ASWeakMapEntry *entry){
ASLockScopeSelf();
_weakCacheEntry = entry;
};
}

// We need to unlock before we access the other accessor.
// Especially tintColor because it needs to walk up the view hierarchy
drawParameters->_bounds = [self threadSafeBounds];
drawParameters->_opaque = self.opaque;
drawParameters->_contentsScale = _contentsScaleForDisplay;
drawParameters->_backgroundColor = self.backgroundColor;
drawParameters->_tintColor = self.tintColor;
drawParameters->_contentMode = self.contentMode;
drawParameters->_cropEnabled = _imageNodeFlags.cropEnabled;
drawParameters->_forceUpscaling = _imageNodeFlags.forceUpscaling;
drawParameters->_forcedSize = _forcedSize;
drawParameters->_cropRect = _cropRect;
drawParameters->_cropDisplayBounds = _cropDisplayBounds;
drawParameters->_imageModificationBlock = _imageModificationBlock;
drawParameters->_willDisplayNodeContentWithRenderingContext = _willDisplayNodeContentWithRenderingContext;
drawParameters->_didDisplayNodeContentWithRenderingContext = _didDisplayNodeContentWithRenderingContext;
drawParameters->_traitCollection = _primitiveTraitCollection;


// Hack for now to retain the weak entry that was created while this drawing happened
drawParameters->_didDrawBlock = ^(ASWeakMapEntry *entry){
ASLockScopeSelf();
_weakCacheEntry = entry;
};
drawParameters->_tintColor = self.tintColor;

return drawParameters;
}
Expand Down
4 changes: 3 additions & 1 deletion Source/ASTextNode.mm
Original file line number Diff line number Diff line change
Expand Up @@ -546,9 +546,11 @@ - (NSArray *)exclusionPaths

- (NSObject *)drawParametersForAsyncLayer:(_ASDisplayLayer *)layer
{
/// have to access tintColor outside of the lock to prevent dead lock when accessing up the view hierarchy
UIColor *tintColor = self.tintColor;
ASLockScopeSelf();
if (_textColorFollowsTintColor) {
_cachedTintColor = self.tintColor;
_cachedTintColor = tintColor;
} else {
_cachedTintColor = nil;
}
Expand Down
37 changes: 24 additions & 13 deletions Source/ASTextNode2.mm
Original file line number Diff line number Diff line change
Expand Up @@ -517,23 +517,34 @@ - (void)prepareAttributedString:(NSMutableAttributedString *)attributedString is

- (NSObject *)drawParametersForAsyncLayer:(_ASDisplayLayer *)layer
{
ASLockScopeSelf();
[self _ensureTruncationText];

// Unlike layout, here we must copy the container since drawing is asynchronous.
ASTextContainer *copiedContainer = [_textContainer copy];
copiedContainer.size = self.bounds.size;
[copiedContainer makeImmutable];
NSMutableAttributedString *mutableText = [_attributedText mutableCopy] ?: [[NSMutableAttributedString alloc] init];

[self prepareAttributedString:mutableText isForIntrinsicSize:NO];

ASTextContainer *copiedContainer;
NSMutableAttributedString *mutableText;
BOOL needsTintColor;
id bgColor;
{
// Wrapping all the other access here, because we can't lock while accessing tintColor.
ASLockScopeSelf();
[self _ensureTruncationText];

// Unlike layout, here we must copy the container since drawing is asynchronous.
copiedContainer = [_textContainer copy];
copiedContainer.size = self.bounds.size;
[copiedContainer makeImmutable];
mutableText = [_attributedText mutableCopy] ?: [[NSMutableAttributedString alloc] init];

[self prepareAttributedString:mutableText isForIntrinsicSize:NO];
needsTintColor = self.textColorFollowsTintColor && mutableText.length > 0;
bgColor = self.backgroundColor ?: [NSNull null];
}

// After all other attributes are set, apply tint color if needed and foreground color is not already specified
if (self.textColorFollowsTintColor && mutableText.length > 0) {
if (needsTintColor) {
// Apply tint color if specified and if foreground color is undefined for attributedString
NSRange limit = NSMakeRange(0, mutableText.length);
// Look for previous attributes that define foreground color
UIColor *attributeValue = (UIColor *)[mutableText attribute:NSForegroundColorAttributeName atIndex:limit.location effectiveRange:NULL];

// we need to unlock before accessing tintColor
UIColor *tintColor = self.tintColor;
if (attributeValue == nil && tintColor) {
// None are found, apply tint color if available. Fallback to "black" text color
Expand All @@ -544,7 +555,7 @@ - (NSObject *)drawParametersForAsyncLayer:(_ASDisplayLayer *)layer
return @{
@"container": copiedContainer,
@"text": mutableText,
@"bgColor": self.backgroundColor ?: [NSNull null]
@"bgColor": bgColor
};
}

Expand Down