clean up

crates/bevy_core_pipeline/src/core_3d/main_opaque_pass_3d_node.rs

@@ -84,7 +84,7 @@ impl ViewNode for MainOpaquePass3dNode {
                     load: match camera_3d.clear_color {
                         ClearColorConfig::None => LoadOp::Load,
                         // TODO clear this earlier?
-                        _ => LoadOp::Clear(EntityTextures::clear_color()),
+                        _ => LoadOp::Clear(EntityTextures::no_entity_color()),
                     },
                     store: true,
                 })));

crates/bevy_core_pipeline/src/entity_index_buffer_copy/mod.rs

@@ -24,6 +24,7 @@ impl ViewNode for EntityIndexBufferCopyNode {
         let Some(buffers) = gpu_picking_camera.buffers.as_ref() else {
             return Ok(());
         };
+
         // copy entity index texture
         buffers.copy_texture_to_buffer(
             render_context.command_encoder(),
@@ -38,10 +39,7 @@ impl ViewNode for EntityIndexBufferCopyNode {
 pub struct EntityIndexBufferCopyPlugin;
 impl Plugin for EntityIndexBufferCopyPlugin {
     fn build(&self, app: &mut bevy_app::App) {
-        let render_app = match app.get_sub_app_mut(RenderApp) {
-            Ok(render_app) => render_app,
-            Err(_) => return,
-        };
+        let Ok(render_app) = app.get_sub_app_mut(RenderApp) else { return; };
 
         // 3D
         use crate::core_3d::graph::node::*;

crates/bevy_render/src/picking.rs

@@ -1,7 +1,7 @@
 use crate::{
     camera::ExtractedCamera,
     extract_component::ExtractComponentPlugin,
-    render_resource::{Buffer, Texture},
+    render_resource::{Buffer, BufferSlice, Texture},
     renderer::RenderDevice,
     texture::{CachedTexture, TextureFormatPixelInfo},
     Render, RenderApp, RenderSet,
@@ -50,28 +50,21 @@ fn send_buffer_to_main_world(
             return;
         };
 
-        // sync
-        if false {
-            let buffer_slice = buffers.entity_buffer.slice(..);
-            buffer_slice.map_async(MapMode::Read, move |result| {
-                if let Err(e) = result {
-                    panic!("MAP ERROR: {e}");
-                }
-            });
-
-            // WARN This is blocking
-            render_device.poll(Maintain::Wait);
-
+        // Send the data to the main world
+        fn send(
+            sender: Sender<GpuPickingData>,
+            padded_bytes_per_row: usize,
+            entity_buffer: &Buffer,
+            buffer_slice: BufferSlice,
+        ) {
             let buffer_view = buffer_slice.get_mapped_range();
-            // we immediately move the data to CPU to avoid holding the mapped view for long
+            // immediately copy the data to CPU to avoid holding the mapped view for long
             let entity_data = Vec::from(&*buffer_view);
             drop(buffer_view);
-            buffers.entity_buffer.unmap();
-            // drop(entity_buffer);
+            entity_buffer.unmap();
 
-            // Send the buffer to the main world
-            if let Err(err) = gpu_picking_camera.sender.try_send(GpuPickingData {
-                padded_bytes_per_row: buffers.buffer_dimensions.padded_bytes_per_row,
+            if let Err(err) = sender.try_send(GpuPickingData {
+                padded_bytes_per_row,
                 entity_data,
             }) {
                 match err {
@@ -81,18 +74,42 @@ fn send_buffer_to_main_world(
                     }
                 }
             }
+
+            // This can only fail if the sender is full or closed
+            // and we can't do anything if either of those things happen
+            // let _ = sender.try_send(GpuPickingData {
+            //     padded_bytes_per_row,
+            //     entity_data,
+            // });
+        }
+
+        if true {
+            // sync
+            let buffer_slice = buffers.entity_buffer.slice(..);
+            buffer_slice.map_async(MapMode::Read, move |result| match result {
+                Ok(_) => {}
+                Err(err) => bevy_log::error!("Failed to map entity buffer: {err}"),
+            });
+
+            // WARN This is blocking
+            render_device.poll(Maintain::Wait);
+
+            // Send the buffer to the main world
+            send(
+                gpu_picking_camera.sender.clone(),
+                buffers.buffer_dimensions.padded_bytes_per_row,
+                &buffers.entity_buffer,
+                buffer_slice,
+            );
         } else {
+            // async
             let entity_buffer = buffers.entity_buffer.clone();
             let sender = gpu_picking_camera.sender.clone();
             let padded_bytes_per_row = buffers.buffer_dimensions.padded_bytes_per_row;
 
             // Mapping the buffer is an asynchronous process.
             // This means we need to wait until the buffer is mapped before sending it to the main world
             let task = async move {
-                if sender.is_full() {
-                    return;
-                }
-
                 let (tx, rx) = async_channel::bounded(1);
 
                 // map entity buffer
@@ -105,46 +122,35 @@ fn send_buffer_to_main_world(
                 // Buffer is mapped and ready to be sent
                 rx.recv().await.unwrap();
 
-                let buffer_view = buffer_slice.get_mapped_range();
-                // immediately move the data to CPU to avoid holding the mapped view for long
-                let entity_data = Vec::from(&*buffer_view);
-                drop(buffer_view);
-                entity_buffer.unmap();
-                drop(entity_buffer);
-
                 // Send the buffer to the main world
-                if let Err(err) = sender.try_send(GpuPickingData {
-                    padded_bytes_per_row,
-                    entity_data,
-                }) {
-                    match err {
-                        TrySendError::Full(_) => bevy_log::error!("GPU Picking channel is full"),
-                        TrySendError::Closed(_) => {
-                            bevy_log::error!("GPU Picking channel is closed");
-                        }
-                    }
-                }
+                send(sender, padded_bytes_per_row, &entity_buffer, buffer_slice);
             };
             AsyncComputeTaskPool::get().spawn(task).detach();
         }
     }
 }
 
+/// Marker component to indicate that a mesh should be available for gpu picking
 #[derive(Component, ExtractComponent, Clone)]
 pub struct GpuPickingMesh;
 
+/// Data needed in the render world to manage the entity buffer
 #[derive(Component)]
 pub struct ExtractedGpuPickingCamera {
     pub buffers: Option<GpuPickingCameraBuffers>,
     sender: Sender<GpuPickingData>,
 }
 
+/// Data sent between the render world and main world
 #[derive(Default)]
 struct GpuPickingData {
     padded_bytes_per_row: usize,
     entity_data: Vec<u8>,
 }
 
+/// This component is used to indicate if a camera should support gpu picking.
+/// Any mesh with the [`GpuPickingMesh`] component that is visible from this camera
+/// will be pickable.
 #[derive(Component)]
 pub struct GpuPickingCamera {
     channel: (Sender<GpuPickingData>, Receiver<GpuPickingData>),
@@ -160,7 +166,7 @@ impl Default for GpuPickingCamera {
 impl GpuPickingCamera {
     pub fn new() -> Self {
         Self {
-            channel: async_channel::bounded(1),
+            channel: async_channel::unbounded(),
             data: GpuPickingData::default(),
         }
     }
@@ -211,9 +217,10 @@ impl crate::extract_component::ExtractComponent for GpuPickingCamera {
     }
 }
 
+/// Contains the buffer and it's dimension required for gpu picking
 pub struct GpuPickingCameraBuffers {
     pub entity_buffer: Buffer,
-    pub buffer_dimensions: BufferDimensions,
+    buffer_dimensions: BufferDimensions,
 }
 
 impl GpuPickingCameraBuffers {
@@ -223,6 +230,8 @@ impl GpuPickingCameraBuffers {
         texture: &Texture,
         buffer: &Buffer,
     ) {
+        // This can't be in the Node because it needs access to wgpu but
+        // bevy_core_pipeline doesn't depend on wgpu
         encoder.copy_texture_to_buffer(
             texture.as_image_copy(),
             wgpu::ImageCopyBuffer {
@@ -275,7 +284,7 @@ pub struct EntityTextures {
 
 impl EntityTextures {
     /// This is the color that will represent "no entity" in the entity buffer
-    pub fn clear_color() -> wgpu::Color {
+    pub fn no_entity_color() -> wgpu::Color {
         let v = entity_as_uvec2(Entity::PLACEHOLDER);
         // The texture only uses the red and green channel
         Color {
@@ -315,16 +324,13 @@ fn prepare_gpu_picking_buffers(
     for (entity, camera, mut gpu_picking_camera) in &mut cameras {
         let Some(size) = camera.physical_target_size else { continue; };
 
+        // TODO create 2 buffers and altenate between each buffer each frame
+
         // Only create a buffer if it doesn't already exist or the size is different
-        let mut create_buffer = false;
+        let mut create_buffer = true;
         if let Some((buffer_dimensions, _)) = buffer_cache.get(&entity) {
-            if buffer_dimensions.width != size.x as usize
-                || buffer_dimensions.height != size.y as usize
-            {
-                create_buffer = true;
-            }
-        } else {
-            create_buffer = true;
+            create_buffer = buffer_dimensions.width != size.x as usize
+                || buffer_dimensions.height != size.y as usize;
         }
 
         if create_buffer {

examples/input/gpu_picking.rs

@@ -11,7 +11,7 @@ use bevy_internal::{
 
 fn main() {
     App::new()
-        .insert_resource(Msaa::Off)
+        // .insert_resource(Msaa::Off)
         .add_plugins(DefaultPlugins.set(WindowPlugin {
             primary_window: Some(Window {
                 present_mode: PresentMode::AutoNoVsync,