Simplify PersistentPipelineState and application of stencil states. Fix

stencil mask usage. D3D12 does not support separate front/back masks.
All APIs support separate read/write masks.

examples/HelloDepthStencil.cpp

@@ -102,7 +102,7 @@ void initBuffers() {
     planeBuffer = CreateFrozenBufferFromData(device, (void*)planeData, sizeof(planeData), nxt::BufferUsageBit::Vertex);
 }
 
-struct {
+struct CameraData {
     glm::mat4 view;
     glm::mat4 proj;
 } cameraData;
@@ -167,31 +167,17 @@ void init() {
     cameraBuffer = device.CreateBufferBuilder()
         .SetAllowedUsage(nxt::BufferUsageBit::Mapped | nxt::BufferUsageBit::Uniform)
         .SetInitialUsage(nxt::BufferUsageBit::Mapped)
-        .SetSize(sizeof(cameraData))
+        .SetSize(sizeof(CameraData))
         .GetResult();
 
     glm::mat4 transform(1.0);
-
+    transformBuffer[0] = CreateFrozenBufferFromData(device, (void*)&transform, sizeof(glm::mat4), nxt::BufferUsageBit::Uniform);
-    transformBuffer[0] = device.CreateBufferBuilder()
-        .SetAllowedUsage(nxt::BufferUsageBit::Mapped | nxt::BufferUsageBit::Uniform)
-        .SetInitialUsage(nxt::BufferUsageBit::Mapped)
-        .SetSize(sizeof(glm::mat4))
-        .GetResult();
-    transformBuffer[0].SetSubData(0, sizeof(glm::mat4) / sizeof(uint32_t), reinterpret_cast<uint32_t*>(&transform));
-    transformBuffer[0].FreezeUsage(nxt::BufferUsageBit::Uniform);
 
     transform = glm::translate(transform, glm::vec3(0.f, -2.f, 0.f));
-
+    transformBuffer[1] = CreateFrozenBufferFromData(device, (void*)&transform, sizeof(glm::mat4), nxt::BufferUsageBit::Uniform);
-    transformBuffer[1] = device.CreateBufferBuilder()
-        .SetAllowedUsage(nxt::BufferUsageBit::Mapped | nxt::BufferUsageBit::Uniform)
-        .SetInitialUsage(nxt::BufferUsageBit::Mapped)
-        .SetSize(sizeof(glm::mat4))
-        .GetResult();
-    transformBuffer[1].SetSubData(0, sizeof(glm::mat4) / sizeof(uint32_t), reinterpret_cast<uint32_t*>(&transform));
-    transformBuffer[1].FreezeUsage(nxt::BufferUsageBit::Uniform);
 
     nxt::BufferView cameraBufferView = cameraBuffer.CreateBufferViewBuilder()
-        .SetExtent(0, sizeof(cameraData))
+        .SetExtent(0, sizeof(CameraData))
         .GetResult();
 
     nxt::BufferView transformBufferView[2] = {
@@ -280,7 +266,7 @@ void frame() {
     );
 
     cameraBuffer.TransitionUsage(nxt::BufferUsageBit::Mapped);
-    cameraBuffer.SetSubData(0, sizeof(cameraData) / sizeof(uint32_t), reinterpret_cast<uint32_t*>(&cameraData));
+    cameraBuffer.SetSubData(0, sizeof(CameraData) / sizeof(uint32_t), reinterpret_cast<uint32_t*>(&cameraData));
 
     nxt::CommandBuffer commands = device.CreateCommandBufferBuilder()
         .BeginRenderPass(renderpass, framebuffer)

next.json

@@ -475,8 +475,8 @@
             {
                 "name": "set stencil mask",
                 "args": [
-                    {"name": "face", "type": "face"},
+                    {"name": "readMask", "type": "uint32_t"},
-                    {"name": "mask", "type": "uint32_t"}
+                    {"name": "writeMask", "type": "uint32_t"}
                 ]
             }
         ]

src/backend/CMakeLists.txt

@@ -83,6 +83,8 @@ SetPIC(opengl_autogen)
 list(APPEND BACKEND_SOURCES
     ${OPENGL_DIR}/CommandBufferGL.cpp
     ${OPENGL_DIR}/CommandBufferGL.h
+    ${OPENGL_DIR}/DepthStencilStateGL.cpp
+    ${OPENGL_DIR}/DepthStencilStateGL.h
     ${OPENGL_DIR}/OpenGLBackend.cpp
     ${OPENGL_DIR}/OpenGLBackend.h
     ${OPENGL_DIR}/PersistentPipelineStateGL.cpp

src/backend/common/CommandBuffer.cpp

@@ -573,10 +573,6 @@ namespace backend {
                 case Command::SetStencilReference:
                     {
                         SetStencilReferenceCmd* cmd = iterator.NextCommand<SetStencilReferenceCmd>();
-                        if (lastPipeline->IsCompute()) {
-                            HandleError("Can't set stencil reference in a compute pipeline");
-                            return false;
-                        }
                         if (currentRenderPass == nullptr) {
                             HandleError("Can't set stencil reference without an active render pass");
                             return false;

src/backend/common/DepthStencilState.cpp

@@ -29,14 +29,16 @@ namespace backend {
     }
 
     bool DepthStencilStateBase::StencilTestEnabled() const {
-        return (stencilInfo.back.compareFunction != nxt::CompareFunction::Always ||
+        return (
-        stencilInfo.back.stencilFail != nxt::StencilOperation::Keep ||
+            stencilInfo.back.compareFunction != nxt::CompareFunction::Always ||
-        stencilInfo.back.depthFail != nxt::StencilOperation::Keep ||
+            stencilInfo.back.stencilFail != nxt::StencilOperation::Keep ||
-        stencilInfo.back.depthStencilPass != nxt::StencilOperation::Keep ||
+            stencilInfo.back.depthFail != nxt::StencilOperation::Keep ||
-        stencilInfo.front.compareFunction != nxt::CompareFunction::Always ||
+            stencilInfo.back.depthStencilPass != nxt::StencilOperation::Keep ||
-        stencilInfo.front.stencilFail != nxt::StencilOperation::Keep ||
+            stencilInfo.front.compareFunction != nxt::CompareFunction::Always ||
-        stencilInfo.front.depthFail != nxt::StencilOperation::Keep ||
+            stencilInfo.front.stencilFail != nxt::StencilOperation::Keep ||
-        stencilInfo.front.depthStencilPass != nxt::StencilOperation::Keep);
+            stencilInfo.front.depthFail != nxt::StencilOperation::Keep ||
+            stencilInfo.front.depthStencilPass != nxt::StencilOperation::Keep
+        );
     }
 
     const DepthStencilStateBase::DepthInfo& DepthStencilStateBase::GetDepth() const {
@@ -53,9 +55,8 @@ namespace backend {
         DEPTH_STENCIL_STATE_PROPERTY_DEPTH_COMPARE_FUNCTION = 0x1,
         DEPTH_STENCIL_STATE_PROPERTY_DEPTH_WRITE_ENABLED = 0x2,
         DEPTH_STENCIL_STATE_PROPERTY_STENCIL_BACK_FUNCTION = 0x4,
-        DEPTH_STENCIL_STATE_PROPERTY_STENCIL_BACK_MASK = 0x8,
+        DEPTH_STENCIL_STATE_PROPERTY_STENCIL_FRONT_FUNCTION = 0x08,
-        DEPTH_STENCIL_STATE_PROPERTY_STENCIL_FRONT_FUNCTION = 0x10,
+        DEPTH_STENCIL_STATE_PROPERTY_STENCIL_MASK = 0x10,
-        DEPTH_STENCIL_STATE_PROPERTY_STENCIL_FRONT_MASK = 0x20,
     };
 
     DepthStencilStateBuilder::DepthStencilStateBuilder(DeviceBase* device) : Builder(device) {
@@ -122,32 +123,15 @@ namespace backend {
         }
     }
 
-    void DepthStencilStateBuilder::SetStencilMask(nxt::Face face, uint32_t mask) {
+    void DepthStencilStateBuilder::SetStencilMask(uint32_t readMask, uint32_t writeMask) {
-        if (face == nxt::Face::None) {
+         if ((propertiesSet & DEPTH_STENCIL_STATE_PROPERTY_STENCIL_MASK) != 0) {
-            HandleError("Can't set stencil mask of None face");
+            HandleError("Stencilmask property set multiple times");
             return;
         }
 
-        if (face & nxt::Face::Back) {
+        propertiesSet |= DEPTH_STENCIL_STATE_PROPERTY_STENCIL_MASK;
-            if ((propertiesSet & DEPTH_STENCIL_STATE_PROPERTY_STENCIL_BACK_MASK) != 0) {
+        stencilInfo.readMask = readMask;
-                HandleError("Stencil back mask property set multiple times");
+        stencilInfo.writeMask = writeMask;
-                return;
-            }
-
-            propertiesSet |= DEPTH_STENCIL_STATE_PROPERTY_STENCIL_BACK_MASK;
-
-            stencilInfo.back.mask = mask;
-        }
-        if (face & nxt::Face::Front) {
-            if ((propertiesSet & DEPTH_STENCIL_STATE_PROPERTY_STENCIL_FRONT_MASK) != 0) {
-                HandleError("Stencil front mask property set multiple times");
-                return;
-            }
-
-            propertiesSet |= DEPTH_STENCIL_STATE_PROPERTY_STENCIL_FRONT_MASK;
-
-            stencilInfo.front.mask = mask;
-        }
     }
 
 }

src/backend/common/DepthStencilState.h

@@ -38,13 +38,13 @@ namespace backend {
                 nxt::StencilOperation stencilFail = nxt::StencilOperation::Keep;
                 nxt::StencilOperation depthFail = nxt::StencilOperation::Keep;
                 nxt::StencilOperation depthStencilPass = nxt::StencilOperation::Keep;
-                uint32_t mask = 0xff;
             };
 
             struct StencilInfo {
-                bool stencilTestEnabled;
                 StencilFaceInfo back;
                 StencilFaceInfo front;
+                uint32_t readMask = 0xff;
+                uint32_t writeMask = 0xff;
             };
 
             bool DepthTestEnabled() const;
@@ -66,7 +66,7 @@ namespace backend {
             void SetDepthWriteEnabled(bool enabled);
             void SetStencilFunction(nxt::Face face, nxt::CompareFunction stencilCompareFunction,
                 nxt::StencilOperation stencilFail, nxt::StencilOperation depthFail, nxt::StencilOperation depthStencilPass);
-            void SetStencilMask(nxt::Face face, uint32_t mask);
+            void SetStencilMask(uint32_t readMask, uint32_t writeMask);
 
         private:
             friend class DepthStencilStateBase;

src/backend/common/Pipeline.cpp

@@ -125,7 +125,7 @@ namespace backend {
         if (!layout) {
             layout = device->CreatePipelineLayoutBuilder()->GetResult();
         }
-        if (!inputState) {
+        if (!inputState && !IsCompute()) {
             inputState = device->CreateInputStateBuilder()->GetResult();
         }
         if (!depthStencilState && !IsCompute()) {

src/backend/metal/MetalBackend.mm

@@ -727,15 +727,15 @@ namespace metal {
             backFaceStencil.stencilFailureOperation = MetalStencilOperation(stencil.back.stencilFail);
             backFaceStencil.depthFailureOperation = MetalStencilOperation(stencil.back.depthFail);
             backFaceStencil.depthStencilPassOperation = MetalStencilOperation(stencil.back.depthStencilPass);
-            backFaceStencil.readMask = stencil.back.mask;
+            backFaceStencil.readMask = stencil.readMask;
-            backFaceStencil.writeMask = stencil.back.mask;
+            backFaceStencil.writeMask = stencil.writeMask;
 
             frontFaceStencil.stencilCompareFunction = MetalDepthStencilCompareFunction(stencil.front.compareFunction);
             frontFaceStencil.stencilFailureOperation = MetalStencilOperation(stencil.front.stencilFail);
             frontFaceStencil.depthFailureOperation = MetalStencilOperation(stencil.front.depthFail);
             frontFaceStencil.depthStencilPassOperation = MetalStencilOperation(stencil.front.depthStencilPass);
-            frontFaceStencil.readMask = stencil.front.mask;
+            frontFaceStencil.readMask = stencil.readMask;
-            frontFaceStencil.writeMask = stencil.front.mask;
+            frontFaceStencil.writeMask = stencil.writeMask;
 
             mtlDepthStencilDescriptor.backFaceStencil = backFaceStencil;
             mtlDepthStencilDescriptor.frontFaceStencil = frontFaceStencil;

src/backend/opengl/CommandBufferGL.cpp

@@ -27,8 +27,6 @@
 namespace backend {
 namespace opengl {
 
-    PersistentPipelineState persistentPipelineState;
-
     CommandBuffer::CommandBuffer(Device* device, CommandBufferBuilder* builder)
         : CommandBufferBase(builder), device(device), commands(builder->AcquireCommands()) {
     }
@@ -61,6 +59,9 @@ namespace opengl {
         uint32_t indexBufferOffset = 0;
         nxt::IndexFormat indexBufferFormat = nxt::IndexFormat::Uint16;
 
+        PersistentPipelineState persistentPipelineState;
+        persistentPipelineState.SetDefaultState();
+
         while(commands.NextCommandId(&type)) {
             switch (type) {
                 case Command::AdvanceSubpass:
@@ -184,9 +185,8 @@ namespace opengl {
                 case Command::SetStencilReference:
                     {
                         SetStencilReferenceCmd* cmd = commands.NextCommand<SetStencilReferenceCmd>();
-                        DepthStencilState* depthStencilState = ToBackend(lastPipeline->GetDepthStencilState());
+                        // DepthStencilState* depthStencilState = ToBackend(lastPipeline->GetDepthStencilState());
-                        persistentPipelineState.UpdateStencilReference(cmd->reference);
+                        persistentPipelineState.SetStencilReference(cmd->reference);
-                        persistentPipelineState.ApplyStencilNow();
                     }
                     break;
 

src/backend/opengl/DepthStencilStateGL.cpp

@@ -0,0 +1,128 @@
+// Copyright 2017 The NXT Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#include "DepthStencilStateGL.h"
+
+#include "OpenGLBackend.h"
+#include "PersistentPipelineStateGL.h"
+
+namespace backend {
+namespace opengl {
+
+    namespace {
+        GLuint OpenGLCompareFunction(nxt::CompareFunction compareFunction) {
+            switch (compareFunction) {
+                case nxt::CompareFunction::Never:
+                    return GL_NEVER;
+                case nxt::CompareFunction::Less:
+                    return GL_LESS;
+                case nxt::CompareFunction::LessEqual:
+                    return GL_LEQUAL;
+                case nxt::CompareFunction::Greater:
+                    return GL_GREATER;
+                case nxt::CompareFunction::GreaterEqual:
+                    return GL_GEQUAL;
+                case nxt::CompareFunction::NotEqual:
+                    return GL_NOTEQUAL;
+                case nxt::CompareFunction::Equal:
+                    return GL_EQUAL;
+                case nxt::CompareFunction::Always:
+                    return GL_ALWAYS;
+            }
+        }
+
+        GLuint OpenGLStencilOperation(nxt::StencilOperation stencilOperation) {
+            switch (stencilOperation) {
+                case nxt::StencilOperation::Keep:
+                    return GL_KEEP;
+                case nxt::StencilOperation::Zero:
+                    return GL_ZERO;
+                case nxt::StencilOperation::Replace:
+                    return GL_REPLACE;
+                case nxt::StencilOperation::Invert:
+                    return GL_INVERT;
+                case nxt::StencilOperation::IncrementClamp:
+                    return GL_INCR;
+                case nxt::StencilOperation::DecrementClamp:
+                    return GL_DECR;
+                case nxt::StencilOperation::IncrementWrap:
+                    return GL_INCR_WRAP;
+                case nxt::StencilOperation::DecrementWrap:
+                    return GL_DECR_WRAP;
+            }
+        }
+    }
+
+    DepthStencilState::DepthStencilState(Device* device, DepthStencilStateBuilder* builder)
+        : DepthStencilStateBase(builder), device(device) {
+    }
+
+    void DepthStencilState::ApplyNow(PersistentPipelineState &persistentPipelineState) const {
+        if (DepthTestEnabled()) {
+            glEnable(GL_DEPTH_TEST);
+        } else {
+            glDisable(GL_DEPTH_TEST);
+        }
+
+        auto& depthInfo = GetDepth();
+
+        if (depthInfo.depthWriteEnabled) {
+            glDepthMask(GL_TRUE);
+        } else {
+            glDepthMask(GL_FALSE);
+        }
+
+        glDepthFunc(OpenGLCompareFunction(depthInfo.compareFunction));
+
+        if (StencilTestEnabled()) {
+            glEnable(GL_STENCIL_TEST);
+        } else {
+            glDisable(GL_STENCIL_TEST);
+        }
+
+        auto& stencilInfo = GetStencil();
+
+        GLenum backCompareFunction = OpenGLCompareFunction(stencilInfo.back.compareFunction);
+        GLenum frontCompareFunction = OpenGLCompareFunction(stencilInfo.front.compareFunction);
+
+        persistentPipelineState.CacheStencilFuncsAndMask(backCompareFunction, frontCompareFunction, stencilInfo.readMask);
+
+        glStencilOpSeparate(GL_BACK,
+            OpenGLStencilOperation(stencilInfo.back.stencilFail),
+            OpenGLStencilOperation(stencilInfo.back.depthFail),
+            OpenGLStencilOperation(stencilInfo.back.depthStencilPass)
+        );
+        glStencilFuncSeparate(GL_BACK,
+            backCompareFunction,
+            persistentPipelineState.GetCachedStencilReference(),
+            stencilInfo.readMask
+        );
+        glStencilMaskSeparate(GL_BACK, stencilInfo.writeMask);
+
+        glStencilOpSeparate(GL_FRONT,
+            OpenGLStencilOperation(stencilInfo.front.stencilFail),
+            OpenGLStencilOperation(stencilInfo.front.depthFail),
+            OpenGLStencilOperation(stencilInfo.front.depthStencilPass)
+        );
+        glStencilFuncSeparate(GL_FRONT,
+            frontCompareFunction,
+            persistentPipelineState.GetCachedStencilReference(),
+            stencilInfo.readMask
+        );
+        glStencilMaskSeparate(GL_FRONT, stencilInfo.writeMask);
+
+    }
+
+}
+}

src/backend/opengl/DepthStencilStateGL.h

@@ -0,0 +1,39 @@
+// Copyright 2017 The NXT Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#ifndef BACKEND_OPENGL_DEPTHSTENCILSTATEGL_H_
+#define BACKEND_OPENGL_DEPTHSTENCILSTATEGL_H_
+
+#include "common/DepthStencilState.h"
+
+namespace backend {
+namespace opengl {
+
+    class Device;
+    class PersistentPipelineState;
+
+    class DepthStencilState : public DepthStencilStateBase {
+        public:
+            DepthStencilState(Device* device, DepthStencilStateBuilder* builder);
+
+            void ApplyNow(PersistentPipelineState &persistentPipelineState) const;
+
+        private:
+            Device* device;
+    };
+
+}
+}
+
+#endif // BACKEND_OPENGL_DEPTHSTENCILSTATEGL_H_

src/backend/opengl/GeneratedCodeIncludes.h

@@ -14,6 +14,7 @@
 
 #include "OpenGLBackend.h"
 #include "CommandBufferGL.h"
+#include "DepthStencilStateGL.h"
 #include "PersistentPipelineStateGL.h"
 #include "PipelineGL.h"
 #include "PipelineLayoutGL.h"

src/backend/opengl/OpenGLBackend.cpp

@@ -15,6 +15,7 @@
 #include "OpenGLBackend.h"
 
 #include "CommandBufferGL.h"
+#include "DepthStencilStateGL.h"
 #include "PipelineGL.h"
 #include "PipelineLayoutGL.h"
 #include "ShaderModuleGL.h"
@@ -45,52 +46,6 @@ namespace opengl {
         *device = reinterpret_cast<nxtDevice>(new Device);
     }
 
-    GLuint OpenGLCompareFunction(nxt::CompareFunction compareFunction) {
-        switch (compareFunction) {
-            case nxt::CompareFunction::Never:
-                return GL_NEVER;
-            case nxt::CompareFunction::Less:
-                return GL_LESS;
-            case nxt::CompareFunction::LessEqual:
-                return GL_LEQUAL;
-            case nxt::CompareFunction::Greater:
-                return GL_GREATER;
-            case nxt::CompareFunction::GreaterEqual:
-                return GL_GEQUAL;
-            case nxt::CompareFunction::NotEqual:
-                return GL_NOTEQUAL;
-            case nxt::CompareFunction::Equal:
-                return GL_EQUAL;
-            case nxt::CompareFunction::Always:
-                return GL_ALWAYS;
-            default:
-                ASSERT(false);
-        }
-    }
-
-    GLuint OpenGLStencilOperation(nxt::StencilOperation stencilOperation) {
-        switch (stencilOperation) {
-            case nxt::StencilOperation::Keep:
-                return GL_KEEP;
-            case nxt::StencilOperation::Zero:
-                return GL_ZERO;
-            case nxt::StencilOperation::Replace:
-                return GL_REPLACE;
-            case nxt::StencilOperation::Invert:
-                return GL_INVERT;
-            case nxt::StencilOperation::IncrementClamp:
-                return GL_INCR;
-            case nxt::StencilOperation::DecrementClamp:
-                return GL_DECR;
-            case nxt::StencilOperation::IncrementWrap:
-                return GL_INCR_WRAP;
-            case nxt::StencilOperation::DecrementWrap:
-                return GL_DECR_WRAP;
-            default:
-                ASSERT(false);
-        }
-    }
-
     // Device
 
     BindGroupBase* Device::CreateBindGroup(BindGroupBuilder* builder) {
@@ -184,12 +139,6 @@ namespace opengl {
         : BufferViewBase(builder), device(device) {
     }
 
-    // DepthStencilState
-
-    DepthStencilState::DepthStencilState(Device* device, DepthStencilStateBuilder* builder)
-        : DepthStencilStateBase(builder), device(device) {
-    }
-
     // InputState
 
     InputState::InputState(Device* device, InputStateBuilder* builder)

src/backend/opengl/OpenGLBackend.h

@@ -75,9 +75,6 @@ namespace opengl {
         return ToBackendBase<OpenGLBackendTraits>(common);
     }
 
-    GLuint OpenGLCompareFunction(nxt::CompareFunction compareFunction);
-    GLuint OpenGLStencilOperation(nxt::StencilOperation stencilOperation);
-
     // Definition of backend types
     class Device : public DeviceBase {
         public:
@@ -140,14 +137,6 @@ namespace opengl {
             Device* device;
     };
 
-    class DepthStencilState : public DepthStencilStateBase {
-        public:
-            DepthStencilState(Device* device, DepthStencilStateBuilder* builder);
-
-        private:
-            Device* device;
-    };
-
     class Framebuffer : public FramebufferBase {
         public:
             Framebuffer(Device* device, FramebufferBuilder* builder);

src/backend/opengl/PersistentPipelineStateGL.cpp

@@ -19,151 +19,38 @@
 namespace backend {
 namespace opengl {
 
-    PersistentPipelineState::PersistentPipelineState() {
+    void PersistentPipelineState::SetDefaultState() {
-        dirtyFields.set(); // initialize all fields as dirty
+        stencilBackCompareFunction = GL_ALWAYS;
+        stencilFrontCompareFunction = GL_ALWAYS;
+        stencilReadMask = 0xff;
+        SetStencilReference(0);
     }
 
-    // when a field on PersistentPipelineState::State changes, mark its starting location as dirty
+    void PersistentPipelineState::CacheStencilFuncsAndMask(GLenum stencilBackCompareFunction, GLenum stencilFrontCompareFunction, uint32_t stencilReadMask) {
-    #define SET_FIELD(field, destination, value) { \
+        this->stencilBackCompareFunction = stencilBackCompareFunction;
-        if (state.destination != value) { \
+        this->stencilFrontCompareFunction = stencilFrontCompareFunction;
-            dirtyFields.set(field); \
+        this->stencilReadMask = stencilReadMask;
-            state.destination = value; \
-        } \
     }
 
-    void PersistentPipelineState::UpdateDepthStencilInfo(const DepthStencilStateBase* const depthStencilState) {
+    void PersistentPipelineState::SetStencilReference(uint32_t stencilReference) {
-        auto& depth = depthStencilState->GetDepth();
+        if (this->stencilReference != stencilReference) {
-        SET_FIELD(DEPTH_COMPARE_FUNCTION, depthInfo.compareFunction, depth.compareFunction)
+            this->stencilReference = stencilReference;
-        SET_FIELD(DEPTH_WRITE_ENABLED, depthInfo.depthWriteEnabled, depth.depthWriteEnabled)
-        SET_FIELD(DEPTH_ENABLED, depthEnabled, depthStencilState->DepthTestEnabled())
-
-        auto& stencil = depthStencilState->GetStencil();
-        SET_FIELD(STENCIL_ENABLED, stencilEnabled, depthStencilState->StencilTestEnabled())
-
-        SET_FIELD(STENCIL_BACK_COMPARE_FUNCTION, stencilInfo.back.compareFunction, stencil.back.compareFunction)
-        SET_FIELD(STENCIL_BACK_STENCIL_FAIL, stencilInfo.back.stencilFail, stencil.back.stencilFail)
-        SET_FIELD(STENCIL_BACK_DEPTH_FAIL, stencilInfo.back.depthFail, stencil.back.depthFail)
-        SET_FIELD(STENCIL_BACK_DEPTH_STENCIL_PASS, stencilInfo.back.depthStencilPass, stencil.back.depthStencilPass)
-        SET_FIELD(STENCIL_BACK_MASK, stencilInfo.back.mask, stencil.back.mask)
-
-        SET_FIELD(STENCIL_FRONT_COMPARE_FUNCTION, stencilInfo.front.compareFunction, stencil.front.compareFunction)
-        SET_FIELD(STENCIL_FRONT_STENCIL_FAIL, stencilInfo.front.stencilFail, stencil.front.stencilFail)
-        SET_FIELD(STENCIL_FRONT_DEPTH_FAIL, stencilInfo.front.depthFail, stencil.front.depthFail)
-        SET_FIELD(STENCIL_FRONT_DEPTH_STENCIL_PASS, stencilInfo.front.depthStencilPass, stencil.front.depthStencilPass)
-        SET_FIELD(STENCIL_FRONT_MASK, stencilInfo.front.mask, stencil.front.mask)
-    }
-
-    void PersistentPipelineState::UpdateStencilReference(uint32_t stencilReference) {
-        SET_FIELD(STENCIL_REFERENCE, stencilReference, stencilReference)
-    }
-
-    #undef SET_FIELD
-
-    bool PersistentPipelineState::IsDirty(Field field) const {
-        return dirtyFields.test(field);
-    }
-
-    void PersistentPipelineState::CleanField(Field field) {
-        dirtyFields.reset(field);
-    }
-
-    void PersistentPipelineState::ApplyDepthNow() {
-        if (IsDirty(DEPTH_ENABLED)) {
-            if (state.depthEnabled) {
-                glEnable(GL_DEPTH_TEST);
-            } else {
-                glDisable(GL_DEPTH_TEST);
-            }
-            CleanField(DEPTH_ENABLED);
-        }
-        if (IsDirty(DEPTH_WRITE_ENABLED)) {
-            if (state.depthInfo.depthWriteEnabled) {
-                glDepthMask(GL_TRUE);
-            } else {
-                glDepthMask(GL_FALSE);
-            }
-            CleanField(DEPTH_WRITE_ENABLED);
-        }
-        if (IsDirty(DEPTH_COMPARE_FUNCTION)) {
-            glDepthFunc(OpenGLCompareFunction(state.depthInfo.compareFunction));
-            CleanField(DEPTH_COMPARE_FUNCTION);
-        }
-    }
-
-    void PersistentPipelineState::ApplyStencilNow() {
-        if (IsDirty(STENCIL_ENABLED)) {
-            if (state.stencilEnabled) {
-                glEnable(GL_STENCIL_TEST);
-            } else {
-                glDisable(GL_STENCIL_TEST);
-            }
-            CleanField(STENCIL_ENABLED);
-        }
-
-        if (IsDirty(STENCIL_BACK_STENCIL_FAIL) ||
-            IsDirty(STENCIL_BACK_DEPTH_FAIL) ||
-            IsDirty(STENCIL_BACK_DEPTH_STENCIL_PASS)) {
-
-            glStencilOpSeparate(GL_BACK,
-                OpenGLStencilOperation(state.stencilInfo.back.stencilFail),
-                OpenGLStencilOperation(state.stencilInfo.back.depthFail),
-                OpenGLStencilOperation(state.stencilInfo.back.depthStencilPass)
-            );
-
-            CleanField(STENCIL_BACK_STENCIL_FAIL);
-            CleanField(STENCIL_BACK_DEPTH_FAIL);
-            CleanField(STENCIL_BACK_DEPTH_STENCIL_PASS);
-        }
-        if (IsDirty(STENCIL_BACK_COMPARE_FUNCTION) ||
-            IsDirty(STENCIL_REFERENCE) ||
-            IsDirty(STENCIL_BACK_MASK)) {
-
             glStencilFuncSeparate(GL_BACK,
-                OpenGLCompareFunction(state.stencilInfo.back.compareFunction),
+                stencilBackCompareFunction,
-                state.stencilReference,
+                stencilReference,
-                state.stencilInfo.back.mask
+                stencilReadMask
             );
-            if (IsDirty(STENCIL_BACK_MASK)) {
-                glStencilMaskSeparate(GL_BACK, state.stencilInfo.back.mask);
-            }
-
-            CleanField(STENCIL_BACK_COMPARE_FUNCTION);
-            CleanField(STENCIL_BACK_MASK);
-        }
-
-        if (IsDirty(STENCIL_FRONT_STENCIL_FAIL) ||
-            IsDirty(STENCIL_FRONT_DEPTH_FAIL) ||
-            IsDirty(STENCIL_FRONT_DEPTH_STENCIL_PASS)) {
-
-            glStencilOpSeparate(GL_FRONT,
-                OpenGLStencilOperation(state.stencilInfo.front.stencilFail),
-                OpenGLStencilOperation(state.stencilInfo.front.depthFail),
-                OpenGLStencilOperation(state.stencilInfo.front.depthStencilPass)
-            );
-
-            CleanField(STENCIL_FRONT_STENCIL_FAIL);
-            CleanField(STENCIL_FRONT_DEPTH_FAIL);
-            CleanField(STENCIL_FRONT_DEPTH_STENCIL_PASS);
-        }
-        if (IsDirty(STENCIL_FRONT_COMPARE_FUNCTION) ||
-            IsDirty(STENCIL_REFERENCE) ||
-            IsDirty(STENCIL_FRONT_MASK)) {
-
             glStencilFuncSeparate(GL_FRONT,
-                OpenGLCompareFunction(state.stencilInfo.front.compareFunction),
+                stencilFrontCompareFunction,
-                state.stencilReference,
+                stencilReference,
-                state.stencilInfo.front.mask
+                stencilReadMask
             );
-            if (IsDirty(STENCIL_FRONT_MASK)) {
-                glStencilMaskSeparate(GL_FRONT, state.stencilInfo.front.mask);
-            }
-
-            CleanField(STENCIL_FRONT_COMPARE_FUNCTION);
-            CleanField(STENCIL_FRONT_MASK);
         }
+    }
 
-        CleanField(STENCIL_REFERENCE); // clean this last because its used for both the back and front functions
+    GLuint PersistentPipelineState::GetCachedStencilReference() const {
+        return stencilReference;
     }
 
 }
-}
+}

src/backend/opengl/PersistentPipelineStateGL.h

@@ -15,58 +15,28 @@
 #ifndef BACKEND_OPENGL_PERSISTENTPIPELINESTATE_H_
 #define BACKEND_OPENGL_PERSISTENTPIPELINESTATE_H_
 
-#include "common/DepthStencilState.h"
+#include "nxt/nxtcpp.h"
 
-#include <bitset>
+#include "glad/glad.h"
 
 namespace backend {
 namespace opengl {
 
     class PersistentPipelineState {
         public:
-            PersistentPipelineState();
+            void SetDefaultState();
-            void UpdateDepthStencilInfo(const DepthStencilStateBase* const depthStencilState);
+            void CacheStencilFuncsAndMask(GLenum stencilBackCompareFunction, GLenum stencilFrontCompareFunction, uint32_t stencilReadMask);
-            void UpdateStencilReference(uint32_t stencilReference);
+            void SetStencilReference(uint32_t stencilReference);
-
+            GLuint GetCachedStencilReference() const;
-            void ApplyDepthNow();
-            void ApplyStencilNow();
-
-            enum Field {
-                DEPTH_COMPARE_FUNCTION,
-                DEPTH_WRITE_ENABLED,
-                DEPTH_ENABLED,
-                STENCIL_ENABLED,
-                STENCIL_BACK_COMPARE_FUNCTION,
-                STENCIL_BACK_STENCIL_FAIL,
-                STENCIL_BACK_DEPTH_FAIL,
-                STENCIL_BACK_DEPTH_STENCIL_PASS,
-                STENCIL_BACK_MASK,
-                STENCIL_FRONT_COMPARE_FUNCTION,
-                STENCIL_FRONT_STENCIL_FAIL,
-                STENCIL_FRONT_DEPTH_FAIL,
-                STENCIL_FRONT_DEPTH_STENCIL_PASS,
-                STENCIL_FRONT_MASK,
-                STENCIL_REFERENCE,
-                Count
-            };
-
-            struct State {
-                bool depthEnabled;
-                bool stencilEnabled;
-                DepthStencilStateBase::DepthInfo depthInfo;
-                DepthStencilStateBase::StencilInfo stencilInfo;
-                uint32_t stencilReference;
-            };
 
         private:
-            State state;
+            GLenum stencilBackCompareFunction;
-            std::bitset<Field::Count> dirtyFields;
+            GLenum stencilFrontCompareFunction;
-
+            GLuint stencilReadMask;
-            inline bool IsDirty(Field field) const;
+            GLuint stencilReference;
-            inline void CleanField(Field field);
     };
 
 }
 }
 
-#endif // BACKEND_OPENGL_PERSISTENTPIPELINESTATE_H_
+#endif // BACKEND_OPENGL_PERSISTENTPIPELINESTATE_H_

src/backend/opengl/PipelineGL.cpp

@@ -14,6 +14,7 @@
 
 #include "PipelineGL.h"
 
+#include "DepthStencilStateGL.h"
 #include "OpenGLBackend.h"
 #include "PersistentPipelineStateGL.h"
 #include "PipelineLayoutGL.h"
@@ -209,9 +210,8 @@ namespace opengl {
         auto inputState = ToBackend(GetInputState());
         glBindVertexArray(inputState->GetVAO());
 
-        persistentPipelineState.UpdateDepthStencilInfo(GetDepthStencilState());
+        auto depthStencilState = ToBackend(GetDepthStencilState());
-        persistentPipelineState.ApplyDepthNow();
+        depthStencilState->ApplyNow(persistentPipelineState);
-        persistentPipelineState.ApplyStencilNow();
     }
 
 }

src/tests/unittests/validation/DepthStencilStateValidationTest.cpp

@@ -17,66 +17,79 @@
 class DepthStencilStateValidationTest : public ValidationTest {
 };
 
-TEST_F(DepthStencilStateValidationTest, Creation) {
+TEST_F(DepthStencilStateValidationTest, CreationSuccess) {
-    // Success
+    // Success for setting all properties
-    nxt::DepthStencilState buf0 = AssertWillBeSuccess(device.CreateDepthStencilStateBuilder())
+    {
-        .SetDepthCompareFunction(nxt::CompareFunction::Less)
+        nxt::DepthStencilState ds = AssertWillBeSuccess(device.CreateDepthStencilStateBuilder())
-        .SetDepthWriteEnabled(true)
+            .SetDepthCompareFunction(nxt::CompareFunction::Less)
-        .SetStencilFunction(nxt::Face::Both, nxt::CompareFunction::Greater,
+            .SetDepthWriteEnabled(true)
-            nxt::StencilOperation::Keep, nxt::StencilOperation::Keep, nxt::StencilOperation::Replace)
+            .SetStencilFunction(nxt::Face::Both, nxt::CompareFunction::Greater,
-        .SetStencilMask(nxt::Face::Both, 0x1)
+                nxt::StencilOperation::Keep, nxt::StencilOperation::Keep, nxt::StencilOperation::Replace)
-        .GetResult();
+            .SetStencilMask(0x0, 0x1)
+            .GetResult();
+    }
 
     // Success for empty builder
-    nxt::DepthStencilState buf1 = AssertWillBeSuccess(device.CreateDepthStencilStateBuilder())
+    {
-        .GetResult();
+        nxt::DepthStencilState ds = AssertWillBeSuccess(device.CreateDepthStencilStateBuilder())
+            .GetResult();
+    }
 
-    // Test failure when specifying properties multiple times
+    // Test success when setting stencil function on separate faces
-    nxt::DepthStencilState buf2 = AssertWillBeError(device.CreateDepthStencilStateBuilder())
+    {
-        .SetDepthWriteEnabled(true)
+        nxt::DepthStencilState ds = AssertWillBeSuccess(device.CreateDepthStencilStateBuilder())
-        .SetDepthWriteEnabled(false)
+            .SetStencilFunction(nxt::Face::Front, nxt::CompareFunction::Less,
-        .GetResult();
+                nxt::StencilOperation::Replace, nxt::StencilOperation::Replace, nxt::StencilOperation::Replace)
-
+            .SetStencilFunction(nxt::Face::Back, nxt::CompareFunction::Greater,
-    nxt::DepthStencilState buf3 = AssertWillBeError(device.CreateDepthStencilStateBuilder())
+                nxt::StencilOperation::Replace, nxt::StencilOperation::Replace, nxt::StencilOperation::Replace)
-        .SetDepthCompareFunction(nxt::CompareFunction::Less)
+            .GetResult();
-        .SetDepthCompareFunction(nxt::CompareFunction::Greater)
+    }
-        .GetResult();
-
-    // Test success when setting properties on separate faces
-    nxt::DepthStencilState buf4 = AssertWillBeSuccess(device.CreateDepthStencilStateBuilder())
-        .SetStencilMask(nxt::Face::Front, 0x00)
-        .SetStencilMask(nxt::Face::Back, 0xff)
-        .GetResult();
-
-    nxt::DepthStencilState buf5 = AssertWillBeSuccess(device.CreateDepthStencilStateBuilder())
-        .SetStencilFunction(nxt::Face::Front, nxt::CompareFunction::Less,
-            nxt::StencilOperation::Replace, nxt::StencilOperation::Replace, nxt::StencilOperation::Replace)
-        .SetStencilFunction(nxt::Face::Back, nxt::CompareFunction::Greater,
-            nxt::StencilOperation::Replace, nxt::StencilOperation::Replace, nxt::StencilOperation::Replace)
-        .GetResult();
-
-    // Test failure when setting properties on a face multiple times
-    nxt::DepthStencilState buf6 = AssertWillBeError(device.CreateDepthStencilStateBuilder())
-        .SetStencilMask(nxt::Face::Back, 0x00)
-        .SetStencilMask(nxt::Face::Back, 0xff)
-        .GetResult();
-
-    nxt::DepthStencilState buf7 = AssertWillBeError(device.CreateDepthStencilStateBuilder())
-        .SetStencilFunction(nxt::Face::Back, nxt::CompareFunction::Less,
-            nxt::StencilOperation::Replace, nxt::StencilOperation::Replace, nxt::StencilOperation::Replace)
-        .SetStencilFunction(nxt::Face::Back, nxt::CompareFunction::Greater,
-            nxt::StencilOperation::Replace, nxt::StencilOperation::Replace, nxt::StencilOperation::Replace)
-        .GetResult();
-
-    nxt::DepthStencilState buf8 = AssertWillBeError(device.CreateDepthStencilStateBuilder())
-        .SetStencilMask(nxt::Face::Both, 0x00)
-        .SetStencilMask(nxt::Face::Back, 0xff)
-        .GetResult();
-
-    nxt::DepthStencilState buf9 = AssertWillBeError(device.CreateDepthStencilStateBuilder())
-        .SetStencilFunction(nxt::Face::Both, nxt::CompareFunction::Less,
-            nxt::StencilOperation::Replace, nxt::StencilOperation::Replace, nxt::StencilOperation::Replace)
-        .SetStencilFunction(nxt::Face::Back, nxt::CompareFunction::Greater,
-            nxt::StencilOperation::Replace, nxt::StencilOperation::Replace, nxt::StencilOperation::Replace)
-        .GetResult();
 }
+
+TEST_F(DepthStencilStateValidationTest, CreationDuplicates) {
+    // Test failure when specifying depth write enabled multiple times
+    {
+        nxt::DepthStencilState ds = AssertWillBeError(device.CreateDepthStencilStateBuilder())
+            .SetDepthWriteEnabled(true)
+            .SetDepthWriteEnabled(false)
+            .GetResult();
+    }
+
+    // Test failure when specifying depth compare function multiple times
+    {
+        nxt::DepthStencilState ds = AssertWillBeError(device.CreateDepthStencilStateBuilder())
+            .SetDepthCompareFunction(nxt::CompareFunction::Less)
+            .SetDepthCompareFunction(nxt::CompareFunction::Greater)
+            .GetResult();
+    }
+
+    // Test failure when setting stencil mask  multiple times
+    {
+        nxt::DepthStencilState ds = AssertWillBeError(device.CreateDepthStencilStateBuilder())
+            .SetStencilMask(0x00, 0x00)
+            .SetStencilMask(0xff, 0xff)
+            .GetResult();
+    }
+
+    // Test failure when directly setting stencil function on a face multiple times
+    {
+        nxt::DepthStencilState ds = AssertWillBeError(device.CreateDepthStencilStateBuilder())
+            .SetStencilFunction(nxt::Face::Back, nxt::CompareFunction::Less,
+                nxt::StencilOperation::Replace, nxt::StencilOperation::Replace, nxt::StencilOperation::Replace)
+            .SetStencilFunction(nxt::Face::Back, nxt::CompareFunction::Greater,
+                nxt::StencilOperation::Replace, nxt::StencilOperation::Replace, nxt::StencilOperation::Replace)
+            .GetResult();
+    }
+
+    // Test failure when indirectly setting stencil function on a face multiple times
+    {
+        nxt::DepthStencilState ds = AssertWillBeError(device.CreateDepthStencilStateBuilder())
+            .SetStencilFunction(nxt::Face::Both, nxt::CompareFunction::Less,
+                nxt::StencilOperation::Replace, nxt::StencilOperation::Replace, nxt::StencilOperation::Replace)
+            .SetStencilFunction(nxt::Face::Back, nxt::CompareFunction::Greater,
+                nxt::StencilOperation::Replace, nxt::StencilOperation::Replace, nxt::StencilOperation::Replace)
+            .GetResult();
+    }
+}
+
+// TODO(enga@google.com) Test failure when set in a compute pipeline