godot icon indicating copy to clipboard operation
godot copied to clipboard

Account for physics interpolation and transform snapping when Y-sorting

Open kleonc opened this issue 8 months ago • 10 comments

Makes the transforms used for Y-sorting canvas items match the transforms used for such items in RendererCanvasCull::_cull_canvas_item, aka it accounts for physics interpolation and transform snapping.

Fixes #92982.

This is done in the first commit as a simple change which results in calculating physics interpolation and transform snapping twice for each y-sorted item: when determining transforms used for y-sorting while gathering to-be-y-sorted items, and when actually handling them after they're y-sorted (also note that calculating transforms twice introduces a chance of using different transforms for sorting/rendering in case there's some mismatch in calculations).


The second commit makes the transform calculations (physics interpolation, transform snapping) happen once for each y-sorted item, before y-sorting (so it's sorted according to the position used later for rendering).

For reviewing the second commit I recommend looking at combined first+second commits diff, should be simpler.

In more detail:

RendererCanvasCull::Item::ysort_xform was previously storing a transform from the parent of the given item to the y-sorted subtree's root, now it's changed to store a transform from the given item to the y-sorted subtree's root instead. Meaning previously y-sorted item's final transform was ysorted_subtree_root.final_xform * item.ysort_xform * item.self_xform, now it's ysorted_subtree_root.final_xform * item.ysort_xform as item.self_xform is calculated before y-sorting and it's included into item.ysort_xform to not need to recalculate it later.

As a result RendererCanvasCull::Item::ysort_pos was removed as no longer needed, now item.ysort_xform.origin is the exact same position (the item's position in y-sorted subtree's root coordinate space) which can be used for sorting instead.

Renamed RendererCanvasCull::_cull_canvas_item's p_allow_y_sort parameter to p_is_already_y_sorted as it seemed more intuitive to me, given it's usage (and negated the passed value everywhere).

So now when p_is_already_y_sorted == true the item's final transform was already calculated during y-sorting, and it's passed as p_parent_xform to RendererCanvasCull::_cull_canvas_item (kinda disliking this naming/behavior discrepancy but oh well). Thus in such case self/local transform is not being recalculated, and thus it's not available (only the final transform is passed).

Also extracted the count-only part of RendererCanvasCull::_collect_ysort_children into separate RendererCanvasCull::_count_ysort_children as with the transform calculations added it seemed too "ify". And moved _mark_ysort_dirty into RendererCanvasCull as well.

I guess that's it, feel free to ask for any further explanation/clarification in case something is unclear.

Oh, and I'd understand if the second commit is considered too risky for cherry-picking into 4.3.stable etc. so of course I could split it into 2 PRs or whatever else is desired.

kleonc avatar Jun 11 '24 09:06 kleonc