diff --git a/AsyncDisplayKit/ASDisplayNode.mm b/AsyncDisplayKit/ASDisplayNode.mm index 0b2f6de2b0..e9ebbab682 100644 --- a/AsyncDisplayKit/ASDisplayNode.mm +++ b/AsyncDisplayKit/ASDisplayNode.mm @@ -986,11 +986,20 @@ static inline void filterNodesInLayoutAtIndexesWithIntersectingNodes( { ASDisplayNodeAssertMainThread(); ASDN::MutexLocker l(_propertyLock); + + if (_pendingViewState.hasSetNeedsLayout) { + [self __setNeedsLayout]; + } + if (self.layerBacked) { [_pendingViewState applyToLayer:self.layer]; } else { [_pendingViewState applyToView:self.view]; } + + if (_pendingViewState.hasSetNeedsDisplay) { + [self __setNeedsDisplay]; + } [_pendingViewState clearChanges]; } @@ -1049,6 +1058,21 @@ static inline void filterNodesInLayoutAtIndexesWithIntersectingNodes( } } +// If not rasterized (and therefore we certainly have a view or layer), +// Send the message to the view/layer first, as scheduleNodeForDisplay may call -displayIfNeeded. +// Wrapped / synchronous nodes created with initWithView/LayerBlock: do not need scheduleNodeForDisplay, +// as they don't need to display in the working range at all - since at all times onscreen, one +// -setNeedsDisplay to the CALayer will result in a synchronous display in the next frame. +- (void)__setNeedsDisplay +{ + BOOL nowDisplay = ASInterfaceStateIncludesDisplay(_interfaceState); + // FIXME: This should not need to recursively display, so create a non-recursive variant. + // The semantics of setNeedsDisplay (as defined by CALayer behavior) are not recursive. + if (_layer && !_flags.synchronous && nowDisplay && [self __implementsDisplay]) { + [ASDisplayNode scheduleNodeForRecursiveDisplay:self]; + } +} + // These private methods ensure that subclasses are not required to call super in order for _renderingSubnodes to be properly managed. - (void)__layout diff --git a/AsyncDisplayKit/Private/ASDisplayNode+UIViewBridge.mm b/AsyncDisplayKit/Private/ASDisplayNode+UIViewBridge.mm index f33f56ca20..d850f2887c 100644 --- a/AsyncDisplayKit/Private/ASDisplayNode+UIViewBridge.mm +++ b/AsyncDisplayKit/Private/ASDisplayNode+UIViewBridge.mm @@ -76,9 +76,10 @@ if (shouldApply) { _view.viewAndPendingViewStateProperty = (viewAndPendingViewSt #define _getFromLayer(layerProperty) __loaded ? _layer.layerProperty : self.pendingViewState.layerProperty -#define _setToLayer(layerProperty, layerValueExpr) __loaded ? _layer.layerProperty = (layerValueExpr) : self.pendingViewState.layerProperty = (layerValueExpr) +#define _setToLayer(layerProperty, layerValueExpr) BOOL shouldApply = ASDisplayNodeShouldApplyBridgedWriteToView(self); \ +if (shouldApply) { _layer.layerProperty = (layerValueExpr); } else { _pendingViewState.layerProperty = (layerValueExpr); } -#define _messageToViewOrLayer(viewAndLayerSelector) __loaded ? (_view ? [_view viewAndLayerSelector] : [_layer viewAndLayerSelector]) : [self.pendingViewState viewAndLayerSelector] +#define _messageToViewOrLayer(viewAndLayerSelector) (_view ? [_view viewAndLayerSelector] : [_layer viewAndLayerSelector]) #define _messageToLayer(layerSelector) __loaded ? [_layer layerSelector] : [self.pendingViewState layerSelector] @@ -281,7 +282,6 @@ if (shouldApply) { _view.viewAndPendingViewStateProperty = (viewAndPendingViewSt - (void)setNeedsDisplay { _bridge_prologue_write; - if (_hierarchyState & ASHierarchyStateRasterized) { ASPerformBlockOnMainThread(^{ // The below operation must be performed on the main thread to ensure against an extremely rare deadlock, where a parent node @@ -299,19 +299,14 @@ if (shouldApply) { _view.viewAndPendingViewStateProperty = (viewAndPendingViewSt [rasterizedContainerNode setNeedsDisplay]; }); } else { - // If not rasterized (and therefore we certainly have a view or layer), - // Send the message to the view/layer first, as scheduleNodeForDisplay may call -displayIfNeeded. - // Wrapped / synchronous nodes created with initWithView/LayerBlock: do not need scheduleNodeForDisplay, - // as they don't need to display in the working range at all - since at all times onscreen, one - // -setNeedsDisplay to the CALayer will result in a synchronous display in the next frame. - - _messageToViewOrLayer(setNeedsDisplay); - - BOOL nowDisplay = ASInterfaceStateIncludesDisplay(_interfaceState); - // FIXME: This should not need to recursively display, so create a non-recursive variant. - // The semantics of setNeedsDisplay (as defined by CALayer behavior) are not recursive. - if (_layer && !_flags.synchronous && nowDisplay && [self __implementsDisplay]) { - [ASDisplayNode scheduleNodeForRecursiveDisplay:self]; + BOOL shouldApply = ASDisplayNodeShouldApplyBridgedWriteToView(self); + if (shouldApply) { + _messageToViewOrLayer(setNeedsDisplay); + [self __setNeedsDisplay]; + } else { + /// We will call `__setNeedsDisplay` just after the pending state + /// gets applied. + [_pendingViewState setNeedsDisplay]; } } } @@ -319,9 +314,15 @@ if (shouldApply) { _view.viewAndPendingViewStateProperty = (viewAndPendingViewSt - (void)setNeedsLayout { _bridge_prologue_write; - // FIXME: This method currently asserts on the node's thread affinity. - [self __setNeedsLayout]; - _messageToViewOrLayer(setNeedsLayout); + BOOL shouldApply = ASDisplayNodeShouldApplyBridgedWriteToView(self); + if (shouldApply) { + [self __setNeedsLayout]; + _messageToViewOrLayer(setNeedsLayout); + } else { + /// We will call `__setNeedsLayout` just before the pending state + /// gets applied. + [_pendingViewState setNeedsLayout]; + } } - (BOOL)isOpaque diff --git a/AsyncDisplayKit/Private/ASDisplayNodeInternal.h b/AsyncDisplayKit/Private/ASDisplayNodeInternal.h index 498b0ff0cc..bc53ff77c0 100644 --- a/AsyncDisplayKit/Private/ASDisplayNodeInternal.h +++ b/AsyncDisplayKit/Private/ASDisplayNodeInternal.h @@ -152,10 +152,15 @@ FOUNDATION_EXPORT NSString * const ASRenderingEngineDidDisplayNodesScheduledBefo - (BOOL)__shouldSize; /** - Invoked by a call to setNeedsLayout to the underlying view + Invoked before a call to setNeedsLayout to the underlying view */ - (void)__setNeedsLayout; +/** + Invoked after a call to setNeedsDisplay to the underlying view + */ +- (void)__setNeedsDisplay; + - (void)__layout; - (void)__setSupernode:(ASDisplayNode *)supernode; diff --git a/AsyncDisplayKit/Private/ASPendingStateController.mm b/AsyncDisplayKit/Private/ASPendingStateController.mm index 58d20068ba..41a11aed0a 100644 --- a/AsyncDisplayKit/Private/ASPendingStateController.mm +++ b/AsyncDisplayKit/Private/ASPendingStateController.mm @@ -76,7 +76,9 @@ } _flags.pendingFlush = YES; - [self performSelectorOnMainThread:@selector(flushNow) withObject:nil waitUntilDone:NO modes:@[ NSRunLoopCommonModes ]]; + dispatch_async(dispatch_get_main_queue(), ^{ + [self flushNow]; + }); } /** diff --git a/AsyncDisplayKit/Private/_ASPendingState.h b/AsyncDisplayKit/Private/_ASPendingState.h index 531e81f169..3178747afb 100644 --- a/AsyncDisplayKit/Private/_ASPendingState.h +++ b/AsyncDisplayKit/Private/_ASPendingState.h @@ -30,6 +30,9 @@ + (_ASPendingState *)pendingViewStateFromLayer:(CALayer *)layer; + (_ASPendingState *)pendingViewStateFromView:(UIView *)view; +@property (nonatomic, readonly) BOOL hasSetNeedsLayout; +@property (nonatomic, readonly) BOOL hasSetNeedsDisplay; + @property (nonatomic, readonly) BOOL hasChanges; - (void)clearChanges; diff --git a/AsyncDisplayKit/Private/_ASPendingState.m b/AsyncDisplayKit/Private/_ASPendingState.m index 85deb9c107..0e522e9f28 100644 --- a/AsyncDisplayKit/Private/_ASPendingState.m +++ b/AsyncDisplayKit/Private/_ASPendingState.m @@ -891,6 +891,16 @@ static UIColor *defaultTintColor = nil; _flags = (ASPendingStateFlags){ 0 }; } +- (BOOL)hasSetNeedsLayout +{ + return _flags.needsLayout; +} + +- (BOOL)hasSetNeedsDisplay +{ + return _flags.needsDisplay; +} + - (BOOL)hasChanges { ASPendingStateFlags flags = _flags; diff --git a/AsyncDisplayKitTests/ASBridgedPropertiesTests.mm b/AsyncDisplayKitTests/ASBridgedPropertiesTests.mm index ae80fd81ff..1e49ac1aa3 100644 --- a/AsyncDisplayKitTests/ASBridgedPropertiesTests.mm +++ b/AsyncDisplayKitTests/ASBridgedPropertiesTests.mm @@ -12,27 +12,42 @@ #import "ASThread.h" #import "ASDisplayNodeInternal.h" #import "_ASPendingState.h" +#import "ASCellNode.h" @interface ASPendingStateController (Testing) - (BOOL)test_isFlushScheduled; @end -@interface ASBridgedPropertiesTests : XCTestCase +@interface ASBridgedPropertiesTestView : UIView +@property (nonatomic, readonly) BOOL receivedSetNeedsLayout; +@end +@implementation ASBridgedPropertiesTestView + +- (void)setNeedsLayout +{ + _receivedSetNeedsLayout = YES; + [super setNeedsLayout]; +} + +@end + +@interface ASBridgedPropertiesTests : XCTestCase @end /// Dispatches the given block synchronously onto a different thread. /// This is useful for testing non-main-thread behavior because `dispatch_sync` /// will often use the current thread. static inline void ASDispatchSyncOnOtherThread(dispatch_block_t block) { - dispatch_semaphore_t sem = dispatch_semaphore_create(0); + dispatch_group_t group = dispatch_group_create(); dispatch_queue_t q = dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0); + dispatch_group_enter(group); dispatch_async(q, ^{ ASDisplayNodeCAssertNotMainThread(); block(); - dispatch_semaphore_signal(sem); + dispatch_group_leave(group); }); - dispatch_semaphore_wait(sem, DISPATCH_TIME_FOREVER); + dispatch_group_wait(group, DISPATCH_TIME_FOREVER); } @implementation ASBridgedPropertiesTests @@ -42,7 +57,26 @@ static inline void ASDispatchSyncOnOtherThread(dispatch_block_t block) { XCTAssertNotNil([ASPendingStateController sharedInstance]); } -- (void)testThatSettingABridgedPropertyInBackgroundGetsFlushedOnNextRunLoop +- (void)testThatDirtyNodesAreNotRetained +{ + ASPendingStateController *ctrl = [ASPendingStateController sharedInstance]; + __weak ASDisplayNode *weakNode = nil; + @autoreleasepool { + ASDisplayNode *node = [ASDisplayNode new]; + weakNode = node; + [node view]; + XCTAssertEqual(node.alpha, 1); + ASDispatchSyncOnOtherThread(^{ + node.alpha = 0; + }); + XCTAssertEqual(node.alpha, 1); + XCTAssert(ctrl.test_isFlushScheduled); + XCTAssertNotNil(weakNode); + } + XCTAssertNil(weakNode); +} + +- (void)testThatSettingABridgedViewPropertyInBackgroundGetsFlushedOnNextRunLoop { ASDisplayNode *node = [ASDisplayNode new]; [node view]; @@ -50,11 +84,25 @@ static inline void ASDispatchSyncOnOtherThread(dispatch_block_t block) { ASDispatchSyncOnOtherThread(^{ node.alpha = 0; }); - [[NSRunLoop mainRunLoop] runMode:NSDefaultRunLoopMode beforeDate:[NSDate distantFuture]]; + XCTAssertEqual(node.alpha, 1); + [self waitForMainDispatchQueueToFlush]; XCTAssertEqual(node.alpha, 0); } -- (void)testThatReadingABridgedPropertyInBackgroundThrowsAnException +- (void)testThatSettingABridgedLayerPropertyInBackgroundGetsFlushedOnNextRunLoop +{ + ASDisplayNode *node = [ASDisplayNode new]; + [node view]; + XCTAssertEqual(node.shadowOpacity, 0); + ASDispatchSyncOnOtherThread(^{ + node.shadowOpacity = 1; + }); + XCTAssertEqual(node.shadowOpacity, 0); + [self waitForMainDispatchQueueToFlush]; + XCTAssertEqual(node.shadowOpacity, 1); +} + +- (void)testThatReadingABridgedViewPropertyInBackgroundThrowsAnException { ASDisplayNode *node = [ASDisplayNode new]; [node view]; @@ -63,6 +111,15 @@ static inline void ASDispatchSyncOnOtherThread(dispatch_block_t block) { }); } +- (void)testThatReadingABridgedLayerPropertyInBackgroundThrowsAnException +{ + ASDisplayNode *node = [ASDisplayNode new]; + [node view]; + ASDispatchSyncOnOtherThread(^{ + XCTAssertThrows(node.contentsScale); + }); +} + - (void)testThatManuallyFlushingTheSyncControllerImmediatelyAppliesChanges { ASPendingStateController *ctrl = [ASPendingStateController sharedInstance]; @@ -104,4 +161,53 @@ static inline void ASDispatchSyncOnOtherThread(dispatch_block_t block) { XCTAssertFalse(ctrl.test_isFlushScheduled); } +- (void)testThatCallingSetNeedsLayoutFromBackgroundCausesItToHappenLater +{ + ASDisplayNode *node = [[ASDisplayNode alloc] initWithViewClass:ASBridgedPropertiesTestView.class]; + ASBridgedPropertiesTestView *view = (ASBridgedPropertiesTestView *)node.view; + XCTAssertFalse(view.receivedSetNeedsLayout); + ASDispatchSyncOnOtherThread(^{ + XCTAssertNoThrow([node setNeedsLayout]); + }); + XCTAssertFalse(view.receivedSetNeedsLayout); + [self waitForMainDispatchQueueToFlush]; + XCTAssertTrue(view.receivedSetNeedsLayout); +} + +- (void)testThatCallingSetNeedsLayoutOnACellNodeFromBackgroundIsSafe +{ + ASCellNode *node = [ASCellNode new]; + [node view]; + ASDispatchSyncOnOtherThread(^{ + XCTAssertNoThrow([node setNeedsLayout]); + }); +} + +- (void)testThatCallingSetNeedsDisplayFromBackgroundCausesItToHappenLater +{ + ASDisplayNode *node = [ASDisplayNode new]; + [node.layer displayIfNeeded]; + XCTAssertFalse(node.layer.needsDisplay); + ASDispatchSyncOnOtherThread(^{ + XCTAssertNoThrow([node setNeedsDisplay]); + }); + XCTAssertFalse(node.layer.needsDisplay); + [self waitForMainDispatchQueueToFlush]; + XCTAssertTrue(node.layer.needsDisplay); +} + +/// [XCTExpectation expectationWithPredicate:] should handle this +/// but under Xcode 7.2.1 its polling interval is 1 second +/// which makes the tests really slow and I'm impatient. +- (void)waitForMainDispatchQueueToFlush +{ + __block BOOL done = NO; + dispatch_async(dispatch_get_main_queue(), ^{ + done = YES; + }); + while (!done) { + [NSRunLoop.mainRunLoop runMode:NSDefaultRunLoopMode beforeDate:[NSDate distantFuture]]; + } +} + @end