YGNodeFree does not mark parent as dirty

[robbert-ef]

Report

• I have searched existing issues and this is not a duplicate

Issues and Steps to Reproduce

YGNodeRef root = YGNodeNew();

YGNodeRef child0 = YGNodeNew();
YGNodeInsertChild(root, child0, 0);

YGNodeRef child1 = YGNodeNew();
YGNodeInsertChild(root, child1, 1);

YGNodeRef child2 = YGNodeNew();
YGNodeInsertChild(root, child2, 2);

YGNodeRef child3 = YGNodeNew();
YGNodeInsertChild(child2, child3, 0);

// Calculate so all nodes are clean
YGNodeCalculateLayout(root, YGUndefined, YGUndefined, YGDirectionLTR);

bool dirty_before = YGNodeIsDirty(root); // == false

// YGNodeFree does not mark root as dirty
YGNodeFree(child0);
bool dirty_after_free_child0 = YGNodeIsDirty(root); // == false

// YGNodeFreeRecursive does not mark as dirty when node has no children
YGNodeFreeRecursive(child1);
bool dirty_after_free_child1 = YGNodeIsDirty(root);  // == false

// YGNodeFreeRecursive marks as dirty because it calls YGNodeRemoveChild which marks dirty
YGNodeFreeRecursive(child2);
bool dirty_after_free_child2 = YGNodeIsDirty(root); // == true

Expected Behavior

I would expect YGNodeFree and YGNodeFreeRecursive to always mark its parents as dirty as it affects the positioning and sizing of other nodes.

Actual Behavior

YGNodeFree never marks its parent as dirty
YGNodeFreeRecursive only marks its parents as dirty when it has children, this is because it calls YGNodeRemoveChild which marks the parent as dirty.

[NickGerleman]

This seems like a sane enough behavior to change, with the current model where YGNodeFree will mutate the tree around it to disconnect the node first.

An aside, I don't like that YGNodeFree manipulates the tree around it, instead of just freeing the node. E.g., we needed to add a separate YGNodeFinalize which doesn't do this, because GC'ing Yoga nodes could happen in parallel in Java bindings. It's probably not worth breaking the existing folks relying on this behavior, but I would consider doing any sort of unhooking of nodes from your Yoga tree, before freeing parts of it.

Probably not going to happen any time soon, but I have had some desire to add an intrinsic ref-count to the nodes, and deprecate YGNodeFree for something like YGNodeRelease. I think it could be done in a backwards-compatible way.