commit 7268e7d36f8e1ae868916c4f663e2a68039cacf0
author Corentin Wallez <cwallez@chromium.org> Tue Aug 18 12:30:06 2020 +0000
committer Commit Bot service account <commit-bot@chromium.org> Tue Aug 18 12:30:06 2020 +0000
tree 2f73b51e4171a5db1ff5aa7e24d214038cbd9322
parent d95180deded12701abc5e13ee7fa9bd1bcdbb9cb

OpenGL: Unconditionally set glFrontFace

Previously glFrontFace was called only if some cull mode was set. This
was incorrect because the front face also influences whether a triangle
uses stencilFront or stencilBack.

Because OpenGL default to GL_CCW (which with the Y-flip is the inverse
of wgpu::FrontFace::CCW that's default in the descriptor), if
stencilFront != stencilBack and cull mode is none, then the incorrect
stencil face descriptor was used.

Also adds a regression test for this issue.

Bug: dawn:508
Change-Id: I00d93bda6d4f030cf9db472a9f2b0deefc72707f
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/26880
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
Reviewed-by: Austin Eng <enga@chromium.org>
Commit-Queue: Corentin Wallez <cwallez@chromium.org>

src/dawn_native/opengl/RenderPipelineGL.cpp

diff --git a/src/dawn_native/opengl/RenderPipelineGL.cpp b/src/dawn_native/opengl/RenderPipelineGL.cpp
index 943dfbb..7c70396 100644
--- a/src/dawn_native/opengl/RenderPipelineGL.cpp
+++ b/src/dawn_native/opengl/RenderPipelineGL.cpp
@@ -43,14 +43,15 @@
         void ApplyFrontFaceAndCulling(const OpenGLFunctions& gl,
                                       wgpu::FrontFace face,
                                       wgpu::CullMode mode) {
+            // Note that we invert winding direction in OpenGL. Because Y axis is up in OpenGL,
+            // which is different from WebGPU and other backends (Y axis is down).
+            GLenum direction = (face == wgpu::FrontFace::CCW) ? GL_CW : GL_CCW;
+            gl.FrontFace(direction);
+
             if (mode == wgpu::CullMode::None) {
                 gl.Disable(GL_CULL_FACE);
             } else {
                 gl.Enable(GL_CULL_FACE);
-                // Note that we invert winding direction in OpenGL. Because Y axis is up in OpenGL,
-                // which is different from WebGPU and other backends (Y axis is down).
-                GLenum direction = (face == wgpu::FrontFace::CCW) ? GL_CW : GL_CCW;
-                gl.FrontFace(direction);
 
                 GLenum cullMode = (mode == wgpu::CullMode::Front) ? GL_FRONT : GL_BACK;
                 gl.CullFace(cullMode);

src/tests/end2end/DepthStencilStateTests.cpp

diff --git a/src/tests/end2end/DepthStencilStateTests.cpp b/src/tests/end2end/DepthStencilStateTests.cpp
index 4788094..5f5895a 100644
--- a/src/tests/end2end/DepthStencilStateTests.cpp
+++ b/src/tests/end2end/DepthStencilStateTests.cpp
@@ -85,6 +85,7 @@
         RGBA8 color;
         float depth;
         uint32_t stencil;
+        wgpu::FrontFace frontFace = wgpu::FrontFace::CCW;
     };
 
     // Check whether a depth comparison function works as expected
@@ -288,6 +289,7 @@
             descriptor.cFragmentStage.module = fsModule;
             descriptor.cDepthStencilState = test.depthStencilState;
             descriptor.cDepthStencilState.format = wgpu::TextureFormat::Depth24PlusStencil8;
+            descriptor.cRasterizationState.frontFace = test.frontFace;
             descriptor.depthStencilState = &descriptor.cDepthStencilState;
 
             wgpu::RenderPipeline pipeline = device.CreateRenderPipeline(&descriptor);
@@ -734,6 +736,17 @@
     }
 }
 
+// Test that the front and back stencil states are set correctly (and take frontFace into account)
+TEST_P(DepthStencilStateTest, StencilFrontAndBackFace) {
+    wgpu::DepthStencilStateDescriptor state;
+    state.stencilFront.compare = wgpu::CompareFunction::Always;
+    state.stencilBack.compare = wgpu::CompareFunction::Never;
+
+    // The front facing triangle passes the stencil comparison but the back facing one doesn't.
+    DoTest({{state, RGBA8::kRed, 0.f, 0u, wgpu::FrontFace::CCW}}, RGBA8::kRed, RGBA8::kZero);
+    DoTest({{state, RGBA8::kRed, 0.f, 0u, wgpu::FrontFace::CW}}, RGBA8::kZero, RGBA8::kRed);
+}
+
 DAWN_INSTANTIATE_TEST(DepthStencilStateTest,
                       D3D12Backend(),
                       MetalBackend(),