[ASDisplayNode] Keepalive reference for layer-backed nodes about asyncdisplaykit HOT 7 CLOSED

Thanks for reporting this—I genuinely didn't realize this hadn't been wired up. Fortunately it's a rare case that is generally avoidable by slight tweaks in app code, but you're absolutely right that it should be clearly documented if there must be an inconsistency here.

from asyncdisplaykit.

The kCAOnOrderIn and kCAOnOrderOut actions on CALayer are almost analogous to -[UIView willMoveToWindow:].

from asyncdisplaykit.

Thanks for chiming in @grp -- I figured the same thing.

@ianolito do you know of behavior of the kCA* actions that implies they will be unsuitable for implementing the keepalive? Shortly before launch (and before this task) I noticed that the -willEnterHierarchy and -didExitHierarchy calls behaved slightly different for view-backed and layer-backed nodes, and I have a personal task filed to unify both of them so they are driven by the kCA actions (even when the UIKit calls are available, just don't use them for consistency).

My hope is that in the same diff I can just change the keepalive to be driven by the same methods, and hold a strict guarantee that there are no unintuitive behavior changes when enabling layer-backing (the only ones being core UIView features not being called / used, like touch handling and fine-grained accessibility).

from asyncdisplaykit.

The problem with the kCAOrderIn style actions is that they also trigger for .hidden, so it's not a complete replacement. Also, UIView wants to be the delegate of the layer so you'd need to handle those and pass them on within AsyncView to the node.

I think conceptually the .hidden is surprising for users but I didn't see another simple way to accomplish this with layer apis.

Le Oct 19, 2014 à 19:21, appleguy [email protected] a écrit :

Thanks for chiming in @grp -- I figured the same thing.

@ianolito do you know of behavior of the kCA* actions that implies they will be unsuitable for implementing the keepalive? Shortly before launch (and before this task) I noticed that the -willEnterHierarchy and -didExitHierarchy calls behaved slightly different for view-backed and layer-backed nodes, and I have a personal task filed to unify both of them so they are driven by the kCA actions (even when the UIKit calls are available, just don't use them for consistency).

My hope is that in the same diff I can just change the keepalive to be driven by the same methods, and hold a strict guarantee that there are no unintuitive behavior changes when enabling layer-backing (the only ones being core UIView features not being called / used, like touch handling and fine-grained accessibility).

—
Reply to this email directly or view it on GitHub.

from asyncdisplaykit.

A regular view gets retained as soon as it is passed to addSubview:. But if we were to rely on the onOrder events, a node would only be retained properly when addSubview: is called on a hierarchy that is already visible.

For instance, the unit test testAddingNodeToHierarchyRetainsNode would not pass anymore.

Typically, such operations happen in a view controller's viewDidLoad method, where self.view is not attached to any window yet. Any node that is not retained would end up being deallocated right at the end of the method, before it even receives the onOrderIn event.

Also, there are other cases where view/layer hierarchies are moved in and out of windows. For example, presenting a fullscreen view controller causes the onOrderOut event to be sent to the whole hierarchy that gets hidden beneath. This would cause the keepalive reference to be released.

I don't know enough about CA internals to come up with other ideas here, except intercepting all calls to -[CALayer addSublayer:] (yuck). Then there's the simpler option of requiring all nodes to be retained as long as they are used in a hierarchy.

@darknoon I don't think the onOrder events are triggered for changes to the hidden property. (Seems the documentation is wrong about this. Since 2010.)

from asyncdisplaykit.

Would it work if we added a category on UIView that offered: -addSubnode:, which would retain the node? The issue with this is you probably want to insert it at positions relative to other subviews/sublayers and so the api might not be very simple. -removeFromSupernode would also work in this case.

Le Oct 19, 2014 à 21:43, Ian Cloutier [email protected] a écrit :

Exactly. A regular view gets retained as soon as it is passed to addSubview. But if we were to rely on the onOrder events, a node would only be retained properly when addSubview is called on a hierarchy that is already visible.

For instance, the unit test testAddingNodeToHierarchyRetainsNode would not pass anymore.

Typically, such operations happen in a view controller's viewDidLoad method, where self.view is not attached to any window yet. Any node that is not retained would end up being deallocated right at the end of the method, before it even receives the onOrderIn event.

Also, there are other cases where view/layer hierarchies are moved in and out of windows. For example, presenting a fullscreen view controller causes the onOrderOut event to be sent to the whole hierarchy that gets hidden beneath. This would cause the keepalive reference to be released.

I don't know enough about CA internals to come up with other ideas here, except intercepting all calls to CALayer addSublayer (yuck). Then there's the simpler option of requiring all nodes to be retained as long as they used in a hierarchy.

—
Reply to this email directly or view it on GitHub.

from asyncdisplaykit.

Even with extensive use of layer-backed nodes, I really haven't been hitting this issue. You should have at least one other thing retaining the node - at the minimum, a subnode array. This is one reason why ASCellNode refuses to be layer-backed, and neither can collection/table/pagernode or ASViewController.

That said, I will move this issue to our new tracker, primarily to openly document the subtle behavior differences that come with layer backing and also as a place holder for a future investigation of possible improvements in this area.

from asyncdisplaykit.