[ASDisplayNode] Notify rasterized subnodes that render pass has completed #532
Conversation
|
|
There was a problem hiding this comment.
Thanks for submitting this diff, especially with a unit test!
One question for you. Should displayWillStart be called on rasterized subnodes as well, probably here? If not, I think we'll have imbalance calls on the two delegations (e.g displayWillStart and displayDidFinish).
While this change looks good to me, I'd like to have another review from a core member before merging.
| @@ -347,6 +349,12 @@ - (void)displayAsyncLayer:(_ASDisplayLayer *)asyncLayer asynchronously:(BOOL)asy | |||
| layer.contents = (id)image.CGImage; | |||
| } | |||
| [self didDisplayAsyncLayer:self.asyncLayer]; | |||
|
|
|||
| if (_flags.rasterizesSubtree) { | |||
There was a problem hiding this comment.
Since we no longer hold the instance lock by the time this completionBlock is executed, I think we should either grab this flag at the beginning of this method and use it here, or call self.rasterizesSubtree here.
|
Thanks for your review @nguyenhuy ! I've added a commit to grab the flag with the instance lock held. I agree that we should balance the delegate calls. While updating the unit test for this I noticed that |
|
Awesome, looking forward to the tests! To answer your question, no it's not expected. I submitted a PR yesterday to address it: #536. |
|
Ah yes I see! I've added a call to |
…eted (TextureGroup#532) * Notify rasterized subsides that render pass has completed * Traverse entire subnode tree notifying all subnodes * Add entry in changelog * Retrieve rasterizesSubtree flag while holding instance lock * Balance display delegate calls for rasterized subnodes
After the rendering pass of an
ASDisplayNodehas completed thedidDisplayAsyncLayer:method is invoked which will triggerdisplayDidFinish. However, in a hierarchy that is rasterized the rendering of the subnodes is done by rasterizing them, hencedidDisplayAsyncLayer:anddisplayDidFinishwill only be invoked on the root node and not on the subnodes.So subnodes that are in a rasterized hierarchy will not be notified that the rendering pass has completed. I came across this issue when working with
ASNetworkImageNode. BothASNetworkImageNodeandASMultiplexImageNodedepend ondisplayDidFinishto notify their delegates that they finished displaying their image, which won't happen if they're in a rasterized tree.