Remove the concept of push constants

BUG=dawn:14

Change-Id: I20587081ec806034ce4f90457c3d475a6fbe834d
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/7180
Commit-Queue: Corentin Wallez <cwallez@chromium.org>
Reviewed-by: Kai Ninomiya <kainino@chromium.org>

BUILD.gn

@@ -573,7 +573,6 @@ test("dawn_unittests") {
     "src/tests/unittests/validation/DebugMarkerValidationTests.cpp",
     "src/tests/unittests/validation/DynamicStateCommandValidationTests.cpp",
     "src/tests/unittests/validation/FenceValidationTests.cpp",
-    "src/tests/unittests/validation/PushConstantsValidationTests.cpp",
     "src/tests/unittests/validation/QueueSubmitValidationTests.cpp",
     "src/tests/unittests/validation/RenderPassDescriptorValidationTests.cpp",
     "src/tests/unittests/validation/RenderPassValidationTests.cpp",
@@ -647,7 +646,6 @@ test("dawn_end2end_tests") {
     "src/tests/end2end/NonzeroTextureCreationTests.cpp",
     "src/tests/end2end/ObjectCachingTests.cpp",
     "src/tests/end2end/PrimitiveTopologyTests.cpp",
-    "src/tests/end2end/PushConstantTests.cpp",
     "src/tests/end2end/RenderPassLoadOpTests.cpp",
     "src/tests/end2end/RenderPassTests.cpp",
     "src/tests/end2end/SamplerTests.cpp",

dawn.json

@@ -336,19 +336,6 @@
                     {"name": "pipeline", "type": "compute pipeline"}
                 ]
             },
-            {
-                "name": "set push constants",
-                "TODO": [
-                    "data should be void*",
-                    "TODO Vulkan has an additional stage mask"
-                ],
-                "args": [
-                    {"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"}
-                ]
-            },
             {
                 "name": "set bind group",
                 "args": [
@@ -751,19 +738,6 @@
                     {"name": "pipeline", "type": "render pipeline"}
                 ]
             },
-            {
-                "name": "set push constants",
-                "TODO": [
-                    "data should be void*",
-                    "TODO Vulkan has an additional stage mask"
-                ],
-                "args": [
-                    {"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"}
-                ]
-            },
             {
                 "name": "set bind group",
                 "args": [

examples/Animometer.cpp

@@ -25,15 +25,18 @@
 dawn::Device device;
 dawn::Queue queue;
 dawn::SwapChain swapchain;
-dawn::TextureView depthStencilView;
 dawn::RenderPipeline pipeline;
+dawn::BindGroup bindGroup;
+dawn::Buffer ubo;
 
 float RandomFloat(float min, float max) {
     float zeroOne = rand() / float(RAND_MAX);
     return zeroOne * (max - min) + min;
 }
 
-struct ShaderData {
+constexpr size_t kNumTriangles = 10000;
+
+struct alignas(kMinDynamicBufferOffsetAlignment) ShaderData {
     float scale;
     float time;
     float offsetX;
@@ -55,7 +58,7 @@ void init() {
     dawn::ShaderModule vsModule = utils::CreateShaderModule(device, dawn::ShaderStage::Vertex, R"(
         #version 450
 
-        layout(push_constant) uniform ConstantsBlock {
+        layout(std140, set = 0, binding = 0) uniform Constants {
             float scale;
             float time;
             float offsetX;
@@ -97,8 +100,7 @@ void init() {
             ypos = yrot + c.offsetY;
             v_color = vec4(fade, 1.0 - fade, 0.0, 1.0) + color;
             gl_Position = vec4(xpos, ypos, 0.0, 1.0);
-        })"
+        })");
-    );
 
     dawn::ShaderModule fsModule = utils::CreateShaderModule(device, dawn::ShaderStage::Fragment, R"(
         #version 450
@@ -108,18 +110,18 @@ void init() {
             fragColor = v_color;
         })");
 
-    depthStencilView = CreateDefaultDepthStencilView(device);
+    dawn::BindGroupLayout bgl = utils::MakeBindGroupLayout(
+        device, {{0, dawn::ShaderStageBit::Vertex, dawn::BindingType::DynamicUniformBuffer}});
 
     utils::ComboRenderPipelineDescriptor descriptor(device);
+    descriptor.layout = utils::MakeBasicPipelineLayout(device, &bgl);
     descriptor.cVertexStage.module = vsModule;
     descriptor.cFragmentStage.module = fsModule;
-    descriptor.depthStencilState = &descriptor.cDepthStencilState;
-    descriptor.cDepthStencilState.format = dawn::TextureFormat::D32FloatS8Uint;
     descriptor.cColorStates[0]->format = GetPreferredSwapChainTextureFormat();
 
     pipeline = device.CreateRenderPipeline(&descriptor);
 
-    shaderData.resize(10000);
+    shaderData.resize(kNumTriangles);
     for (auto& data : shaderData) {
         data.scale = RandomFloat(0.2f, 0.4f);
         data.time = 0.0;
@@ -128,6 +130,14 @@ void init() {
         data.scalar = RandomFloat(0.5f, 2.0f);
         data.scalarOffset = RandomFloat(0.0f, 10.0f);
     }
+
+    dawn::BufferDescriptor bufferDesc;
+    bufferDesc.size = kNumTriangles * sizeof(ShaderData);
+    bufferDesc.usage = dawn::BufferUsageBit::TransferDst | dawn::BufferUsageBit::Uniform;
+    ubo = device.CreateBuffer(&bufferDesc);
+
+    bindGroup =
+        utils::MakeBindGroup(device, bgl, {{0, ubo, 0, kNumTriangles * sizeof(ShaderData)}});
 }
 
 void frame() {
@@ -135,21 +145,22 @@ void frame() {
 
     static int f = 0;
     f++;
+    for (auto& data : shaderData) {
+        data.time = f / 60.0f;
+    }
+    ubo.SetSubData(0, kNumTriangles * sizeof(ShaderData),
+                   reinterpret_cast<const uint8_t*>(shaderData.data()));
 
-    size_t i = 0;
+    utils::ComboRenderPassDescriptor renderPass({backbuffer.CreateDefaultView()});
-
-    utils::ComboRenderPassDescriptor renderPass({backbuffer.CreateDefaultView()},
-                                                depthStencilView);
     dawn::CommandEncoder encoder = device.CreateCommandEncoder();
     {
         dawn::RenderPassEncoder pass = encoder.BeginRenderPass(&renderPass);
         pass.SetPipeline(pipeline);
 
-        for (int k = 0; k < 10000; k++) {
+        for (size_t i = 0; i < kNumTriangles; i++) {
-            shaderData[i].time = f / 60.0f;
+            uint64_t offset = i * sizeof(ShaderData);
-            pass.SetPushConstants(dawn::ShaderStageBit::Vertex, 0, 6, reinterpret_cast<uint32_t*>(&shaderData[i]));
+            pass.SetBindGroup(0, bindGroup, 1, &offset);
             pass.Draw(3, 1, 0, 0);
-            i++;
         }
 
         pass.EndPass();

src/common/Constants.h

@@ -17,7 +17,6 @@
 
 #include <cstdint>
 
-static constexpr uint32_t kMaxPushConstants = 32u;
 static constexpr uint32_t kMaxBindGroups = 4u;
 // TODO(cwallez@chromium.org): investigate bindgroup limits
 static constexpr uint32_t kMaxBindingsPerGroup = 16u;

src/dawn_native/CommandEncoder.cpp

@@ -1016,18 +1016,6 @@ namespace dawn_native {
                     persistentState.SetComputePipeline(pipeline);
                 } break;
 
-                case Command::SetPushConstants: {
-                    SetPushConstantsCmd* cmd = mIterator.NextCommand<SetPushConstantsCmd>();
-                    mIterator.NextData<uint32_t>(cmd->count);
-                    // Validation of count and offset has already been done when the command was
-                    // recorded because it impacts the size of an allocation in the
-                    // CommandAllocator.
-                    if (cmd->stages & ~dawn::ShaderStageBit::Compute) {
-                        return DAWN_VALIDATION_ERROR(
-                            "SetPushConstants stage must be compute or 0 in compute passes");
-                    }
-                } break;
-
                 case Command::SetBindGroup: {
                     SetBindGroupCmd* cmd = mIterator.NextCommand<SetBindGroupCmd>();
                     if (cmd->dynamicOffsetCount > 0) {
@@ -1120,20 +1108,6 @@ namespace dawn_native {
                     persistentState.SetRenderPipeline(pipeline);
                 } break;
 
-                case Command::SetPushConstants: {
-                    SetPushConstantsCmd* cmd = mIterator.NextCommand<SetPushConstantsCmd>();
-                    mIterator.NextData<uint32_t>(cmd->count);
-                    // Validation of count and offset has already been done when the command was
-                    // recorded because it impacts the size of an allocation in the
-                    // CommandAllocator.
-                    if (cmd->stages &
-                        ~(dawn::ShaderStageBit::Vertex | dawn::ShaderStageBit::Fragment)) {
-                        return DAWN_VALIDATION_ERROR(
-                            "SetPushConstants stage must be a subset of (vertex|fragment) in "
-                            "render passes");
-                    }
-                } break;
-
                 case Command::SetStencilReference: {
                     mIterator.NextCommand<SetStencilReferenceCmd>();
                 } break;

src/dawn_native/Commands.cpp

@@ -96,11 +96,6 @@ namespace dawn_native {
                     SetRenderPipelineCmd* cmd = commands->NextCommand<SetRenderPipelineCmd>();
                     cmd->~SetRenderPipelineCmd();
                 } break;
-                case Command::SetPushConstants: {
-                    SetPushConstantsCmd* cmd = commands->NextCommand<SetPushConstantsCmd>();
-                    commands->NextData<uint32_t>(cmd->count);
-                    cmd->~SetPushConstantsCmd();
-                } break;
                 case Command::SetStencilReference: {
                     SetStencilReferenceCmd* cmd = commands->NextCommand<SetStencilReferenceCmd>();
                     cmd->~SetStencilReferenceCmd();
@@ -206,11 +201,6 @@ namespace dawn_native {
                 commands->NextCommand<SetRenderPipelineCmd>();
                 break;
 
-            case Command::SetPushConstants: {
-                auto* cmd = commands->NextCommand<SetPushConstantsCmd>();
-                commands->NextData<uint32_t>(cmd->count);
-            } break;
-
             case Command::SetStencilReference:
                 commands->NextCommand<SetStencilReferenceCmd>();
                 break;

src/dawn_native/Commands.h

@@ -47,7 +47,6 @@ namespace dawn_native {
         PushDebugGroup,
         SetComputePipeline,
         SetRenderPipeline,
-        SetPushConstants,
         SetStencilReference,
         SetScissorRect,
         SetBlendColor,
@@ -169,12 +168,6 @@ namespace dawn_native {
         Ref<RenderPipelineBase> pipeline;
     };
 
-    struct SetPushConstantsCmd {
-        dawn::ShaderStageBit stages;
-        uint32_t offset;
-        uint32_t count;
-    };
-
     struct SetStencilReferenceCmd {
         uint32_t reference;
     };

src/dawn_native/ComputePipeline.cpp

@@ -40,7 +40,6 @@ namespace dawn_native {
           mModule(descriptor->computeStage->module),
           mEntryPoint(descriptor->computeStage->entryPoint),
           mIsBlueprint(blueprint) {
-        ExtractModuleData(dawn::ShaderStage::Compute, descriptor->computeStage->module);
     }
 
     ComputePipelineBase::ComputePipelineBase(DeviceBase* device, ObjectBase::ErrorTag tag)

src/dawn_native/Forward.h

@@ -48,8 +48,6 @@ namespace dawn_native {
 
     template <typename T>
     class PerStage;
-
-    enum PushConstantType : uint8_t;
 }  // namespace dawn_native
 
 #endif  // DAWNNATIVE_FORWARD_H_

src/dawn_native/Pipeline.cpp

@@ -50,33 +50,6 @@ namespace dawn_native {
         : ObjectBase(device, tag) {
     }
 
-    void PipelineBase::ExtractModuleData(dawn::ShaderStage stage, ShaderModuleBase* module) {
-        ASSERT(!IsError());
-
-        PushConstantInfo* info = &mPushConstants[stage];
-
-        const auto& moduleInfo = module->GetPushConstants();
-        info->mask = moduleInfo.mask;
-
-        for (uint32_t i = 0; i < moduleInfo.names.size(); i++) {
-            uint32_t size = moduleInfo.sizes[i];
-            if (size == 0) {
-                continue;
-            }
-
-            for (uint32_t offset = 0; offset < size; offset++) {
-                info->types[i + offset] = moduleInfo.types[i];
-            }
-            i += size - 1;
-        }
-    }
-
-    const PipelineBase::PushConstantInfo& PipelineBase::GetPushConstants(
-        dawn::ShaderStage stage) const {
-        ASSERT(!IsError());
-        return mPushConstants[stage];
-    }
-
     dawn::ShaderStageBit PipelineBase::GetStageMask() const {
         ASSERT(!IsError());
         return mStageMask;

src/dawn_native/Pipeline.h

@@ -28,12 +28,6 @@
 
 namespace dawn_native {
 
-    enum PushConstantType : uint8_t {
-        Int,
-        UInt,
-        Float,
-    };
-
     MaybeError ValidatePipelineStageDescriptor(DeviceBase* device,
                                                const PipelineStageDescriptor* descriptor,
                                                const PipelineLayoutBase* layout,
@@ -41,11 +35,6 @@ namespace dawn_native {
 
     class PipelineBase : public ObjectBase {
       public:
-        struct PushConstantInfo {
-            std::bitset<kMaxPushConstants> mask;
-            std::array<PushConstantType, kMaxPushConstants> types;
-        };
-        const PushConstantInfo& GetPushConstants(dawn::ShaderStage stage) const;
         dawn::ShaderStageBit GetStageMask() const;
         PipelineLayoutBase* GetLayout();
         const PipelineLayoutBase* GetLayout() const;
@@ -54,12 +43,9 @@ namespace dawn_native {
         PipelineBase(DeviceBase* device, PipelineLayoutBase* layout, dawn::ShaderStageBit stages);
         PipelineBase(DeviceBase* device, ObjectBase::ErrorTag tag);
 
-        void ExtractModuleData(dawn::ShaderStage stage, ShaderModuleBase* module);
-
       private:
         dawn::ShaderStageBit mStageMask;
         Ref<PipelineLayoutBase> mLayout;
-        PerStage<PushConstantInfo> mPushConstants;
     };
 
 }  // namespace dawn_native

src/dawn_native/ProgrammablePassEncoder.cpp

@@ -124,34 +124,6 @@ namespace dawn_native {
         }
     }
 
-    void ProgrammablePassEncoder::SetPushConstants(dawn::ShaderStageBit stages,
-                                                   uint32_t offset,
-                                                   uint32_t count,
-                                                   const void* data) {
-        if (mTopLevelEncoder->ConsumedError(ValidateCanRecordCommands())) {
-            return;
-        }
-
-        if (mTopLevelEncoder->ConsumedError(ValidateShaderStageBit(stages))) {
-            return;
-        }
-
-        // TODO(cwallez@chromium.org): check for overflows
-        if (offset + count > kMaxPushConstants) {
-            mTopLevelEncoder->HandleError("Setting too many push constants");
-            return;
-        }
-
-        SetPushConstantsCmd* cmd =
-            mAllocator->Allocate<SetPushConstantsCmd>(Command::SetPushConstants);
-        cmd->stages = stages;
-        cmd->offset = offset;
-        cmd->count = count;
-
-        uint32_t* values = mAllocator->AllocateData<uint32_t>(count);
-        memcpy(values, data, count * sizeof(uint32_t));
-    }
-
     MaybeError ProgrammablePassEncoder::ValidateCanRecordCommands() const {
         if (mAllocator == nullptr) {
             return DAWN_VALIDATION_ERROR("Recording in an error or already ended pass encoder");

src/dawn_native/ProgrammablePassEncoder.h

@@ -43,10 +43,6 @@ namespace dawn_native {
                           BindGroupBase* group,
                           uint32_t dynamicOffsetCount,
                           const uint64_t* dynamicOffsets);
-        void SetPushConstants(dawn::ShaderStageBit stages,
-                              uint32_t offset,
-                              uint32_t count,
-                              const void* data);
 
       protected:
         // Construct an "error" programmable pass encoder.

src/dawn_native/RenderPipeline.cpp

@@ -377,8 +377,6 @@ namespace dawn_native {
             mDepthStencilState.stencilReadMask = 0xff;
             mDepthStencilState.stencilWriteMask = 0xff;
         }
-        ExtractModuleData(dawn::ShaderStage::Vertex, descriptor->vertexStage->module);
-        ExtractModuleData(dawn::ShaderStage::Fragment, descriptor->fragmentStage->module);
 
         for (uint32_t i = 0; i < descriptor->colorStateCount; ++i) {
             mColorAttachmentsSet.set(i);

src/dawn_native/ShaderModule.cpp

@@ -125,56 +125,8 @@ namespace dawn_native {
                 UNREACHABLE();
         }
 
-        // Extract push constants
-        mPushConstants.mask.reset();
-        mPushConstants.sizes.fill(0);
-        mPushConstants.types.fill(PushConstantType::Int);
-
         if (resources.push_constant_buffers.size() > 0) {
-            auto interfaceBlock = resources.push_constant_buffers[0];
+            GetDevice()->HandleError("Push constants aren't supported.");
-
-            const auto& blockType = compiler.get_type(interfaceBlock.type_id);
-            ASSERT(blockType.basetype == spirv_cross::SPIRType::Struct);
-
-            for (uint32_t i = 0; i < blockType.member_types.size(); i++) {
-                ASSERT(compiler.get_member_decoration_bitset(blockType.self, i)
-                           .get(spv::DecorationOffset));
-
-                uint32_t offset =
-                    compiler.get_member_decoration(blockType.self, i, spv::DecorationOffset);
-                ASSERT(offset % 4 == 0);
-                offset /= 4;
-
-                auto memberType = compiler.get_type(blockType.member_types[i]);
-                PushConstantType constantType;
-                if (memberType.basetype == spirv_cross::SPIRType::Int) {
-                    constantType = PushConstantType::Int;
-                } else if (memberType.basetype == spirv_cross::SPIRType::UInt) {
-                    constantType = PushConstantType::UInt;
-                } else {
-                    ASSERT(memberType.basetype == spirv_cross::SPIRType::Float);
-                    constantType = PushConstantType::Float;
-                }
-
-                // TODO(cwallez@chromium.org): check for overflows and make the logic better take
-                // into account things like the array of types with padding.
-                uint32_t size = memberType.vecsize * memberType.columns;
-                // Handle unidimensional arrays
-                if (!memberType.array.empty()) {
-                    size *= memberType.array[0];
-                }
-
-                if (offset + size > kMaxPushConstants) {
-                    device->HandleError("Push constant block too big in the SPIRV");
-                    return;
-                }
-
-                mPushConstants.mask.set(offset);
-                mPushConstants.names[offset] =
-                    interfaceBlock.name + "." + compiler.get_member_name(blockType.self, i);
-                mPushConstants.sizes[offset] = size;
-                mPushConstants.types[offset] = constantType;
-            }
         }
 
         // Fill in bindingInfo with the SPIRV bindings
@@ -247,11 +199,6 @@ namespace dawn_native {
         }
     }
 
-    const ShaderModuleBase::PushConstantInfo& ShaderModuleBase::GetPushConstants() const {
-        ASSERT(!IsError());
-        return mPushConstants;
-    }
-
     const ShaderModuleBase::ModuleBindingInfo& ShaderModuleBase::GetBindingInfo() const {
         ASSERT(!IsError());
         return mBindingInfo;

src/dawn_native/ShaderModule.h

@@ -46,14 +46,6 @@ namespace dawn_native {
 
         void ExtractSpirvInfo(const spirv_cross::Compiler& compiler);
 
-        struct PushConstantInfo {
-            std::bitset<kMaxPushConstants> mask;
-
-            std::array<std::string, kMaxPushConstants> names;
-            std::array<uint32_t, kMaxPushConstants> sizes;
-            std::array<PushConstantType, kMaxPushConstants> types;
-        };
-
         struct BindingInfo {
             // The SPIRV ID of the resource.
             uint32_t id;
@@ -64,7 +56,6 @@ namespace dawn_native {
         using ModuleBindingInfo =
             std::array<std::array<BindingInfo, kMaxBindingsPerGroup>, kMaxBindGroups>;
 
-        const PushConstantInfo& GetPushConstants() const;
         const ModuleBindingInfo& GetBindingInfo() const;
         const std::bitset<kMaxVertexAttributes>& GetUsedVertexAttributes() const;
         dawn::ShaderStage GetExecutionModel() const;
@@ -89,7 +80,6 @@ namespace dawn_native {
         std::vector<uint32_t> mCode;
         bool mIsBlueprint = false;
 
-        PushConstantInfo mPushConstants = {};
         ModuleBindingInfo mBindingInfo;
         std::bitset<kMaxVertexAttributes> mUsedVertexAttributes;
         dawn::ShaderStage mExecutionModel;

src/dawn_native/metal/CommandBufferMTL.mm

@@ -628,15 +628,10 @@ namespace dawn_native { namespace metal {
 
     void CommandBuffer::EncodeComputePass(id<MTLCommandBuffer> commandBuffer) {
         ComputePipeline* lastPipeline = nullptr;
-        std::array<uint32_t, kMaxPushConstants> pushConstants;
 
         // Will be autoreleased
         id<MTLComputeCommandEncoder> encoder = [commandBuffer computeCommandEncoder];
 
-        // Set default values for push constants
-        pushConstants.fill(0);
-        [encoder setBytes:&pushConstants length:sizeof(uint32_t) * kMaxPushConstants atIndex:0];
-
         Command type;
         while (mCommands.NextCommandId(&type)) {
             switch (type) {
@@ -659,19 +654,6 @@ namespace dawn_native { namespace metal {
                     lastPipeline->Encode(encoder);
                 } break;
 
-                case Command::SetPushConstants: {
-                    SetPushConstantsCmd* cmd = mCommands.NextCommand<SetPushConstantsCmd>();
-                    uint32_t* values = mCommands.NextData<uint32_t>(cmd->count);
-
-                    if (cmd->stages & dawn::ShaderStageBit::Compute) {
-                        memcpy(&pushConstants[cmd->offset], values, cmd->count * sizeof(uint32_t));
-
-                        [encoder setBytes:&pushConstants
-                                   length:sizeof(uint32_t) * kMaxPushConstants
-                                  atIndex:0];
-                    }
-                } break;
-
                 case Command::SetBindGroup: {
                     SetBindGroupCmd* cmd = mCommands.NextCommand<SetBindGroupCmd>();
                     uint64_t* dynamicOffsets = nullptr;
@@ -792,24 +774,10 @@ namespace dawn_native { namespace metal {
         id<MTLBuffer> indexBuffer = nil;
         uint32_t indexBufferBaseOffset = 0;
 
-        std::array<uint32_t, kMaxPushConstants> vertexPushConstants;
-        std::array<uint32_t, kMaxPushConstants> fragmentPushConstants;
-
         // This will be autoreleased
         id<MTLRenderCommandEncoder> encoder =
             [commandBuffer renderCommandEncoderWithDescriptor:mtlRenderPass];
 
-        // Set default values for push constants
-        vertexPushConstants.fill(0);
-        fragmentPushConstants.fill(0);
-
-        [encoder setVertexBytes:&vertexPushConstants
-                         length:sizeof(uint32_t) * kMaxPushConstants
-                        atIndex:0];
-        [encoder setFragmentBytes:&fragmentPushConstants
-                           length:sizeof(uint32_t) * kMaxPushConstants
-                          atIndex:0];
-
         Command type;
         while (mCommands.NextCommandId(&type)) {
             switch (type) {
@@ -885,27 +853,6 @@ namespace dawn_native { namespace metal {
                     lastPipeline->Encode(encoder);
                 } break;
 
-                case Command::SetPushConstants: {
-                    SetPushConstantsCmd* cmd = mCommands.NextCommand<SetPushConstantsCmd>();
-                    uint32_t* values = mCommands.NextData<uint32_t>(cmd->count);
-
-                    if (cmd->stages & dawn::ShaderStageBit::Vertex) {
-                        memcpy(&vertexPushConstants[cmd->offset], values,
-                               cmd->count * sizeof(uint32_t));
-                        [encoder setVertexBytes:&vertexPushConstants
-                                         length:sizeof(uint32_t) * kMaxPushConstants
-                                        atIndex:0];
-                    }
-
-                    if (cmd->stages & dawn::ShaderStageBit::Fragment) {
-                        memcpy(&fragmentPushConstants[cmd->offset], values,
-                               cmd->count * sizeof(uint32_t));
-                        [encoder setFragmentBytes:&fragmentPushConstants
-                                           length:sizeof(uint32_t) * kMaxPushConstants
-                                          atIndex:0];
-                    }
-                } break;
-
                 case Command::SetStencilReference: {
                     SetStencilReferenceCmd* cmd = mCommands.NextCommand<SetStencilReferenceCmd>();
                     [encoder setStencilReferenceValue:cmd->reference];

src/dawn_native/opengl/CommandBufferGL.cpp

@@ -138,77 +138,6 @@ namespace dawn_native { namespace opengl {
             }
         }
 
-        // Push constants are implemented using OpenGL uniforms, however they aren't part of the
-        // global OpenGL state but are part of the program state instead. This means that we have to
-        // reapply push constants on pipeline change.
-        //
-        // This structure tracks the current values of push constants as well as dirty bits for push
-        // constants that should be applied before the next draw or dispatch.
-        class PushConstantTracker {
-          public:
-            PushConstantTracker() {
-                for (auto stage : IterateStages(kAllStages)) {
-                    mValues[stage].fill(0);
-                    // No need to set dirty bits as a pipeline will be set before the next operation
-                    // using push constants.
-                }
-            }
-
-            void OnSetPushConstants(dawn::ShaderStageBit stages,
-                                    uint32_t count,
-                                    uint32_t offset,
-                                    const uint32_t* data) {
-                for (auto stage : IterateStages(stages)) {
-                    memcpy(&mValues[stage][offset], data, count * sizeof(uint32_t));
-
-                    // Use 64 bit masks and make sure there are no shift UB
-                    static_assert(kMaxPushConstants <= 8 * sizeof(unsigned long long) - 1, "");
-                    mDirtyBits[stage] |= ((1ull << count) - 1ull) << offset;
-                }
-            }
-
-            void OnSetPipeline(PipelineBase* pipeline) {
-                for (auto stage : IterateStages(kAllStages)) {
-                    mDirtyBits[stage] = pipeline->GetPushConstants(stage).mask;
-                }
-            }
-
-            void Apply(PipelineBase* pipeline, PipelineGL* glPipeline) {
-                for (auto stage : IterateStages(kAllStages)) {
-                    const auto& pushConstants = pipeline->GetPushConstants(stage);
-                    const auto& glPushConstants = glPipeline->GetGLPushConstants(stage);
-
-                    for (uint32_t constant :
-                         IterateBitSet(mDirtyBits[stage] & pushConstants.mask)) {
-                        GLint location = glPushConstants[constant];
-                        switch (pushConstants.types[constant]) {
-                            case PushConstantType::Int:
-                                glUniform1i(location,
-                                            *reinterpret_cast<GLint*>(&mValues[stage][constant]));
-                                break;
-                            case PushConstantType::UInt:
-                                glUniform1ui(location,
-                                             *reinterpret_cast<GLuint*>(&mValues[stage][constant]));
-                                break;
-                            case PushConstantType::Float:
-                                float value;
-                                // Use a memcpy to avoid strict-aliasing warnings, even if it is
-                                // still technically undefined behavior.
-                                memcpy(&value, &mValues[stage][constant], sizeof(value));
-                                glUniform1f(location, value);
-                                break;
-                        }
-                    }
-
-                    mDirtyBits[stage].reset();
-                }
-            }
-
-          private:
-            PerStage<std::array<uint32_t, kMaxPushConstants>> mValues;
-            PerStage<std::bitset<kMaxPushConstants>> mDirtyBits;
-        };
-
         // Vertex buffers and index buffers are implemented as part of an OpenGL VAO that
         // corresponds to an VertexInput. On the contrary in Dawn they are part of the global state.
         // This means that we have to re-apply these buffers on an VertexInput change.
@@ -555,7 +484,6 @@ namespace dawn_native { namespace opengl {
     }
 
     void CommandBuffer::ExecuteComputePass() {
-        PushConstantTracker pushConstants;
         ComputePipeline* lastPipeline = nullptr;
 
         Command type;
@@ -568,7 +496,6 @@ namespace dawn_native { namespace opengl {
 
                 case Command::Dispatch: {
                     DispatchCmd* dispatch = mCommands.NextCommand<DispatchCmd>();
-                    pushConstants.Apply(lastPipeline, lastPipeline);
                     glDispatchCompute(dispatch->x, dispatch->y, dispatch->z);
                     // TODO(cwallez@chromium.org): add barriers to the API
                     glMemoryBarrier(GL_ALL_BARRIER_BITS);
@@ -577,15 +504,7 @@ namespace dawn_native { namespace opengl {
                 case Command::SetComputePipeline: {
                     SetComputePipelineCmd* cmd = mCommands.NextCommand<SetComputePipelineCmd>();
                     lastPipeline = ToBackend(cmd->pipeline).Get();
-
                     lastPipeline->ApplyNow();
-                    pushConstants.OnSetPipeline(lastPipeline);
-                } break;
-
-                case Command::SetPushConstants: {
-                    SetPushConstantsCmd* cmd = mCommands.NextCommand<SetPushConstantsCmd>();
-                    uint32_t* data = mCommands.NextData<uint32_t>(cmd->count);
-                    pushConstants.OnSetPushConstants(cmd->stages, cmd->count, cmd->offset, data);
                 } break;
 
                 case Command::SetBindGroup: {
@@ -732,7 +651,6 @@ namespace dawn_native { namespace opengl {
         RenderPipeline* lastPipeline = nullptr;
         uint64_t indexBufferBaseOffset = 0;
 
-        PushConstantTracker pushConstants;
         InputBufferTracker inputBuffers;
 
         Command type;
@@ -751,7 +669,6 @@ namespace dawn_native { namespace opengl {
 
                 case Command::Draw: {
                     DrawCmd* draw = mCommands.NextCommand<DrawCmd>();
-                    pushConstants.Apply(lastPipeline, lastPipeline);
                     inputBuffers.Apply();
 
                     if (draw->firstInstance > 0) {
@@ -768,7 +685,6 @@ namespace dawn_native { namespace opengl {
 
                 case Command::DrawIndexed: {
                     DrawIndexedCmd* draw = mCommands.NextCommand<DrawIndexedCmd>();
-                    pushConstants.Apply(lastPipeline, lastPipeline);
                     inputBuffers.Apply();
 
                     dawn::IndexFormat indexFormat =
@@ -805,16 +721,9 @@ namespace dawn_native { namespace opengl {
                     lastPipeline = ToBackend(cmd->pipeline).Get();
                     lastPipeline->ApplyNow(persistentPipelineState);
 
-                    pushConstants.OnSetPipeline(lastPipeline);
                     inputBuffers.OnSetPipeline(lastPipeline);
                 } break;
 
-                case Command::SetPushConstants: {
-                    SetPushConstantsCmd* cmd = mCommands.NextCommand<SetPushConstantsCmd>();
-                    uint32_t* data = mCommands.NextData<uint32_t>(cmd->count);
-                    pushConstants.OnSetPushConstants(cmd->stages, cmd->count, cmd->offset, data);
-                } break;
-
                 case Command::SetStencilReference: {
                     SetStencilReferenceCmd* cmd = mCommands.NextCommand<SetStencilReferenceCmd>();
                     persistentPipelineState.SetStencilReference(cmd->reference);

src/dawn_native/opengl/PipelineGL.cpp

@@ -70,29 +70,6 @@ namespace dawn_native { namespace opengl {
             return shader;
         };
 
-        auto FillPushConstants = [](const ShaderModule* module, GLPushConstantInfo* info,
-                                    GLuint program) {
-            const auto& moduleInfo = module->GetPushConstants();
-            for (uint32_t i = 0; i < moduleInfo.names.size(); i++) {
-                (*info)[i] = -1;
-
-                unsigned int size = moduleInfo.sizes[i];
-                if (size == 0) {
-                    continue;
-                }
-
-                GLint location = glGetUniformLocation(program, moduleInfo.names[i].c_str());
-                if (location == -1) {
-                    continue;
-                }
-
-                for (uint32_t offset = 0; offset < size; offset++) {
-                    (*info)[i + offset] = location + offset;
-                }
-                i += size - 1;
-            }
-        };
-
         mProgram = glCreateProgram();
 
         dawn::ShaderStageBit activeStages = dawn::ShaderStageBit::None;
@@ -123,10 +100,6 @@ namespace dawn_native { namespace opengl {
             }
         }
 
-        for (dawn::ShaderStage stage : IterateStages(activeStages)) {
-            FillPushConstants(modules[stage], &mGlPushConstants[stage], mProgram);
-        }
-
         glUseProgram(mProgram);
 
         // The uniforms are part of the program state so we can pre-bind buffer units, texture units
@@ -200,11 +173,6 @@ namespace dawn_native { namespace opengl {
         }
     }
 
-    const PipelineGL::GLPushConstantInfo& PipelineGL::GetGLPushConstants(
-        dawn::ShaderStage stage) const {
-        return mGlPushConstants[stage];
-    }
-
     const std::vector<GLuint>& PipelineGL::GetTextureUnitsForSampler(GLuint index) const {
         ASSERT(index < mUnitsForSamplers.size());
         return mUnitsForSamplers[index];

src/dawn_native/opengl/PipelineGL.h

@@ -34,11 +34,9 @@ namespace dawn_native { namespace opengl {
 
         void Initialize(const PipelineLayout* layout, const PerStage<const ShaderModule*>& modules);
 
-        using GLPushConstantInfo = std::array<GLint, kMaxPushConstants>;
         using BindingLocations =
             std::array<std::array<GLint, kMaxBindingsPerGroup>, kMaxBindGroups>;
 
-        const GLPushConstantInfo& GetGLPushConstants(dawn::ShaderStage stage) const;
         const std::vector<GLuint>& GetTextureUnitsForSampler(GLuint index) const;
         const std::vector<GLuint>& GetTextureUnitsForTextureView(GLuint index) const;
         GLuint GetProgramHandle() const;
@@ -47,7 +45,6 @@ namespace dawn_native { namespace opengl {
 
       private:
         GLuint mProgram;
-        PerStage<GLPushConstantInfo> mGlPushConstants;
         std::vector<std::vector<GLuint>> mUnitsForSamplers;
         std::vector<std::vector<GLuint>> mUnitsForTextures;
     };

src/dawn_native/vulkan/PipelineLayoutVk.cpp

@@ -34,21 +34,14 @@ namespace dawn_native { namespace vulkan {
             numSetLayouts++;
         }
 
-        // Specify Dawn's push constant range on all pipeline layouts because we don't know which
-        // pipelines might use it.
-        VkPushConstantRange pushConstantRange;
-        pushConstantRange.stageFlags = VK_SHADER_STAGE_ALL;
-        pushConstantRange.offset = 0;
-        pushConstantRange.size = 4 * kMaxPushConstants;
-
         VkPipelineLayoutCreateInfo createInfo;
         createInfo.sType = VK_STRUCTURE_TYPE_PIPELINE_LAYOUT_CREATE_INFO;
         createInfo.pNext = nullptr;
         createInfo.flags = 0;
         createInfo.setLayoutCount = numSetLayouts;
         createInfo.pSetLayouts = setLayouts.data();
-        createInfo.pushConstantRangeCount = 1;
+        createInfo.pushConstantRangeCount = 0;
-        createInfo.pPushConstantRanges = &pushConstantRange;
+        createInfo.pPushConstantRanges = nullptr;
 
         if (device->fn.CreatePipelineLayout(device->GetVkDevice(), &createInfo, nullptr,
                                             &mHandle) != VK_SUCCESS) {

src/tests/end2end/PushConstantTests.cpp

@@ -1,416 +0,0 @@
-// Copyright 2017 The Dawn 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 "tests/DawnTest.h"
-
-#include "common/Assert.h"
-#include "common/Constants.h"
-#include "utils/ComboRenderPipelineDescriptor.h"
-#include "utils/DawnHelpers.h"
-
-#include <array>
-
-class PushConstantTest: public DawnTest {
-    protected:
-        // Layout, bind group and friends to store results for compute tests, can have an extra buffer
-        // so that two different pipeline layout can be created.
-        struct TestBindings {
-            dawn::PipelineLayout layout;
-            dawn::BindGroup bindGroup;
-            dawn::Buffer resultBuffer;
-        };
-        TestBindings MakeTestBindings(bool extraBuffer) {
-            uint32_t one = 1;
-            dawn::Buffer buf1 = utils::CreateBufferFromData(device, &one, 4, dawn::BufferUsageBit::Storage |
-                                                                             dawn::BufferUsageBit::TransferSrc |
-                                                                             dawn::BufferUsageBit::TransferDst);
-            dawn::BufferDescriptor buf2Desc;
-            buf2Desc.size = 4;
-            buf2Desc.usage = dawn::BufferUsageBit::Storage;
-            dawn::Buffer buf2 = device.CreateBuffer(&buf2Desc);
-
-            dawn::ShaderStageBit kAllStages = dawn::ShaderStageBit::Compute | dawn::ShaderStageBit::Fragment | dawn::ShaderStageBit::Vertex;
-            constexpr dawn::ShaderStageBit kNoStages{};
-
-            dawn::BindGroupLayout bgl = utils::MakeBindGroupLayout(
-                device,
-                {
-                    {0, kAllStages, dawn::BindingType::StorageBuffer},
-                    {1, extraBuffer ? kAllStages : kNoStages, dawn::BindingType::StorageBuffer},
-                });
-
-            dawn::PipelineLayout pl = utils::MakeBasicPipelineLayout(device, &bgl);
-
-            dawn::BindGroup bg;
-            if (extraBuffer) {
-                bg = utils::MakeBindGroup(device, bgl, {
-                    {0, buf1, 0, 4},
-                    {1, buf2, 0, 4},
-                });
-            } else {
-                bg = utils::MakeBindGroup(device, bgl, {
-                    {0, buf1, 0, 4},
-                });
-            }
-
-            return {std::move(pl), std::move(bg), std::move(buf1)};
-        }
-
-        // A test spec is a bunch of push constant types and expected values
-        enum PushConstantType {
-            Float,
-            Int,
-            UInt,
-        };
-        struct PushConstantSpecItem {
-            PushConstantType type;
-            int value;
-        };
-        using PushConstantSpec = std::vector<PushConstantSpecItem>;
-
-        PushConstantSpec MakeAllZeroSpec() const {
-            PushConstantSpec allZeros;
-            for (uint32_t i = 0; i < kMaxPushConstants; ++i) {
-                allZeros.push_back({Int, 0});
-            }
-            return allZeros;
-        }
-
-        // The GLSL code to define the push constant block for a given test spec
-        std::string MakePushConstantBlock(PushConstantSpec spec) {
-            std::string block = "layout(push_constant) uniform ConstantsBlock {\n";
-            for (size_t i = 0; i < spec.size(); ++i) {
-                block += "    ";
-                switch (spec[i].type) {
-                    case Float:
-                        block += "float";
-                        break;
-                    case Int:
-                        block += "int";
-                        break;
-                    case UInt:
-                        block += "uint";
-                        break;
-                }
-                block += " val" + std::to_string(i) + ";\n";
-            }
-            block += "} c;\n";
-            return block;
-        }
-
-        // The GLSL code to define the push constant test for a given test spec
-        std::string MakePushConstantTest(PushConstantSpec spec, std::string varName) {
-            std::string test = "bool " + varName + " = true;\n";
-            for (size_t i = 0; i < spec.size(); ++i) {
-                test += varName + " = " + varName + " && (c.val" + std::to_string(i) + " == ";
-                switch (spec[i].type) {
-                    case Float:
-                        test += "float";
-                        break;
-                    case Int:
-                        test += "int";
-                        break;
-                    case UInt:
-                        test += "uint";
-                        break;
-                }
-                test += "(" + std::to_string(spec[i].value) + "));\n";
-            }
-            return test;
-        }
-
-        // The compute pipeline ANDs the result of the test in the SSBO
-        dawn::ComputePipeline MakeTestComputePipeline(const dawn::PipelineLayout& pl, PushConstantSpec spec) {
-            dawn::ShaderModule module = utils::CreateShaderModule(device, dawn::ShaderStage::Compute, (R"(
-                #version 450
-                layout(set = 0, binding = 0) buffer Result {
-                    int success;
-                } result;
-                )" + MakePushConstantBlock(spec) + R"(
-                void main() {
-                    )" + MakePushConstantTest(spec, "success") + R"(
-                    if (success && result.success == 1) {
-                        result.success = 1;
-                    } else {
-                        result.success = 0;
-                    }
-                })").c_str()
-            );
-
-            dawn::ComputePipelineDescriptor descriptor;
-            descriptor.layout = pl;
-
-            dawn::PipelineStageDescriptor computeStage;
-            computeStage.module = module;
-            computeStage.entryPoint = "main";
-            descriptor.computeStage = &computeStage;
-
-            return device.CreateComputePipeline(&descriptor);
-        }
-
-        dawn::PipelineLayout MakeEmptyLayout() {
-            return utils::MakeBasicPipelineLayout(device, nullptr);
-        }
-
-        // The render pipeline adds one to the red channel for successful vertex push constant test
-        // and adds one to green for the frgament test.
-        dawn::RenderPipeline MakeTestRenderPipeline(dawn::PipelineLayout& layout, PushConstantSpec vsSpec, PushConstantSpec fsSpec) {
-            dawn::ShaderModule vsModule = utils::CreateShaderModule(device, dawn::ShaderStage::Vertex, (R"(
-                #version 450
-                )" + MakePushConstantBlock(vsSpec) + R"(
-                layout(location = 0) out float red;
-                void main() {
-                    red = 0.0f;
-                    )" + MakePushConstantTest(vsSpec, "success") + R"(
-                    if (success) {red = 1.0f / 255.0f;}
-                    gl_Position = vec4(0.0f, 0.0f, 0.0f, 1.0f);
-                })").c_str()
-            );
-            dawn::ShaderModule fsModule = utils::CreateShaderModule(device, dawn::ShaderStage::Fragment, (R"(
-                #version 450
-                )" + MakePushConstantBlock(fsSpec) + R"(
-                layout(location = 0) out vec4 color;
-                layout(location = 0) in float red;
-                void main() {
-                    color = vec4(red, 0.0f, 0.0f, 0.0f);
-                    )" + MakePushConstantTest(fsSpec, "success") + R"(
-                    if (success) {color.g = 1.0f / 255.0f;}
-                })").c_str()
-            );
-
-            utils::ComboRenderPipelineDescriptor descriptor(device);
-            descriptor.layout = layout;
-            descriptor.cVertexStage.module = vsModule;
-            descriptor.cFragmentStage.module = fsModule;
-            descriptor.primitiveTopology = dawn::PrimitiveTopology::PointList;
-
-            return device.CreateRenderPipeline(&descriptor);
-        }
-};
-
-// Test that push constants default to zero at the beginning of every compute passes.
-TEST_P(PushConstantTest, ComputePassDefaultsToZero) {
-    auto binding = MakeTestBindings(false);
-
-    // Expect push constants to be zero in all dispatches of this test.
-    dawn::ComputePipeline pipeline = MakeTestComputePipeline(binding.layout, MakeAllZeroSpec());
-
-    uint32_t notZero = 42;
-    dawn::CommandEncoder encoder = device.CreateCommandEncoder();
-    {
-        dawn::ComputePassEncoder pass = encoder.BeginComputePass();
-
-        // Test compute push constants are set to zero by default.
-        pass.SetPipeline(pipeline);
-        pass.SetBindGroup(0, binding.bindGroup, 0, nullptr);
-        pass.Dispatch(1, 1, 1);
-        // Set push constants to non-zero value to check they will be reset to zero
-        // on the next BeginComputePass
-        pass.SetPushConstants(dawn::ShaderStageBit::Compute, 0, 1, &notZero);
-
-        pass.EndPass();
-    }
-    {
-        dawn::ComputePassEncoder pass = encoder.BeginComputePass();
-
-        pass.SetPipeline(pipeline);
-        pass.SetBindGroup(0, binding.bindGroup, 0, nullptr);
-        pass.Dispatch(1, 1, 1);
-
-        pass.EndPass();
-    }
-
-    dawn::CommandBuffer commands = encoder.Finish();
-    queue.Submit(1, &commands);
-
-    EXPECT_BUFFER_U32_EQ(1, binding.resultBuffer, 0);
-}
-
-// Test that push constants default to zero at the beginning of render passes.
-TEST_P(PushConstantTest, RenderPassDefaultsToZero) {
-    utils::BasicRenderPass renderPass = utils::CreateBasicRenderPass(device, 1, 1);
-
-    // Expect push constants to be zero in all draws of this test.
-    PushConstantSpec allZeros = MakeAllZeroSpec();
-    dawn::PipelineLayout layout = MakeEmptyLayout();
-    dawn::RenderPipeline pipeline = MakeTestRenderPipeline(layout, MakeAllZeroSpec(), MakeAllZeroSpec());
-
-    dawn::CommandEncoder encoder = device.CreateCommandEncoder();
-    {
-        dawn::RenderPassEncoder pass = encoder.BeginRenderPass(&renderPass.renderPassInfo);
-        // Test render push constants are set to zero by default.
-        pass.SetPipeline(pipeline);
-        pass.Draw(1, 1, 0, 0);
-        pass.EndPass();
-    }
-
-    dawn::CommandBuffer commands = encoder.Finish();
-    queue.Submit(1, &commands);
-
-    EXPECT_PIXEL_RGBA8_EQ(RGBA8(1, 1, 0, 0), renderPass.color, 0, 0);
-}
-
-// Test setting push constants of various 32bit types.
-TEST_P(PushConstantTest, VariousConstantTypes) {
-
-    struct {
-        int32_t v1;
-        uint32_t v2;
-        float v3;
-    } values = {-1, 3, 4.0f};
-    static_assert(sizeof(values) == 3 * sizeof(uint32_t), "");
-
-    auto binding = MakeTestBindings(false);
-    PushConstantSpec spec = {{Int, -1}, {UInt, 3}, {Float, 4}};
-    dawn::ComputePipeline pipeline = MakeTestComputePipeline(binding.layout, spec);
-
-
-    dawn::CommandEncoder encoder = device.CreateCommandEncoder();
-    {
-        dawn::ComputePassEncoder pass = encoder.BeginComputePass();
-
-        pass.SetPushConstants(dawn::ShaderStageBit::Compute, 0, 3, reinterpret_cast<uint32_t*>(&values));
-        pass.SetPipeline(pipeline);
-        pass.SetBindGroup(0, binding.bindGroup, 0, nullptr);
-        pass.Dispatch(1, 1, 1);
-
-        pass.EndPass();
-    }
-
-    dawn::CommandBuffer commands = encoder.Finish();
-    queue.Submit(1, &commands);
-
-    EXPECT_BUFFER_U32_EQ(1, binding.resultBuffer, 0);
-}
-
-// Test that the push constants stay in between pipeline layout changes.
-TEST_P(PushConstantTest, InheritThroughPipelineLayoutChange) {
-    // These bindings will have a different pipeline layout because binding 2 has an extra buffer.
-    auto binding1 = MakeTestBindings(false);
-    auto binding2 = MakeTestBindings(true);
-    PushConstantSpec spec1 = {{Int, 1}};
-    PushConstantSpec spec2 = {{Int, 2}};
-    dawn::ComputePipeline pipeline1 = MakeTestComputePipeline(binding1.layout, spec1);
-    dawn::ComputePipeline pipeline2 = MakeTestComputePipeline(binding2.layout, spec2);
-
-    uint32_t one = 1;
-    uint32_t two = 2;
-    dawn::CommandEncoder encoder = device.CreateCommandEncoder();
-    {
-        dawn::ComputePassEncoder pass = encoder.BeginComputePass();
-
-        // Set Push constant before there is a pipeline set
-        pass.SetPushConstants(dawn::ShaderStageBit::Compute, 0, 1, &one);
-        pass.SetPipeline(pipeline1);
-        pass.SetBindGroup(0, binding1.bindGroup, 0, nullptr);
-        pass.Dispatch(1, 1, 1);
-        // Change the push constant before changing pipeline layout
-        pass.SetPushConstants(dawn::ShaderStageBit::Compute, 0, 1, &two);
-        pass.SetPipeline(pipeline2);
-        pass.SetBindGroup(0, binding2.bindGroup, 0, nullptr);
-        pass.Dispatch(1, 1, 1);
-
-        pass.EndPass();
-    }
-
-    dawn::CommandBuffer commands = encoder.Finish();
-    queue.Submit(1, &commands);
-
-    EXPECT_BUFFER_U32_EQ(1, binding1.resultBuffer, 0);
-    EXPECT_BUFFER_U32_EQ(1, binding2.resultBuffer, 0);
-}
-
-// Try setting all push constants
-TEST_P(PushConstantTest, SetAllConstantsToNonZero) {
-    PushConstantSpec spec;
-    std::array<uint32_t, kMaxPushConstants> values;
-    for (uint32_t i = 0; i < kMaxPushConstants; ++i) {
-        spec.push_back({Int, static_cast<int>(i + 1)});
-        values[i] = i + 1;
-    }
-
-    auto binding = MakeTestBindings(false);
-    dawn::ComputePipeline pipeline = MakeTestComputePipeline(binding.layout, spec);
-
-    dawn::CommandEncoder encoder = device.CreateCommandEncoder();
-    {
-        dawn::ComputePassEncoder pass = encoder.BeginComputePass();
-
-        pass.SetPushConstants(dawn::ShaderStageBit::Compute, 0, kMaxPushConstants, &values[0]);
-        pass.SetPipeline(pipeline);
-        pass.SetBindGroup(0, binding.bindGroup, 0, nullptr);
-        pass.Dispatch(1, 1, 1);
-
-        pass.EndPass();
-    }
-
-    dawn::CommandBuffer commands = encoder.Finish();
-    queue.Submit(1, &commands);
-
-    EXPECT_BUFFER_U32_EQ(1, binding.resultBuffer, 0);
-}
-
-// Try setting separate push constants for vertex and fragment stage
-TEST_P(PushConstantTest, SeparateVertexAndFragmentConstants) {
-    PushConstantSpec vsSpec = {{Int, 1}};
-    PushConstantSpec fsSpec = {{Int, 2}};
-
-    utils::BasicRenderPass renderPass = utils::CreateBasicRenderPass(device, 1, 1);
-
-    dawn::PipelineLayout layout = MakeEmptyLayout();
-    dawn::RenderPipeline pipeline = MakeTestRenderPipeline(layout, vsSpec, fsSpec);
-
-    uint32_t one = 1;
-    uint32_t two = 2;
-    dawn::CommandEncoder encoder = device.CreateCommandEncoder();
-    {
-        dawn::RenderPassEncoder pass = encoder.BeginRenderPass(&renderPass.renderPassInfo);
-        pass.SetPushConstants(dawn::ShaderStageBit::Vertex, 0, 1, &one);
-        pass.SetPushConstants(dawn::ShaderStageBit::Fragment, 0, 1, &two);
-        pass.SetPipeline(pipeline);
-        pass.Draw(1, 1, 0, 0);
-        pass.EndPass();
-    }
-
-    dawn::CommandBuffer commands = encoder.Finish();
-    queue.Submit(1, &commands);
-
-    EXPECT_PIXEL_RGBA8_EQ(RGBA8(1, 1, 0, 0), renderPass.color, 0, 0);
-}
-
-// Try setting push constants for vertex and fragment stage simulteanously
-TEST_P(PushConstantTest, SimultaneousVertexAndFragmentConstants) {
-    PushConstantSpec spec = {{Int, 2}};
-
-    utils::BasicRenderPass renderPass = utils::CreateBasicRenderPass(device, 1, 1);
-
-    dawn::PipelineLayout layout = MakeEmptyLayout();
-    dawn::RenderPipeline pipeline = MakeTestRenderPipeline(layout, spec, spec);
-
-    uint32_t two = 2;
-    dawn::CommandEncoder encoder = device.CreateCommandEncoder();
-    {
-        dawn::RenderPassEncoder pass = encoder.BeginRenderPass(&renderPass.renderPassInfo);
-        pass.SetPushConstants(dawn::ShaderStageBit::Vertex | dawn::ShaderStageBit::Fragment, 0, 1, &two);
-        pass.SetPipeline(pipeline);
-        pass.Draw(1, 1, 0, 0);
-        pass.EndPass();
-    }
-
-    dawn::CommandBuffer commands = encoder.Finish();
-    queue.Submit(1, &commands);
-
-    EXPECT_PIXEL_RGBA8_EQ(RGBA8(1, 1, 0, 0), renderPass.color, 0, 0);
-}
-DAWN_INSTANTIATE_TEST(PushConstantTest, MetalBackend, OpenGLBackend);

src/tests/unittests/validation/PushConstantsValidationTests.cpp

@@ -1,244 +0,0 @@
-// Copyright 2017 The Dawn 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 "tests/unittests/validation/ValidationTest.h"
-
-#include "common/Constants.h"
-#include "utils/DawnHelpers.h"
-
-#include <gmock/gmock.h>
-
-using namespace testing;
-
-class PushConstantTest : public ValidationTest {
-    protected:
-        dawn::Queue queue;
-        uint32_t constants[kMaxPushConstants] = {0};
-
-        void TestCreateShaderModule(bool success, std::string vertexSource) {
-            dawn::ShaderModule module;
-            if (success) {
-                module = utils::CreateShaderModule(device, dawn::ShaderStage::Vertex, vertexSource.c_str());
-            } else {
-                ASSERT_DEVICE_ERROR(module = utils::CreateShaderModule(device, dawn::ShaderStage::Vertex, vertexSource.c_str()));
-            }
-        }
-
-    private:
-        void SetUp() override {
-            ValidationTest::SetUp();
-            queue = device.CreateQueue();
-        }
-};
-
-// Test valid usage of the parameters to SetPushConstants
-TEST_F(PushConstantTest, Success) {
-    DummyRenderPass renderpassData(device);
-
-    dawn::CommandEncoder encoder = device.CreateCommandEncoder();
-    // PushConstants in a compute pass
-    {
-        dawn::ComputePassEncoder pass = encoder.BeginComputePass();
-        pass.SetPushConstants(dawn::ShaderStageBit::Compute, 0, 1, constants);
-        pass.EndPass();
-    }
-
-    // PushConstants in a render pass
-    {
-        dawn::RenderPassEncoder pass = encoder.BeginRenderPass(&renderpassData);
-        pass.SetPushConstants(dawn::ShaderStageBit::Vertex | dawn::ShaderStageBit::Fragment, 0, 1, constants);
-        pass.EndPass();
-    }
-
-    // Setting all constants
-    {
-        dawn::ComputePassEncoder pass = encoder.BeginComputePass();
-        pass.SetPushConstants(dawn::ShaderStageBit::Compute, 0, kMaxPushConstants, constants);
-        pass.EndPass();
-    }
-
-    // Setting constants at an offset
-    {
-        dawn::ComputePassEncoder pass = encoder.BeginComputePass();
-        pass.SetPushConstants(dawn::ShaderStageBit::Compute, kMaxPushConstants - 1, 1, constants);
-        pass.EndPass();
-    }
-
-    encoder.Finish();
-}
-
-// Test check for constants being set out of bounds
-TEST_F(PushConstantTest, SetPushConstantsOOB) {
-    uint32_t constants[kMaxPushConstants] = {0};
-
-    // Control case: setting all constants
-    {
-        dawn::CommandEncoder encoder = device.CreateCommandEncoder();
-        dawn::ComputePassEncoder pass = encoder.BeginComputePass();
-        pass.SetPushConstants(dawn::ShaderStageBit::Compute, 0, kMaxPushConstants, constants);
-        pass.EndPass();
-        encoder.Finish();
-    }
-
-    // OOB because count is too big.
-    {
-        dawn::CommandEncoder encoder = device.CreateCommandEncoder();
-        dawn::ComputePassEncoder pass = encoder.BeginComputePass();
-        pass.SetPushConstants(dawn::ShaderStageBit::Compute, 0, kMaxPushConstants + 1, constants);
-        pass.EndPass();
-        ASSERT_DEVICE_ERROR(encoder.Finish());
-    }
-
-    // OOB because of the offset.
-    {
-        dawn::CommandEncoder encoder = device.CreateCommandEncoder();
-        dawn::ComputePassEncoder pass = encoder.BeginComputePass();
-        pass.SetPushConstants(dawn::ShaderStageBit::Compute, 1, kMaxPushConstants, constants);
-        pass.EndPass();
-        ASSERT_DEVICE_ERROR(encoder.Finish());
-    }
-}
-
-// Test valid stages for compute pass
-TEST_F(PushConstantTest, StageForComputePass) {
-    // Control case: setting to the compute stage in compute passes
-    {
-        dawn::CommandEncoder encoder = device.CreateCommandEncoder();
-        dawn::ComputePassEncoder pass = encoder.BeginComputePass();
-        pass.SetPushConstants(dawn::ShaderStageBit::Compute, 0, 1, constants);
-        pass.EndPass();
-        encoder.Finish();
-    }
-
-    // Graphics stages are disallowed
-    {
-        dawn::CommandEncoder encoder = device.CreateCommandEncoder();
-        dawn::ComputePassEncoder pass = encoder.BeginComputePass();
-        pass.SetPushConstants(dawn::ShaderStageBit::Vertex, 0, 1, constants);
-        pass.EndPass();
-        ASSERT_DEVICE_ERROR(encoder.Finish());
-    }
-
-    // A None shader stage mask is valid.
-    {
-        dawn::CommandEncoder encoder = device.CreateCommandEncoder();
-        dawn::ComputePassEncoder pass = encoder.BeginComputePass();
-        pass.SetPushConstants(dawn::ShaderStageBit::None, 0, 1, constants);
-        pass.EndPass();
-        encoder.Finish();
-    }
-}
-
-// Test valid stages for render passes
-TEST_F(PushConstantTest, StageForRenderPass) {
-    DummyRenderPass renderpassData(device);
-
-    // Control case: setting to vertex and fragment in render pass
-    {
-        dawn::CommandEncoder encoder = device.CreateCommandEncoder();
-        dawn::RenderPassEncoder pass = encoder.BeginRenderPass(&renderpassData);
-        pass.SetPushConstants(dawn::ShaderStageBit::Vertex | dawn::ShaderStageBit::Fragment, 0, 1, constants);
-        pass.EndPass();
-        encoder.Finish();
-    }
-
-    // Compute stage is disallowed
-    {
-        dawn::CommandEncoder encoder = device.CreateCommandEncoder();
-        dawn::RenderPassEncoder pass = encoder.BeginRenderPass(&renderpassData);
-        pass.SetPushConstants(dawn::ShaderStageBit::Compute, 0, 1, constants);
-        pass.EndPass();
-        ASSERT_DEVICE_ERROR(encoder.Finish());
-    }
-
-    // A None shader stage mask is valid.
-    {
-        dawn::CommandEncoder encoder = device.CreateCommandEncoder();
-        dawn::RenderPassEncoder pass = encoder.BeginRenderPass(&renderpassData);
-        pass.SetPushConstants(dawn::ShaderStageBit::None, 0, 1, constants);
-        pass.EndPass();
-        encoder.Finish();
-    }
-}
-
-// Valid shaders that use pushconstants
-TEST_F(PushConstantTest, ShaderCompilationSuccess) {
-    // Test shader module not using any push constants
-    TestCreateShaderModule(true, R"(
-        #version 450
-        void main() {
-            gl_Position = vec4(0.0);
-        }
-    )");
-
-    // Test one push constant
-    TestCreateShaderModule(true, R"(
-        #version 450
-        layout(push_constant) uniform ConstantsBlock {
-            float a;
-        } c;
-        void main() {
-            gl_Position = vec4(0.0);
-        }
-    )");
-
-    // Test one push constant with an offset
-    TestCreateShaderModule(true, R"(
-        #version 450
-        layout(push_constant) uniform ConstantsBlock {
-            float a;
-        } c;
-        void main() {
-            gl_Position = vec4(0.0);
-        }
-    )");
-
-    // Test max push constants
-    TestCreateShaderModule(true, R"(
-        #version 450
-        layout(push_constant) uniform ConstantsBlock {
-            float a[)" + std::to_string(kMaxPushConstants) + R"(];
-        } c;
-        void main() {
-            gl_Position = vec4(0.0);
-        }
-    )");
-}
-
-// Test that shaders using a push constant block too big fail compilation
-// TODO(cwallez@chromium.org): Currently disabled because ShaderModule error handling needs refactoring
-TEST_F(PushConstantTest, DISABLED_ShaderCompilationOOB) {
-    // Test one push constant over the max
-    TestCreateShaderModule(false, R"(
-        #version 450
-        layout(push_constant) uniform ConstantsBlock {
-            float a[)" + std::to_string(kMaxPushConstants + 1) + R"(];
-        } c;
-        void main() {
-            gl_Position = vec4(0.0);
-        }
-    )");
-
-    // Test two variables in the push constant block that together overflow
-    TestCreateShaderModule(false, R"(
-        #version 450
-        layout(push_constant) uniform ConstantsBlock {
-            float a[)" + std::to_string(kMaxPushConstants) + R"(];
-            float b;
-        } c;
-        void main() {
-            gl_Position = vec4(0.0);
-        }
-    )");
-}

src/tests/unittests/wire/WireArgumentTests.cpp

@@ -16,6 +16,8 @@
 
 #include "common/Constants.h"
 
+#include <array>
+
 using namespace testing;
 using namespace dawn_wire;
 
@@ -44,22 +46,33 @@ TEST_F(WireArgumentTests, ValueArgument) {
 }
 
 // Test that the wire is able to send arrays of numerical values
-static constexpr uint32_t testPushConstantValues[4] = {0, 42, 0xDEADBEEFu, 0xFFFFFFFFu};
-
-bool CheckPushConstantValues(const uint32_t* values) {
-    for (int i = 0; i < 4; ++i) {
-        if (values[i] != testPushConstantValues[i]) {
-            return false;
-        }
-    }
-    return true;
-}
-
 TEST_F(WireArgumentTests, ValueArrayArgument) {
+    // Create a bindgroup.
+    DawnBindGroupLayoutDescriptor bglDescriptor;
+    bglDescriptor.nextInChain = nullptr;
+    bglDescriptor.bindingCount = 0;
+    bglDescriptor.bindings = nullptr;
+
+    DawnBindGroupLayout bgl = dawnDeviceCreateBindGroupLayout(device, &bglDescriptor);
+    DawnBindGroupLayout apiBgl = api.GetNewBindGroupLayout();
+    EXPECT_CALL(api, DeviceCreateBindGroupLayout(apiDevice, _)).WillOnce(Return(apiBgl));
+
+    DawnBindGroupDescriptor bindGroupDescriptor;
+    bindGroupDescriptor.nextInChain = nullptr;
+    bindGroupDescriptor.layout = bgl;
+    bindGroupDescriptor.bindingCount = 0;
+    bindGroupDescriptor.bindings = nullptr;
+
+    DawnBindGroup bindGroup = dawnDeviceCreateBindGroup(device, &bindGroupDescriptor);
+    DawnBindGroup apiBindGroup = api.GetNewBindGroup();
+    EXPECT_CALL(api, DeviceCreateBindGroup(apiDevice, _)).WillOnce(Return(apiBindGroup));
+
+    // Use the bindgroup in SetBindGroup that takes an array of value offsets.
     DawnCommandEncoder encoder = dawnDeviceCreateCommandEncoder(device);
     DawnComputePassEncoder pass = dawnCommandEncoderBeginComputePass(encoder);
-    dawnComputePassEncoderSetPushConstants(pass, DAWN_SHADER_STAGE_BIT_VERTEX, 0, 4,
+
-                                           testPushConstantValues);
+    std::array<uint64_t, 4> testOffsets = {0, 42, 0xDEAD'BEEF'DEAD'BEEFu, 0xFFFF'FFFF'FFFF'FFFFu};
+    dawnComputePassEncoderSetBindGroup(pass, 0, bindGroup, testOffsets.size(), testOffsets.data());
 
     DawnCommandEncoder apiEncoder = api.GetNewCommandEncoder();
     EXPECT_CALL(api, DeviceCreateCommandEncoder(apiDevice)).WillOnce(Return(apiEncoder));
@@ -67,9 +80,16 @@ TEST_F(WireArgumentTests, ValueArrayArgument) {
     DawnComputePassEncoder apiPass = api.GetNewComputePassEncoder();
     EXPECT_CALL(api, CommandEncoderBeginComputePass(apiEncoder)).WillOnce(Return(apiPass));
 
-    EXPECT_CALL(api,
+    EXPECT_CALL(api, ComputePassEncoderSetBindGroup(
-                ComputePassEncoderSetPushConstants(apiPass, DAWN_SHADER_STAGE_BIT_VERTEX, 0, 4,
+                         apiPass, 0, apiBindGroup, testOffsets.size(),
-                                                   ResultOf(CheckPushConstantValues, Eq(true))));
+                         MatchesLambda([testOffsets](const uint64_t* offsets) -> bool {
+                             for (size_t i = 0; i < testOffsets.size(); i++) {
+                                 if (offsets[i] != testOffsets[i]) {
+                                     return false;
+                                 }
+                             }
+                             return true;
+                         })));
 
     FlushClient();
 }
@@ -280,6 +300,7 @@ TEST_F(WireArgumentTests, StructureOfValuesArgument) {
 // Test that the wire is able to send structures that contain objects
 TEST_F(WireArgumentTests, StructureOfObjectArrayArgument) {
     DawnBindGroupLayoutDescriptor bglDescriptor;
+    bglDescriptor.nextInChain = nullptr;
     bglDescriptor.bindingCount = 0;
     bglDescriptor.bindings = nullptr;