From de86f2a72fb85a0d4ae661a1b99f956e00f92199 Mon Sep 17 00:00:00 2001 From: Dzmitry Malyshau Date: Mon, 19 Apr 2021 23:35:33 -0400 Subject: [PATCH] Mark unused implicit BGL ids as vacant --- player/src/lib.rs | 4 +- wgpu-core/src/device/mod.rs | 100 ++++++++++++++++++------------------ wgpu-core/src/hub.rs | 11 +++- 3 files changed, 63 insertions(+), 52 deletions(-) diff --git a/player/src/lib.rs b/player/src/lib.rs index fe2573e2c..41719759a 100644 --- a/player/src/lib.rs +++ b/player/src/lib.rs @@ -265,7 +265,7 @@ impl GlobalPlay for wgc::hub::Global { root_id: ic.root_id, group_ids: &ic.group_ids, }); - let (_, _, error) = + let (_, error) = self.device_create_compute_pipeline::(device, &desc, id, implicit_ids); if let Some(e) = error { panic!("{:?}", e); @@ -287,7 +287,7 @@ impl GlobalPlay for wgc::hub::Global { root_id: ic.root_id, group_ids: &ic.group_ids, }); - let (_, _, error) = + let (_, error) = self.device_create_render_pipeline::(device, &desc, id, implicit_ids); if let Some(e) = error { panic!("{:?}", e); diff --git a/wgpu-core/src/device/mod.rs b/wgpu-core/src/device/mod.rs index 8abce9cc3..5e1f943e9 100644 --- a/wgpu-core/src/device/mod.rs +++ b/wgpu-core/src/device/mod.rs @@ -1816,13 +1816,7 @@ impl Device { mut derived_group_layouts: ArrayVec<[binding_model::BindEntryMap; MAX_BIND_GROUPS]>, bgl_guard: &mut Storage, id::BindGroupLayoutId>, pipeline_layout_guard: &mut Storage, id::PipelineLayoutId>, - ) -> Result< - (id::PipelineLayoutId, pipeline::ImplicitBindGroupCount), - pipeline::ImplicitLayoutError, - > { - let derived_bind_group_count = - derived_group_layouts.len() as pipeline::ImplicitBindGroupCount; - + ) -> Result { while derived_group_layouts .last() .map_or(false, |map| map.is_empty()) @@ -1837,9 +1831,7 @@ impl Device { ids.group_ids.len(), derived_group_layouts.len() ); - return Err(pipeline::ImplicitLayoutError::MissingIds( - derived_bind_group_count, - )); + return Err(pipeline::ImplicitLayoutError::MissingIds(group_count as _)); } for (bgl_id, map) in ids.group_ids.iter_mut().zip(derived_group_layouts) { @@ -1861,7 +1853,7 @@ impl Device { }; let layout = self.create_pipeline_layout(self_id, &layout_desc, bgl_guard)?; pipeline_layout_guard.insert(ids.root_id, layout); - Ok((ids.root_id, derived_bind_group_count)) + Ok(ids.root_id) } fn create_compute_pipeline( @@ -1871,14 +1863,20 @@ impl Device { implicit_context: Option, hub: &Hub, token: &mut Token, - ) -> Result< - ( - pipeline::ComputePipeline, - pipeline::ImplicitBindGroupCount, - id::PipelineLayoutId, - ), - pipeline::CreateComputePipelineError, - > { + ) -> Result, pipeline::CreateComputePipelineError> { + //TODO: only lock mutable if the layout is derived + let (mut pipeline_layout_guard, mut token) = hub.pipeline_layouts.write(token); + let (mut bgl_guard, mut token) = hub.bind_group_layouts.write(&mut token); + + // This has to be done first, or otherwise the IDs may be pointing to entries + // that are not even in the storage. + if let Some(ref ids) = implicit_context { + pipeline_layout_guard.make_room(ids.root_id); + for &bgl_id in ids.group_ids.iter() { + bgl_guard.make_room(bgl_id); + } + } + if !self .downlevel .flags @@ -1887,10 +1885,6 @@ impl Device { return Err(pipeline::CreateComputePipelineError::ComputeShadersUnsupported); } - //TODO: only lock mutable if the layout is derived - let (mut pipeline_layout_guard, mut token) = hub.pipeline_layouts.write(token); - let (mut bgl_guard, mut token) = hub.bind_group_layouts.write(&mut token); - let mut derived_group_layouts = ArrayVec::<[binding_model::BindEntryMap; MAX_BIND_GROUPS]>::new(); @@ -1942,8 +1936,8 @@ impl Device { // TODO let parent = hal::pso::BasePipeline::None; - let (pipeline_layout_id, derived_bind_group_count) = match desc.layout { - Some(id) => (id, 0), + let pipeline_layout_id = match desc.layout { + Some(id) => id, None => self.derive_pipeline_layout( self_id, implicit_context, @@ -1992,7 +1986,7 @@ impl Device { }, life_guard: LifeGuard::new(desc.label.borrow_or_default()), }; - Ok((pipeline, derived_bind_group_count, pipeline_layout_id)) + Ok(pipeline) } fn create_render_pipeline( @@ -2002,18 +1996,20 @@ impl Device { implicit_context: Option, hub: &Hub, token: &mut Token, - ) -> Result< - ( - pipeline::RenderPipeline, - pipeline::ImplicitBindGroupCount, - id::PipelineLayoutId, - ), - pipeline::CreateRenderPipelineError, - > { + ) -> Result, pipeline::CreateRenderPipelineError> { //TODO: only lock mutable if the layout is derived let (mut pipeline_layout_guard, mut token) = hub.pipeline_layouts.write(token); let (mut bgl_guard, mut token) = hub.bind_group_layouts.write(&mut token); + // This has to be done first, or otherwise the IDs may be pointing to entries + // that are not even in the storage. + if let Some(ref ids) = implicit_context { + pipeline_layout_guard.make_room(ids.root_id); + for &bgl_id in ids.group_ids.iter() { + bgl_guard.make_room(bgl_id); + } + } + let mut derived_group_layouts = ArrayVec::<[binding_model::BindEntryMap; MAX_BIND_GROUPS]>::new(); @@ -2380,8 +2376,8 @@ impl Device { // TODO let parent = hal::pso::BasePipeline::None; - let (pipeline_layout_id, derived_bind_group_count) = match desc.layout { - Some(id) => (id, 0), + let pipeline_layout_id = match desc.layout { + Some(id) => id, None => self.derive_pipeline_layout( self_id, implicit_context, @@ -2482,7 +2478,7 @@ impl Device { vertex_strides, life_guard: LifeGuard::new(desc.label.borrow_or_default()), }; - Ok((pipeline, derived_bind_group_count, pipeline_layout_id)) + Ok(pipeline) } fn wait_for_submit( @@ -4029,7 +4025,6 @@ impl Global { implicit_pipeline_ids: Option>, ) -> ( id::RenderPipelineId, - pipeline::ImplicitBindGroupCount, Option, ) { profiling::scope!("Device::create_render_pipeline"); @@ -4055,19 +4050,23 @@ impl Global { }); } - let (pipeline, derived_bind_group_count, _layout_id) = match device - .create_render_pipeline(device_id, desc, implicit_context, &hub, &mut token) - { + let pipeline = match device.create_render_pipeline( + device_id, + desc, + implicit_context, + &hub, + &mut token, + ) { Ok(pair) => pair, Err(e) => break e, }; let id = fid.assign(pipeline, &mut token); - return (id.0, derived_bind_group_count, None); + return (id.0, None); }; let id = fid.assign_error(desc.label.borrow_or_default(), &mut token); - (id, 0, Some(error)) + (id, Some(error)) } /// Get an ID of one of the bind group layouts. The ID adds a refcount, @@ -4157,7 +4156,6 @@ impl Global { implicit_pipeline_ids: Option>, ) -> ( id::ComputePipelineId, - pipeline::ImplicitBindGroupCount, Option, ) { profiling::scope!("Device::create_compute_pipeline"); @@ -4183,19 +4181,23 @@ impl Global { }); } - let (pipeline, derived_bind_group_count, _layout_id) = match device - .create_compute_pipeline(device_id, desc, implicit_context, &hub, &mut token) - { + let pipeline = match device.create_compute_pipeline( + device_id, + desc, + implicit_context, + &hub, + &mut token, + ) { Ok(pair) => pair, Err(e) => break e, }; let id = fid.assign(pipeline, &mut token); - return (id.0, derived_bind_group_count, None); + return (id.0, None); }; let id = fid.assign_error(desc.label.borrow_or_default(), &mut token); - (id, 0, Some(error)) + (id, Some(error)) } /// Get an ID of one of the bind group layouts. The ID adds a refcount, diff --git a/wgpu-core/src/hub.rs b/wgpu-core/src/hub.rs index 5e71604fb..d6a1dc785 100644 --- a/wgpu-core/src/hub.rs +++ b/wgpu-core/src/hub.rs @@ -160,10 +160,19 @@ impl Storage { } } - fn insert_impl(&mut self, index: usize, element: Element) { + fn make_room_impl(&mut self, index: usize) { if index >= self.map.len() { self.map.resize_with(index + 1, || Element::Vacant); } + } + + pub(crate) fn make_room(&mut self, id: I) { + let (index, _, _) = id.unzip(); + self.make_room_impl(index as usize) + } + + fn insert_impl(&mut self, index: usize, element: Element) { + self.make_room_impl(index); match std::mem::replace(&mut self.map[index], element) { Element::Vacant => {} _ => panic!("Index {:?} is already occupied", index),