Skip to content
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

Consistent screen-space coordinates #8306

Merged

Conversation

tim-blackbird
Copy link
Contributor

Objective

Make the coordinate systems of screen-space items (cursor position, UI, viewports, etc.) consistent.

Solution

Remove the weird double inversion of the cursor position's Y origin. Once in bevy_winit to the bottom and then again in bevy_ui back to the top.
This leaves the origin at the top left like it is in every other popular app framework.

Update the world_to_viewport, viewport_to_world, and viewport_to_world_2d methods to flip the Y origin (as they should since the viewport coordinates were always relative to the top left).

Migration Guide

Window::cursor_position now returns the position of the cursor relative to the top left instead of the bottom left.
This now matches other screen-space coordinates like RelativeCursorPosition, UI, and viewports.

The world_to_viewport, viewport_to_world, and viewport_to_world_2d methods on Camera now return/take the viewport position relative to the top left instead of the bottom left.

If you were using world_to_viewport to position a UI node the returned y value should now be passed into the top field on Style instead of the bottom field.
Note that this might shift the position of the UI node as it is now anchored at the top.

If you were passing Window::cursor_position to viewport_to_world or viewport_to_world_2d no change is necessary.

@alice-i-cecile alice-i-cecile added A-Rendering Drawing game state to the screen A-Windowing Platform-agnostic interface layer to run your app in A-UI Graphical user interfaces, styles, layouts, and widgets C-Code-Quality A section of code that is hard to understand or change 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 labels Apr 5, 2023
@alice-i-cecile alice-i-cecile requested a review from superdump April 5, 2023 18:04
parent.spawn(
TextBundle::from_section(label, label_text_style.clone()).with_style(Style {
position_type: PositionType::Absolute,
bottom: Val::Px(0.),
Copy link
Contributor Author

@tim-blackbird tim-blackbird Apr 5, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had to rework the labels in the blend_modes example a bit to keep their position the same as before.
Not a usability regression, it's just that for the labels to display correctly they need to be 'anchored' to the bottom left which happened to align with what world_to_viewport returned before.

Solved by adding a parent node and using bottom: Val::Px(0.), for the text itself.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I'm happy with the changes here!

Copy link
Contributor

@superdump superdump left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this looks good but need to recheck when I’m less tired.

crates/bevy_render/src/camera/camera.rs Show resolved Hide resolved
@cart cart added this pull request to the merge queue Apr 5, 2023
Merged via the queue into bevyengine:main with commit 585baf0 Apr 5, 2023
@tim-blackbird tim-blackbird deleted the consistent-screen-space-coordinates branch April 9, 2023 19:24
github-merge-queue bot pushed a commit that referenced this pull request Aug 17, 2023
# Objective

Fixes #9455

This change has probably been forgotten in
#8306.

## Solution

Remove the inversion of the Y axis when propagates window change back to
winit.
cart pushed a commit that referenced this pull request Aug 18, 2023
# Objective

Fixes #9455

This change has probably been forgotten in
#8306.

## Solution

Remove the inversion of the Y axis when propagates window change back to
winit.
nelson137 added a commit to nelson137/gambit that referenced this pull request Nov 8, 2023
The mouse origin moved from the bottom left to the top left. See
[relevant PR](bevyengine/bevy#8306).
Subserial pushed a commit to Subserial/bevy_winit_hook that referenced this pull request Jan 24, 2024
# Objective

Fixes #9455

This change has probably been forgotten in
bevyengine/bevy#8306.

## Solution

Remove the inversion of the Y axis when propagates window change back to
winit.
@nablabla
Copy link

nablabla commented Feb 20, 2024

please provide a formula to convert, guessing is so frustrating.
That is what i did, it should be a minor change. Or is there a easier way?

let Some(screen_pos) = window.cursor_position()
let window_size = Vec2::new(window.width() as f32, window.height() as f32);
// convert screen position [0..resolution] to ndc [-1..1] (gpu coordinates)
let ndc = - (screen_pos / window_size); // * 2.0 - Vec2::ONE;
// matrix for undoing the projection and camera transform
let ndc_to_world = camera_transform.compute_matrix() * camera.projection_matrix().inverse();
// use it to convert ndc to world-space coordinates
let world_pos_3d = ndc_to_world.project_point3(ndc.extend(-1.0));
println!("world_pos_3d: {}/{}", world_pos_3d.x, world_pos_3d.y);
for (clickable, trans) in clickable.iter_mut() {
  // find the top-most instance
  let model_pos = trans.compute_matrix().inverse().project_point3(world_pos_3d).truncate();			
  if   clickable.check_clicked(model_pos) && trans.translation.z > clicked_z {
    clicked_z = trans.translation.z;
    clicked = Some(clickable);
  }
}

@nablabla
Copy link

nablabla commented Feb 21, 2024

please provide a formula to convert, guessing is so frustrating. That is what i did, it should be a minor change. Or is there a easier way?

let Some(screen_pos) = window.cursor_position()
let window_size = Vec2::new(window.width() as f32, window.height() as f32);
// convert screen position [0..resolution] to ndc [-1..1] (gpu coordinates)
let ndc = - (screen_pos / window_size); // * 2.0 - Vec2::ONE;
// matrix for undoing the projection and camera transform
let ndc_to_world = camera_transform.compute_matrix() * camera.projection_matrix().inverse();
// use it to convert ndc to world-space coordinates
let world_pos_3d = ndc_to_world.project_point3(ndc.extend(-1.0));
println!("world_pos_3d: {}/{}", world_pos_3d.x, world_pos_3d.y);
for (clickable, trans) in clickable.iter_mut() {
  // find the top-most instance
  let model_pos = trans.compute_matrix().inverse().project_point3(world_pos_3d).truncate();			
  if   clickable.check_clicked(model_pos) && trans.translation.z > clicked_z {
    clicked_z = trans.translation.z;
    clicked = Some(clickable);
  }
}

Okay, I found it out, to restore the previous behaviour, simply do this:
let screen_pos = Vec2::new(screen_pos.x, window.height() as f32 - screen_pos.y);
I think something like this should be in the migration guide

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Rendering Drawing game state to the screen A-UI Graphical user interfaces, styles, layouts, and widgets A-Windowing Platform-agnostic interface layer to run your app in C-Code-Quality A section of code that is hard to understand or change 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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants