Skip to content
This repository was archived by the owner on Feb 2, 2023. It is now read-only.

Conversation

appleguy
Copy link
Contributor

Improve UIView & CALayer handling of -addSubnode:, and ensure node hierarchies are hooked up even when addSubview: is used directly.

@1nput0utput, could you please test this? I have already done so with your app and am pretty sure it is the complete fix. This one was a hell of a hard time to develop. I tried all combinations of addSubview / addSubnode for both the ingoing and outgoing of the animation (including mismatched ones - going wild over here!)

I did quite a bit of debugging for the flash, but sadly and strangely have not been able to locate it yet. The contents of the image node is NOT being cleared and the image is NOT re-decoding. So I think the actual cause is likely that the view controllers are not swapped out in the same runloop or something like that.

Just so you know, the main reason this was so hard is that at the end of the animation you were adding the node back to the KittenNode while the hierarchy that owned the KittenNode (ASTableView etc) itself was not onscreen - its window was considered nil - and so this introduced some trickiness, but I think that is now handled.

I want to make sure the flicker is fixed, but I'm running low on time to debug that specifically. Could you please take a look at a technique such as marking the other view .hidden or .alpha = 0.0, instead of removing it from the window completely, or at least verify that it is added and in the hierarchy behind the view that is being faded out before the transition starts?

…erarchies are hooked up even when addSubview: is used directly.
@appleguy
Copy link
Contributor Author

This is intended to address #958

@1nput0utput
Copy link

@appleguy Thanks a lot and I am sorry that this bug put you through the torture :). I am going to test this one on my main project tonight.

As for the flickering, I think you might be right about view controllers swapping, I'll try a custom view controller transition, it can maybe fix this flicker. I was also thinking about overlaying the image view controller on tap with its on image view and then do the transition by either hiding or changing the alpha as you suggested. This way, maybe, I can avoid the whole mess I made in the image view controller.

… of view-backed nodes as children of layer-backed ones.
@appleguy
Copy link
Contributor Author

@1nput0utput no worries - in the end this was actually a very important improvement to the framework's robustness. As you can see if you look closely, I even had to update some of the expected values from a couple unit tests because of the new behavior! It did take me half the day to get all the details right, but it is very satisfying to know that even the bizarre edge cases of something like a view-backed node contained within a layer-backed node (which actually asserts / is not supposed to happen, but could be forced at runtime by a crazy-ass user) should work correctly!!

As you are debugging the flickers, a key tip is to set a breakpoint in the -clearContents method of ASDisplayNode and also in -setNeedsDisplay on _ASAsyncLayer, and possibly the + drawing method (full selector is in ASDisplayNode+Subclasses.h) of the subclass in question, like ASImageNode. If any of these trigger then you'll quickly be able to see what is triggering the node to redraw, and if it does redraw it will often flicker.

If you come up with a case where you see those triggering and you don't want them to, please let me know, because I can almost certainly fix it quickly. The case in your test app right now is flicking WITHOUT any of those happening so I'm pretty sure it is the app, but it is VERY possible that as you are swapping nodes between different view and node hosting containers that the specific call order involved and other details could cause -clearContents to be triggered when you don't want it to. In any case I'll help you get past that issue ASAP.

Do I have your email address? Shoot me a note at asyncdisplaykit@gmail.com so I can ping you directly with other thoughts.

@1nput0utput
Copy link

@appleguy my main app is working as expected without making any changes mentioned in #928. I am sending you an email as I type this comment.

appleguy added a commit that referenced this pull request Dec 20, 2015
[ASDisplayNode] Automatically find and hook up node hierarchies, even when -addSubview: is used directly.
@appleguy appleguy merged commit b787020 into master Dec 20, 2015
@appleguy appleguy deleted the AutomagicalNodeHierarchyHookups branch December 20, 2015 20:06
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants