Add a function for computing a world point from a screen point #1799
Conversation
Co-authored-by: MinerSebas <66798382+MinerSebas@users.noreply.github.com>
alice-i-cecile
added
C-Usability
|
@jamadazi once this is merged this will obsolete https://bevy-cheatbook.github.io/cookbook/cursor2world.html :) |
examples/3d/3d_scene.rs
Outdated
|
|
||
| fn follow( | ||
| mut q: Query<&mut Transform, With<Follow>>, | ||
| q_camera: Query<(&Camera, &GlobalTransform)>, |
There was a problem hiding this comment.
You probably want to tweak this to exclude UI cameras to make it easier to port to user code.
There was a problem hiding this comment.
I haven't played with UI yet, tip for what struct exactly I should exclude?
|
I would very much like to see a 2D example of this as well :) It's not obvious to many beginners that 3D code tends to also work in 2D. Remember to add new examples to examples/README.md as well! |
I was actually curious about your thoughts on this. Should there be a function specialized for 2D users, since we can assume they probably want to use the plane that passes through the origin and faces the camera? If so, how should we prevent 3D users from accidentally using the 2D version and vice/versa? |
| @@ -61,6 +62,48 @@ impl Camera { | |||
| let screen_space_coords = (ndc_space_coords.truncate() + Vec2::ONE) / 2.0 * window_size; | |||
| Some(screen_space_coords) | |||
| } | |||
|
|
|||
| /// Given a position in screen space, compute the world-space line that corresponds to it. | |||
| pub fn screen_to_world_line( | |||
There was a problem hiding this comment.
Both of these functions feel like they should be a method on the Camera, not a function that takes a camera as the argument.
There was a problem hiding this comment.
I wrote it like that originally, but it felt weird that it was a method on the camera but still required the camera's global transform. I thought it was cleaner as a function but I'm happy to change it if you think it's better or more consistent that way (I'm not sure how this is normally handled in bevy).
|
You could argue that there is no such thing as 2D in bevy, considering layers are positioned with real z-depth, and some games use the fact that 2d is actually 3d as a feature for rendering effects. That said, if someone is truly making a 2D-only game, they probably only need the cursor coordinates. |
Yes, this sounds very useful. And it's particularly helpful for beginners.
The simplest solution would be to extend The other idea would be to inspect the properties of the camera, and then determine which approach to take based on their properties. I don't think this is the right approach though: the extra check will be wasted work and the 2D version needs significantly less information than the 3D version. |
I'd totally forgotten that #1258 was merged. It's probably saner to just use that for 2D. I still think it deserves an example for discoverability (and API consistency and doc crosslinking). IMO that's within scope for this PR, since it will make the big picture much clearer for the end user. |
Done!
Done!
I didn't see the code for #1258 in my branch - not sure if it got moved or if my branch just isn't up to date. Either way, I think the implementation of the 2D version should just call into the 3D version so I've rewritten it, but the interface doesn't need to change (although it's different right now because mine is a function and the other is a method). Thoughts? |
anchpop
force-pushed
the
screen_point_to_ray
branch
5 times, most recently
from
34709ba
to
c22b434
Compare
I just realized something. #1258 converts world-space coordinates to screen-space, my function does the opposite. So there's need for both. |
|
Happy to see a proper solution making its way into bevy, for something so common. I really don't like having that ugly cursor2world cheatsheet page in cheatbook. 😄 |
anchpop
force-pushed
the
screen_point_to_ray
branch
3 times, most recently
from
7b61246
to
96645dd
Compare
This can save users from having to type `&*X` all the time at the cost of some complexity in the type signature. For instance, this allows me to accommodate @jakobhellermann's suggestion in #1799 without requiring users to type `&*windows` 99% of the time.
anchpop
force-pushed
the
screen_point_to_ray
branch
from
96645dd
to
5b263ff
Compare
@alice-i-cecile Do you think Bevy's UICameraBundle should come with a |
anchpop
force-pushed
the
screen_point_to_ray
branch
2 times, most recently
from
ba6da4a
to
5b263ff
Compare
Quite strongly yes :) I would love to see that change, but I think it should be part of a seperate PR. |
|
Is there an update on this? I'd love too see it merged, if it gets approved. Do you need help with fixing conflicts / get CI to pass? (No hurry though) |
| } | ||
| impl Error for PrimitiveError {} | ||
| impl fmt::Display for PrimitiveError { |
There was a problem hiding this comment.
| } | |
| impl Error for PrimitiveError {} | |
| impl fmt::Display for PrimitiveError { | |
| } | |
| impl Error for PrimitiveError {} | |
| impl fmt::Display for PrimitiveError { |
just a small nit-pick, add some newlines here :)
There was a problem hiding this comment.
(Also this is here in a few other places, so adding newlines between impls would be nice)
| radius: f32, | ||
| } | ||
|
|
||
| impl Sphere { |
There was a problem hiding this comment.
These seem like quite simple almost unnecessary getters/setters for Sphere.
IMO just having the Sphere's origin and radius field be public would be better here.
There was a problem hiding this comment.
ditto for a lot of the other simple structs in the PR
| for vertex in self.vertices().iter() { | ||
| if plane.distance_to_point(*vertex) <= 0.0 { | ||
| return false; | ||
| } | ||
| } | ||
| true |
There was a problem hiding this comment.
if self.vertices()
.iter()
.find(|vert| plane.distance_to_point(*vertex) <= 0.0)
.map_or(true, |_| false)I would personally prefer using find() then map here.
| Err(PrimitiveError::MinGreaterThanMax) | ||
| } | ||
| } | ||
| /// Construct an [AxisAlignedBox] from the origin at the minimum corner, and the extents - the |
There was a problem hiding this comment.
| /// Construct an [AxisAlignedBox] from the origin at the minimum corner, and the extents - the | |
| /// Construct an [`AxisAlignedBox`] from the origin at the minimum corner, and the extents - the |
having the ticks (`) in the doc comments helps highlight the fact this is a type :)
| } | ||
| impl Primitive3d for Frustum { | ||
| fn outside_plane(&self, plane: Plane) -> bool { | ||
| for vertex in self.vertices().iter() { |
There was a problem hiding this comment.
ditto here with the find() then map_or
| pub fn intersection_line(&self, line: &Line) -> Option<Vec3> { | ||
| let d = line.direction.dot(self.normal); | ||
| if d == 0. { | ||
| // Should probably check if they're approximately equal, not strictly equal |
There was a problem hiding this comment.
yep, I agree with this
| } | ||
| impl Primitive3d for OBB { | ||
| fn outside_plane(&self, plane: Plane) -> bool { | ||
| for vertex in self.vertices().iter() { |
| MinGreaterThanMax, | ||
| NonPositiveExtents, | ||
| } | ||
| impl Error for PrimitiveError {} |
There was a problem hiding this comment.
instead of implementing Error and Display for this type.
You can use the thiserror crate. (we use it in other places of the bevy code base)
#[derive(Error, Debug, Clone)]
pub enum PrimitiveError {
#[error("this is the error message")]
MinGreaterThanMax,
#[error("this is another error message")]
NonPositiveExtents,
}
|
@NathanSWard Most of those comments are related to #1621, pinging @aevyrie to make sure they see them |
cart
added
the
S-Pre-Relicense
mockersf
removed
the
S-Pre-Relicense
This can save users from having to type `&*X` all the time at the cost of some complexity in the type signature. For instance, this allows me to accommodate @jakobhellermann's suggestion in bevyengine#1799 without requiring users to type `&*windows` 99% of the time.
|
Are there plans for this PR? IMO this should focus exclusively on space transformation and not 'object picking' which the presence of the geometry primitives seems to imply. That's a much harder problem to solve for arbitrary geometry and generally relies on either a physics engine ray-casting or color-readback from offscreen buffers using an "object ID" shader. This (below) seems sufficient. Or possibly return a pub fn screen_to_world_point_at_distance(
pos_screen: Vec2,
window: &Window,
camera: &Camera,
camera_transform: &GlobalTransform,
distance: f32,
) -> Vec3 {
let camera_position = camera_transform.compute_matrix();
let screen_size = Vec2::from([window.width() as f32, window.height() as f32]);
let projection_matrix = camera.projection_matrix;
// Normalized device coordinate cursor position from (-1, -1, -1) to (1, 1, 1)
let cursor_ndc = (pos_screen / screen_size) * 2.0 - Vec2::from([1.0, 1.0]);
let cursor_pos_ndc_near: Vec3 = cursor_ndc.extend(-1.0);
let cursor_pos_ndc_far: Vec3 = cursor_ndc.extend(1.0);
// Use near and far ndc points to generate a ray in world space
// This method is more robust than using the location of the camera as the start of
// the ray, because ortho cameras have a focal point at infinity!
let ndc_to_world: Mat4 = camera_position * projection_matrix.inverse();
let cursor_pos_near: Vec3 = ndc_to_world.project_point3(cursor_pos_ndc_near);
let cursor_pos_far: Vec3 = ndc_to_world.project_point3(cursor_pos_ndc_far);
let ray_direction = cursor_pos_far - cursor_pos_near;
cursor_pos_near + (ray_direction * distance)
} |
This PR doesn't have anything to do with object picking or raycasting (as it's normally interpreted). The issue is that any screen pixel corresponds to a line in world space (really a ray). It's often useful to have that line, and it's often useful to see where that line intersects with a plane. For example, imagine a top-down shooter like hotline miami, but with a perspective camera at a slight tilt. To determine where the player is aiming their gun, you need to find the plane parallel to the ground that intersects the player, then calculate the intersection of that plane and the line corresponding to the pixel the user's mouse is on. |
|
@anchpop Indeed, that's what I said 😋 A space transform from screen to world creates a ray (hence "or possibly return a This certainly isn't meant ad hominem of course! Both are needed, I'm just casting a vote to keep those separate and distinct concepts instead of munging them together 😁 |
|
Hi all, is there any chance of this getting added soon, or should I implement my own for now? |
I'm guiding @omarbassam88 through creating a scoped version of this. However, there's no guarantee on when that will be merged in, so I would suggest unblocking yourself by coding your own simple implementation (or using |
alice-i-cecile
added
the
X-Controversial
|
Closing in favor of the fresher and more scoped #4177. |
Uses geometric primitives from #1621 (so get that merged first haha). The example at
screen_to_world_positioningcontains a demonstrationThis is a feature that a large proportion of 3D games will need at some point, so it's useful to have it in Bevy proper (rather than in a 3rd-party extension like bevy_mod_raycast from which some of the code was borrowed).
Thank you to @aevyrie and @alice-i-cecile for helping me out with this one in the Discord :P