Fix ui depth system

[svents]

The current UI depth system looks a bit odd in some places. In particular, it treats nodes without children and nodes with empty children lists differently, and it effectively discards the results of previous computations when it progresses from one hierarchy tree to the next. I think that was not intentional and changed it to a system where the global z for each node increases by 1 in each step in a depth first traversal. I also added a unit test to make the changes in behavior transparent.
Two open points:

• The ordering of the root nodes is somewhat arbitrary (depends on archetypes and entity index, I suppose). From a user perspective I would expect that more recently spawned UI elements are shown in front, or alternatively that I could set an order and put elements in front.
• The depth system traverses the children of nodes without transforms, the transform-propagate system does not. This could be fixed by moving the children query inside the if-let statement that checks for the transform. Alternatively, the propagate system could be changed to treat missing transforms as the identity.

[cart]

I do like the improved legibility of a hard-coded approach over the previous "overly generic" hierarchy traversal abstraction. I also like that we have tests now to illustrate the behavior and prevent regressions 😄

I'm less a fan of using a heap-allocated stack on each frame to process these changes. I think I would prefer to use a recursive algorithm (to be "allocation free"). Alternatively we could cache the heap-allocated stack across frames, but this has the downside of (1) not resizing itself when the number of ui nodes fluctuates and (2) being a burst-ey allocation (see this discussion).

I think that was not intentional and changed it to a system where the global z for each node increases by 1 in each step in a depth first traversal.

Haha this was actually intentional, because the order really is arbitrary. Its "insertion order" for trivial examples, but it can quickly become effectively random once entities start changing archetypes. In general I want to encourage people to build their UI beneath a "root node".

Making it work "as expected" for trivial cases, but break down for non-trivial cases is actively harmful imo.

Rather than arbitrarily ordering two root nodes, we would instead let people set the "base z" value of the root node. This encourages people to fix the problem right away in a way that is predictable and easy to control (either by grouping the nodes under a root, or manually specifying base z values).

[cart]

It would also probably be good to detect when roots are at the same z-value and produce a warning.

[svents]

I'm less a fan of using a heap-allocated stack on each frame to process these changes. I think I would prefer to use a recursive algorithm (to be "allocation free").

Good point and thanks for the links! I am fine with a recursive solution, but technically you could also solve the issue with a sufficiently large SmallVec, I suppose?

Haha this was actually intentional, because the order really is arbitrary. Its "insertion order" for trivial examples, but it can quickly become effectively random once entities start changing archetypes.

OK, then I clearly misunderstood that code. 😁 I was confused because all root nodes end up at z=1.

Making it work "as expected" for trivial cases, but break down for non-trivial cases is actively harmful imo.

👍

Rather than arbitrarily ordering two root nodes, we would instead let people set the "base z" value of the root node. This encourages people to fix the problem right away in a way that is predictable and easy to control (either by grouping the nodes under a root, or manually specifying base z values).

If this base z is the z component of the transform, there would be edge cases for root nodes with close z values and many children, where some children are in front and some behind the next root node. If it is a new component/field in nodes, then it would matter only for root nodes.
Maybe a resource with a list of all root nodes? The ordering would avoid the issue with two roots having the same depth, and changing the order would be very convenient for the user. The disadvantage would be that the list has to be synced either by a system or when inserting/deleting nodes.

Also, what do you think about nodes without transform? I think it makes sense to stop the recursion at these nodes since the computed depth will not be propagated later anyway.

[cart]

Maybe a resource with a list of all root nodes?

That would work, but its overly centralized imo. I'm thinking we could add a new Style::z_position: Option<f32>, which would allow any node to manually specify a z value to override the "default" behavior.

Also, what do you think about nodes without transform? I think it makes sense to stop the recursion at these nodes since the computed depth will not be propagated later anyway.

This one is hard. I kind of want to let people use arbitrary entities to organize other entities (even nodes), but supporting "arbitrary container entities" means we are doing way more work to check component existence.

[cart]

Good point and thanks for the links! I am fine with a recursive solution, but technically you could also solve the issue with a sufficiently large SmallVec, I suppose?

Yup you could definitely do that. The downside is that we would want to make the smallvec very big to avoid allocations. this means that we would claim a large amount of stack space, even when that isn't necessary. The plus is that stack overflows would become entirely out of the question.

[cart]

Awesome. This looks good to me. Thanks!