-
-
Notifications
You must be signed in to change notification settings - Fork 4.3k
Use RayMap and RenderLayers in bevy_sprite/picking_backend
#18069
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
base: main
Are you sure you want to change the base?
Conversation
|
Will this be also ported to |
|
I wouldn't expect so but that's not my call. You can take the code changes and build a custom sprite picking backend that implements the same fixes if you need this to work asap :) |
|
I wouldn't expect any further backports to 0.15 at this stage, except maybe for the most serious of problems. |
alice-i-cecile
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This makes sense to me as a fix. I'd prefer more automated tests for this area of the code, but given that we don't have any set up for picking broadly I won't block on this.
This won't work on non-primary windows, which is annoying, but that's a pre-existing bug that should be fixed seperately.
| if let Some(cam_render_layers) = cam_render_layers { | ||
| if let Some(layers) = layers { | ||
| if !cam_render_layers.intersects(layers) { | ||
| return None; | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| if let Some(cam_render_layers) = cam_render_layers { | |
| if let Some(layers) = layers { | |
| if !cam_render_layers.intersects(layers) { | |
| return None; | |
| } | |
| } | |
| } | |
| if !cam_render_layers | |
| .unwrap_or_default() | |
| .intersects(layers.unwrap_or_default()) | |
| { | |
| return None; | |
| } |
Entities without RenderLayers are assumed to be RenderLayers::layer(0) (which is the default() value). This is needed to skip cases like camera = None, sprite = Some(RenderLayers::layer(1)) .
Mesh picking backend does this:
| let cam_layers = cam_layers.to_owned().unwrap_or_default(); |
bevy/crates/bevy_picking/src/mesh_picking/mod.rs
Lines 106 to 107 in ea7e868
| let entity_layers = layers.get(entity).cloned().unwrap_or_default(); | |
| let render_layers_match = cam_layers.intersects(&entity_layers); |
|
Hi, I would like to adopt this PR (as was recommended to me on bevy's discord). I'm not sure if I've done everything right, but here is my new PR. |
Objective
Fixes #18005
Utilize
RayMapin sprite picking in order to get picking working with differing viewport scaling. AddsRenderLayersto the sprite query for layer-exclusive picking control.Solution
Small rewrite of the loops in
bevy_sprite/picking_backend.rs. Implementing eitherRayMaporRenderLayerson their own didn't have the correct results, so both solutions are rolled into one PR.Testing
CI & an updated version of the reproduction repo in the original issue, along with the
sprite_pickingexample.