[ASImageNode] Move to class method of displayWithParameters:isCancell…

…ed: for drawing (#244) * Remove instance displayWithParameters:isCancelled: * Address comments
TextureGroup · May 9, 2017 · 3738f1f · 3738f1f
1 parent 8692428
commit 3738f1f
Show file tree

Hide file tree

Showing 7 changed files with 90 additions and 87 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -13,3 +13,4 @@
 - Remove finalLayoutElement [Michael Schneider] (https://github.com/maicki)[#96](https://github.com/TextureGroup/Texture/pull/96)
 - Add ASPageTable - A map table for fast retrieval of objects within a certain page [Huy Nguyen](https://github.com/nguyenhuy)
 - [ASDisplayNode] Pass drawParameter in rendering context callbacks [Michael Schneider](https://github.com/maicki)[#248](https://github.com/TextureGroup/Texture/pull/248)
+- [ASTextNode] Move to class method of drawRect:withParameters:isCancelled:isRasterizing: for drawing [Michael Schneider] (https://github.com/maicki)[#232](https://github.com/TextureGroup/Texture/pull/232)
diff --git a/Source/ASDisplayNode.mm b/Source/ASDisplayNode.mm
@@ -143,11 +143,9 @@ static struct ASDisplayNodeFlags GetASDisplayNodeFlags(Class c, ASDisplayNode *i
   if (instance) {
     flags.implementsDrawParameters = ([instance respondsToSelector:@selector(drawParametersForAsyncLayer:)] ? 1 : 0);
     flags.implementsInstanceDrawRect = ([instance respondsToSelector:@selector(drawRect:withParameters:isCancelled:isRasterizing:)] ? 1 : 0);
-    flags.implementsInstanceImageDisplay = ([instance respondsToSelector:@selector(displayWithParameters:isCancelled:)] ? 1 : 0);
   } else {
     flags.implementsDrawParameters = ([c instancesRespondToSelector:@selector(drawParametersForAsyncLayer:)] ? 1 : 0);
     flags.implementsInstanceDrawRect = ([c instancesRespondToSelector:@selector(drawRect:withParameters:isCancelled:isRasterizing:)] ? 1 : 0);
-    flags.implementsInstanceImageDisplay = ([c instancesRespondToSelector:@selector(displayWithParameters:isCancelled:)] ? 1 : 0);
   }
   return flags;
 }
@@ -832,6 +830,11 @@ - (void)setShouldAnimateSizeChanges:(BOOL)shouldAnimateSizeChanges
 - (CGRect)threadSafeBounds
 {
   ASDN::MutexLocker l(__instanceLock__);
+  return [self _locked_threadSafeBounds];
+}
+
+- (CGRect)_locked_threadSafeBounds
+{
   return _threadSafeBounds;
 }
 
@@ -2001,8 +2004,7 @@ - (BOOL)_implementsDisplay
 {
   ASDN::MutexLocker l(__instanceLock__);
 
-  return _flags.implementsDrawRect || _flags.implementsImageDisplay || _flags.rasterizesSubtree ||
-  _flags.implementsInstanceDrawRect || _flags.implementsInstanceImageDisplay;
+  return _flags.implementsDrawRect || _flags.implementsImageDisplay || _flags.rasterizesSubtree || _flags.implementsInstanceDrawRect;
 }
 
 // Track that a node will be displayed as part of the current node hierarchy.

diff --git a/Source/ASImageNode.mm b/Source/ASImageNode.mm
@@ -41,26 +41,37 @@
 
 #include <functional>
 
-struct ASImageNodeDrawParameters {
-  BOOL opaque;
-  CGRect bounds;
-  CGFloat contentsScale;
-  UIColor *backgroundColor;
-  UIViewContentMode contentMode;
-  BOOL cropEnabled;
-  BOOL forceUpscaling;
-  CGSize forcedSize;
-  CGRect cropRect;
-  CGRect cropDisplayBounds;
-  asimagenode_modification_block_t imageModificationBlock;
-  ASDisplayNodeContextModifier willDisplayNodeContentWithRenderingContext;
-  ASDisplayNodeContextModifier didDisplayNodeContentWithRenderingContext;
-};
+typedef void (^ASImageNodeDrawParametersBlock)(ASWeakMapEntry *entry);
+
+@interface ASImageNodeDrawParameters : NSObject {
+@package
+  UIImage *_image;
+  BOOL _opaque;
+  CGRect _bounds;
+  CGFloat _contentsScale;
+  UIColor *_backgroundColor;
+  UIViewContentMode _contentMode;
+  BOOL _cropEnabled;
+  BOOL _forceUpscaling;
+  CGSize _forcedSize;
+  CGRect _cropRect;
+  CGRect _cropDisplayBounds;
+  asimagenode_modification_block_t _imageModificationBlock;
+  ASDisplayNodeContextModifier _willDisplayNodeContentWithRenderingContext;
+  ASDisplayNodeContextModifier _didDisplayNodeContentWithRenderingContext;
+  ASImageNodeDrawParametersBlock _didDrawBlock;
+}
+
+@end
+
+@implementation ASImageNodeDrawParameters
+
+@end
 
 /**
  * Contains all data that is needed to generate the content bitmap.
  */
-@interface ASImageNodeContentsKey : NSObject {}
+@interface ASImageNodeContentsKey : NSObject
 
 @property (nonatomic, strong) UIImage *image;
 @property CGSize backingSize;
@@ -137,7 +148,6 @@ @implementation ASImageNode
   void (^_displayCompletionBlock)(BOOL canceled);
 
   // Drawing
-  ASImageNodeDrawParameters _drawParameter;
   ASTextNode *_debugLabelNode;
 
   // Cropping.
@@ -151,17 +161,7 @@ @implementation ASImageNode
 @synthesize image = _image;
 @synthesize imageModificationBlock = _imageModificationBlock;
 
-#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]));
-  }
-}
+#pragma mark - Lifecycle
 
 - (instancetype)init
 {
@@ -194,6 +194,8 @@ - (void)dealloc
   [self invalidateAnimatedImage];
 }
 
+#pragma mark - Placeholder
+
 - (UIImage *)placeholderImage
 {
   // FIXME: Replace this implementation with reusable CALayers that have .backgroundColor set.
@@ -290,57 +292,53 @@ - (NSObject *)drawParametersForAsyncLayer:(_ASDisplayLayer *)layer
 {
   ASDN::MutexLocker l(__instanceLock__);
 
-  _drawParameter = {
-    .bounds = self.bounds,
-    .opaque = self.opaque,
-    .contentsScale = self.contentsScaleForDisplay,
-    .backgroundColor = self.backgroundColor,
-    .contentMode = self.contentMode,
-    .cropEnabled = _cropEnabled,
-    .forceUpscaling = _forceUpscaling,
-    .forcedSize = _forcedSize,
-    .cropRect = _cropRect,
-    .cropDisplayBounds = _cropDisplayBounds,
-    .imageModificationBlock = _imageModificationBlock,
-    .willDisplayNodeContentWithRenderingContext = _willDisplayNodeContentWithRenderingContext,
-    .didDisplayNodeContentWithRenderingContext = _didDisplayNodeContentWithRenderingContext
+  ASImageNodeDrawParameters *drawParameters = [[ASImageNodeDrawParameters alloc] init];
+  drawParameters->_image = [self _locked_Image];
+  drawParameters->_bounds = [self threadSafeBounds];
+  drawParameters->_opaque = self.opaque;
+  drawParameters->_contentsScale = _contentsScaleForDisplay;
+  drawParameters->_backgroundColor = self.backgroundColor;
+  drawParameters->_contentMode = self.contentMode;
+  drawParameters->_cropEnabled = _cropEnabled;
+  drawParameters->_forceUpscaling = _forceUpscaling;
+  drawParameters->_forcedSize = _forcedSize;
+  drawParameters->_cropRect = _cropRect;
+  drawParameters->_cropDisplayBounds = _cropDisplayBounds;
+  drawParameters->_imageModificationBlock = _imageModificationBlock;
+  drawParameters->_willDisplayNodeContentWithRenderingContext = _willDisplayNodeContentWithRenderingContext;
+  drawParameters->_didDisplayNodeContentWithRenderingContext = _didDisplayNodeContentWithRenderingContext;
+
+  // Hack for now to retain the weak entry that was created while this drawing happened
+  drawParameters->_didDrawBlock = ^(ASWeakMapEntry *entry){
+    ASDN::MutexLocker l(__instanceLock__);
+    _weakCacheEntry = entry;
   };
 
-  return nil;
+  return drawParameters;
 }
 
-- (NSDictionary *)debugLabelAttributes
++ (UIImage *)displayWithParameters:(id<NSObject>)parameter isCancelled:(asdisplaynode_iscancelled_block_t)isCancelled
 {
-  return @{
-    NSFontAttributeName: [UIFont systemFontOfSize:15.0],
-    NSForegroundColorAttributeName: [UIColor redColor]
-  };
-}
+  ASImageNodeDrawParameters *drawParameter = (ASImageNodeDrawParameters *)parameter;
 
-- (UIImage *)displayWithParameters:(id<NSObject>)parameter isCancelled:(asdisplaynode_iscancelled_block_t)isCancelled
-{
-  UIImage *image = self.image;
+  UIImage *image = drawParameter->_image;
   if (image == nil) {
     return nil;
   }
 
-  __instanceLock__.lock();
-  ASImageNodeDrawParameters drawParameter = _drawParameter;
-  __instanceLock__.unlock();
-
-  CGRect drawParameterBounds       = drawParameter.bounds;
-  BOOL forceUpscaling              = drawParameter.forceUpscaling;
-  CGSize forcedSize                = drawParameter.forcedSize;
-  BOOL cropEnabled                 = drawParameter.cropEnabled;
-  BOOL isOpaque                    = drawParameter.opaque;
-  UIColor *backgroundColor         = drawParameter.backgroundColor;
-  UIViewContentMode contentMode    = drawParameter.contentMode;
-  CGFloat contentsScale            = drawParameter.contentsScale;
-  CGRect cropDisplayBounds         = drawParameter.cropDisplayBounds;
-  CGRect cropRect                  = drawParameter.cropRect;
-  asimagenode_modification_block_t imageModificationBlock    = drawParameter.imageModificationBlock;
-  ASDisplayNodeContextModifier willDisplayNodeContentWithRenderingContext = drawParameter.willDisplayNodeContentWithRenderingContext;
-  ASDisplayNodeContextModifier didDisplayNodeContentWithRenderingContext = drawParameter.didDisplayNodeContentWithRenderingContext;
+  CGRect drawParameterBounds       = drawParameter->_bounds;
+  BOOL forceUpscaling              = drawParameter->_forceUpscaling;
+  CGSize forcedSize                = drawParameter->_forcedSize;
+  BOOL cropEnabled                 = drawParameter->_cropEnabled;
+  BOOL isOpaque                    = drawParameter->_opaque;
+  UIColor *backgroundColor         = drawParameter->_backgroundColor;
+  UIViewContentMode contentMode    = drawParameter->_contentMode;
+  CGFloat contentsScale            = drawParameter->_contentsScale;
+  CGRect cropDisplayBounds         = drawParameter->_cropDisplayBounds;
+  CGRect cropRect                  = drawParameter->_cropRect;
+  asimagenode_modification_block_t imageModificationBlock                 = drawParameter->_imageModificationBlock;
+  ASDisplayNodeContextModifier willDisplayNodeContentWithRenderingContext = drawParameter->_willDisplayNodeContentWithRenderingContext;
+  ASDisplayNodeContextModifier didDisplayNodeContentWithRenderingContext  = drawParameter->_didDisplayNodeContentWithRenderingContext;
 
   BOOL hasValidCropBounds = cropEnabled && !CGRectIsEmpty(cropDisplayBounds);
   CGRect bounds = (hasValidCropBounds ? cropDisplayBounds : drawParameterBounds);
@@ -421,9 +419,9 @@ - (UIImage *)displayWithParameters:(id<NSObject>)parameter isCancelled:(asdispla
     return nil;
   }
 
-  __instanceLock__.lock();
-    _weakCacheEntry = entry; // Retain so that the entry remains in the weak cache
-  __instanceLock__.unlock();
+  if (drawParameter->_didDrawBlock) {
+    drawParameter->_didDrawBlock(entry);
+  }
 
   return entry.value;
 }
@@ -440,7 +438,6 @@ + (ASWeakMapEntry *)contentsForkey:(ASImageNodeContentsKey *)key drawParameters:
     }
     ASWeakMapEntry *entry = [cache entryForKey:key];
     if (entry != nil) {
-      // cache hit
       return entry;
     }
   }
@@ -713,6 +710,15 @@ - (void)layout
     _debugLabelNode.frame    = (CGRect) {debugLabelOrigin, debugLabelSize};
   }
 }
+
+- (NSDictionary *)debugLabelAttributes
+{
+  return @{
+    NSFontAttributeName: [UIFont systemFontOfSize:15.0],
+    NSForegroundColorAttributeName: [UIColor redColor]
+  };
+}
+
 @end
 
 #pragma mark - Extras

diff --git a/Source/Details/_ASDisplayLayer.h b/Source/Details/_ASDisplayLayer.h
@@ -125,12 +125,6 @@
  */
 - (void)drawRect:(CGRect)bounds withParameters:(id <NSObject>)parameters isCancelled:(AS_NOESCAPE asdisplaynode_iscancelled_block_t)isCancelledBlock isRasterizing:(BOOL)isRasterizing;
 
-/**
- * @abstract instance version of display class method
- * @see displayWithParameters:isCancelled class method
- */
-- (UIImage *)displayWithParameters:(id <NSObject>)parameters isCancelled:(AS_NOESCAPE asdisplaynode_iscancelled_block_t)isCancelled;
-
 // Called on the main thread only
 
 /**

diff --git a/Source/Private/ASDisplayNode+AsyncDisplay.mm b/Source/Private/ASDisplayNode+AsyncDisplay.mm
@@ -168,10 +168,10 @@ - (asyncdisplaykit_async_transaction_operation_block_t)_displayBlockWithAsynchro
   flags = _flags;
 
   // We always create a graphics context, unless a -display method is used, OR if we are a subnode drawing into a rasterized parent.
-  BOOL shouldCreateGraphicsContext = (flags.implementsInstanceImageDisplay == NO && flags.implementsImageDisplay == NO && rasterizing == NO);
+  BOOL shouldCreateGraphicsContext = (flags.implementsImageDisplay == NO && rasterizing == NO);
   BOOL shouldBeginRasterizing = (rasterizing == NO && flags.rasterizesSubtree);
-  BOOL usesInstanceMethodDisplay = (flags.implementsInstanceDrawRect || flags.implementsInstanceImageDisplay);
-  BOOL usesImageDisplay = (flags.implementsImageDisplay || flags.implementsInstanceImageDisplay);
+  BOOL usesInstanceMethodDisplay = (flags.implementsInstanceDrawRect);
+  BOOL usesImageDisplay = (flags.implementsImageDisplay);
   BOOL usesDrawRect = (flags.implementsDrawRect || flags.implementsInstanceDrawRect);
 
   if (usesImageDisplay == NO && usesDrawRect == NO && shouldBeginRasterizing == NO) {

diff --git a/Source/Private/ASDisplayNode+FrameworkPrivate.h b/Source/Private/ASDisplayNode+FrameworkPrivate.h
@@ -123,6 +123,7 @@ __unused static NSString * _Nonnull NSStringFromASHierarchyState(ASHierarchyStat
 
 // Thread safe way to access the bounds of the node
 @property (nonatomic, assign) CGRect threadSafeBounds;
+- (CGRect)_locked_threadSafeBounds;
 
 // delegate to inform of ASInterfaceState changes (used by ASNodeController)
 @property (nonatomic, weak) id<ASInterfaceStateDelegate> interfaceStateDelegate;

diff --git a/Source/Private/ASDisplayNodeInternal.h b/Source/Private/ASDisplayNodeInternal.h
@@ -110,7 +110,6 @@ FOUNDATION_EXPORT NSString * const ASRenderingEngineDidDisplayNodesScheduledBefo
     // whether custom drawing is enabled
     unsigned implementsInstanceDrawRect:1;
     unsigned implementsDrawRect:1;
-    unsigned implementsInstanceImageDisplay:1;
     unsigned implementsImageDisplay:1;
     unsigned implementsDrawParameters:1;