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.

Sign up for GitHub

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

Jump to bottom

GPU Picking v2 #8784

Open

IceSentry wants to merge 7 commits into bevyengine:main from IceSentry:gpu-picking

+949 −29

Contributor

IceSentry commented Jun 8, 2023 •

edited

Loading

Objective

Add native support for gpu picking to bevy
This is an updated version of GPU picking #6991

Solution

This happens over multiple frames

Frame N:
- For each visible mesh, generate a mesh id.
- For each mesh being rendered, output it's mesh id to a texture. This is just a new render attachment to the main pass.
- Once everything is rendered copy that texture to the cpu
Frame N + 1:
- Map the mesh id buffer and send it to the main world.
- This step will poll the gpu if necessary, so it could block here
Frame N + 2:
- From the main world you can give it a position like the current mouse position and know exactly which entity was rendered at that specific screen location. Since this takes multiple frames, the exact entity under the mouse might not be the same as the one visible.
This works at the Camera level, so it will work with multiple windows or split-screen.
The texture format is Rg16Uint, this means it can only render up to 65536 visible entities
Currently only implemented for 3d to keep it simple

Works with transparency, opaque, alpha mask and custom materials:

gpu_picking.mp4

Here's the mesh id texture generated for the above video

Multiple window support:

Code_LMlDGjcjWh.mp4

Open Questions

Should this be in the prepass instead of the main pass?
Naming could probably be improved
- I tried to name most things GpuPicking instead of just Picking because this is not intended as an end user api and more of a low level api for things like bevy_mod_picking to build on top.
What to do with the mesh id field added to MeshUniform?
- Should it be a separate uniform?
- Just feature flag it?
- Just keep it, it could be useful for things like occlusion culling and maybe even a debug view

Changelog

Added GpuPickingPlugin, GpuPickingCamera, GpuPickingMesh

Future Work

2d
Copy depth buffer and use it to determine world position under mouse
Various performance improvements
Web support (currently untested, it might already work)

Co-Authored By: @torsteingrindvik

IceSentry added A-Rendering C-Usability C-Enhancement and removed C-Usability labels

IceSentry requested a review from superdump

June 8, 2023 01:40

Contributor

github-actions bot commented Jun 8, 2023

Example alien_cake_addict failed to run, please try running it locally and check the result.

Contributor

github-actions bot commented Jun 8, 2023

You added a new example but didn't update the readme. Please run cargo run -p build-templated-pages -- update examples to update it, and commit the file change.

1 similar comment

Contributor

github-actions bot commented Jun 8, 2023

You added a new example but didn't update the readme. Please run cargo run -p build-templated-pages -- update examples to update it, and commit the file change.

Contributor

github-actions bot commented Jun 8, 2023

Example alien_cake_addict failed to run, please try running it locally and check the result.

1 similar comment

Contributor

github-actions bot commented Jun 8, 2023

Example alien_cake_addict failed to run, please try running it locally and check the result.

Contributor

github-actions bot commented Jun 8, 2023

You added a new example but didn't update the readme. Please run cargo run -p build-templated-pages -- update examples to update it, and commit the file change.

Contributor

github-actions bot commented Jun 8, 2023

Example alien_cake_addict failed to run, please try running it locally and check the result.

1 similar comment

Contributor

github-actions bot commented Jun 8, 2023

Example alien_cake_addict failed to run, please try running it locally and check the result.

IceSentry force-pushed the gpu-picking branch from a34a91c to 143fb61 Compare

June 12, 2023 17:15

Contributor

github-actions bot commented Jun 12, 2023

You added a new feature but didn't add a description for it. Please update the root Cargo.toml file.

IceSentry force-pushed the gpu-picking branch 2 times, most recently from db97ffc to f4dc8e1 Compare

June 12, 2023 19:10

Contributor

github-actions bot commented Jun 12, 2023

You added a new feature but didn't add a description for it. Please update the root Cargo.toml file.

1 similar comment

Contributor

github-actions bot commented Jun 12, 2023

You added a new feature but didn't add a description for it. Please update the root Cargo.toml file.

IceSentry force-pushed the gpu-picking branch from 2ce013b to 0fb1a46 Compare

June 13, 2023 18:11

Contributor

github-actions bot commented Jun 13, 2023

You added a new feature but didn't add a description for it. Please update the root Cargo.toml file.

IceSentry force-pushed the gpu-picking branch from 0fb1a46 to 5b4cab1 Compare

June 13, 2023 19:45

IceSentry marked this pull request as ready for review

June 13, 2023 20:31

IceSentry added this to the 0.12 milestone

aevyrie reviewed

View reviewed changes

crates/bevy_render/src/picking.rs Outdated Show resolved Hide resolved

IceSentry force-pushed the gpu-picking branch from 0953072 to 11c7075 Compare

June 16, 2023 18:44

Jeff425 commented Jun 25, 2023

Any chance that this PR will be able to get into 0.11? Looks like you have done a lot of good work on it @IceSentry and would be great to be able to use it with an actual Bevy release soon!

Contributor Author

IceSentry commented Jun 25, 2023

It's pretty much ready, but it needs reviews and since 0.11 is so close people are focusing on older PR and getting the release ready. It will most likely get merged pretty fast after 0.11 is released though.

Contributor Author

IceSentry commented Jun 25, 2023

Oh, just saw the conflicts, I'll need to fix that first.

IceSentry force-pushed the gpu-picking branch from 11c7075 to 41cb513 Compare

June 26, 2023 18:34

Kethku reviewed

View reviewed changes

examples/input/gpu_picking.rs Outdated Show resolved Hide resolved

IceSentry force-pushed the gpu-picking branch from 41cb513 to 208e388 Compare

July 19, 2023 22:27

Kethku reviewed

View reviewed changes

crates/bevy_core_pipeline/src/core_3d/main_transparent_pass_3d_node.rs Outdated Show resolved Hide resolved

nicopap self-requested a review

July 31, 2023 13:21

torsteingrindvik mentioned this pull request

GPU picking #6991

Closed

nicopap suggested changes

View reviewed changes

Contributor

nicopap left a comment

Looks very good! Just a few questions and remarks concerning perf/memory usage.

assets/shaders/gpu_picking_material.wgsl Outdated

Comment on lines 24 to 26

+              #ifdef GPU_PICKING
+                  out.mesh_id = mesh.id;
+              #endif

Contributor

nicopap Aug 3, 2023

Maybe update the custom material examples in addition to this example. Since it will inform anyone looking to make any kind of material. Otherwise users might end up surprised that their material doesn't work with picking.

With a separate example, someone needs to specifically be looking to make their material compatible with GPU picking.

Contributor Author

IceSentry Aug 3, 2023

I kinda like keeping examples as single-focus as possible but I guess you're right that people making third party shader should know about this even if they don't use picking.

crates/bevy_core_pipeline/src/core_3d/main_opaque_pass_3d_node.rs Outdated Show resolved Hide resolved

crates/bevy_core_pipeline/src/core_3d/mod.rs Outdated Show resolved Hide resolved

crates/bevy_core_pipeline/src/core_3d/mod.rs Outdated Show resolved Hide resolved

crates/bevy_render/src/picking.rs Outdated Show resolved Hide resolved

crates/bevy_render/src/picking.rs Outdated Show resolved Hide resolved

crates/bevy_pbr/src/render/mesh.rs

@@ @@ -192,8 +194,10 @@ pub fn extract_meshes( @@
                   let mut not_caster_commands = Vec::with_capacity(*prev_not_caster_commands_len);
                   let visible_meshes = meshes_query.iter().filter(|(_, vis, ..)| vis.is_visible());
-                  for (entity, _, transform, previous_transform, handle, not_receiver, not_caster) in
-                      visible_meshes
+                  let mut visible_mesh_entities = vec![Entity::PLACEHOLDER];

Contributor

nicopap Aug 3, 2023

Instead of having VisibleMeshEntities in the render world, and pass it to the main world for each picking camera, what about just updating a VisibleMeshEntities in the Main world?

It's a big deal, given this might be a pretty large array and it's allocated & cloned every frame (currently).

Also keeping it in the main world will not prevent you from updating it from this system (so that you still have correct ordering to match the mesh_id)

Contributor Author

IceSentry Aug 3, 2023

Good question, I don't remember why I did it this way 😅. I'll look into doin that instead

Contributor

nicopap Sep 8, 2023

Did you look into it?

Contributor Author

IceSentry Sep 8, 2023

Not yet. I started looking into it a bit, but hit a blocker and stopped. I'm planning on trying again next week.

crates/bevy_render/src/picking.rs Outdated Show resolved Hide resolved

crates/bevy_render/src/picking.rs

Comment on lines +156 to +154

+                      if entity != Entity::PLACEHOLDER {
+                          Some(entity)
+                      } else {
+                          None
+                      }

Contributor

nicopap Aug 3, 2023

Personally, instead of storing an Entity::PLACEHOLDER at index zero, I would do index.checked_sub(1).map(|i| self.data.visible_mesh_entities[i])

crates/bevy_pbr/src/render/mesh.rs Outdated Show resolved Hide resolved

IceSentry force-pushed the gpu-picking branch from b7f0702 to b48645a Compare

August 30, 2023 23:48

IceSentry and others added 7 commits

September 4, 2023 22:15


          add gpu picking

7e749d8

Co-authored-by: Kaylee Simmons <kay@the-simmons.net>

Co-authored-by: Nicola Papale <nicopap@users.noreply.github.com>


          clear buffer cache

1bb7131


          use entry api and some misc fixes

7efa0ee


          use const clear color

76db53b


          use smallvec

eb30901


          clarify comment

19c3350


          yeet bevy internals

7d1a5cc

IceSentry force-pushed the gpu-picking branch from 39cf863 to 7d1a5cc Compare

September 5, 2023 02:20

happydpc commented Sep 5, 2023

The mouse position should use the physical coordinate because the readback buffer is using the physical size.

// ***** now the result is correct  ****//
let mouse_position = (moved_event.position.as_dvec2() * moved_event.scale_factor).as_uvec2();

    for gpu_picking_camera in &gpu_picking_cameras {
        // This will read the entity texture and get the entity that is at the given position
        if let Some(entity) = gpu_picking_camera.get_entity(mouse_position) {
            if let Some(hovered) = *hovered {
                if entity != hovered {
                    set_color(hovered, Color::BLUE);
                }
            }
            set_color(entity, Color::RED);
            *hovered = Some(entity);
        } else {
            if let Some(hovered) = *hovered {
                set_color(hovered, Color::BLUE);
            }
            *hovered = None;
        }
    }

happydpc commented Sep 5, 2023

I have made the example working on web/wasm, but the blend function is a problem because we use the TextureFormat::R16Uint, this format doesn't support the blend function. So the blend function had to be set to none when using gpu picking, you can see the left cube is not transparent.

Contributor Author

IceSentry commented Sep 5, 2023

@happydpc can you make a PR to my branch with the web support

happydpc commented Sep 5, 2023

Sure, I am still trying modify the copied buffer size, if we only want to pick the point nearby, we don't need to copy the entire texture then.

Contributor Author

IceSentry commented Sep 5, 2023 •

edited

Loading

I intentionally left that out as a feature for 2 reasons:

I don't want to overcomplicate a PR that is already really big. This can be done in a future PR
It won't work with multi touch so it needs to be optional.

alice-i-cecile requested a review from aevyrie

September 29, 2023 16:01

alice-i-cecile modified the milestones: 0.12, 0.13

Contributor

mtsr commented Nov 4, 2023

Should this be in the prepass instead of the main pass?
I see definite uses for this if it can be done in the prepass when enabled. Accurate outlines are one example.

torsteingrindvik mentioned this pull request

Animated meshes disapear if their original position is not within the field of view #4971

Open

alice-i-cecile removed this from the 0.13 milestone

Jeff425 commented Feb 1, 2024

What is the current status of this PR? Hoping to see this merged at some point, lots of good work has been done already

Contributor

DasLixou commented Mar 24, 2024

Hello! I want to ask what the current status of this PR is and how it will behave with custom shaders. Will it also be possible to render something in the picking that is not visible in the normal render mesh? For example a bigger interaction circle for mobile/touch input but without rendering that in the real scene. And it also seems like it just directly results in the entity id, I wonder if it would be easy to add support for multiple picking textures which then not directly hold the entity id but an id created with the picking texture in a follow-up PR.
Thanks

Contributor Author

IceSentry commented Mar 24, 2024

The branch is currently very outdated so I'll need to update it. It's not high on my priority list right now but I want to get back to this at some point in the next few months

janhohenheim added the S-Waiting-on-Author label

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment