diff --git a/BUILD.gn b/BUILD.gn index 6277625c7e..b66d8079e3 100644 --- a/BUILD.gn +++ b/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", diff --git a/dawn.json b/dawn.json index c1e26f87e0..6cd052feaf 100644 --- a/dawn.json +++ b/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": [ diff --git a/examples/Animometer.cpp b/examples/Animometer.cpp index 263f35cb11..610544bcd2 100644 --- a/examples/Animometer.cpp +++ b/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(shaderData.data())); - size_t i = 0; - - utils::ComboRenderPassDescriptor renderPass({backbuffer.CreateDefaultView()}, - depthStencilView); + utils::ComboRenderPassDescriptor renderPass({backbuffer.CreateDefaultView()}); dawn::CommandEncoder encoder = device.CreateCommandEncoder(); { dawn::RenderPassEncoder pass = encoder.BeginRenderPass(&renderPass); pass.SetPipeline(pipeline); - for (int k = 0; k < 10000; k++) { - shaderData[i].time = f / 60.0f; - pass.SetPushConstants(dawn::ShaderStageBit::Vertex, 0, 6, reinterpret_cast(&shaderData[i])); + for (size_t i = 0; i < kNumTriangles; i++) { + uint64_t offset = i * sizeof(ShaderData); + pass.SetBindGroup(0, bindGroup, 1, &offset); pass.Draw(3, 1, 0, 0); - i++; } pass.EndPass(); diff --git a/src/common/Constants.h b/src/common/Constants.h index aa04501abf..7924a70e2e 100644 --- a/src/common/Constants.h +++ b/src/common/Constants.h @@ -17,7 +17,6 @@ #include -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; diff --git a/src/dawn_native/CommandEncoder.cpp b/src/dawn_native/CommandEncoder.cpp index 58f22ee0d9..ee0fe9c9eb 100644 --- a/src/dawn_native/CommandEncoder.cpp +++ b/src/dawn_native/CommandEncoder.cpp @@ -1016,18 +1016,6 @@ namespace dawn_native { persistentState.SetComputePipeline(pipeline); } break; - case Command::SetPushConstants: { - SetPushConstantsCmd* cmd = mIterator.NextCommand(); - mIterator.NextData(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(); if (cmd->dynamicOffsetCount > 0) { @@ -1120,20 +1108,6 @@ namespace dawn_native { persistentState.SetRenderPipeline(pipeline); } break; - case Command::SetPushConstants: { - SetPushConstantsCmd* cmd = mIterator.NextCommand(); - mIterator.NextData(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(); } break; diff --git a/src/dawn_native/Commands.cpp b/src/dawn_native/Commands.cpp index 98a978be1b..512e7e4191 100644 --- a/src/dawn_native/Commands.cpp +++ b/src/dawn_native/Commands.cpp @@ -96,11 +96,6 @@ namespace dawn_native { SetRenderPipelineCmd* cmd = commands->NextCommand(); cmd->~SetRenderPipelineCmd(); } break; - case Command::SetPushConstants: { - SetPushConstantsCmd* cmd = commands->NextCommand(); - commands->NextData(cmd->count); - cmd->~SetPushConstantsCmd(); - } break; case Command::SetStencilReference: { SetStencilReferenceCmd* cmd = commands->NextCommand(); cmd->~SetStencilReferenceCmd(); @@ -206,11 +201,6 @@ namespace dawn_native { commands->NextCommand(); break; - case Command::SetPushConstants: { - auto* cmd = commands->NextCommand(); - commands->NextData(cmd->count); - } break; - case Command::SetStencilReference: commands->NextCommand(); break; diff --git a/src/dawn_native/Commands.h b/src/dawn_native/Commands.h index 6dc25e2a66..e36262299a 100644 --- a/src/dawn_native/Commands.h +++ b/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 pipeline; }; - struct SetPushConstantsCmd { - dawn::ShaderStageBit stages; - uint32_t offset; - uint32_t count; - }; - struct SetStencilReferenceCmd { uint32_t reference; }; diff --git a/src/dawn_native/ComputePipeline.cpp b/src/dawn_native/ComputePipeline.cpp index 8f527865fe..468928b947 100644 --- a/src/dawn_native/ComputePipeline.cpp +++ b/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) diff --git a/src/dawn_native/Forward.h b/src/dawn_native/Forward.h index 3f32c4a641..db539048ab 100644 --- a/src/dawn_native/Forward.h +++ b/src/dawn_native/Forward.h @@ -48,8 +48,6 @@ namespace dawn_native { template class PerStage; - - enum PushConstantType : uint8_t; } // namespace dawn_native #endif // DAWNNATIVE_FORWARD_H_ diff --git a/src/dawn_native/Pipeline.cpp b/src/dawn_native/Pipeline.cpp index 8c245e1aa9..bc79cf06db 100644 --- a/src/dawn_native/Pipeline.cpp +++ b/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; diff --git a/src/dawn_native/Pipeline.h b/src/dawn_native/Pipeline.h index 55d57bf02e..ceacdf11ae 100644 --- a/src/dawn_native/Pipeline.h +++ b/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 mask; - std::array 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 mLayout; - PerStage mPushConstants; }; } // namespace dawn_native diff --git a/src/dawn_native/ProgrammablePassEncoder.cpp b/src/dawn_native/ProgrammablePassEncoder.cpp index 3ae7b113a0..5330b6c990 100644 --- a/src/dawn_native/ProgrammablePassEncoder.cpp +++ b/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(Command::SetPushConstants); - cmd->stages = stages; - cmd->offset = offset; - cmd->count = count; - - uint32_t* values = mAllocator->AllocateData(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"); diff --git a/src/dawn_native/ProgrammablePassEncoder.h b/src/dawn_native/ProgrammablePassEncoder.h index d37f9bec63..0389447571 100644 --- a/src/dawn_native/ProgrammablePassEncoder.h +++ b/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. diff --git a/src/dawn_native/RenderPipeline.cpp b/src/dawn_native/RenderPipeline.cpp index 699c992eec..457cf301e3 100644 --- a/src/dawn_native/RenderPipeline.cpp +++ b/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); diff --git a/src/dawn_native/ShaderModule.cpp b/src/dawn_native/ShaderModule.cpp index 598d4f6177..ac025e6594 100644 --- a/src/dawn_native/ShaderModule.cpp +++ b/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]; - - 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; - } + GetDevice()->HandleError("Push constants aren't supported."); } // 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; diff --git a/src/dawn_native/ShaderModule.h b/src/dawn_native/ShaderModule.h index ab00c273fb..035c2d71a4 100644 --- a/src/dawn_native/ShaderModule.h +++ b/src/dawn_native/ShaderModule.h @@ -46,14 +46,6 @@ namespace dawn_native { void ExtractSpirvInfo(const spirv_cross::Compiler& compiler); - struct PushConstantInfo { - std::bitset mask; - - std::array names; - std::array sizes; - std::array types; - }; - struct BindingInfo { // The SPIRV ID of the resource. uint32_t id; @@ -64,7 +56,6 @@ namespace dawn_native { using ModuleBindingInfo = std::array, kMaxBindGroups>; - const PushConstantInfo& GetPushConstants() const; const ModuleBindingInfo& GetBindingInfo() const; const std::bitset& GetUsedVertexAttributes() const; dawn::ShaderStage GetExecutionModel() const; @@ -89,7 +80,6 @@ namespace dawn_native { std::vector mCode; bool mIsBlueprint = false; - PushConstantInfo mPushConstants = {}; ModuleBindingInfo mBindingInfo; std::bitset mUsedVertexAttributes; dawn::ShaderStage mExecutionModel; diff --git a/src/dawn_native/metal/CommandBufferMTL.mm b/src/dawn_native/metal/CommandBufferMTL.mm index 2bff478f1b..a0a35491a3 100644 --- a/src/dawn_native/metal/CommandBufferMTL.mm +++ b/src/dawn_native/metal/CommandBufferMTL.mm @@ -628,15 +628,10 @@ namespace dawn_native { namespace metal { void CommandBuffer::EncodeComputePass(id commandBuffer) { ComputePipeline* lastPipeline = nullptr; - std::array pushConstants; // Will be autoreleased id 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(); - uint32_t* values = mCommands.NextData(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(); uint64_t* dynamicOffsets = nullptr; @@ -792,24 +774,10 @@ namespace dawn_native { namespace metal { id indexBuffer = nil; uint32_t indexBufferBaseOffset = 0; - std::array vertexPushConstants; - std::array fragmentPushConstants; - // This will be autoreleased id 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(); - uint32_t* values = mCommands.NextData(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(); [encoder setStencilReferenceValue:cmd->reference]; diff --git a/src/dawn_native/opengl/CommandBufferGL.cpp b/src/dawn_native/opengl/CommandBufferGL.cpp index 7973d44201..a1b5e547c4 100644 --- a/src/dawn_native/opengl/CommandBufferGL.cpp +++ b/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(&mValues[stage][constant])); - break; - case PushConstantType::UInt: - glUniform1ui(location, - *reinterpret_cast(&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> mValues; - PerStage> 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(); - 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(); lastPipeline = ToBackend(cmd->pipeline).Get(); - lastPipeline->ApplyNow(); - pushConstants.OnSetPipeline(lastPipeline); - } break; - - case Command::SetPushConstants: { - SetPushConstantsCmd* cmd = mCommands.NextCommand(); - uint32_t* data = mCommands.NextData(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(); - 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(); - 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(); - uint32_t* data = mCommands.NextData(cmd->count); - pushConstants.OnSetPushConstants(cmd->stages, cmd->count, cmd->offset, data); - } break; - case Command::SetStencilReference: { SetStencilReferenceCmd* cmd = mCommands.NextCommand(); persistentPipelineState.SetStencilReference(cmd->reference); diff --git a/src/dawn_native/opengl/PipelineGL.cpp b/src/dawn_native/opengl/PipelineGL.cpp index 11cb5b9dfd..6394ea1c43 100644 --- a/src/dawn_native/opengl/PipelineGL.cpp +++ b/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& PipelineGL::GetTextureUnitsForSampler(GLuint index) const { ASSERT(index < mUnitsForSamplers.size()); return mUnitsForSamplers[index]; diff --git a/src/dawn_native/opengl/PipelineGL.h b/src/dawn_native/opengl/PipelineGL.h index b3feeb96a5..892dc6aba1 100644 --- a/src/dawn_native/opengl/PipelineGL.h +++ b/src/dawn_native/opengl/PipelineGL.h @@ -34,11 +34,9 @@ namespace dawn_native { namespace opengl { void Initialize(const PipelineLayout* layout, const PerStage& modules); - using GLPushConstantInfo = std::array; using BindingLocations = std::array, kMaxBindGroups>; - const GLPushConstantInfo& GetGLPushConstants(dawn::ShaderStage stage) const; const std::vector& GetTextureUnitsForSampler(GLuint index) const; const std::vector& GetTextureUnitsForTextureView(GLuint index) const; GLuint GetProgramHandle() const; @@ -47,7 +45,6 @@ namespace dawn_native { namespace opengl { private: GLuint mProgram; - PerStage mGlPushConstants; std::vector> mUnitsForSamplers; std::vector> mUnitsForTextures; }; diff --git a/src/dawn_native/vulkan/PipelineLayoutVk.cpp b/src/dawn_native/vulkan/PipelineLayoutVk.cpp index 3c4ad1bc17..4b5615ef3b 100644 --- a/src/dawn_native/vulkan/PipelineLayoutVk.cpp +++ b/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.pPushConstantRanges = &pushConstantRange; + createInfo.pushConstantRangeCount = 0; + createInfo.pPushConstantRanges = nullptr; if (device->fn.CreatePipelineLayout(device->GetVkDevice(), &createInfo, nullptr, &mHandle) != VK_SUCCESS) { diff --git a/src/tests/end2end/PushConstantTests.cpp b/src/tests/end2end/PushConstantTests.cpp deleted file mode 100644 index 0bd371c366..0000000000 --- a/src/tests/end2end/PushConstantTests.cpp +++ /dev/null @@ -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 - -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; - - 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, ¬Zero); - - 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(&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 values; - for (uint32_t i = 0; i < kMaxPushConstants; ++i) { - spec.push_back({Int, static_cast(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); diff --git a/src/tests/unittests/validation/PushConstantsValidationTests.cpp b/src/tests/unittests/validation/PushConstantsValidationTests.cpp deleted file mode 100644 index 030deaef1e..0000000000 --- a/src/tests/unittests/validation/PushConstantsValidationTests.cpp +++ /dev/null @@ -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 - -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); - } - )"); -} diff --git a/src/tests/unittests/wire/WireArgumentTests.cpp b/src/tests/unittests/wire/WireArgumentTests.cpp index c63435a15d..49735f2e2c 100644 --- a/src/tests/unittests/wire/WireArgumentTests.cpp +++ b/src/tests/unittests/wire/WireArgumentTests.cpp @@ -16,6 +16,8 @@ #include "common/Constants.h" +#include + 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 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, - ComputePassEncoderSetPushConstants(apiPass, DAWN_SHADER_STAGE_BIT_VERTEX, 0, 4, - ResultOf(CheckPushConstantValues, Eq(true)))); + EXPECT_CALL(api, ComputePassEncoderSetBindGroup( + apiPass, 0, apiBindGroup, testOffsets.size(), + 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;