bevyengine / bevy

A refreshingly simple data-driven game engine built in Rust
https://bevyengine.org
Apache License 2.0
36.4k stars 3.59k forks source link

Only use physical coords internally in `bevy_ui` #16375

Closed ickshonpe closed 3 days ago

ickshonpe commented 1 week ago

Objective

We switch back and forwards between logical and physical coordinates all over the place. Systems have to query for cameras and the UiScale when they shouldn't need to. It's confusing and fragile and new scale factor bugs get found constantly.

Solution

Migration Guide

ComputedNode's fields and methods now use physical coordinates. ComputedNode has a new field inverse_scale_factor. Multiplying the physical coordinates by the inverse_scale_factor will give the logical values.

github-actions[bot] commented 5 days ago

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.

ickshonpe commented 4 days ago

@atlv24 I ran the UI testbed and a couple more examples. But, now I checked some other random example and there are indeed issues. 😬

So… I've run all the UI examples and this is what I've found:

  • border width are different (example: borders, ui_texture_atlas)
  • texture atlas slice border width (example: ui_texture_atlas_slice, ui_texture_slice_flip_and_tile, ui_texture_slice)
  • border radius are different (example: box_shadow)
  • text wrapping (examples: grid sidebar text, scroll, text_debug, text_wrap_debug)
  • wrong text/box (?) alignment, could be because of the wrapping issue (examples: display_and_visibility, text, size_constraints)
  • overflow clip margin (example: overflow_clip_margin)
  • a panic! (example: overflow_debug)

All of these should be fixed now I think.

doup commented 4 days ago

I've checked the examples that had issues, I still see some:

thread 'Compute Task Pool (1)' panicked at /rustc/f6e511eec7342f59a25f7c0534f1dbea00d01b14/library/core/src/num/f32.rs:1433:9:
min > max, or either was NaN. min = 0.0, max = NaN
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
Encountered a panic in system `bevy_ui::layout::ui_layout_system`!
Encountered a panic in system `bevy_app::main_schedule::Main::run_main`!