Skip to content

Specialized UI transform #16615

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 69 commits into
base: main
Choose a base branch
from

Conversation

ickshonpe
Copy link
Contributor

@ickshonpe ickshonpe commented Dec 2, 2024

Objective

Add specialized UI transform components and fix some related problems:

  • Animating UI elements by modifying the Transform component of UI nodes doesn't work very well because ui_layout_system overwrites the translations each frame. The overflow_debug example uses a horrible hack where it copies the transform into the position that'll likely cause a panic if any users naively copy it.
  • Picking ignores rotation and scaling and assumes UI nodes are always axis aligned.
  • The clipping geometry stored in CalculatedClip is wrong for rotated and scaled elements.
  • Transform propagation is unnecessary for the UI, the transforms can be updated during layout updates.
  • The UI internals use both object-centered and top-left-corner-based coordinates systems for UI nodes. Depending on the context you have to add or subtract the half-size sometimes before transforming between coordinate spaces. We should just use one system consistantly so that the transform can always be directly applied.
  • Transform doesn't support responsive coordinates.

Solution

  • Unrequire Transform from Node.
  • New components UiTransform, UiGlobalTransform:
    • Node requires UiTransform, UiTransform requires UiGlobalTransform
    • UiTransform is a 2d-only equivalent of Transform with a translation in Vals.
    • UiGlobalTransform newtypes Affine2 and is updated in ui_layout_system.
  • New helper functions on ComputedNode for mapping between viewport and local node space.
  • The cursor position is transformed to local node space during picking so that it respects rotations and scalings.
  • To check if the cursor hovers a node recursively walk up the tree to the root checking if any of the ancestor nodes clip the point at the cursor. If the point is clipped the interaction is ignored.
  • Use object-centered coordinates for UI nodes.
  • RelativeCursorPosition's coordinates are now object-centered with (0,0) at the the center of the node and the corners at (±0.5, ±0.5).
  • Replaced the normalized_visible_node_rect: Rect field of RelativeCursorPosition with cursor_over: bool, which is set to true when the cursor is over an unclipped point on the node. The visible area of the node is not necessarily a rectangle, so the previous implementation didn't work.

This should fix all the logical bugs with non-axis aligned interactions and clipping. Rendering still needs changes but they are far outside the scope of this PR.

Tried and abandoned two other approaches:

  • New transform field on Node, require GlobalTransform on Node, and unrequire Transform on Node. Unrequiring Transform opts out of transform propagation so there is then no conflict with updating the GlobalTransform in ui_layout_system. This was a nice change in its simplicity but potentially confusing for users I think, all the GlobalTransform docs mention Transform and having special rules for how it's updated just for the UI is unpleasently surprising.
  • New transform field on Node. Unrequire Transform on Node. New transform: Affine2 field on ComputedNode.
    This was okay but I think most users want a separate specialized UI transform components. The fat ComputedNode doesn't work well with change detection.

Fixes #18929, #18930

Migration Guide

New specialized 2D UI transform components UiTransform and UiGlobalTransform. UiTransform is a 2d-only equivalent of Transform with a translation in Vals. UiGlobalTransform newtypes Affine2 and is updated in ui_layout_system.
Node now requires UiTransform instead of Transform. UiTransform requires UiGlobalTransform.

In previous versions of Bevy ui_layout_system would overwrite UI node's Transform::translation each frame. UiTransforms aren't overwritten and there is no longer any need for systems that cache and rewrite the transform for translated UI elements.

RelativeCursorPosition's coordinates are now object-centered with (0,0) at the the center of the node and the corners at (±0.5, ±0.5). Its normalized_visible_node_rect field has been removed and replaced with a new cursor_over: bool field which is set to true when the cursor is hovering an unclipped area of the UI node.

@alice-i-cecile alice-i-cecile added A-UI Graphical user interfaces, styles, layouts, and widgets X-Contentious There are nontrivial implications that should be thought through C-Usability A targeted quality-of-life change that makes Bevy easier to use D-Straightforward Simple bug fixes and API improvements, docs, test and examples S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Dec 2, 2024
@alice-i-cecile
Copy link
Member

alice-i-cecile commented Dec 2, 2024

I like this as an incremental move forward. We shouldn't be storing a full Transform here, but that's easy to migrate from this PR.

Should this be on ComputedNode instead though? I don't think this is intended to be user-modifiable.

@alice-i-cecile alice-i-cecile added the M-Needs-Migration-Guide A breaking change to Bevy's public API that needs to be noted in a migration guide label Dec 2, 2024
Copy link
Contributor

github-actions bot commented Dec 2, 2024

It looks like your PR is a breaking change, but you didn't provide a migration guide.

Could you add some context on what users should update when this change get released in a new version of Bevy?
It will be used to help writing the migration guide for the version. Putting it after a ## Migration Guide will help it get automatically picked up by our tooling.

@NthTensor
Copy link
Contributor

This does seem like the least-bad option. But I thought there was code in bevy_transform to specifically filter out ui elements from propagation. Could you check if that's still there and remove it if it is?

I believe this should fix the issues big_space had with the use of Transform by ui elements, which is nice.

@ickshonpe
Copy link
Contributor Author

ickshonpe commented Dec 2, 2024

I like this as an incremental move forward. We shouldn't be storing a full Transform here, but that's easy to migrate from this PR.

Should this be on ComputedNode instead though? I don't think this is intended to be user-modifiable.

The new transform field on Node is intended to be user-modifiable. It's meant to be simar to the CSS transform property and allows users to add simple UI animations or whatever without weird hacks to get around the conflicts between the transformation propagation and UI layout update systems.

I made another branch first just before this one that removed both Transform and GlobalTransform from UI nodes and instead added the transform to ComputedNode but it felt like it would be difficult to get through review as it required lots of changes to extraction and focus and new a UI specific visibility system. Performance is much better than main with that approach though and would allow for some nice helper methods on Node like contains_point etc which would simplify a lot of the systems.

@ickshonpe
Copy link
Contributor Author

ickshonpe commented Dec 2, 2024

This does seem like the least-bad option. But I thought there was code in bevy_transform to specifically filter out ui elements from propagation. Could you check if that's still there and remove it if it is?

I believe this should fix the issues big_space had with the use of Transform by ui elements, which is nice.

I haven't seen anything like this. Afaik the only way to opt-out of transform propagation is by removing Transform and there's no UI specific filter.

@alice-i-cecile alice-i-cecile added the A-Animation Make things move and change over time label Dec 2, 2024
@BenjaminBrienen BenjaminBrienen added S-Waiting-on-Author The author needs to make changes or address concerns before this can be merged and removed S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Jan 22, 2025
@alice-i-cecile alice-i-cecile removed the S-Waiting-on-Author The author needs to make changes or address concerns before this can be merged label Apr 23, 2025
… switching between corner-based and object-centered coords.

`RelativeCursorPosition`'s coordinates are now object-centered with (0,0) at the the center of the node and the corners at  (±0.5, ±0.5).
@ickshonpe ickshonpe changed the title New UI transform Specialized UI transform Apr 25, 2025
@ickshonpe
Copy link
Contributor Author

@alice-i-cecile I think this is more or less good enough now. Abandoned the weird ideas I had, just got a UiTransform component that replaces Transform. I wanted to keep this a simple PR but ended up making a lot of other changes, figured it's better to fix everything here than migrate all the bugs to the new setup.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Animation Make things move and change over time A-Rendering Drawing game state to the screen A-Transform Translations, rotations and scales A-UI Graphical user interfaces, styles, layouts, and widgets C-Bug An unexpected or incorrect behavior C-Feature A new feature, making something new possible C-Usability A targeted quality-of-life change that makes Bevy easier to use M-Needs-Migration-Guide A breaking change to Bevy's public API that needs to be noted in a migration guide M-Needs-Release-Note Work that should be called out in the blog due to impact S-Needs-Review Needs reviewer attention (from anyone!) to move forward X-Contentious There are nontrivial implications that should be thought through
Projects
Status: Widget-ready
Status: No status
Development

Successfully merging this pull request may close these issues.

Responsive translation support for UI nodes
5 participants