P4
In a custom control that doesn't change size and lays out several nodes, it's possible to trigger a situation where layoutChildren stops being called despite having called `setNeedsLayout(true)`. This function is part of the public API and is documented as:
> Indicates that this Node and its subnodes requires a layout pass on the next pulse.
Replacing this call with either `requestLayout()` or a direct call to layoutChildren (or a similar method that contains the exact same logic) does not experience the same problem suggesting a problem with the dirty state tracking of layouts. Printing the current state of the node before calling `setNeedsLayout(true)` confirms this; normally the node will be clean and not in need of layout, but when the bug occurs, the node is dirty and requires layout. Querying the parent of the node (which normally should be clean) also reveals it needs layout, yet no layout occurs until some other layout invalidation method occurs (like resizing the window).
It's common for nodes that do not depend for their size on the size of their children to want to avoid doing a full layout pass. `requestLayout` will mark all ancestors and the current node with "NEEDS_LAYOUT", while `setNeedsLayout(true)` instead marks only the current node with "NEEDS_LAYOUT" and all ancestors with "DIRTY_BRANCH". Both will schedule a layout pass.
I've traced this problem to specific code in `Node` which monitors the layoutX and layoutY properties. This listener code can be called while a layout is being performed, typically because a layoutChildren method is called which uses something like layoutInArea which adjusts layoutX/Y -- this is correct and expected usage.
A safeguard in this listener code will not trigger another layout if the current child being laid out is the current node, however, for siblings this protection won't work as they're not the current child (or in case where layoutChildren is overridden this is simply not set as expected). When the safeguard fails, the listener code will then proceed to attempt to schedule a new layout for a managed child by calling `requestLayout(true)`. This immediately sets the NEEDS_LAYOUT flag on the node, but is unable to schedule a layout as we're already in a layout.
After the current layout completes, no further layouts are scheduled, yet some nodes in the scene graph have NEEDS_LAYOUT set - an inconsistent state. As this flag can block propagation to trigger layouts, setNeedsLayout(true) no longer functions for this node. Resizing the window (or triggering a full layout in some other way) gets the state consistent again, and it will function until next the bug occurs.
Workaround:
- Make absolutely sure that all children are unmanaged if intending to use setNeedsLayout(true); even a single managed child (even though layoutChildren is overridden) can cause this bug.
Expected:
- The managed or unmanaged state is irrelevant as layoutChildren is fully overridden, and setNeedsLayout makes no mention of this (and I don't think it should).
Solution:
- Node listeners should probably not be calling `p.requestLayout(true)` as it immediately sets layout flags but may not be able to schedule a new layout. They probably should call `requestParentLayout` (as it does in the other branch) as this method takes care to not mess up the state.
> Indicates that this Node and its subnodes requires a layout pass on the next pulse.
Replacing this call with either `requestLayout()` or a direct call to layoutChildren (or a similar method that contains the exact same logic) does not experience the same problem suggesting a problem with the dirty state tracking of layouts. Printing the current state of the node before calling `setNeedsLayout(true)` confirms this; normally the node will be clean and not in need of layout, but when the bug occurs, the node is dirty and requires layout. Querying the parent of the node (which normally should be clean) also reveals it needs layout, yet no layout occurs until some other layout invalidation method occurs (like resizing the window).
It's common for nodes that do not depend for their size on the size of their children to want to avoid doing a full layout pass. `requestLayout` will mark all ancestors and the current node with "NEEDS_LAYOUT", while `setNeedsLayout(true)` instead marks only the current node with "NEEDS_LAYOUT" and all ancestors with "DIRTY_BRANCH". Both will schedule a layout pass.
I've traced this problem to specific code in `Node` which monitors the layoutX and layoutY properties. This listener code can be called while a layout is being performed, typically because a layoutChildren method is called which uses something like layoutInArea which adjusts layoutX/Y -- this is correct and expected usage.
A safeguard in this listener code will not trigger another layout if the current child being laid out is the current node, however, for siblings this protection won't work as they're not the current child (or in case where layoutChildren is overridden this is simply not set as expected). When the safeguard fails, the listener code will then proceed to attempt to schedule a new layout for a managed child by calling `requestLayout(true)`. This immediately sets the NEEDS_LAYOUT flag on the node, but is unable to schedule a layout as we're already in a layout.
After the current layout completes, no further layouts are scheduled, yet some nodes in the scene graph have NEEDS_LAYOUT set - an inconsistent state. As this flag can block propagation to trigger layouts, setNeedsLayout(true) no longer functions for this node. Resizing the window (or triggering a full layout in some other way) gets the state consistent again, and it will function until next the bug occurs.
Workaround:
- Make absolutely sure that all children are unmanaged if intending to use setNeedsLayout(true); even a single managed child (even though layoutChildren is overridden) can cause this bug.
Expected:
- The managed or unmanaged state is irrelevant as layoutChildren is fully overridden, and setNeedsLayout makes no mention of this (and I don't think it should).
Solution:
- Node listeners should probably not be calling `p.requestLayout(true)` as it immediately sets layout flags but may not be able to schedule a new layout. They probably should call `requestParentLayout` (as it does in the other branch) as this method takes care to not mess up the state.