Skip to content

Fix viewport_to_world and world_to_viewport#6526

Closed
tim-blackbird wants to merge 1 commit intobevyengine:mainfrom
tim-blackbird:fix-viewport-to-world-and-back
Closed

Fix viewport_to_world and world_to_viewport#6526
tim-blackbird wants to merge 1 commit intobevyengine:mainfrom
tim-blackbird:fix-viewport-to-world-and-back

Conversation

@tim-blackbird
Copy link
Contributor

@tim-blackbird tim-blackbird commented Nov 9, 2022

Objective

The origin of the viewport lies at the top-left, as documented here.

Test program.
use bevy::prelude::*;

fn main() {
    App::new()
        .add_plugins(DefaultPlugins)
        .add_startup_system(setup)
        .add_system(system)
        .run();
}

fn setup(mut commands: Commands) {
    commands.spawn(Camera2dBundle::default());

    commands.spawn(NodeBundle {
        background_color: Color::BLACK.into(),
        style: Style {
            size: Size::new(Val::Px(20.), Val::Px(20.)),
            position_type: PositionType::Absolute,
            ..default()
        },
        ..default()
    });
}

fn system(query: Query<(&Camera, &GlobalTransform)>, mut ui_query: Query<&mut Style>) {
    let (camera, global_transform) = query.single();

    let world_pos = Vec3::new(0., 150., 0.);

    let Some(pos) = camera.world_to_viewport(global_transform, world_pos) else {
        println!("B");
        return;
    };

    // UI element should be 150 pixels up from the center, instead it was down 150 pixels.
    ui_query.single_mut().position = UiRect {
        left: Val::Px(pos.x),
        top: Val::Px(pos.y),
        ..default()
    };
}

Migration Guide

Camera::world_to_viewport now correctly returns the viewport position with Y pointing downwards, instead of up.
You may need to adjust your code if you were using this method.

@alice-i-cecile alice-i-cecile added this to the Bevy 0.9 milestone Nov 9, 2022
@tim-blackbird tim-blackbird marked this pull request as draft November 9, 2022 09:17
@tim-blackbird
Copy link
Contributor Author

tim-blackbird commented Nov 9, 2022

Needs more testing D:
edit: Sanity-check completed. See the test program in the PR description :)

@tim-blackbird tim-blackbird marked this pull request as ready for review November 9, 2022 09:29
@mockersf
Copy link
Member

mockersf commented Nov 9, 2022

this may be reverted for the 0.9, see #6522

@tim-blackbird
Copy link
Contributor Author

tim-blackbird commented Nov 9, 2022

@mockersf While this change is somewhat related, that issue is only about reverting the origin of the cursor back to the bottom-left.
#6000 did not touch viewports; world_to_viewport was already wrong in Bevy 0.8 (Hence the migration guide).

@alice-i-cecile alice-i-cecile added A-Rendering Drawing game state to the screen C-Bug An unexpected or incorrect behavior A-Input Player input via keyboard, mouse, gamepad, and more labels Nov 9, 2022
@alice-i-cecile alice-i-cecile added S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it and removed S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it labels Nov 12, 2022
@alice-i-cecile
Copy link
Member

I'm less convinced by this. it feels like the right fix is that the viewport's y should match the world's and be at the bottom.

@alice-i-cecile alice-i-cecile removed this from the Bevy 0.9 milestone Nov 12, 2022
@bzm3r
Copy link
Contributor

bzm3r commented Nov 12, 2022

@devil-ira sorry, actually, I don't quite understand this. Could you provide some more commentary on why your solution does what is intended?

From a mathematical perspective, I get that if I want to mirror a coordinate system along a particular axis, we multiply the values of that axis by -1.

But suppose we have a grid an N x M grid, and we want to mirror in the y-direction.

Then, the mapping from (x, y) to mirrored coordinates is: (x, M - y), right? How come we don't have to do something like that there?

@tim-blackbird
Copy link
Contributor Author

I'm less convinced by this. it feels like the right fix is that the viewport's y should match the world's and be at the bottom.

Pls no. I really want screen space stuff to have consistent coordinates, unless it really makes sense for them to be different. (UI and viewports are aligned currently).

@bzm3r Note that this is a breaking change as I believed(I'm rethinking atm) the usage of the existing method was technically incorrect, even if it did work. It's just the sign/direction of the Y axis that is off. The comments in the code should be changed to reflect that better.

I'm closing this as I realize the way it is now is not necessarily incorrect, and merging this would make using these methods with the cursor position more cumbersome.

I do want to make an argument in favor of making the coordinate systems of screen space stuff in Bevy(UI, cursor position, and viewports) be consistent/least surprising, but I'll do that in a dedicated issue once I've written something up :)

@lewiszlw
Copy link
Member

lewiszlw commented Feb 8, 2023

I do want to make an argument in favor of making the coordinate systems of screen space stuff in Bevy(UI, cursor position, and viewports) be consistent/least surprising, but I'll do that in a dedicated issue once I've written something up :)

Yes, I agree that making the coordinate systems of screen space stuff be consistent. It's weird that when I first using world_to_viewport method. Do you create issue or something to track this? @devil-ira

@tim-blackbird tim-blackbird deleted the fix-viewport-to-world-and-back branch March 13, 2023 16:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-Input Player input via keyboard, mouse, gamepad, and more A-Rendering Drawing game state to the screen C-Bug An unexpected or incorrect behavior

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants