gfx-rs / wgpu

A cross-platform, safe, pure-Rust graphics API.
https://wgpu.rs
Apache License 2.0
12.07k stars 874 forks source link

set_stencil_reference does not work with execute_bundles #4104

Open Pauan opened 12 months ago

Pauan commented 12 months ago

Description

Using set_stencil_reference does not work properly with execute_bundles.

It gives the incorrect result, and also produces nasty Vulkan error messages in the console.

Repro steps

I created a fork of wgpu, and then I slightly modified the stencil-triangles example.

  1. Pull the stencil-bundle-bug branch on my fork.
  2. Run cargo run --bin stencil-triangles

Expected vs observed behavior

This is how it's supposed to look:

Screenshot 2023-08-31 17:40:51

But instead it looks like this:

Screenshot 2023-08-31 17:41:43

In addition, I get these Vulkan error messages in the console:

[2023-09-01T00:42:16Z ERROR wgpu_hal::vulkan::instance] VALIDATION [VUID-vkCmdDraw-None-07839 (0x56f23620)]
        Validation Error: [ VUID-vkCmdDraw-None-07839 ] Object 0: handle = 0x55ee70b3ef20, type = VK_OBJECT_TYPE_COMMAND_BUFFER; | MessageID = 0x56f23620 | vkCmdDraw: VK_DYNAMIC_STATE_STENCIL_REFERENCE state not set for this command buffer. The Vulkan spec states: If the bound graphics pipeline state was created with the VK_DYNAMIC_STATE_STENCIL_REFERENCE dynamic state enabled then vkCmdSetStencilReference must have been called in the current command buffer prior to this drawing command (https://www.khronos.org/registry/vulkan/specs/1.3-extensions/html/vkspec.html#VUID-vkCmdDraw-None-07839)
[2023-09-01T00:42:16Z ERROR wgpu_hal::vulkan::instance]         objects: (type: COMMAND_BUFFER, hndl: 0x55ee70b3ef20, name: ?)

I also get incorrect results with the WebGL backend, so it's not a problem with Vulkan specifically.

Extra information

In the modified stencil-triangles example, there are two chunks of code:

rpass.set_stencil_reference(0);
rpass.execute_bundles(std::iter::once(&bundle1));

rpass.set_stencil_reference(1);
rpass.execute_bundles(std::iter::once(&bundle2));
rpass.set_stencil_reference(0);
rpass.set_pipeline(&self.mask_pipeline);
rpass.set_vertex_buffer(0, self.mask_vertex_buffer.slice(..));
rpass.draw(0..3, 0..1);

rpass.set_stencil_reference(1);
rpass.set_pipeline(&self.outer_pipeline);
rpass.set_vertex_buffer(0, self.outer_vertex_buffer.slice(..));
rpass.draw(0..3, 0..1);

Both chunks are identical, except that one chunk uses bundles and the other does not. You can use the if to switch between them.

API trace log:

wgpu-trace.zip

Platform

OS: NixOS Unstable 64-bit wgpu: https://github.com/gfx-rs/wgpu/commit/625afc3b4299c92c79e0b3881e22a0bb780678ed

Pauan commented 12 months ago

By the way, is there a particular reason why RenderBundleEncoder doesn't have a set_stencil_reference method?

cwfitzgerald commented 12 months ago

By the way, is there a particular reason why RenderBundleEncoder doesn't have a set_stencil_reference method?

WebGPU doesn't have it - not sure the specifics.

Pauan commented 12 months ago

P.S. Based on my reading of the spec, I believe that my code example should work.

It specifies that the render pass pipeline, bind groups, vertex buffers, and index buffer are not inherited. That implies that everything except for those things is inherited.

In addition, it says that the bundle cannot mutate the renderState, but it does inherit the renderState, which means that the bundle should inherit the [[stencilReference]] (which is a part of the renderState).

The setStencilReference method doesn't immediately change the renderState.[[stencilReference]], instead it adds a step into the queue which later sets the renderState.[[stencilReference]].

And the executeBundles method also adds a step into the queue.

So because they are both run on the queue, they should be processed in order:

  1. rpass.set_stencil_reference(0);
  2. rpass.execute_bundles(std::iter::once(&bundle1));
  3. rpass.set_stencil_reference(1);
  4. rpass.execute_bundles(std::iter::once(&bundle2));

Which means that when bundle1 is executed, it will inherit the stencil reference of 0, and when bundle2 is executed it will inherit the stencil reference of 1.

Pauan commented 12 months ago

So my reading of the spec is correct, which means that this is a confirmed bug in wgpu.

cwfitzgerald commented 11 months ago

@Pauan it would be really helpful if you could spin your existing test code into a proper regression test that fails by default on vulkan (it would go in tests/tests/regression/issue_4104.rs) - we can merge in the expected-failing test and fixing it will be significantly easier.