Validate SetPushConstants is done inside subpass and compute passes

next.json

@@ -380,7 +380,7 @@
                     "TODO Vulkan has an additional stage mask"
                 ],
                 "args": [
-                    {"name": "stage", "type": "shader stage bit"},
+                    {"name": "stages", "type": "shader stage bit"},
                     {"name": "offset", "type": "uint32_t"},
                     {"name": "count", "type": "uint32_t"},
                     {"name": "data", "type": "uint32_t", "annotation": "const*", "length": "count"}

src/backend/CommandBuffer.cpp

@@ -552,8 +552,9 @@ namespace backend {
                     {
                         SetPushConstantsCmd* cmd = iterator.NextCommand<SetPushConstantsCmd>();
                         iterator.NextData<uint32_t>(cmd->count);
-                        if (cmd->count + cmd->offset > kMaxPushConstants) {
+                        // Validation of count and offset has already been done when the command was recorded
-                            HandleError("Setting pushconstants past the limit");
+                        // because it impacts the size of an allocation in the CommandAllocator.
+                        if (!state->ValidateSetPushConstants(cmd->stages)) {
                             return false;
                         }
                     }
@@ -755,7 +756,8 @@ namespace backend {
         cmd->pipeline = pipeline;
     }
 
-    void CommandBufferBuilder::SetPushConstants(nxt::ShaderStageBit stage, uint32_t offset, uint32_t count, const void* data) {
+    void CommandBufferBuilder::SetPushConstants(nxt::ShaderStageBit stages, uint32_t offset, uint32_t count, const void* data) {
+        // TODO(cwallez@chromium.org): check for overflows
         if (offset + count > kMaxPushConstants) {
             HandleError("Setting too many push constants");
             return;
@@ -763,7 +765,7 @@ namespace backend {
 
         SetPushConstantsCmd* cmd = allocator.Allocate<SetPushConstantsCmd>(Command::SetPushConstants);
         new(cmd) SetPushConstantsCmd;
-        cmd->stage = stage;
+        cmd->stages = stages;
         cmd->offset = offset;
         cmd->count = count;
 

src/backend/CommandBuffer.h

@@ -77,7 +77,7 @@ namespace backend {
             void EndComputePass();
             void EndRenderPass();
             void EndRenderSubpass();
-            void SetPushConstants(nxt::ShaderStageBit stage, uint32_t offset, uint32_t count, const void* data);
+            void SetPushConstants(nxt::ShaderStageBit stages, uint32_t offset, uint32_t count, const void* data);
             void SetComputePipeline(ComputePipelineBase* pipeline);
             void SetRenderPipeline(RenderPipelineBase* pipeline);
             void SetStencilReference(uint32_t reference);

src/backend/CommandBufferStateTracker.cpp

@@ -127,6 +127,24 @@ namespace backend {
         return true;
     }
 
+    bool CommandBufferStateTracker::ValidateSetPushConstants(nxt::ShaderStageBit stages) {
+        if (aspects[VALIDATION_ASPECT_COMPUTE_PASS]) {
+            if (stages & ~nxt::ShaderStageBit::Compute) {
+                builder->HandleError("SetPushConstants stage must be compute or 0 in compute passes");
+                return false;
+            }
+        } else if (aspects[VALIDATION_ASPECT_RENDER_SUBPASS]) {
+            if (stages & ~(nxt::ShaderStageBit::Vertex | nxt::ShaderStageBit::Fragment)) {
+                builder->HandleError("SetPushConstants stage must be a subset if (vertex|fragment) in subpasses");
+                return false;
+            }
+        } else {
+            builder->HandleError("PushConstants must be set in either compute passes or subpasses");
+            return false;
+        }
+        return true;
+    }
+
     bool CommandBufferStateTracker::BeginComputePass() {
         if (currentRenderPass != nullptr) {
             builder->HandleError("Cannot begin a compute pass while a render pass is active");

src/backend/CommandBufferStateTracker.h

@@ -37,6 +37,7 @@ namespace backend {
             bool ValidateCanDrawArrays();
             bool ValidateCanDrawElements();
             bool ValidateEndCommandBuffer() const;
+            bool ValidateSetPushConstants(nxt::ShaderStageBit stages);
 
             // State-modifying methods
             bool BeginComputePass();

src/backend/Commands.h

@@ -130,7 +130,7 @@ namespace backend {
     };
 
     struct SetPushConstantsCmd {
-        nxt::ShaderStageBit stage;
+        nxt::ShaderStageBit stages;
         uint32_t offset;
         uint32_t count;
     };

src/backend/opengl/CommandBufferGL.cpp

@@ -314,7 +314,7 @@ namespace opengl {
                         int32_t* valuesInt = reinterpret_cast<int32_t*>(valuesUInt);
                         float* valuesFloat = reinterpret_cast<float*>(valuesUInt);
 
-                        for (auto stage : IterateStages(cmd->stage)) {
+                        for (auto stage : IterateStages(cmd->stages)) {
                             const auto& pushConstants = lastPipeline->GetPushConstants(stage);
                             const auto& glPushConstants = lastGLPipeline->GetGLPushConstants(stage);
                             for (size_t i = 0; i < cmd->count; i++) {