Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WebGPU backend fix to deal with Dawn change #7232

Closed
GrigoryGraborenko opened this issue Jan 17, 2024 · 1 comment
Closed

WebGPU backend fix to deal with Dawn change #7232

GrigoryGraborenko opened this issue Jan 17, 2024 · 1 comment

Comments

@GrigoryGraborenko
Copy link

Version/Branch of Dear ImGui:

Version 1.9.0, Branch: master

Back-ends:

imgui_impl_wgpu.cpp

Compiler, OS:

Windows 10 + MSVC 2022

Full config/build information:

Dear ImGui 1.90 WIP (18993)
--------------------------------
sizeof(size_t): 8, sizeof(ImDrawIdx): 2, sizeof(ImDrawVert): 20
define: __cplusplus=201703
define: _WIN32
define: _WIN64
define: _MSC_VER=1937
define: _MSVC_LANG=201703
--------------------------------
io.BackendPlatformName: imgui_impl_sdl2
io.BackendRendererName: imgui_impl_webgpu
io.ConfigFlags: 0x00000000
io.ConfigInputTextCursorBlink
io.ConfigWindowsResizeFromEdges
io.ConfigMemoryCompactTimer = 60.0
io.BackendFlags: 0x0000000E
 HasMouseCursors
 HasSetMousePos
 RendererHasVtxOffset
--------------------------------
io.Fonts: 1 fonts, Flags: 0x00000000, TexSize: 512,64
io.DisplaySize: 1280.00,720.00
io.DisplayFramebufferScale: 1.00,1.00
--------------------------------
style.WindowPadding: 8.00,8.00
style.WindowBorderSize: 1.00
style.FramePadding: 4.00,3.00
style.FrameRounding: 0.00
style.FrameBorderSize: 0.00
style.ItemSpacing: 8.00,4.00
style.ItemInnerSpacing: 4.00,4.00

Details:

Recently there was a change in Dawn's native WebGPU implementation where they no longer gave sensible defaults to stencil operations for render pipelines:
https://dawn.googlesource.com/dawn/+/628a1eb2a720bef3dd25bf352094c76a74514efe%5E%21/#F1

When validating the render pipeline, this condition gets triggered:

DAWN_INVALID_IF(!format->HasStencil() && StencilTestEnabled(descriptor),
                "Depth stencil format (%s) doesn't have stencil aspect while stencil "
                "test or stencil write is enabled.",
                descriptor->format);

which calls this function:

bool StencilTestEnabled(const DepthStencilState* depthStencil) {
    return depthStencil->stencilBack.compare != wgpu::CompareFunction::Always ||
           depthStencil->stencilBack.failOp != wgpu::StencilOperation::Keep ||
           depthStencil->stencilBack.depthFailOp != wgpu::StencilOperation::Keep ||
           depthStencil->stencilBack.passOp != wgpu::StencilOperation::Keep ||
           depthStencil->stencilFront.compare != wgpu::CompareFunction::Always ||
           depthStencil->stencilFront.failOp != wgpu::StencilOperation::Keep ||
           depthStencil->stencilFront.depthFailOp != wgpu::StencilOperation::Keep ||
           depthStencil->stencilFront.passOp != wgpu::StencilOperation::Keep;
}

Those wgpu::StencilOperation::Keep are actually 1 not 0 so the defaults immediately fail validation if you don't set them.

In imgui_impl_wgpu.cpp on line 651 you create a WGPUDepthStencilState and set the compare functions:

    // Create depth-stencil State
    WGPUDepthStencilState depth_stencil_state = {};
    depth_stencil_state.format = bd->depthStencilFormat;
    depth_stencil_state.depthWriteEnabled = false;
    depth_stencil_state.depthCompare = WGPUCompareFunction_Always;
    depth_stencil_state.stencilFront.compare = WGPUCompareFunction_Always;
    depth_stencil_state.stencilBack.compare = WGPUCompareFunction_Always;

May I humbly request we add these lines below:

	depth_stencil_state.stencilFront.failOp = WGPUStencilOperation_Keep;
	depth_stencil_state.stencilFront.depthFailOp = WGPUStencilOperation_Keep;
	depth_stencil_state.stencilFront.passOp = WGPUStencilOperation_Keep;
	depth_stencil_state.stencilBack.failOp = WGPUStencilOperation_Keep;
	depth_stencil_state.stencilBack.depthFailOp = WGPUStencilOperation_Keep;
	depth_stencil_state.stencilBack.passOp = WGPUStencilOperation_Keep;

I was able to confirm this sets the expected defaults and passes pipeline validation.

Screenshots/Video:

No response

Minimal, Complete and Verifiable Example code:

No response

ocornut added a commit that referenced this issue Jan 17, 2024
… as a recent Dawn update stopped setting default values. (#7232)
@ocornut
Copy link
Owner

ocornut commented Jan 17, 2024

Thank you Grigory for your careful and researched issue!
I have pushed your suggested fix as 03417cc

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants