From bb9e874c837e070584e355c5d54ee91c1d88c0ea Mon Sep 17 00:00:00 2001 From: valadaptive <79560998+valadaptive@users.noreply.github.com> Date: Thu, 19 Sep 2024 03:16:42 -0400 Subject: [PATCH] Update sampler along with texture on wgpu backend (#5122) * Closes #5121 * [x] I have followed the instructions in the PR template This unifies the code paths in `update_texture` somewhat, so that the texture sampler and bind group are always replaced. Not sure whether removing and reinserting the texture from and into the `textures` map, or creating a new bind group, has much of a performance impact. An alternative, as described in #5121, would be to split the functionality for updating a texture's data from updating its options, so that we don't have to unconditionally update the bind group (or do something like store the options to check if they're changed). --- crates/egui-wgpu/src/renderer.rs | 96 ++++++++++++++++++++++++-------- 1 file changed, 72 insertions(+), 24 deletions(-) diff --git a/crates/egui-wgpu/src/renderer.rs b/crates/egui-wgpu/src/renderer.rs index d4e0e7062db..eda934944ea 100644 --- a/crates/egui-wgpu/src/renderer.rs +++ b/crates/egui-wgpu/src/renderer.rs @@ -163,6 +163,18 @@ struct SlicedBuffer { capacity: wgpu::BufferAddress, } +pub struct Texture { + /// The texture may be None if the `TextureId` is just a handle to a user-provided bind-group. + pub texture: Option, + + /// Bindgroup for the texture + sampler. + pub bind_group: wgpu::BindGroup, + + /// Options describing the sampler used in the bind group. This may be None if the `TextureId` + /// is just a handle to a user-provided bind-group. + pub options: Option, +} + /// Renderer for a egui based GUI. pub struct Renderer { pipeline: wgpu::RenderPipeline, @@ -178,7 +190,7 @@ pub struct Renderer { /// Map of egui texture IDs to textures and their associated bindgroups (texture view + /// sampler). The texture may be None if the `TextureId` is just a handle to a user-provided /// sampler. - textures: HashMap, wgpu::BindGroup)>, + textures: HashMap, next_user_texture_id: u64, samplers: HashMap, @@ -454,7 +466,7 @@ impl Renderer { let index_buffer_slice = index_buffer_slices.next().unwrap(); let vertex_buffer_slice = vertex_buffer_slices.next().unwrap(); - if let Some((_texture, bind_group)) = self.textures.get(&mesh.texture_id) { + if let Some(Texture { bind_group, .. }) = self.textures.get(&mesh.texture_id) { render_pass.set_bind_group(1, bind_group, &[]); render_pass.set_index_buffer( self.index_buffer.buffer.slice( @@ -577,26 +589,41 @@ impl Renderer { ); }; - if let Some(pos) = image_delta.pos { + // Use same label for all resources associated with this texture id (no point in retyping the type) + let label_str = format!("egui_texid_{id:?}"); + let label = Some(label_str.as_str()); + + let (texture, origin, bind_group) = if let Some(pos) = image_delta.pos { // update the existing texture - let (texture, _bind_group) = self + let Texture { + texture, + bind_group, + options, + } = self .textures - .get(&id) + .remove(&id) .expect("Tried to update a texture that has not been allocated yet."); + let texture = texture.expect("Tried to update user texture."); + let options = options.expect("Tried to update user texture."); let origin = wgpu::Origin3d { x: pos[0] as u32, y: pos[1] as u32, z: 0, }; - queue_write_data_to_texture( - texture.as_ref().expect("Tried to update user texture."), + + ( + texture, origin, - ); + // If the TextureOptions are the same as the previous ones, we can reuse the bind group. Otherwise we + // have to recreate it. + if image_delta.options == options { + Some(bind_group) + } else { + None + }, + ) } else { // allocate a new texture - // Use same label for all resources associated with this texture id (no point in retyping the type) - let label_str = format!("egui_texid_{id:?}"); - let label = Some(label_str.as_str()); let texture = { crate::profile_scope!("create_texture"); device.create_texture(&wgpu::TextureDescriptor { @@ -610,11 +637,16 @@ impl Renderer { view_formats: &[wgpu::TextureFormat::Rgba8UnormSrgb], }) }; + let origin = wgpu::Origin3d::ZERO; + (texture, origin, None) + }; + + let bind_group = bind_group.unwrap_or_else(|| { let sampler = self .samplers .entry(image_delta.options) .or_insert_with(|| create_sampler(image_delta.options, device)); - let bind_group = device.create_bind_group(&wgpu::BindGroupDescriptor { + device.create_bind_group(&wgpu::BindGroupDescriptor { label, layout: &self.texture_bind_group_layout, entries: &[ @@ -629,25 +661,31 @@ impl Renderer { resource: wgpu::BindingResource::Sampler(sampler), }, ], - }); - let origin = wgpu::Origin3d::ZERO; - queue_write_data_to_texture(&texture, origin); - self.textures.insert(id, (Some(texture), bind_group)); - }; + }) + }); + + queue_write_data_to_texture(&texture, origin); + self.textures.insert( + id, + Texture { + texture: Some(texture), + bind_group, + options: Some(image_delta.options), + }, + ); } pub fn free_texture(&mut self, id: &epaint::TextureId) { - self.textures.remove(id); + if let Some(texture) = self.textures.remove(id).and_then(|t| t.texture) { + texture.destroy(); + } } /// Get the WGPU texture and bind group associated to a texture that has been allocated by egui. /// /// This could be used by custom paint hooks to render images that have been added through /// [`epaint::Context::load_texture`](https://docs.rs/egui/latest/egui/struct.Context.html#method.load_texture). - pub fn texture( - &self, - id: &epaint::TextureId, - ) -> Option<&(Option, wgpu::BindGroup)> { + pub fn texture(&self, id: &epaint::TextureId) -> Option<&Texture> { self.textures.get(id) } @@ -735,7 +773,14 @@ impl Renderer { }); let id = epaint::TextureId::User(self.next_user_texture_id); - self.textures.insert(id, (None, bind_group)); + self.textures.insert( + id, + Texture { + texture: None, + bind_group, + options: None, + }, + ); self.next_user_texture_id += 1; id @@ -755,7 +800,10 @@ impl Renderer { ) { crate::profile_function!(); - let (_user_texture, user_texture_binding) = self + let Texture { + bind_group: user_texture_binding, + .. + } = self .textures .get_mut(&id) .expect("Tried to update a texture that has not been allocated yet.");