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

GPU Picking v2 #8784

Closed
wants to merge 7 commits into from
Closed

GPU Picking v2 #8784

wants to merge 7 commits into from

Conversation

IceSentry
Copy link
Contributor

@IceSentry IceSentry commented Jun 8, 2023

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
image

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 IceSentry added A-Rendering Drawing game state to the screen C-Usability A targeted quality-of-life change that makes Bevy easier to use C-Feature A new feature, making something new possible and removed C-Usability A targeted quality-of-life change that makes Bevy easier to use labels Jun 8, 2023
@IceSentry IceSentry requested a review from superdump June 8, 2023 01:40
@github-actions
Copy link
Contributor

github-actions bot commented Jun 8, 2023

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

@github-actions
Copy link
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
@github-actions
Copy link
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.

@github-actions
Copy link
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
@github-actions
Copy link
Contributor

github-actions bot commented Jun 8, 2023

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

@github-actions
Copy link
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.

@github-actions
Copy link
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
@github-actions
Copy link
Contributor

github-actions bot commented Jun 8, 2023

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

@github-actions
Copy link
Contributor

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

@IceSentry IceSentry force-pushed the gpu-picking branch 2 times, most recently from db97ffc to f4dc8e1 Compare June 12, 2023 19:10
@github-actions
Copy link
Contributor

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

1 similar comment
@github-actions
Copy link
Contributor

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

@github-actions
Copy link
Contributor

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

@IceSentry IceSentry marked this pull request as ready for review June 13, 2023 20:31
@IceSentry IceSentry added this to the 0.12 milestone Jun 14, 2023
@Jeff425
Copy link

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!

@IceSentry
Copy link
Contributor Author

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.

@IceSentry
Copy link
Contributor Author

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

@torsteingrindvik torsteingrindvik mentioned this pull request Jul 31, 2023
Copy link
Contributor

@nicopap nicopap left a comment

Choose a reason for hiding this comment

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

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

Comment on lines 24 to 26
#ifdef GPU_PICKING
out.mesh_id = mesh.id;
#endif
Copy link
Contributor

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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/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
@@ -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];
Copy link
Contributor

Choose a reason for hiding this comment

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

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)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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

Copy link
Contributor

Choose a reason for hiding this comment

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

Did you look into it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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
Comment on lines +156 to +154
if entity != Entity::PLACEHOLDER {
Some(entity)
} else {
None
}
Copy link
Contributor

Choose a reason for hiding this comment

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

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
@happydpc
Copy link

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
Copy link

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.

1693917750215

@IceSentry
Copy link
Contributor Author

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

@happydpc
Copy link

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.

@IceSentry
Copy link
Contributor Author

IceSentry commented Sep 5, 2023

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

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

@alice-i-cecile alice-i-cecile modified the milestones: 0.12, 0.13 Sep 29, 2023
@mtsr
Copy link
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.

@Jeff425
Copy link

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

@DasLixou
Copy link
Contributor

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

@IceSentry
Copy link
Contributor Author

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 janhohenheim added the S-Waiting-on-Author The author needs to make changes or address concerns before this can be merged label Jun 25, 2024
@IceSentry
Copy link
Contributor Author

Bevy's internals have change too much since this PR and this would most likely need a rewrite. The general principle would still be the same though.

This is currently low priority for me, so I'll close the PR but if anyone wants this feel free to adopt it.

@IceSentry IceSentry closed this Aug 8, 2024
@IceSentry IceSentry added S-Adopt-Me The original PR author has no intent to complete this work. Pick me up! and removed S-Waiting-on-Author The author needs to make changes or address concerns before this can be merged labels Aug 8, 2024
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 C-Feature A new feature, making something new possible S-Adopt-Me The original PR author has no intent to complete this work. Pick me up!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants