3 problems with the ASCellNode Layout updating/diffing + example project

[djblake]

I have recently encountered some issues with ASCellNodes not updating properly when I believe they should, and created a simple example project which demonstrates these issues.

The first issue shows what happens if you try to add a subNode of an ASCellNode to a separate layer without properly removing it first (can no longer update the ASCellNode)

The second issue shows what happens if you update a sub node of an ASCellNode with a different ASDisplayNode of the same dimensions (setNeedsLayout has no effect).

The third issue shows what happens if a sub-sub-node of an ASCellNode is the same size as its parent node's minHeight (setNeedsLayout has no effect).

I have attached an example project showing these below:

TextureLayoutDifBug.zip

At the top of the 'ViewController' class I have described the steps to go through to observe each bug. The same text also pasted below here:

_

Layout Diffing Bugs

Each ASCellNode has 2 ASDisplayNodes, 'display1' and 'display2'. Whichever one of these that is displayed is controlled by a BOOL flag 'showDisplay1' in the TestCellNode class. Each cell has slightly different properties to show what elicits the bugs, you can see these in the 'formatForIndex:' method in the TestCellNode class.

The first example (Cells 0 and 1) demonstrate what happens if you try and add 'display1' to a new parent without removing it properly from the ASCellNode first. I'm not sure if this is a bug but I am told that both scenarios should work (Cell 1 doesnt). When you tap Cell 0 (blue), didSelectItemAtIndexPath: updates 'showDisplay1 = NO' and calls 'setNeedsLayout' so that 'display1' is properly removed from its parent ASCellNode. It is then added directly to 'ViewController' (via 'addSubnode:') for 1 second, and then removed with 'removeFromSupernode', 'showDisplay1 = YES' is called on its original ASCellNode and 'setNeedsLayout' redisplays it back in its original ASCellNode. All works fine. Cell 1 (yellow) skips the removing process, and directly adds it to the viewController via 'addSubnode:' - this works, but when calling 'setNeedsLayout' on the original cell one second later you'll notice it is never re-added.

The second example (Cells 2-4) demonstrate updating your layoutSpecThatFits with different layers of equivalent size, and the problems that occur. When these cells are tapped, the cell is updated with 'showDisplay1 = YES' and 'setNeedsDisplay' (causing display2 to be displayed instead of display1), and then 1 second later reverted back with 'showDisplay1 = NO' 'setNeedsDisplay'. You can see this in the 'didSelectItemAtIndexPath:' method.

Cell 2 (green) updates the 'display1' and 'display2' layers that are added directly to the 'TestCellNode' node. These are both the same size. Tapping this cell you will see the green display1 node disappear for a second to be replaced by the red display2 node. All works as expected. Cell 3 (orange) is exactly the same, except this time 'display1' and 'display2' are added to a further subnode (called 'InnerNode') in the TestCellNode. Although you can see layoutSpecThatFits is called by the NSLogs, the cell is not updated and remains orange (the colour of display1). Cell 4 (purple) is the same conditions as Cell 3, except the display2 layer has a different height to display1. This updates accordingly when you tap it.

The third example (Cells 5-6) demonstrates what happens if the 'display1' in the InnerNode is the same height (or lower) as the InnerNodes 'minHeight' property. This example is similar to the previous example, in that when you tap the cell it should switch display1 to display2 for a second, then switch back. For both Cells 5 and 6, display2 is a different size to display1, so it should be unaffected by the bug shown in the previous example and it should behave just like Cell 4..

Cell 5 (cyan) however shows that tapping it has no effect (even though the logs show layoutSpecThatFits is called). The height of 'display2' is set to the minHeight of its parent InnerNode, plus the value of 'self.contentHeightAdjust' which in this case is '0'. Cell 6 (magenta) has a 'self.contentHeightAdjust' of '1', making it 1 pointer greater than the minHeight of its parent InnerNode. As you can see, tapping Cell 6 updates with no problems.

_

[qqppooqq]

I do not understand what you want to express If you are so tangled, do not use automationManagesSubnodes, forcing yourself to manage nodes

[djblake]

My apologies, I will explain better.

What I am demonstrating is there a bug in the layout diffing algorithm when using automaticallyManageSubnodes. Presumably when someone calls setNeedsLayout, it calls layoutSpecThatFits:, and then compares the layoutSpec that was returned to the current one, analyses the differences between these, and then animates the changes. I am showing that in a certain situation, it fails to analyse the difference between two layouts.

The help bring some context to this issue, I will describe the project I am working on as an example.

In my project, I have an ASCellNode. This cell has a subnode 'Container' class added to it, which allows for dynamic content to be added to it. For example if a user was posting text, it would be the 'textContainer' subclass, if they were posting an image, the 'imageContainer' subclass. When its the imageContainer subclass, the heirarchy looks like this:

[ ASCellNode
    [imageContainer
         [image A]
    ]
] 

Suppose I want to change the image displayed to be a different image. I could have a BOOL flag which controls if image A or image B is shown. To update I could simply check the BOOL flag in layoutSpecThatFits: and add either image A or image B into the hierarchy when setNeedsLayout is called. Except this is where the bug lies. If image A and image B are the same dimensions, then the diffing algorithm will think they are the same and the layout will not actually get updated. You have to make the two images at least 1 pixel different for the setNeedsLayout to actually have an effect. This is that 'second' issue I mentioned originally above.

The third issue is something I came across whilst trying to work out what causes the second one. If the imageContainer has a 'minHeight' property set, and the height of image A or image B is the same as this minHeight or lower, then setNeedsDisplay will also fail to update. Presumably this is because image A and B are both forced to be the minHeight of their parent, making them both the same height, and thus failing to be determined to be different by the layout diffing bug.

I am not tangled - I understand the issues and how to avoid them, in my project I am just forcing the dimensions to be 1 point different when I call setNeedsLayout to avoid this issue. But I was asked to create an issue/example project for this bug so I did.

[djblake]

Upon further reflection on this issue, I have realised the issue is caused because the diffing algorithm does not presumably check subnodes recursively. Setting a 1 point different on image A vs image B causes the imageContainer parent to be a different height than it was before, and it is this difference that it is detecting has changed between the layouts, rather than image A vs image B changing size. I'm betting you could theoretically have both image A and image B appear simultaneously stacked next to each other, and as long as the sum of their dimensions does not change (ie [image A width = 20][image B width = 80] = 100 vs [image A width = 60][image B width = 40] = 100 ) it would not detect a difference.

The solution to this then would be to call setNeedsLayout directly on node that has changed - in my case the imageContainer node, rather that on its parent ASCellNode. I have just tried implementing this in my code and can confirm it works properly without the workaround setting the dimension difference.

Whether then if this is a bug or it is just an unavoidable consequence of not being able to call the diffing algorithm on subnodes recursively for performance reasons I will leave to someone with more experience!