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

Tighten Rasterization API, Undeprecate It #82

Merged
merged 6 commits into from
May 1, 2017
Merged
Show file tree
Hide file tree
Changes from 3 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
5 changes: 3 additions & 2 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
## master

* Add your own contributions to the next release on the line below this with your name.
- Add support for IGListKit post-removal-of-IGListSectionType, in preparation for IGListKit 3.0.0 release. (Adlai-Holler)[https://github.com/Adlai-Holler] (#49)[https://github.com/TextureGroup/Texture/pull/49]
- Add support for IGListKit post-removal-of-IGListSectionType, in preparation for IGListKit 3.0.0 release. [Adlai Holler](https://github.com/Adlai-Holler) [#49](https://github.com/TextureGroup/Texture/pull/49)
- Fix `__has_include` check in ASLog.h [Philipp Smorygo]([email protected])
- Fix potential deadlock in ASControlNode [Garrett Moon](https://github.com/garrettmoon)
- [Yoga Beta] Improvements to the experimental support for Yoga layout [Scott Goodson](appleguy)
- [Yoga Beta] Improvements to the experimental support for Yoga layout [Scott Goodson](appleguy)
- Update the rasterization API and un-deprecate it. [Adlai Holler](https://github.com/Adlai-Holler)[#82](https://github.com/TextureGroup/Texture/pull/49)
11 changes: 7 additions & 4 deletions Source/ASDisplayNode+Beta.h
Original file line number Diff line number Diff line change
Expand Up @@ -136,11 +136,11 @@ typedef struct {
+ (void)setRangeModeForMemoryWarnings:(ASLayoutRangeMode)rangeMode;

/**
* @abstract Whether to draw all descendant nodes' layers/views into this node's layer/view's backing store.
* @abstract Whether to draw all descendent nodes' contents into this node's layer's backing store.
*
* @discussion
* When set to YES, causes all descendant nodes' layers/views to be drawn directly into this node's layer/view's backing
* store. Defaults to NO.
* When called, causes all descendent nodes' contents to be drawn directly into this node's layer's backing
* store.
*
* If a node's descendants are static (never animated or never change attributes after creation) then that node is a
* good candidate for rasterization. Rasterizing descendants has two main benefits:
Expand All @@ -154,8 +154,11 @@ typedef struct {
*
* Note: this has nothing to do with -[CALayer shouldRasterize], which doesn't work with ASDisplayNode's asynchronous
* rendering model.
*
* Note: You cannot add subnodes whose layers/views are already loaded to a rasterized node.
* Note: You cannot call this method after the receiver's layer/view is loaded.
*/
@property (nonatomic, assign) BOOL shouldRasterizeDescendants ASDISPLAYNODE_DEPRECATED_MSG("Deprecated in version 2.2");
- (void)setShouldRasterizeDescendants;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Adlai-Holler I think it's a good idea to make this an enable-only method. However, this name seems kinda sucky because it sounds exactly like a property that expects an argument.

How about something like -enableSubtreeRasterization, -enableRasterizingDescendants, etc? I think coming up with the right term for a subtree of subnodes could help here. .automaticallyManagesSubnodes is one reference point, but it does not apply recursively to all levels below, so Subtree might be better.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmmm OK! I'll change it to enableSubtreeRasterization.


@end

Expand Down
129 changes: 45 additions & 84 deletions Source/ASDisplayNode.mm
Original file line number Diff line number Diff line change
Expand Up @@ -541,28 +541,7 @@ + (NSValue * _Nonnull)_ivarsThatMayNeedMainDeallocation
return result;
}

#pragma mark - Loading / Unloading

- (void)__unloadNode
{
ASDisplayNodeAssertMainThread();
ASDisplayNodeAssert([self isNodeLoaded], @"Implementation shouldn't call __unloadNode if not loaded: %@", self);
ASDisplayNodeAssert(_flags.synchronous == NO, @"Node created using -initWithViewBlock:/-initWithLayerBlock: cannot be unloaded. Node: %@", self);
ASDN::MutexLocker l(__instanceLock__);

if (_flags.layerBacked) {
_pendingViewState = [_ASPendingState pendingViewStateFromLayer:_layer];
} else {
_pendingViewState = [_ASPendingState pendingViewStateFromView:_view];
}

[_view removeFromSuperview];
_view = nil;
if (_flags.layerBacked)
_layer.delegate = nil;
[_layer removeFromSuperlayer];
_layer = nil;
}
#pragma mark - Loading

- (BOOL)_locked_shouldLoadViewOrLayer
{
Expand Down Expand Up @@ -1972,60 +1951,41 @@ - (void)setDisplaysAsynchronously:(BOOL)displaysAsynchronously
- (BOOL)shouldRasterizeDescendants
{
ASDN::MutexLocker l(__instanceLock__);
ASDisplayNodeAssert(!((_hierarchyState & ASHierarchyStateRasterized) && _flags.shouldRasterizeDescendants),
@"Subnode of a rasterized node should not have redundant shouldRasterizeDescendants enabled");
return _flags.shouldRasterizeDescendants;
}

- (void)setShouldRasterizeDescendants:(BOOL)shouldRasterize
- (void)setShouldRasterizeDescendants
{
ASDisplayNodeAssertThreadAffinity(self);
BOOL rasterizedFromSelfOrAncestor = NO;
{
ASDN::MutexLocker l(__instanceLock__);

if (_flags.shouldRasterizeDescendants == shouldRasterize)
return;

_flags.shouldRasterizeDescendants = shouldRasterize;
rasterizedFromSelfOrAncestor = shouldRasterize || ASHierarchyStateIncludesRasterized(_hierarchyState);
ASDN::MutexLocker l(__instanceLock__);
// Already rasterized from self.
if (_flags.shouldRasterizeDescendants) {
return;
}

if (self.isNodeLoaded) {
// Recursively tear down or build up subnodes.
// TODO: When disabling rasterization, preserve rasterized backing store as placeholderImage
// while the newly materialized subtree finishes rendering. Then destroy placeholderImage to save memory.
[self recursivelyClearContents];

ASDisplayNodePerformBlockOnEverySubnode(self, NO, ^(ASDisplayNode *node) {
if (rasterizedFromSelfOrAncestor) {
[node enterHierarchyState:ASHierarchyStateRasterized];
if (node.isNodeLoaded) {
[node __unloadNode];
}
} else {
[node exitHierarchyState:ASHierarchyStateRasterized];
// We can avoid eagerly loading this node. We will load it on-demand as usual.
}
});
if (!rasterizedFromSelfOrAncestor) {
// If we are not going to rasterize at all, go ahead and set up our view hierarchy.
[self _addSubnodeViewsAndLayers];
}

if (ASInterfaceStateIncludesVisible(self.interfaceState)) {
// TODO: Change this to recursivelyEnsureDisplay - but need a variant that does not skip
// nodes that have shouldBypassEnsureDisplay set (such as image nodes) so they are rasterized.
[self recursivelyDisplayImmediately];

// If rasterized from above, bail.
if (ASHierarchyStateIncludesRasterized(_hierarchyState)) {
ASDisplayNodeFailAssert(@"Subnode of a rasterized node should not have redundant -setShouldRasterizeDescendants.");
return;
}

// Ensure not loaded.
if ([self _locked_isNodeLoaded]) {
ASDisplayNodeFailAssert(@"Cannot call %@ on loaded node: %@", NSStringFromSelector(_cmd), self);
return;
}

// Ensure no loaded subnodes
for (ASDisplayNode *subnode in _subnodes) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this loop enough to check all subnodes' subnodes? Should we use ASPerformBlockOnEverySubnode, or otherwise recursively call _enableRasterizingToParent or something on each?

if (subnode.nodeLoaded) {
ASDisplayNodeFailAssert(@"Cannot call %@ on node %@ with loaded subnode %@", NSStringFromSelector(_cmd), self, subnode);
return;
}
} else {
ASDisplayNodePerformBlockOnEverySubnode(self, NO, ^(ASDisplayNode *node) {
if (rasterizedFromSelfOrAncestor) {
[node enterHierarchyState:ASHierarchyStateRasterized];
} else {
[node exitHierarchyState:ASHierarchyStateRasterized];
}
});
}
_flags.shouldRasterizeDescendants = YES;

// Tell subnodes that now they're in a rasterized hierarchy (while holding lock!)
for (ASDisplayNode *subnode in _subnodes) {
[subnode enterHierarchyState:ASHierarchyStateRasterized];
}
}

Expand Down Expand Up @@ -2619,7 +2579,7 @@ ASDISPLAYNODE_INLINE BOOL canUseViewAPI(ASDisplayNode *node, ASDisplayNode *subn
}

/// Returns if node is a member of a rasterized tree
ASDISPLAYNODE_INLINE BOOL nodeIsInRasterizedTree(ASDisplayNode *node) {
ASDISPLAYNODE_INLINE BOOL subtreeIsRasterized(ASDisplayNode *node) {
return (node.shouldRasterizeDescendants || (node.hierarchyState & ASHierarchyStateRasterized));
}

Expand Down Expand Up @@ -2744,6 +2704,12 @@ - (void)_insertSubnode:(ASDisplayNode *)subnode atSubnodeIndex:(NSInteger)subnod
return;
}

BOOL isRasterized = subtreeIsRasterized(self);
if (isRasterized && subnode.nodeLoaded) {
ASDisplayNodeFailAssert(@"Cannot add loaded node %@ to rasterized subtree of node %@", ASObjectDescriptionMakeTiny(subnode), ASObjectDescriptionMakeTiny(self));
return;
}

__instanceLock__.lock();
NSUInteger subnodesCount = _subnodes.count;
__instanceLock__.unlock();
Expand Down Expand Up @@ -2773,15 +2739,10 @@ - (void)_insertSubnode:(ASDisplayNode *)subnode atSubnodeIndex:(NSInteger)subnod
// If we are a managed hierarchy, as in ASCellNode trees, it will also apply our .interfaceState.
[subnode _setSupernode:self];

// If this subnode will be rasterized, update its hierarchy state & enter hierarchy if needed
if (nodeIsInRasterizedTree(self)) {
ASDisplayNodePerformBlockOnEveryNodeBFS(subnode, ^(ASDisplayNode * _Nonnull node) {
[node enterHierarchyState:ASHierarchyStateRasterized];
if (node.isNodeLoaded) {
[node __unloadNode];
}
});
if (self.isInHierarchy) {
// If this subnode will be rasterized, enter hierarchy if needed
// TODO: Move this into _setSupernode: ?
if (isRasterized) {
if (self.inHierarchy) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it may have been important for the original code to enter the Rasterized hierarchy state before __enterHierarchy is called, because it may have invoked delegate methods that check this. I haven't looked closely though, this might not be true - just something to take a close look at.

[subnode __enterHierarchy];
}
} else if (self.nodeLoaded) {
Expand Down Expand Up @@ -2910,7 +2871,7 @@ - (void)_replaceSubnode:(ASDisplayNode *)oldSubnode withSubnode:(ASDisplayNode *

// Don't bother figuring out the sublayerIndex if in a rasterized subtree, because there are no layers in the
// hierarchy and none of this could possibly work.
if (nodeIsInRasterizedTree(self) == NO) {
if (subtreeIsRasterized(self) == NO) {
if (_layer) {
sublayerIndex = [_layer.sublayers indexOfObjectIdenticalTo:oldSubnode.layer];
ASDisplayNodeAssert(sublayerIndex != NSNotFound, @"Somehow oldSubnode's supernode is self, yet we could not find it in our layers to replace");
Expand Down Expand Up @@ -2956,7 +2917,7 @@ - (void)_insertSubnode:(ASDisplayNode *)subnode belowSubnode:(ASDisplayNode *)be

// Don't bother figuring out the sublayerIndex if in a rasterized subtree, because there are no layers in the
// hierarchy and none of this could possibly work.
if (nodeIsInRasterizedTree(self) == NO) {
if (subtreeIsRasterized(self) == NO) {
if (_layer) {
belowSublayerIndex = [_layer.sublayers indexOfObjectIdenticalTo:below.layer];
ASDisplayNodeAssert(belowSublayerIndex != NSNotFound, @"Somehow below's supernode is self, yet we could not find it in our layers to reference");
Expand Down Expand Up @@ -3020,7 +2981,7 @@ - (void)_insertSubnode:(ASDisplayNode *)subnode aboveSubnode:(ASDisplayNode *)ab

// Don't bother figuring out the sublayerIndex if in a rasterized subtree, because there are no layers in the
// hierarchy and none of this could possibly work.
if (nodeIsInRasterizedTree(self) == NO) {
if (subtreeIsRasterized(self) == NO) {
if (_layer) {
aboveSublayerIndex = [_layer.sublayers indexOfObjectIdenticalTo:above.layer];
ASDisplayNodeAssert(aboveSublayerIndex != NSNotFound, @"Somehow above's supernode is self, yet we could not find it in our layers to replace");
Expand Down Expand Up @@ -3078,7 +3039,7 @@ - (void)_insertSubnode:(ASDisplayNode *)subnode atIndex:(NSInteger)idx

// Don't bother figuring out the sublayerIndex if in a rasterized subtree, because there are no layers in the
// hierarchy and none of this could possibly work.
if (nodeIsInRasterizedTree(self) == NO) {
if (subtreeIsRasterized(self) == NO) {
// Account for potentially having other subviews
if (_layer && idx == 0) {
sublayerIndex = 0;
Expand Down
5 changes: 2 additions & 3 deletions Source/Private/ASDisplayNodeInternal.h
Original file line number Diff line number Diff line change
Expand Up @@ -286,10 +286,9 @@ FOUNDATION_EXPORT NSString * const ASRenderingEngineDidDisplayNodesScheduledBefo
- (ASPrimitiveTraitCollection)primitiveTraitCollection;

/**
* This is a non-deprecated internal declaration of the property. Public declaration
* is in ASDisplayNode+Beta.h
* Whether this node rasterizes its descendants. See -setShouldRasterizeDescendants.
*/
@property (nonatomic, assign) BOOL shouldRasterizeDescendants;
@property (atomic, readonly) BOOL shouldRasterizeDescendants;

- (void)nodeViewDidAddGestureRecognizer;

Expand Down
52 changes: 5 additions & 47 deletions Tests/ASDisplayNodeTests.mm
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
//
// ASDisplayNodeTests.m
// ASDisplayNodeTests.mm
// Texture
//
// Copyright (c) 2014-present, Facebook, Inc. All rights reserved.
Expand Down Expand Up @@ -1977,7 +1977,7 @@ - (void)testThatNodeGetsRenderedIfItGoesFromZeroSizeToRealSizeButOnlyOnce
- (void)testThatRasterizedNodesGetInterfaceStateUpdatesWhenContainerEntersHierarchy
{
ASDisplayNode *supernode = [[ASDisplayNode alloc] init];
supernode.shouldRasterizeDescendants = YES;
[supernode setShouldRasterizeDescendants];
ASDisplayNode *subnode = [[ASDisplayNode alloc] init];
ASSetDebugNames(supernode, subnode);
UIWindow *window = [[UIWindow alloc] initWithFrame:[UIScreen mainScreen].bounds];
Expand All @@ -1995,7 +1995,7 @@ - (void)testThatRasterizedNodesGetInterfaceStateUpdatesWhenContainerEntersHierar
- (void)testThatRasterizedNodesGetInterfaceStateUpdatesWhenAddedToContainerThatIsInHierarchy
{
ASDisplayNode *supernode = [[ASDisplayNode alloc] init];
supernode.shouldRasterizeDescendants = YES;
[supernode setShouldRasterizeDescendants];
ASDisplayNode *subnode = [[ASDisplayNode alloc] init];
ASSetDebugNames(supernode, subnode);

Expand All @@ -2010,52 +2010,10 @@ - (void)testThatRasterizedNodesGetInterfaceStateUpdatesWhenAddedToContainerThatI
XCTAssertFalse(subnode.isVisible);
}

- (void)testThatLoadedNodeGetsUnloadedIfSubtreeBecomesRasterized
{
ASDisplayNode *supernode = [[ASDisplayNode alloc] init];
[supernode view];
ASDisplayNode *subnode = [[ASDisplayNode alloc] init];
ASSetDebugNames(supernode, subnode);
[supernode addSubnode:subnode];
XCTAssertTrue(subnode.nodeLoaded);
supernode.shouldRasterizeDescendants = YES;
XCTAssertFalse(subnode.nodeLoaded);
}

- (void)testThatLoadedNodeGetsUnloadedIfAddedToRasterizedSubtree
{
ASDisplayNode *supernode = [[ASDisplayNode alloc] init];
supernode.shouldRasterizeDescendants = YES;
ASDisplayNode *subnode = [[ASDisplayNode alloc] init];
ASSetDebugNames(supernode, subnode);
[subnode view];
XCTAssertTrue(subnode.nodeLoaded);
[supernode addSubnode:subnode];
XCTAssertFalse(subnode.nodeLoaded);
XCTAssertTrue(ASHierarchyStateIncludesRasterized(subnode.hierarchyState));
}

- (void)testThatClearingRasterizationBitMidwayDownTheTreeWorksRight
{
ASDisplayNode *topNode = [[ASDisplayNode alloc] init];
topNode.shouldRasterizeDescendants = YES;
ASDisplayNode *middleNode = [[ASDisplayNode alloc] init];
middleNode.shouldRasterizeDescendants = YES;
ASDisplayNode *bottomNode = [[ASDisplayNode alloc] init];
ASSetDebugNames(topNode, middleNode, bottomNode);
[middleNode addSubnode:bottomNode];
[topNode addSubnode:middleNode];
XCTAssertTrue(ASHierarchyStateIncludesRasterized(bottomNode.hierarchyState));
XCTAssertTrue(ASHierarchyStateIncludesRasterized(middleNode.hierarchyState));
middleNode.shouldRasterizeDescendants = NO;
XCTAssertTrue(ASHierarchyStateIncludesRasterized(bottomNode.hierarchyState));
XCTAssertTrue(ASHierarchyStateIncludesRasterized(middleNode.hierarchyState));
}

- (void)testThatRasterizingWrapperNodesIsNotAllowed
{
ASDisplayNode *rasterizedSupernode = [[ASDisplayNode alloc] init];
rasterizedSupernode.shouldRasterizeDescendants = YES;
[rasterizedSupernode setShouldRasterizeDescendants];
ASDisplayNode *subnode = [[ASDisplayNode alloc] initWithViewBlock:^UIView * _Nonnull{
return [[UIView alloc] init];
}];
Expand Down Expand Up @@ -2110,7 +2068,7 @@ - (void)testThatSubnodeGetsInterfaceStateSetIfRasterized
{
ASTestDisplayNode *node = [[ASTestDisplayNode alloc] init];
node.debugName = @"Node";
node.shouldRasterizeDescendants = YES;
[node setShouldRasterizeDescendants];

ASTestDisplayNode *subnode = [[ASTestDisplayNode alloc] init];
subnode.debugName = @"Subnode";
Expand Down