diff --git a/AsyncDisplayKit/ASDisplayNode+Beta.h b/AsyncDisplayKit/ASDisplayNode+Beta.h index 1ea3d3376f..11cd6d0e70 100644 --- a/AsyncDisplayKit/ASDisplayNode+Beta.h +++ b/AsyncDisplayKit/ASDisplayNode+Beta.h @@ -91,4 +91,11 @@ ASDISPLAYNODE_EXTERN_C_END shouldMeasureAsync:(BOOL)shouldMeasureAsync measurementCompletion:(void(^)())completion; + +/** + * @abstract Currently used by ASNetworkImageNode and ASMultiplexImageNode to allow their placeholders to stay if they are loading an image from the network. + * Otherwise, a display pass is scheduled and completes, but does not actually draw anything - and ASDisplayNode considers the element finished. + */ +- (BOOL)placeholderShouldPersist; + @end diff --git a/AsyncDisplayKit/ASDisplayNode.mm b/AsyncDisplayKit/ASDisplayNode.mm index 6b42181aec..0596eaf510 100644 --- a/AsyncDisplayKit/ASDisplayNode.mm +++ b/AsyncDisplayKit/ASDisplayNode.mm @@ -426,6 +426,7 @@ static ASDisplayNodeMethodOverrides GetASDisplayNodeMethodOverrides(Class c) { CALayer *layer; ASDN::MutexLocker l(_propertyLock); + ASDisplayNodeAssert(_flags.layerBacked, @"_layerToLoad is only for layer-backed nodes"); if (_layerBlock) { layer = _layerBlock(); @@ -481,10 +482,6 @@ static ASDisplayNodeMethodOverrides GetASDisplayNodeMethodOverrides(Class c) TIME_SCOPED(_debugTimeForDidLoad); [self __didLoad]; } - - if (self.placeholderEnabled) { - [self _setupPlaceholderLayer]; - } } - (UIView *)view @@ -749,17 +746,20 @@ static ASDisplayNodeMethodOverrides GetASDisplayNodeMethodOverrides(Class c) ASDN::MutexLocker l(_propertyLock); [self calculatedLayoutDidChange]; - // we generate placeholders at measureWithSizeRange: time so that a node is guaranteed - // to have a placeholder ready to go. Also, if a node has no size it should not have a placeholder - if (self.placeholderEnabled && [self _displaysAsynchronously] && - _layout.size.width > 0.0 && _layout.size.height > 0.0) { + // We generate placeholders at measureWithSizeRange: time so that a node is guaranteed to have a placeholder ready to go. + // This is also because measurement is usually asynchronous, but placeholders need to be set up synchronously. + // First measurement is guaranteed to be before the node is onscreen, so we can create the image async. but still have it appear sync. + if (_placeholderEnabled && [self _displaysAsynchronously] && self.contents == nil) { + + // Zero-sized nodes do not require a placeholder. + CGSize layoutSize = (_layout ? _layout.size : CGSizeZero); + if (CGSizeEqualToSize(layoutSize, CGSizeZero)) { + return; + } + if (!_placeholderImage) { _placeholderImage = [self placeholderImage]; } - - if (_placeholderLayer) { - [self _setupPlaceholderLayerContents]; - } } } @@ -1010,13 +1010,14 @@ static ASDisplayNodeMethodOverrides GetASDisplayNodeMethodOverrides(Class c) { ASDisplayNodeAssertMainThread(); ASDN::MutexLocker l(_propertyLock); - if (CGRectEqualToRect(self.bounds, CGRectZero)) { + CGRect bounds = self.bounds; + if (CGRectEqualToRect(bounds, CGRectZero)) { // Performing layout on a zero-bounds view often results in frame calculations // with negative sizes after applying margins, which will cause // measureWithSizeRange: on subnodes to assert. return; } - _placeholderLayer.frame = self.bounds; + _placeholderLayer.frame = bounds; [self layout]; [self layoutDidFinish]; } @@ -1545,9 +1546,20 @@ static NSInteger incrementIfFound(NSInteger i) { } _flags.isEnteringHierarchy = NO; - CALayer *layer = self.layer; - if (!layer.contents) { + + // If we don't have contents finished drawing by the time we are on screen, immediately add the placeholder (if it is enabled and we do have something to draw). + if (self.contents == nil) { + CALayer *layer = self.layer; [layer setNeedsDisplay]; + + if ([self _shouldHavePlaceholderLayer]) { + [CATransaction begin]; + [CATransaction setDisableActions:YES]; + [self _setupPlaceholderLayerIfNeeded]; + _placeholderLayer.opacity = 1.0; + [CATransaction commit]; + [self.layer addSublayer:_placeholderLayer]; + } } } } @@ -1677,10 +1689,9 @@ static NSInteger incrementIfFound(NSInteger i) { [_pendingDisplayNodes removeObject:node]; - // only trampoline if there is a placeholder and nodes are done displaying - if ([self _pendingDisplayNodesHaveFinished] && _placeholderLayer.superlayer) { + if (_pendingDisplayNodes.count == 0 && _placeholderLayer.superlayer && ![self placeholderShouldPersist]) { void (^cleanupBlock)() = ^{ - [self _tearDownPlaceholderLayer]; + [_placeholderLayer removeFromSuperlayer]; }; if (_placeholderFadeDuration > 0.0 && ASInterfaceStateIncludesVisible(self.interfaceState)) { @@ -1695,33 +1706,46 @@ static NSInteger incrementIfFound(NSInteger i) { } } -// Helper method to check that all nodes that the current node is waiting to display are finished -// Use this method to check to remove any placeholder layers -- (BOOL)_pendingDisplayNodesHaveFinished -{ - return _pendingDisplayNodes.count == 0; -} - // Helper method to summarize whether or not the node run through the display process - (BOOL)__implementsDisplay { return _flags.implementsDrawRect || _flags.implementsImageDisplay || self.shouldRasterizeDescendants || _flags.implementsInstanceDrawRect || _flags.implementsInstanceImageDisplay; } -- (void)_setupPlaceholderLayer +- (BOOL)placeholderShouldPersist { - ASDisplayNodeAssertMainThread(); - - _placeholderLayer = [CALayer layer]; - // do not set to CGFLOAT_MAX in the case that something needs to be overtop the placeholder - _placeholderLayer.zPosition = 9999.0; + return NO; } -- (void)_tearDownPlaceholderLayer +- (BOOL)_shouldHavePlaceholderLayer +{ + return (_placeholderEnabled && [self __implementsDisplay]); +} + +- (void)_setupPlaceholderLayerIfNeeded { ASDisplayNodeAssertMainThread(); - [_placeholderLayer removeFromSuperlayer]; + if (!_placeholderLayer) { + _placeholderLayer = [CALayer layer]; + // do not set to CGFLOAT_MAX in the case that something needs to be overtop the placeholder + _placeholderLayer.zPosition = 9999.0; + } + + if (_placeholderLayer.contents == nil) { + if (!_placeholderImage) { + _placeholderImage = [self placeholderImage]; + } + if (_placeholderImage) { + BOOL stretchable = !UIEdgeInsetsEqualToEdgeInsets(_placeholderImage.capInsets, UIEdgeInsetsZero); + if (stretchable) { + ASDisplayNodeSetupLayerContentsWithResizableImage(_placeholderLayer, _placeholderImage); + } else { + _placeholderLayer.contentsScale = self.contentsScale; + _placeholderLayer.contents = (id)_placeholderImage.CGImage; + } + } + } } void recursivelyTriggerDisplayForLayer(CALayer *layer, BOOL shouldBlock) @@ -2258,26 +2282,6 @@ void recursivelyTriggerDisplayForLayer(CALayer *layer, BOOL shouldBlock) [self _pendingNodeWillDisplay:self]; [_supernode subnodeDisplayWillStart:self]; - - if (_placeholderImage && _placeholderLayer && self.layer.contents == nil) { - [CATransaction begin]; - [CATransaction setDisableActions:YES]; - [self _setupPlaceholderLayerContents]; - _placeholderLayer.opacity = 1.0; - [CATransaction commit]; - [self.layer addSublayer:_placeholderLayer]; - } -} - -- (void)_setupPlaceholderLayerContents -{ - BOOL stretchable = !UIEdgeInsetsEqualToEdgeInsets(_placeholderImage.capInsets, UIEdgeInsetsZero); - if (stretchable) { - ASDisplayNodeSetupLayerContentsWithResizableImage(_placeholderLayer, _placeholderImage); - } else { - _placeholderLayer.contentsScale = self.contentsScale; - _placeholderLayer.contents = (id)_placeholderImage.CGImage; - } } - (void)displayDidFinish diff --git a/AsyncDisplayKit/ASImageNode.mm b/AsyncDisplayKit/ASImageNode.mm index 66fa1fdfc5..7c27ed561a 100644 --- a/AsyncDisplayKit/ASImageNode.mm +++ b/AsyncDisplayKit/ASImageNode.mm @@ -25,6 +25,7 @@ @interface _ASImageNodeDrawParameters : NSObject +@property (nonatomic, retain) UIImage *image; @property (nonatomic, assign) BOOL opaque; @property (nonatomic, assign) CGRect bounds; @property (nonatomic, assign) CGFloat contentsScale; @@ -36,11 +37,17 @@ // TODO: eliminate explicit parameters with a set of keys copied from the node @implementation _ASImageNodeDrawParameters -- (id)initWithBounds:(CGRect)bounds opaque:(BOOL)opaque contentsScale:(CGFloat)contentsScale backgroundColor:(UIColor *)backgroundColor contentMode:(UIViewContentMode)contentMode +- (id)initWithImage:(UIImage *)image + bounds:(CGRect)bounds + opaque:(BOOL)opaque + contentsScale:(CGFloat)contentsScale + backgroundColor:(UIColor *)backgroundColor + contentMode:(UIViewContentMode)contentMode { - self = [self init]; - if (!self) return nil; + if (!(self = [self init])) + return nil; + _image = image; _opaque = opaque; _bounds = bounds; _contentsScale = contentsScale; @@ -133,8 +140,13 @@ _image = image; _imageLock.unlock(); + [self invalidateCalculatedLayout]; - [self setNeedsDisplay]; + if (image) { + [self setNeedsDisplay]; + } else { + self.contents = nil; + } } else { _imageLock.unlock(); // We avoid using MutexUnlocker as it needlessly re-locks at the end of the scope. } @@ -156,11 +168,12 @@ - (NSObject *)drawParametersForAsyncLayer:(_ASDisplayLayer *)layer { - return [[_ASImageNodeDrawParameters alloc] initWithBounds:self.bounds - opaque:self.opaque - contentsScale:self.contentsScaleForDisplay - backgroundColor:self.backgroundColor - contentMode:self.contentMode]; + return [[_ASImageNodeDrawParameters alloc] initWithImage:self.image + bounds:self.bounds + opaque:self.opaque + contentsScale:self.contentsScaleForDisplay + backgroundColor:self.backgroundColor + contentMode:self.contentMode]; } - (NSDictionary *)debugLabelAttributes @@ -171,21 +184,26 @@ - (UIImage *)displayWithParameters:(_ASImageNodeDrawParameters *)parameters isCancelled:(asdisplaynode_iscancelled_block_t)isCancelled { - UIImage *image; - BOOL cropEnabled; - BOOL forceUpscaling; - CGFloat contentsScale; - CGRect cropDisplayBounds; - CGRect cropRect; + UIImage *image = parameters.image; + if (!image) { + return nil; + } + + BOOL forceUpscaling = NO; + BOOL cropEnabled = NO; + BOOL isOpaque = parameters.opaque; + UIColor *backgroundColor = parameters.backgroundColor; + UIViewContentMode contentMode = parameters.contentMode; + CGFloat contentsScale = 0.0; + CGRect cropDisplayBounds = CGRectZero; + CGRect cropRect = CGRectZero; asimagenode_modification_block_t imageModificationBlock; { ASDN::MutexLocker l(_imageLock); - image = _image; - if (!image) { - return nil; - } + // 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; @@ -194,16 +212,12 @@ imageModificationBlock = _imageModificationBlock; } + BOOL hasValidCropBounds = cropEnabled && !CGRectIsNull(cropDisplayBounds) && !CGRectIsEmpty(cropDisplayBounds); + CGRect bounds = (hasValidCropBounds ? cropDisplayBounds : parameters.bounds); + ASDisplayNodeContextModifier preContextBlock = self.willDisplayNodeContentWithRenderingContext; ASDisplayNodeContextModifier postContextBlock = self.didDisplayNodeContentWithRenderingContext; - BOOL hasValidCropBounds = cropEnabled && !CGRectIsNull(cropDisplayBounds) && !CGRectIsEmpty(cropDisplayBounds); - - CGRect bounds = (hasValidCropBounds ? cropDisplayBounds : parameters.bounds); - BOOL isOpaque = parameters.opaque; - UIColor *backgroundColor = parameters.backgroundColor; - UIViewContentMode contentMode = parameters.contentMode; - ASDisplayNodeAssert(contentsScale > 0, @"invalid contentsScale at display time"); // if the image is resizable, bail early since the image has likely already been configured @@ -232,17 +246,15 @@ } } - BOOL contentModeSupported = contentMode == UIViewContentModeScaleAspectFill - || contentMode == UIViewContentModeScaleAspectFit - || contentMode == UIViewContentModeCenter; + BOOL contentModeSupported = contentMode == UIViewContentModeScaleAspectFill || + contentMode == UIViewContentModeScaleAspectFit || + contentMode == UIViewContentModeCenter; - CGSize backingSize; - CGRect imageDrawRect; + CGSize backingSize = CGSizeZero; + CGRect imageDrawRect = CGRectZero; - if (boundsSizeInPixels.width * contentsScale < 1.0f || - boundsSizeInPixels.height * contentsScale < 1.0f || - imageSizeInPixels.width < 1.0f || - imageSizeInPixels.height < 1.0f) { + if (boundsSizeInPixels.width * contentsScale < 1.0f || boundsSizeInPixels.height * contentsScale < 1.0f || + imageSizeInPixels.width < 1.0f || imageSizeInPixels.height < 1.0f) { return nil; } @@ -260,10 +272,8 @@ &imageDrawRect); } - if (backingSize.width <= 0.0f || - backingSize.height <= 0.0f || - imageDrawRect.size.width <= 0.0f || - imageDrawRect.size.height <= 0.0f) { + if (backingSize.width <= 0.0f || backingSize.height <= 0.0f || + imageDrawRect.size.width <= 0.0f || imageDrawRect.size.height <= 0.0f) { return nil; } diff --git a/AsyncDisplayKit/ASMultiplexImageNode.mm b/AsyncDisplayKit/ASMultiplexImageNode.mm index cd7c448e0b..351ff9c7d9 100644 --- a/AsyncDisplayKit/ASMultiplexImageNode.mm +++ b/AsyncDisplayKit/ASMultiplexImageNode.mm @@ -265,6 +265,11 @@ typedef void(^ASMultiplexImageLoadCompletionBlock)(UIImage *image, id imageIdent } } +- (BOOL)placeholderShouldPersist +{ + return (self.image == nil && self.imageIdentifiers.count > 0); +} + /* displayWillStart in ASNetworkImageNode has a very similar implementation. Changes here are likely necessary in ASNetworkImageNode as well. */ - (void)displayWillStart diff --git a/AsyncDisplayKit/ASNetworkImageNode.mm b/AsyncDisplayKit/ASNetworkImageNode.mm index 1a1ce26887..f909de9f82 100755 --- a/AsyncDisplayKit/ASNetworkImageNode.mm +++ b/AsyncDisplayKit/ASNetworkImageNode.mm @@ -173,6 +173,12 @@ static const CGSize kMinReleaseImageOnBackgroundSize = {20.0, 20.0}; return _delegate; } +- (BOOL)placeholderShouldPersist +{ + ASDN::MutexLocker l(_lock); + return (self.image == nil && _URL != nil); +} + /* displayWillStart in ASMultiplexImageNode has a very similar implementation. Changes here are likely necessary in ASMultiplexImageNode as well. */ - (void)displayWillStart @@ -334,6 +340,7 @@ static const CGSize kMinReleaseImageOnBackgroundSize = {20.0, 20.0}; - (void)_lazilyLoadImageIfNecessary { + // FIXME: We should revisit locking in this method (e.g. to access the instance variables at the top, and holding lock while calling delegate) if (!_imageLoaded && _URL != nil && _downloadIdentifier == nil) { { ASDN::MutexLocker l(_lock);