diff --git a/wgpu-hal/src/gles/command.rs b/wgpu-hal/src/gles/command.rs index 7c1eb1c841..8076a13aeb 100644 --- a/wgpu-hal/src/gles/command.rs +++ b/wgpu-hal/src/gles/command.rs @@ -620,6 +620,11 @@ impl crate::CommandEncoder for super::CommandEncoder { self.state.dirty_vbuf_mask = 0; self.state.active_first_instance = 0; self.state.color_targets.clear(); + for index in 0..self.state.vertex_attributes.len() { + self.cmd_buffer + .commands + .push(C::UnsetVertexAttribute(index as u32)); + } self.state.vertex_attributes.clear(); self.state.primitive = super::PrimitiveState::default(); } diff --git a/wgpu/tests/regression/issue_3457.rs b/wgpu/tests/regression/issue_3457.rs new file mode 100644 index 0000000000..f4359fb937 --- /dev/null +++ b/wgpu/tests/regression/issue_3457.rs @@ -0,0 +1,187 @@ +use crate::common::{initialize_test, TestParameters}; + +use wasm_bindgen_test::wasm_bindgen_test; +use wgpu::*; + +/// The core issue here was that we weren't properly disabling vertex attributes on GL +/// when a renderpass ends. This ended up being rather tricky to test for as GL is remarkably +/// tolerant of errors. This test, with the fix not-applied, only fails on WebGL. +/// +/// We need to setup a situation where it's invalid to issue a draw call without the fix. +/// To do this we first make a renderpass using two vertex buffers and draw on it. Then we +/// submit, delete the second vertex buffer and `poll(Wait)`. Because we maintained the device, +/// the actual underlying buffer for the second vertex buffer is deleted, causing a draw call +/// that is invalid if the second attribute is still enabled. +#[wasm_bindgen_test] +#[test] +fn pass_reset_vertex_buffer() { + initialize_test(TestParameters::default(), |ctx| { + let module = ctx + .device + .create_shader_module(include_wgsl!("issue_3457.wgsl")); + + // We use two separate vertex buffers so we can delete one in between submisions + let vertex_buffer1 = ctx.device.create_buffer(&BufferDescriptor { + label: Some("vertex buffer 1"), + size: 3 * 16, + usage: BufferUsages::VERTEX, + mapped_at_creation: false, + }); + + let vertex_buffer2 = ctx.device.create_buffer(&BufferDescriptor { + label: Some("vertex buffer 2"), + size: 3 * 4, + usage: BufferUsages::VERTEX, + mapped_at_creation: false, + }); + + let pipeline_layout = ctx + .device + .create_pipeline_layout(&PipelineLayoutDescriptor { + label: Some("Pipeline Layout"), + bind_group_layouts: &[], + push_constant_ranges: &[], + }); + + let double_pipeline = ctx + .device + .create_render_pipeline(&RenderPipelineDescriptor { + label: Some("Double Pipeline"), + layout: Some(&pipeline_layout), + vertex: VertexState { + module: &module, + entry_point: "double_buffer_vert", + buffers: &[ + VertexBufferLayout { + array_stride: 16, + step_mode: VertexStepMode::Vertex, + attributes: &vertex_attr_array![0 => Float32x4], + }, + VertexBufferLayout { + array_stride: 4, + step_mode: VertexStepMode::Vertex, + attributes: &vertex_attr_array![1 => Float32], + }, + ], + }, + primitive: PrimitiveState::default(), + depth_stencil: None, + multisample: MultisampleState::default(), + fragment: Some(FragmentState { + module: &module, + entry_point: "double_buffer_frag", + targets: &[Some(ColorTargetState { + format: TextureFormat::Rgba8Unorm, + blend: None, + write_mask: ColorWrites::all(), + })], + }), + multiview: None, + }); + + let single_pipeline = ctx + .device + .create_render_pipeline(&RenderPipelineDescriptor { + label: Some("Single Pipeline"), + layout: Some(&pipeline_layout), + vertex: VertexState { + module: &module, + entry_point: "single_buffer_vert", + buffers: &[VertexBufferLayout { + array_stride: 16, + step_mode: VertexStepMode::Vertex, + attributes: &vertex_attr_array![0 => Float32x4], + }], + }, + primitive: PrimitiveState::default(), + depth_stencil: None, + multisample: MultisampleState::default(), + fragment: Some(FragmentState { + module: &module, + entry_point: "single_buffer_frag", + targets: &[Some(ColorTargetState { + format: TextureFormat::Rgba8Unorm, + blend: None, + write_mask: ColorWrites::all(), + })], + }), + multiview: None, + }); + + let view = ctx + .device + .create_texture(&TextureDescriptor { + label: Some("Render texture"), + size: Extent3d { + width: 4, + height: 4, + depth_or_array_layers: 1, + }, + mip_level_count: 1, + sample_count: 1, + dimension: TextureDimension::D2, + format: TextureFormat::Rgba8Unorm, + usage: TextureUsages::RENDER_ATTACHMENT, + view_formats: &[], + }) + .create_view(&TextureViewDescriptor::default()); + + let mut encoder1 = ctx + .device + .create_command_encoder(&CommandEncoderDescriptor::default()); + + let mut double_rpass = encoder1.begin_render_pass(&RenderPassDescriptor { + label: Some("double renderpass"), + color_attachments: &[Some(RenderPassColorAttachment { + view: &view, + resolve_target: None, + ops: Operations { + load: LoadOp::Clear(Color::BLACK), + store: false, + }, + })], + depth_stencil_attachment: None, + }); + + double_rpass.set_pipeline(&double_pipeline); + double_rpass.set_vertex_buffer(0, vertex_buffer1.slice(..)); + double_rpass.set_vertex_buffer(1, vertex_buffer2.slice(..)); + double_rpass.draw(0..3, 0..1); + + drop(double_rpass); + + // Submit the first pass using both buffers + ctx.queue.submit(Some(encoder1.finish())); + // Drop the second buffer, meaning it's invalid to use draw + // unless it's unbound. + drop(vertex_buffer2); + + // Make sure the buffers are actually deleted. + ctx.device.poll(Maintain::Wait); + + let mut encoder2 = ctx + .device + .create_command_encoder(&CommandEncoderDescriptor::default()); + + let mut single_rpass = encoder2.begin_render_pass(&RenderPassDescriptor { + label: Some("single renderpass"), + color_attachments: &[Some(RenderPassColorAttachment { + view: &view, + resolve_target: None, + ops: Operations { + load: LoadOp::Clear(Color::BLACK), + store: false, + }, + })], + depth_stencil_attachment: None, + }); + + single_rpass.set_pipeline(&single_pipeline); + single_rpass.set_vertex_buffer(0, vertex_buffer1.slice(..)); + single_rpass.draw(0..3, 0..1); + + drop(single_rpass); + + ctx.queue.submit(Some(encoder2.finish())); + }) +} diff --git a/wgpu/tests/regression/issue_3457.wgsl b/wgpu/tests/regression/issue_3457.wgsl new file mode 100644 index 0000000000..f02aaa1bb1 --- /dev/null +++ b/wgpu/tests/regression/issue_3457.wgsl @@ -0,0 +1,33 @@ +struct DoubleVertexIn { + @location(0) position: vec4, + @location(1) value: f32, +} + +struct DoubleVertexOut { + @builtin(position) position: vec4, + @location(0) value: f32, +} + +@vertex +fn double_buffer_vert(v_in: DoubleVertexIn) -> DoubleVertexOut { + return DoubleVertexOut(v_in.position, v_in.value); +} + +@fragment +fn double_buffer_frag(v_out: DoubleVertexOut) -> @location(0) vec4 { + return vec4(v_out.value); +} + +struct SingleVertexIn { + @location(0) position: vec4, +} + +@vertex +fn single_buffer_vert(v_in: SingleVertexIn) -> @builtin(position) vec4 { + return v_in.position; +} + +@fragment +fn single_buffer_frag() -> @location(0) vec4 { + return vec4(0.0); +} diff --git a/wgpu/tests/root.rs b/wgpu/tests/root.rs index a8676fb959..277b34eb65 100644 --- a/wgpu/tests/root.rs +++ b/wgpu/tests/root.rs @@ -3,6 +3,9 @@ use wasm_bindgen_test::wasm_bindgen_test_configure; // All files containing tests mod common; +mod regression { + mod issue_3457; +} mod buffer; mod buffer_copy; mod buffer_usages;