diff --git a/CHANGELOG.md b/CHANGELOG.md index d0eb15b5ae..32f90b3d90 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -64,6 +64,28 @@ let render_pass = encoder.begin_render_pass(&wgpu::RenderPassDescriptor { By @Valaphee in [#3402](https://github.com/gfx-rs/wgpu/pull/3402) + +#### Render pass store operation is now an enum + +`wgpu::Operations::store` used to be an underdocumented boolean value, +causing misunderstandings of the effect of setting it to `false`. + +The API now more closely resembles WebGPU which distinguishes between `store` and `discard`, +see [WebGPU spec on GPUStoreOp](https://gpuweb.github.io/gpuweb/#enumdef-gpustoreop). + +```diff +// ... +depth_ops: Some(wgpu::Operations { + load: wgpu::LoadOp::Clear(1.0), +- store: false, ++ store: wgpu::StoreOp::Discard, +}), +// ... +``` + +By @wumpf in [#4147](https://github.com/gfx-rs/wgpu/pull/4147) + + ### Added/New Features - Add `gles_minor_version` field to `wgpu::InstanceDescriptor`. By @PJB3005 in [#3998](https://github.com/gfx-rs/wgpu/pull/3998) diff --git a/examples/boids/src/main.rs b/examples/boids/src/main.rs index e8aa2f71fd..eb5146f8bd 100644 --- a/examples/boids/src/main.rs +++ b/examples/boids/src/main.rs @@ -276,7 +276,7 @@ impl wgpu_example::framework::Example for Example { // Not clearing here in order to test wgpu's zero texture initialization on a surface texture. // Users should avoid loading uninitialized memory since this can cause additional overhead. load: wgpu::LoadOp::Load, - store: true, + store: wgpu::StoreOp::Store, }, })]; let render_pass_descriptor = wgpu::RenderPassDescriptor { diff --git a/examples/bunnymark/src/main.rs b/examples/bunnymark/src/main.rs index 256083eebb..ca8dbbee06 100644 --- a/examples/bunnymark/src/main.rs +++ b/examples/bunnymark/src/main.rs @@ -332,7 +332,7 @@ impl wgpu_example::framework::Example for Example { resolve_target: None, ops: wgpu::Operations { load: wgpu::LoadOp::Clear(clear_color), - store: true, + store: wgpu::StoreOp::Store, }, })], depth_stencil_attachment: None, diff --git a/examples/capture/src/main.rs b/examples/capture/src/main.rs index b783b3af80..47a453de6b 100644 --- a/examples/capture/src/main.rs +++ b/examples/capture/src/main.rs @@ -101,7 +101,7 @@ async fn create_red_image_with_dimensions( resolve_target: None, ops: wgpu::Operations { load: wgpu::LoadOp::Clear(wgpu::Color::RED), - store: true, + store: wgpu::StoreOp::Store, }, })], depth_stencil_attachment: None, diff --git a/examples/conservative-raster/src/main.rs b/examples/conservative-raster/src/main.rs index e5cfb4d775..093740a206 100644 --- a/examples/conservative-raster/src/main.rs +++ b/examples/conservative-raster/src/main.rs @@ -269,7 +269,7 @@ impl wgpu_example::framework::Example for Example { resolve_target: None, ops: wgpu::Operations { load: wgpu::LoadOp::Clear(wgpu::Color::BLACK), - store: true, + store: wgpu::StoreOp::Store, }, })], depth_stencil_attachment: None, @@ -290,7 +290,7 @@ impl wgpu_example::framework::Example for Example { resolve_target: None, ops: wgpu::Operations { load: wgpu::LoadOp::Clear(wgpu::Color::BLACK), - store: true, + store: wgpu::StoreOp::Store, }, })], depth_stencil_attachment: None, diff --git a/examples/cube/src/main.rs b/examples/cube/src/main.rs index a10dfd0fd0..b031e1004c 100644 --- a/examples/cube/src/main.rs +++ b/examples/cube/src/main.rs @@ -375,7 +375,7 @@ impl wgpu_example::framework::Example for Example { b: 0.3, a: 1.0, }), - store: true, + store: wgpu::StoreOp::Store, }, })], depth_stencil_attachment: None, diff --git a/examples/hello-triangle/src/main.rs b/examples/hello-triangle/src/main.rs index c5432acd07..ebb8f6b736 100644 --- a/examples/hello-triangle/src/main.rs +++ b/examples/hello-triangle/src/main.rs @@ -118,7 +118,7 @@ async fn run(event_loop: EventLoop<()>, window: Window) { resolve_target: None, ops: wgpu::Operations { load: wgpu::LoadOp::Clear(wgpu::Color::GREEN), - store: true, + store: wgpu::StoreOp::Store, }, })], depth_stencil_attachment: None, diff --git a/examples/hello-windows/src/main.rs b/examples/hello-windows/src/main.rs index f368804c36..ba28341395 100644 --- a/examples/hello-windows/src/main.rs +++ b/examples/hello-windows/src/main.rs @@ -131,7 +131,7 @@ async fn run(event_loop: EventLoop<()>, viewports: Vec<(Window, wgpu::Color)>) { resolve_target: None, ops: wgpu::Operations { load: wgpu::LoadOp::Clear(viewport.desc.background), - store: true, + store: wgpu::StoreOp::Store, }, })], depth_stencil_attachment: None, diff --git a/examples/mipmap/src/main.rs b/examples/mipmap/src/main.rs index a85110ff14..5536579b0b 100644 --- a/examples/mipmap/src/main.rs +++ b/examples/mipmap/src/main.rs @@ -163,7 +163,7 @@ impl Example { resolve_target: None, ops: wgpu::Operations { load: wgpu::LoadOp::Clear(wgpu::Color::WHITE), - store: true, + store: wgpu::StoreOp::Store, }, })], depth_stencil_attachment: None, @@ -490,7 +490,7 @@ impl wgpu_example::framework::Example for Example { resolve_target: None, ops: wgpu::Operations { load: wgpu::LoadOp::Clear(clear_color), - store: true, + store: wgpu::StoreOp::Store, }, })], depth_stencil_attachment: None, diff --git a/examples/msaa-line/src/main.rs b/examples/msaa-line/src/main.rs index aa7a277418..07cc4eaf57 100644 --- a/examples/msaa-line/src/main.rs +++ b/examples/msaa-line/src/main.rs @@ -282,7 +282,7 @@ impl wgpu_example::framework::Example for Example { resolve_target: None, ops: wgpu::Operations { load: wgpu::LoadOp::Clear(wgpu::Color::BLACK), - store: true, + store: wgpu::StoreOp::Store, }, } } else { @@ -293,7 +293,7 @@ impl wgpu_example::framework::Example for Example { load: wgpu::LoadOp::Clear(wgpu::Color::BLACK), // Storing pre-resolve MSAA data is unnecessary if it isn't used later. // On tile-based GPU, avoid store can reduce your app's memory footprint. - store: false, + store: wgpu::StoreOp::Discard, }, } }; diff --git a/examples/shadow/src/main.rs b/examples/shadow/src/main.rs index 3f963d0c53..c63076e6ac 100644 --- a/examples/shadow/src/main.rs +++ b/examples/shadow/src/main.rs @@ -773,7 +773,7 @@ impl wgpu_example::framework::Example for Example { view: &light.target_view, depth_ops: Some(wgpu::Operations { load: wgpu::LoadOp::Clear(1.0), - store: true, + store: wgpu::StoreOp::Store, }), stencil_ops: None, }), @@ -810,14 +810,14 @@ impl wgpu_example::framework::Example for Example { b: 0.3, a: 1.0, }), - store: true, + store: wgpu::StoreOp::Store, }, })], depth_stencil_attachment: Some(wgpu::RenderPassDepthStencilAttachment { view: &self.forward_depth, depth_ops: Some(wgpu::Operations { load: wgpu::LoadOp::Clear(1.0), - store: false, + store: wgpu::StoreOp::Discard, }), stencil_ops: None, }), diff --git a/examples/skybox/src/main.rs b/examples/skybox/src/main.rs index d09622f53c..5d91c62865 100644 --- a/examples/skybox/src/main.rs +++ b/examples/skybox/src/main.rs @@ -428,14 +428,14 @@ impl wgpu_example::framework::Example for Skybox { b: 0.3, a: 1.0, }), - store: true, + store: wgpu::StoreOp::Store, }, })], depth_stencil_attachment: Some(wgpu::RenderPassDepthStencilAttachment { view: &self.depth_view, depth_ops: Some(wgpu::Operations { load: wgpu::LoadOp::Clear(1.0), - store: false, + store: wgpu::StoreOp::Discard, }), stencil_ops: None, }), diff --git a/examples/stencil-triangles/src/main.rs b/examples/stencil-triangles/src/main.rs index 55aad9c9ba..9d918500d5 100644 --- a/examples/stencil-triangles/src/main.rs +++ b/examples/stencil-triangles/src/main.rs @@ -200,7 +200,7 @@ impl wgpu_example::framework::Example for Triangles { b: 0.3, a: 1.0, }), - store: true, + store: wgpu::StoreOp::Store, }, })], depth_stencil_attachment: Some(wgpu::RenderPassDepthStencilAttachment { @@ -208,7 +208,7 @@ impl wgpu_example::framework::Example for Triangles { depth_ops: None, stencil_ops: Some(wgpu::Operations { load: wgpu::LoadOp::Clear(0), - store: true, + store: wgpu::StoreOp::Store, }), }), timestamp_writes: None, diff --git a/examples/texture-arrays/src/main.rs b/examples/texture-arrays/src/main.rs index 373c2396ae..af9cfa56ff 100644 --- a/examples/texture-arrays/src/main.rs +++ b/examples/texture-arrays/src/main.rs @@ -379,7 +379,7 @@ impl wgpu_example::framework::Example for Example { resolve_target: None, ops: wgpu::Operations { load: wgpu::LoadOp::Clear(wgpu::Color::BLACK), - store: true, + store: wgpu::StoreOp::Store, }, })], depth_stencil_attachment: None, diff --git a/examples/timestamp-queries/src/main.rs b/examples/timestamp-queries/src/main.rs index f8c524f03c..d463ea6579 100644 --- a/examples/timestamp-queries/src/main.rs +++ b/examples/timestamp-queries/src/main.rs @@ -375,7 +375,7 @@ fn render_pass( resolve_target: None, ops: wgpu::Operations { load: wgpu::LoadOp::Clear(wgpu::Color::GREEN), - store: true, + store: wgpu::StoreOp::Store, }, })], depth_stencil_attachment: None, diff --git a/examples/water/src/main.rs b/examples/water/src/main.rs index 5d5daa1f59..da9f1238ab 100644 --- a/examples/water/src/main.rs +++ b/examples/water/src/main.rs @@ -739,7 +739,7 @@ impl wgpu_example::framework::Example for Example { resolve_target: None, ops: wgpu::Operations { load: wgpu::LoadOp::Clear(back_color), - store: true, + store: wgpu::StoreOp::Store, }, })], // We still need to use the depth buffer here @@ -748,7 +748,7 @@ impl wgpu_example::framework::Example for Example { view: &self.depth_buffer, depth_ops: Some(wgpu::Operations { load: wgpu::LoadOp::Clear(1.0), - store: true, + store: wgpu::StoreOp::Store, }), stencil_ops: None, }), @@ -768,14 +768,14 @@ impl wgpu_example::framework::Example for Example { resolve_target: None, ops: wgpu::Operations { load: wgpu::LoadOp::Clear(back_color), - store: true, + store: wgpu::StoreOp::Store, }, })], depth_stencil_attachment: Some(wgpu::RenderPassDepthStencilAttachment { view: &self.depth_buffer, depth_ops: Some(wgpu::Operations { load: wgpu::LoadOp::Clear(1.0), - store: true, + store: wgpu::StoreOp::Store, }), stencil_ops: None, }), @@ -797,7 +797,7 @@ impl wgpu_example::framework::Example for Example { resolve_target: None, ops: wgpu::Operations { load: wgpu::LoadOp::Load, - store: true, + store: wgpu::StoreOp::Store, }, })], depth_stencil_attachment: Some(wgpu::RenderPassDepthStencilAttachment { diff --git a/tests/tests/occlusion_query/mod.rs b/tests/tests/occlusion_query/mod.rs index eab0828e41..7747eaa624 100644 --- a/tests/tests/occlusion_query/mod.rs +++ b/tests/tests/occlusion_query/mod.rs @@ -69,7 +69,7 @@ fn occlusion_query() { view: &depth_texture_view, depth_ops: Some(wgpu::Operations { load: wgpu::LoadOp::Clear(1.0), - store: true, + store: wgpu::StoreOp::Store, }), stencil_ops: None, }), diff --git a/tests/tests/regression/issue_3457.rs b/tests/tests/regression/issue_3457.rs index 2dccd3d427..0d2c086ed5 100644 --- a/tests/tests/regression/issue_3457.rs +++ b/tests/tests/regression/issue_3457.rs @@ -140,7 +140,7 @@ fn pass_reset_vertex_buffer() { resolve_target: None, ops: Operations { load: LoadOp::Clear(Color::BLACK), - store: false, + store: StoreOp::Discard, }, })], depth_stencil_attachment: None, @@ -175,7 +175,7 @@ fn pass_reset_vertex_buffer() { resolve_target: None, ops: Operations { load: LoadOp::Clear(Color::BLACK), - store: false, + store: StoreOp::Discard, }, })], depth_stencil_attachment: None, diff --git a/tests/tests/scissor_tests/mod.rs b/tests/tests/scissor_tests/mod.rs index da050cb61f..a921827e0d 100644 --- a/tests/tests/scissor_tests/mod.rs +++ b/tests/tests/scissor_tests/mod.rs @@ -75,7 +75,7 @@ fn scissor_test_impl(ctx: &TestingContext, scissor_rect: Rect, expected_data: [u b: 0.0, a: 0.0, }), - store: true, + store: wgpu::StoreOp::Store, }, })], depth_stencil_attachment: None, diff --git a/tests/tests/shader_primitive_index/mod.rs b/tests/tests/shader_primitive_index/mod.rs index a05d1cd5f0..2739b2e77d 100644 --- a/tests/tests/shader_primitive_index/mod.rs +++ b/tests/tests/shader_primitive_index/mod.rs @@ -180,7 +180,7 @@ fn pulling_common( color_attachments: &[Some(wgpu::RenderPassColorAttachment { ops: wgpu::Operations { load: wgpu::LoadOp::Clear(wgpu::Color::WHITE), - store: true, + store: wgpu::StoreOp::Store, }, resolve_target: None, view: &color_view, diff --git a/tests/tests/zero_init_texture_after_discard.rs b/tests/tests/zero_init_texture_after_discard.rs index 2b757e069a..e47f1aa0fa 100644 --- a/tests/tests/zero_init_texture_after_discard.rs +++ b/tests/tests/zero_init_texture_after_discard.rs @@ -155,11 +155,11 @@ impl<'ctx> TestCase<'ctx> { view: &texture.create_view(&TextureViewDescriptor::default()), depth_ops: format.has_depth_aspect().then_some(Operations { load: LoadOp::Clear(1.0), - store: true, + store: StoreOp::Store, }), stencil_ops: format.has_stencil_aspect().then_some(Operations { load: LoadOp::Clear(0xFFFFFFFF), - store: true, + store: StoreOp::Store, }), }), timestamp_writes: None, @@ -230,7 +230,7 @@ impl<'ctx> TestCase<'ctx> { resolve_target: None, ops: Operations { load: LoadOp::Load, - store: false, // discard! + store: StoreOp::Discard, }, }, )], @@ -239,11 +239,11 @@ impl<'ctx> TestCase<'ctx> { view: &self.texture.create_view(&TextureViewDescriptor::default()), depth_ops: self.format.has_depth_aspect().then_some(Operations { load: LoadOp::Load, - store: false, // discard! + store: StoreOp::Discard, }), stencil_ops: self.format.has_stencil_aspect().then_some(Operations { load: LoadOp::Load, - store: false, // discard! + store: StoreOp::Discard, }), }, ), @@ -264,11 +264,11 @@ impl<'ctx> TestCase<'ctx> { view: &self.texture.create_view(&TextureViewDescriptor::default()), depth_ops: Some(Operations { load: LoadOp::Load, - store: false, // discard! + store: StoreOp::Discard, }), stencil_ops: self.format.has_stencil_aspect().then_some(Operations { load: LoadOp::Clear(0), - store: true, + store: StoreOp::Store, }), }, ), @@ -289,11 +289,11 @@ impl<'ctx> TestCase<'ctx> { view: &self.texture.create_view(&TextureViewDescriptor::default()), depth_ops: self.format.has_depth_aspect().then_some(Operations { load: LoadOp::Clear(0.0), - store: true, + store: StoreOp::Store, }), stencil_ops: Some(Operations { load: LoadOp::Load, - store: false, // discard! + store: StoreOp::Discard, }), }, ), diff --git a/wgpu/src/backend/direct.rs b/wgpu/src/backend/direct.rs index 2e15e295e8..3d3028d334 100644 --- a/wgpu/src/backend/direct.rs +++ b/wgpu/src/backend/direct.rs @@ -4,7 +4,7 @@ use crate::{ BufferDescriptor, CommandEncoderDescriptor, ComputePassDescriptor, ComputePipelineDescriptor, DownlevelCapabilities, Features, Label, Limits, LoadOp, MapMode, Operations, PipelineLayoutDescriptor, RenderBundleEncoderDescriptor, RenderPipelineDescriptor, - SamplerDescriptor, ShaderModuleDescriptor, ShaderModuleDescriptorSpirV, ShaderSource, + SamplerDescriptor, ShaderModuleDescriptor, ShaderModuleDescriptorSpirV, ShaderSource, StoreOp, SurfaceStatus, TextureDescriptor, TextureViewDescriptor, UncapturedErrorHandler, }; @@ -392,6 +392,13 @@ fn map_texture_tagged_copy_view( } } +fn map_store_op(op: StoreOp) -> wgc::command::StoreOp { + match op { + StoreOp::Store => wgc::command::StoreOp::Store, + StoreOp::Discard => wgc::command::StoreOp::Discard, + } +} + fn map_pass_channel( ops: Option<&Operations>, ) -> wgc::command::PassChannel { @@ -401,11 +408,7 @@ fn map_pass_channel( store, }) => wgc::command::PassChannel { load_op: wgc::command::LoadOp::Clear, - store_op: if store { - wgc::command::StoreOp::Store - } else { - wgc::command::StoreOp::Discard - }, + store_op: map_store_op(store), clear_value, read_only: false, }, @@ -414,11 +417,7 @@ fn map_pass_channel( store, }) => wgc::command::PassChannel { load_op: wgc::command::LoadOp::Load, - store_op: if store { - wgc::command::StoreOp::Store - } else { - wgc::command::StoreOp::Discard - }, + store_op: map_store_op(store), clear_value: V::default(), read_only: false, }, diff --git a/wgpu/src/backend/web.rs b/wgpu/src/backend/web.rs index 2f83d50c55..d457681e57 100644 --- a/wgpu/src/backend/web.rs +++ b/wgpu/src/backend/web.rs @@ -621,11 +621,10 @@ fn map_color(color: wgt::Color) -> web_sys::GpuColorDict { web_sys::GpuColorDict::new(color.a, color.b, color.g, color.r) } -fn map_store_op(store: bool) -> web_sys::GpuStoreOp { - if store { - web_sys::GpuStoreOp::Store - } else { - web_sys::GpuStoreOp::Discard +fn map_store_op(store: crate::StoreOp) -> web_sys::GpuStoreOp { + match store { + crate::StoreOp::Store => web_sys::GpuStoreOp::Store, + crate::StoreOp::Discard => web_sys::GpuStoreOp::Discard, } } diff --git a/wgpu/src/lib.rs b/wgpu/src/lib.rs index 19dc20120d..88b852eaa4 100644 --- a/wgpu/src/lib.rs +++ b/wgpu/src/lib.rs @@ -1006,16 +1006,24 @@ static_assertions::assert_impl_all!(BufferBinding: Send, Sync); /// Operation to perform to the output attachment at the start of a render pass. /// -/// The render target must be cleared at least once before its content is loaded. -/// -/// Corresponds to [WebGPU `GPULoadOp`](https://gpuweb.github.io/gpuweb/#enumdef-gpuloadop). +/// Corresponds to [WebGPU `GPULoadOp`](https://gpuweb.github.io/gpuweb/#enumdef-gpuloadop), +/// plus the corresponding clearValue. #[derive(Copy, Clone, Debug, Hash, Eq, PartialEq)] #[cfg_attr(feature = "trace", derive(serde::Serialize))] #[cfg_attr(feature = "replay", derive(serde::Deserialize))] pub enum LoadOp { - /// Clear with a specified value. + /// Loads the specified value for this attachment into the render pass. + /// + /// On some GPU hardware (primarily mobile), "clear" is significantly cheaper + /// because it avoids loading data from main memory into tile-local memory. + /// + /// On other GPU hardware, there isn’t a significant difference. + /// + /// As a result, it is recommended to use "clear" rather than "load" in cases + /// where the initial value doesn’t matter + /// (e.g. the render target will be cleared using a skybox). Clear(V), - /// Load from memory. + /// Loads the existing value for this attachment into the render pass. Load, } @@ -1025,6 +1033,28 @@ impl Default for LoadOp { } } +/// Operation to perform to the output attachment at the end of a render pass. +/// +/// Corresponds to [WebGPU `GPUStoreOp`](https://gpuweb.github.io/gpuweb/#enumdef-gpustoreop). +#[derive(Copy, Clone, Debug, Hash, Eq, PartialEq, Default)] +#[cfg_attr(feature = "trace", derive(serde::Serialize))] +#[cfg_attr(feature = "replay", derive(serde::Deserialize))] +pub enum StoreOp { + /// Stores the resulting value of the render pass for this attachment. + #[default] + Store, + /// Discards the resulting value of the render pass for this attachment. + /// + /// The attachment will be treated as uninitialized afterwards. + /// (If only either Depth or Stencil texture-aspects is set to `Discard`, + /// the respective other texture-aspect will be preserved.) + /// + /// This can be significantly faster on tile-based render hardware. + /// + /// Prefer this if the attachment is not read by subsequent passes. + Discard, +} + /// Pair of load and store operations for an attachment aspect. /// /// This type is unique to the Rust API of `wgpu`. In the WebGPU specification, @@ -1036,14 +1066,18 @@ pub struct Operations { /// How data should be read through this attachment. pub load: LoadOp, /// Whether data will be written to through this attachment. - pub store: bool, + /// + /// Note that resolve textures (if specified) are always written to, + /// regardless of this setting. + pub store: StoreOp, } impl Default for Operations { + #[inline] fn default() -> Self { Self { - load: Default::default(), - store: true, + load: LoadOp::::default(), + store: StoreOp::default(), } } } @@ -1084,6 +1118,8 @@ pub struct RenderPassColorAttachment<'tex> { /// The view to use as an attachment. pub view: &'tex TextureView, /// The view that will receive the resolved output if multisampling is used. + /// + /// If set, it is always written to, regardless of how [`Self::ops`] is configured. pub resolve_target: Option<&'tex TextureView>, /// What operations will be performed on this color attachment. pub ops: Operations,