From 3384297c589695dc043142b26d2642e9ab85514c Mon Sep 17 00:00:00 2001 From: Michael Schneider Date: Sun, 19 Jun 2016 14:59:27 -0700 Subject: [PATCH 1/5] Move drawing parameters in ASTextNode and ASImageNode to structs --- AsyncDisplayKit/ASImageNode.mm | 140 ++++++++++++++++----------------- AsyncDisplayKit/ASTextNode.mm | 58 ++++++-------- 2 files changed, 95 insertions(+), 103 deletions(-) diff --git a/AsyncDisplayKit/ASImageNode.mm b/AsyncDisplayKit/ASImageNode.mm index 0678a528..8609bf48 100644 --- a/AsyncDisplayKit/ASImageNode.mm +++ b/AsyncDisplayKit/ASImageNode.mm @@ -26,46 +26,18 @@ #import "ASInternalHelpers.h" #import "ASEqualityHelpers.h" -@interface _ASImageNodeDrawParameters : NSObject - -@property (nonatomic, retain) UIImage *image; -@property (nonatomic, assign) BOOL opaque; -@property (nonatomic, assign) CGRect bounds; -@property (nonatomic, assign) CGFloat contentsScale; -@property (nonatomic, strong) UIColor *backgroundColor; -@property (nonatomic, assign) UIViewContentMode contentMode; - -@end - -// TODO: eliminate explicit parameters with a set of keys copied from the node -@implementation _ASImageNodeDrawParameters - -- (instancetype)initWithImage:(UIImage *)image - bounds:(CGRect)bounds - opaque:(BOOL)opaque - contentsScale:(CGFloat)contentsScale - backgroundColor:(UIColor *)backgroundColor - contentMode:(UIViewContentMode)contentMode -{ - if (!(self = [self init])) - return nil; - - _image = image; - _opaque = opaque; - _bounds = bounds; - _contentsScale = contentsScale; - _backgroundColor = backgroundColor; - _contentMode = contentMode; - - return self; -} - -- (NSString *)description -{ - return [NSString stringWithFormat:@"<%@ : %p opaque:%@ bounds:%@ contentsScale:%.2f backgroundColor:%@ contentMode:%@>", [self class], self, @(self.opaque), NSStringFromCGRect(self.bounds), self.contentsScale, self.backgroundColor, ASDisplayNodeNSStringFromUIContentMode(self.contentMode)]; -} - -@end +struct ASImageNodeDrawParameters { + BOOL opaque; + CGRect bounds; + CGFloat contentsScale; + UIColor *backgroundColor; + UIViewContentMode contentMode; + BOOL cropEnabled; + BOOL forceUpscaling; + CGRect cropRect; + CGRect cropDisplayBounds; + asimagenode_modification_block_t imageModificationBlock; +}; @implementation ASImageNode { @@ -75,18 +47,22 @@ void (^_displayCompletionBlock)(BOOL canceled); ASDN::RecursiveMutex _imageLock; + // Drawing + ASImageNodeDrawParameters _drawParameter; + ASTextNode *_debugLabelNode; + // Cropping. BOOL _cropEnabled; // Defaults to YES. BOOL _forceUpscaling; //Defaults to NO. CGRect _cropRect; // Defaults to CGRectMake(0.5, 0.5, 0, 0) - CGRect _cropDisplayBounds; - - ASTextNode *_debugLabelNode; + CGRect _cropDisplayBounds; // Defaults to CGRectNull } @synthesize image = _image; @synthesize imageModificationBlock = _imageModificationBlock; +#pragma mark - NSObject + - (instancetype)init { if (!(self = [super init])) @@ -124,6 +100,8 @@ return nil; } +#pragma mark - Layout and Sizing + - (CGSize)calculateSizeThatFits:(CGSize)constrainedSize { ASDN::MutexLocker l(_imageLock); @@ -136,6 +114,8 @@ return CGSizeZero; } +#pragma mark - Setter / Getter + - (void)setImage:(UIImage *)image { _imageLock.lock(); @@ -177,54 +157,72 @@ self.placeholderEnabled = placeholderColor != nil; } +#pragma mark - Drawing + - (NSObject *)drawParametersForAsyncLayer:(_ASDisplayLayer *)layer { - return [[_ASImageNodeDrawParameters alloc] initWithImage:self.image - bounds:self.bounds - opaque:self.opaque - contentsScale:self.contentsScaleForDisplay - backgroundColor:self.backgroundColor - contentMode:self.contentMode]; + ASDN::MutexLocker l(_imageLock); + + _drawParameter = { + .bounds = self.bounds, + .opaque = self.opaque, + .contentsScale = _contentsScaleForDisplay, + .backgroundColor = self.backgroundColor, + .contentMode = self.contentMode, + .cropEnabled = _cropEnabled, + .forceUpscaling = _forceUpscaling, + .cropRect = _cropRect, + .cropDisplayBounds = _cropDisplayBounds, + .imageModificationBlock = _imageModificationBlock + }; + + return nil; } - (NSDictionary *)debugLabelAttributes { - return @{ NSFontAttributeName: [UIFont systemFontOfSize:15.0], - NSForegroundColorAttributeName: [UIColor redColor] }; + return @{ + NSFontAttributeName: [UIFont systemFontOfSize:15.0], + NSForegroundColorAttributeName: [UIColor redColor] + }; } -- (UIImage *)displayWithParameters:(_ASImageNodeDrawParameters *)parameters isCancelled:(asdisplaynode_iscancelled_block_t)isCancelled +- (UIImage *)displayWithParameters:(id *)parameter isCancelled:(asdisplaynode_iscancelled_block_t)isCancelled { - UIImage *image = parameters.image; - if (!image) { + UIImage *image = self.image; + if (image == nil) { return nil; } + CGRect drawParameterBounds = CGRectZero; BOOL forceUpscaling = NO; - BOOL cropEnabled = NO; - BOOL isOpaque = parameters.opaque; - UIColor *backgroundColor = parameters.backgroundColor; - UIViewContentMode contentMode = parameters.contentMode; + BOOL cropEnabled = YES; + BOOL isOpaque = NO; + UIColor *backgroundColor = nil; + UIViewContentMode contentMode = UIViewContentModeScaleAspectFill; CGFloat contentsScale = 0.0; CGRect cropDisplayBounds = CGRectZero; CGRect cropRect = CGRectZero; asimagenode_modification_block_t imageModificationBlock; - + + ASDN::MutexLocker l(_imageLock); { - ASDN::MutexLocker l(_imageLock); + ASImageNodeDrawParameters drawParameter = _drawParameter; - // FIXME: There is a small risk of these values changing between the main thread creation of drawParameters, and the execution of this method. - // We should package these up into the draw parameters object. Might be easiest to create a struct for the non-objects and make it one property. - cropEnabled = _cropEnabled; - forceUpscaling = _forceUpscaling; - contentsScale = _contentsScaleForDisplay; - cropDisplayBounds = _cropDisplayBounds; - cropRect = _cropRect; - imageModificationBlock = _imageModificationBlock; + drawParameterBounds = drawParameter.bounds; + forceUpscaling = drawParameter.forceUpscaling; + cropEnabled = drawParameter.cropEnabled; + isOpaque = drawParameter.opaque; + backgroundColor = drawParameter.backgroundColor; + contentMode = drawParameter.contentMode; + contentsScale = drawParameter.contentsScale; + cropDisplayBounds = drawParameter.cropDisplayBounds; + cropRect = drawParameter.cropRect; + imageModificationBlock = drawParameter.imageModificationBlock; } BOOL hasValidCropBounds = cropEnabled && !CGRectIsNull(cropDisplayBounds) && !CGRectIsEmpty(cropDisplayBounds); - CGRect bounds = (hasValidCropBounds ? cropDisplayBounds : parameters.bounds); + CGRect bounds = (hasValidCropBounds ? cropDisplayBounds : drawParameterBounds); ASDisplayNodeContextModifier preContextBlock = self.willDisplayNodeContentWithRenderingContext; ASDisplayNodeContextModifier postContextBlock = self.didDisplayNodeContentWithRenderingContext; @@ -359,7 +357,6 @@ } } -#pragma mark - - (void)setNeedsDisplayWithCompletion:(void (^ _Nullable)(BOOL canceled))displayCompletionBlock { if (self.displaySuspended) { @@ -378,6 +375,7 @@ } #pragma mark - Cropping + - (BOOL)isCropEnabled { ASDN::MutexLocker l(_imageLock); @@ -462,6 +460,7 @@ } #pragma mark - Debug + - (void)layout { [super layout]; @@ -477,6 +476,7 @@ @end #pragma mark - Extras + extern asimagenode_modification_block_t ASImageNodeRoundBorderModificationBlock(CGFloat borderWidth, UIColor *borderColor) { return ^(UIImage *originalImage) { diff --git a/AsyncDisplayKit/ASTextNode.mm b/AsyncDisplayKit/ASTextNode.mm index aba0e7e8..9fd9fafb 100644 --- a/AsyncDisplayKit/ASTextNode.mm +++ b/AsyncDisplayKit/ASTextNode.mm @@ -32,25 +32,10 @@ static const CGFloat ASTextNodeHighlightLightOpacity = 0.11; static const CGFloat ASTextNodeHighlightDarkOpacity = 0.22; static NSString *ASTextNodeTruncationTokenAttributeName = @"ASTextNodeTruncationAttribute"; -@interface ASTextNodeDrawParameters : NSObject - -@property (nonatomic, assign, readonly) CGRect bounds; -@property (nonatomic, strong, readonly) UIColor *backgroundColor; - -@end - -@implementation ASTextNodeDrawParameters - -- (instancetype)initWithBounds:(CGRect)bounds backgroundColor:(UIColor *)backgroundColor -{ - if (self = [super init]) { - _bounds = bounds; - _backgroundColor = backgroundColor; - } - return self; -} - -@end +struct ASTextNodeDrawParameter { + CGRect bounds; + UIColor *backgroundColor; +}; @interface ASTextNode () @@ -78,6 +63,8 @@ static NSString *ASTextNodeTruncationTokenAttributeName = @"ASTextNodeTruncation ASTextKitRenderer *_renderer; + ASTextNodeDrawParameter _drawParameter; + UILongPressGestureRecognizer *_longPressGestureRecognizer; } @dynamic placeholderEnabled; @@ -431,27 +418,37 @@ static NSArray *DefaultLinkAttributeNames = @[ NSLinkAttributeName ]; #pragma mark - Drawing -- (void)drawRect:(CGRect)bounds withParameters:(ASTextNodeDrawParameters *)parameters isCancelled:(asdisplaynode_iscancelled_block_t)isCancelledBlock isRasterizing:(BOOL)isRasterizing +- (NSObject *)drawParametersForAsyncLayer:(_ASDisplayLayer *)layer +{ + _drawParameter = { + .backgroundColor = self.backgroundColor, + .bounds = self.bounds + }; + return nil; +} + +- (void)drawRect:(CGRect)bounds withParameters:(id )p isCancelled:(asdisplaynode_iscancelled_block_t)isCancelledBlock isRasterizing:(BOOL)isRasterizing; { std::lock_guard l(_textLock); + + ASTextNodeDrawParameter drawParameter = _drawParameter; + CGRect drawParameterBounds = drawParameter.bounds; + UIColor *backgroundColor = isRasterizing ? nil : drawParameter.backgroundColor; CGContextRef context = UIGraphicsGetCurrentContext(); ASDisplayNodeAssert(context, @"This is no good without a context."); CGContextSaveGState(context); - ASTextKitRenderer *renderer = [self _rendererWithBounds:parameters.bounds]; + ASTextKitRenderer *renderer = [self _rendererWithBounds:drawParameterBounds]; UIEdgeInsets shadowPadding = [self shadowPaddingWithRenderer:renderer]; - CGPoint boundsOrigin = parameters.bounds.origin; + CGPoint boundsOrigin = drawParameterBounds.origin; CGPoint textOrigin = CGPointMake(boundsOrigin.x - shadowPadding.left, boundsOrigin.y - shadowPadding.top); // Fill background - if (!isRasterizing) { - UIColor *backgroundColor = parameters.backgroundColor; - if (backgroundColor) { - [backgroundColor setFill]; - UIRectFillUsingBlendMode(CGContextGetClipBoundingBox(context), kCGBlendModeCopy); - } + if (backgroundColor != nil) { + [backgroundColor setFill]; + UIRectFillUsingBlendMode(CGContextGetClipBoundingBox(context), kCGBlendModeCopy); } // Draw shadow @@ -464,11 +461,6 @@ static NSArray *DefaultLinkAttributeNames = @[ NSLinkAttributeName ]; CGContextRestoreGState(context); } -- (NSObject *)drawParametersForAsyncLayer:(_ASDisplayLayer *)layer -{ - return [[ASTextNodeDrawParameters alloc] initWithBounds:self.threadSafeBounds backgroundColor:self.backgroundColor]; -} - #pragma mark - Attributes - (id)linkAttributeValueAtPoint:(CGPoint)point From fc7cff333ece1ff4ed19ed158dcd66db1d5c609a Mon Sep 17 00:00:00 2001 From: Michael Schneider Date: Sun, 19 Jun 2016 14:59:39 -0700 Subject: [PATCH 2/5] Prevent subclassing of ASTextNode and ASImageNode --- AsyncDisplayKit/ASImageNode.mm | 10 ++++++++++ AsyncDisplayKit/ASTextNode.mm | 10 ++++++++++ 2 files changed, 20 insertions(+) diff --git a/AsyncDisplayKit/ASImageNode.mm b/AsyncDisplayKit/ASImageNode.mm index 8609bf48..024bf137 100644 --- a/AsyncDisplayKit/ASImageNode.mm +++ b/AsyncDisplayKit/ASImageNode.mm @@ -63,6 +63,16 @@ struct ASImageNodeDrawParameters { #pragma mark - NSObject ++ (void)initialize +{ + [super initialize]; + + if (self != [ASImageNode class]) { + // Prevent custom drawing in subclasses + ASDisplayNodeAssert(!ASSubclassOverridesClassSelector([ASImageNode class], self, @selector(displayWithParameters:isCancelled:)), @"Subclass %@ must not override displayWithParameters:isCancelled: method. Custom drawing in %@ subclass is not supported.", NSStringFromClass(self), NSStringFromClass([ASImageNode class])); + } +} + - (instancetype)init { if (!(self = [super init])) diff --git a/AsyncDisplayKit/ASTextNode.mm b/AsyncDisplayKit/ASTextNode.mm index 9fd9fafb..688da55c 100644 --- a/AsyncDisplayKit/ASTextNode.mm +++ b/AsyncDisplayKit/ASTextNode.mm @@ -71,6 +71,16 @@ struct ASTextNodeDrawParameter { #pragma mark - NSObject ++ (void)initialize +{ + [super initialize]; + + if (self != [ASTextNode class]) { + // Prevent custom drawing in subclasses + ASDisplayNodeAssert(!ASSubclassOverridesClassSelector([ASTextNode class], self, @selector(drawRect:withParameters:isCancelled:isRasterizing:)), @"Subclass %@ must not override drawRect:withParameters:isCancelled:isRasterizing: method. Custom drawing in %@ subclass is not supported.", NSStringFromClass(self), NSStringFromClass([ASTextNode class])); + } +} + static NSArray *DefaultLinkAttributeNames = @[ NSLinkAttributeName ]; - (instancetype)init From e7f2edd183c57021beb335a25492e270cdb70ad7 Mon Sep 17 00:00:00 2001 From: Michael Schneider Date: Mon, 20 Jun 2016 15:03:16 -0700 Subject: [PATCH 3/5] Add updating drawing parameter in ASTextNode Update drawing on demand if properties change and not on every drawing cycle. This should reduce the overhead to access properties from the view / layer for the drawing parameters. --- AsyncDisplayKit/ASTextNode.mm | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/AsyncDisplayKit/ASTextNode.mm b/AsyncDisplayKit/ASTextNode.mm index 688da55c..2efe1f5e 100644 --- a/AsyncDisplayKit/ASTextNode.mm +++ b/AsyncDisplayKit/ASTextNode.mm @@ -210,9 +210,16 @@ static NSArray *DefaultLinkAttributeNames = @[ NSLinkAttributeName ]; - (void)setBounds:(CGRect)bounds { [super setBounds:bounds]; + [self updateDrawingParameter]; [self _invalidateRendererIfNeededForBoundsSize:bounds.size]; } +- (void)setBackgroundColor:(UIColor *)backgroundColor +{ + [super setBackgroundColor:backgroundColor]; + [self updateDrawingParameter]; +} + #pragma mark - Renderer Management - (ASTextKitRenderer *)_renderer @@ -428,13 +435,14 @@ static NSArray *DefaultLinkAttributeNames = @[ NSLinkAttributeName ]; #pragma mark - Drawing -- (NSObject *)drawParametersForAsyncLayer:(_ASDisplayLayer *)layer +- (void)updateDrawingParameter { + std::lock_guard l(_textLock); + _drawParameter = { .backgroundColor = self.backgroundColor, .bounds = self.bounds }; - return nil; } - (void)drawRect:(CGRect)bounds withParameters:(id )p isCancelled:(asdisplaynode_iscancelled_block_t)isCancelledBlock isRasterizing:(BOOL)isRasterizing; From 74b9b6b49e2841855774a2977126162e04bf73aa Mon Sep 17 00:00:00 2001 From: Michael Schneider Date: Mon, 20 Jun 2016 15:21:52 -0700 Subject: [PATCH 4/5] Use threadSafeBounds instead of bounds to create drawing parameters --- AsyncDisplayKit/ASImageNode.mm | 2 +- AsyncDisplayKit/ASTextNode.mm | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/AsyncDisplayKit/ASImageNode.mm b/AsyncDisplayKit/ASImageNode.mm index 024bf137..b669f533 100644 --- a/AsyncDisplayKit/ASImageNode.mm +++ b/AsyncDisplayKit/ASImageNode.mm @@ -174,7 +174,7 @@ struct ASImageNodeDrawParameters { ASDN::MutexLocker l(_imageLock); _drawParameter = { - .bounds = self.bounds, + .bounds = self.threadSafeBounds, .opaque = self.opaque, .contentsScale = _contentsScaleForDisplay, .backgroundColor = self.backgroundColor, diff --git a/AsyncDisplayKit/ASTextNode.mm b/AsyncDisplayKit/ASTextNode.mm index 2efe1f5e..466a96ef 100644 --- a/AsyncDisplayKit/ASTextNode.mm +++ b/AsyncDisplayKit/ASTextNode.mm @@ -441,7 +441,7 @@ static NSArray *DefaultLinkAttributeNames = @[ NSLinkAttributeName ]; _drawParameter = { .backgroundColor = self.backgroundColor, - .bounds = self.bounds + .bounds = self.threadSafeBounds }; } From 36e48cf340c2e667a7450e50ed80aefc41206a86 Mon Sep 17 00:00:00 2001 From: Michael Schneider Date: Mon, 20 Jun 2016 16:41:42 -0700 Subject: [PATCH 5/5] Remove caching of _drawParameter and use bounds instead of threadSafeBounds --- AsyncDisplayKit/ASImageNode.mm | 2 +- AsyncDisplayKit/ASTextNode.mm | 13 ++++--------- 2 files changed, 5 insertions(+), 10 deletions(-) diff --git a/AsyncDisplayKit/ASImageNode.mm b/AsyncDisplayKit/ASImageNode.mm index b669f533..024bf137 100644 --- a/AsyncDisplayKit/ASImageNode.mm +++ b/AsyncDisplayKit/ASImageNode.mm @@ -174,7 +174,7 @@ struct ASImageNodeDrawParameters { ASDN::MutexLocker l(_imageLock); _drawParameter = { - .bounds = self.threadSafeBounds, + .bounds = self.bounds, .opaque = self.opaque, .contentsScale = _contentsScaleForDisplay, .backgroundColor = self.backgroundColor, diff --git a/AsyncDisplayKit/ASTextNode.mm b/AsyncDisplayKit/ASTextNode.mm index 466a96ef..95d44145 100644 --- a/AsyncDisplayKit/ASTextNode.mm +++ b/AsyncDisplayKit/ASTextNode.mm @@ -210,16 +210,9 @@ static NSArray *DefaultLinkAttributeNames = @[ NSLinkAttributeName ]; - (void)setBounds:(CGRect)bounds { [super setBounds:bounds]; - [self updateDrawingParameter]; [self _invalidateRendererIfNeededForBoundsSize:bounds.size]; } -- (void)setBackgroundColor:(UIColor *)backgroundColor -{ - [super setBackgroundColor:backgroundColor]; - [self updateDrawingParameter]; -} - #pragma mark - Renderer Management - (ASTextKitRenderer *)_renderer @@ -435,16 +428,18 @@ static NSArray *DefaultLinkAttributeNames = @[ NSLinkAttributeName ]; #pragma mark - Drawing -- (void)updateDrawingParameter +- (NSObject *)drawParametersForAsyncLayer:(_ASDisplayLayer *)layer { std::lock_guard l(_textLock); _drawParameter = { .backgroundColor = self.backgroundColor, - .bounds = self.threadSafeBounds + .bounds = self.bounds }; + return nil; } + - (void)drawRect:(CGRect)bounds withParameters:(id )p isCancelled:(asdisplaynode_iscancelled_block_t)isCancelledBlock isRasterizing:(BOOL)isRasterizing; { std::lock_guard l(_textLock);