fix occasional crash moving ui root nodes

[robtfm]

Objective

fix an occasional crash when moving ui root nodes between cameras.

occasionally, updating the TargetCamera of a ui element and then removing the element causes a crash.

i believe that is because when we assign a child in taffy, the old parent doesn't remove that child from it's children, so we have:

user: create root node N1, camera A
-> layout::set_camera_children(A) : 
	- create implicit node A1
	- assign 1 as child -> taffy.children[A1] = [N1], taffy.parents[1] = A1

user: move root node N1 to camera B
-> layout::set_camera_children(B) :
	- create implicit node B1
	- assign 1 as child -> taffy.children[A1] = [N1], taffy.children[B1] = [N1], taffy.parents[1] = B1
-> layout::set_camera_children(A) :
	- remove implicit node A1 (which still has N1 as a child) -> 
		-> taffy sets parent[N1] = None ***
		-> taffy.children[B1] = [N1], taffy.parents[1] = None

user: remove N1
-> layout::remove_entities(N1)
	- since parent[N1] is None, it's not removed from B1 -> taffy.children[B1] = [N1], taffy.parents[1] is removed
-> layout::set_camera_children(B)
	- remove implicit node B1
	- taffy crash accessing taffy.parents[N1]

Solution

we can work around this by making sure to remove the child from the old parent if one exists (this pr).

i think a better fix may be for taffy to check in Taffy::remove and only set the child's parent to None if it is currently equal to the node being removed but i'm not sure if there's an explicit assumption we're violating here (@nicoburns).

[nicoburns]

i think a better fix may be for taffy to check in Taffy::remove and only set the child's parent to None if it is currently equal to the node being removed but i'm not sure if there's an explicit assumption we're violating here (@nicoburns).

Taffy's tree-manipulation methods need a review in general, and probably to moved closer to a browser-like API where there is no "set_parent", only "appendChild" (possibly with a set of paralell "unsafe/unchecked" methods where explicit invariants must be upheld). For the time being, your suggested workaround seems sensible.