diff --git a/docs/amber_script.md b/docs/amber_script.md index e99093114..51d260ccb 100644 --- a/docs/amber_script.md +++ b/docs/amber_script.md @@ -195,7 +195,6 @@ The following commands are all specified within the `PIPELINE` command. ### Pipeline Buffers #### Buffer Types - * `push_constant` * `uniform` * `storage` @@ -217,6 +216,10 @@ attachment content, depth/stencil content, uniform buffers, etc. # pipelines. BIND BUFFER {buffer_name} AS depth_stencil + # Attach |buffer_name| as the push_constant buffer. There can be only one + # push constant buffer attached to a pipeline. + BIND BUFFER AS push_constant + # Bind the buffer of the given |buffer_type| at the given descriptor set # and binding. The buffer will use a start index of 0. BIND BUFFER {buffer_name} AS {buffer_type} DESCRIPTOR_SET _id_ \ diff --git a/src/amberscript/parser.cc b/src/amberscript/parser.cc index 6d6dcba31..e11252aff 100644 --- a/src/amberscript/parser.cc +++ b/src/amberscript/parser.cc @@ -542,9 +542,7 @@ Result Parser::ParsePipelineFramebufferSize(Pipeline* pipeline) { Result Parser::ToBufferType(const std::string& name, BufferType* type) { assert(type); - if (name == "push_constant") - *type = BufferType::kPushConstant; - else if (name == "uniform") + if (name == "uniform") *type = BufferType::kUniform; else if (name == "storage") *type = BufferType::kStorage; @@ -596,6 +594,11 @@ Result Parser::ParsePipelineBind(Pipeline* pipeline) { Result r = pipeline->SetDepthBuffer(buffer); if (!r.IsSuccess()) return r; + } else if (token->AsString() == "push_constant") { + buffer->SetBufferType(BufferType::kPushConstant); + Result r = pipeline->SetPushConstantBuffer(buffer); + if (!r.IsSuccess()) + return r; } else { BufferType type = BufferType::kColor; Result r = ToBufferType(token->AsString(), &type); @@ -1130,15 +1133,22 @@ Result Parser::ParseExpect() { token->AsString()); } - if (buffer->GetBufferType() != buffer_2->GetBufferType()) + if (!buffer->GetFormat()->Equal(buffer_2->GetFormat())) { return Result( - "EXPECT EQ_BUFFER command cannot compare buffers of different type"); - if (buffer->ElementCount() != buffer_2->ElementCount()) + "EXPECT EQ_BUFFER command cannot compare buffers of differing " + "format"); + } + if (buffer->ElementCount() != buffer_2->ElementCount()) { return Result( - "EXPECT EQ_BUFFER command cannot compare buffers of different size"); - if (buffer->GetWidth() != buffer_2->GetWidth()) + "EXPECT EQ_BUFFER command cannot compare buffers of different " + "size: " + + std::to_string(buffer->ElementCount()) + " vs " + + std::to_string(buffer_2->ElementCount())); + } + if (buffer->GetWidth() != buffer_2->GetWidth()) { return Result( "EXPECT EQ_BUFFER command cannot compare buffers of different width"); + } if (buffer->GetHeight() != buffer_2->GetHeight()) { return Result( "EXPECT EQ_BUFFER command cannot compare buffers of different " diff --git a/src/amberscript/parser_bind_test.cc b/src/amberscript/parser_bind_test.cc index f1754e638..5207c8d10 100644 --- a/src/amberscript/parser_bind_test.cc +++ b/src/amberscript/parser_bind_test.cc @@ -1092,11 +1092,62 @@ PIPELINE graphics my_pipeline INSTANTIATE_TEST_CASE_P( AmberScriptParserBufferTypeTest, AmberScriptParserBufferTypeTest, - testing::Values(BufferTypeData{"push_constant", BufferType::kPushConstant}, - BufferTypeData{"uniform", BufferType::kUniform}, + testing::Values(BufferTypeData{"uniform", BufferType::kUniform}, BufferTypeData{ "storage", BufferType::kStorage}), ); // NOLINT(whitespace/parens) +TEST_F(AmberScriptParserTest, BindPushConstants) { + std::string in = R"( +SHADER vertex my_shader PASSTHROUGH +SHADER fragment my_fragment GLSL +# GLSL Shader +END +BUFFER my_buf DATA_TYPE float SIZE 20 FILL 5 + +PIPELINE graphics my_pipeline + ATTACH my_shader + ATTACH my_fragment + + BIND BUFFER my_buf AS push_constant +END)"; + + Parser parser; + Result r = parser.Parse(in); + ASSERT_TRUE(r.IsSuccess()) << r.Error(); + + auto script = parser.GetScript(); + const auto& pipelines = script->GetPipelines(); + ASSERT_EQ(1U, pipelines.size()); + + const auto* pipeline = pipelines[0].get(); + const auto& buf = pipeline->GetPushConstantBuffer(); + ASSERT_TRUE(buf.buffer != nullptr); + EXPECT_EQ(20, buf.buffer->ElementCount()); + EXPECT_EQ(20, buf.buffer->ValueCount()); + EXPECT_EQ(20 * sizeof(float), buf.buffer->GetSizeInBytes()); +} + +TEST_F(AmberScriptParserTest, BindPushConstantsExtraParams) { + std::string in = R"( +SHADER vertex my_shader PASSTHROUGH +SHADER fragment my_fragment GLSL +# GLSL Shader +END +BUFFER my_buf DATA_TYPE float SIZE 20 FILL 5 + +PIPELINE graphics my_pipeline + ATTACH my_shader + ATTACH my_fragment + + BIND BUFFER my_buf AS push_constant EXTRA +END)"; + + Parser parser; + Result r = parser.Parse(in); + ASSERT_FALSE(r.IsSuccess()); + EXPECT_EQ("12: extra parameters after BIND command", r.Error()); +} + } // namespace amberscript } // namespace amber diff --git a/src/amberscript/parser_expect_test.cc b/src/amberscript/parser_expect_test.cc index e47eb4b85..251b9ab8e 100644 --- a/src/amberscript/parser_expect_test.cc +++ b/src/amberscript/parser_expect_test.cc @@ -795,7 +795,8 @@ EXPECT buf_1 EQ_BUFFER buf_2 Result r = parser.Parse(in); ASSERT_FALSE(r.IsSuccess()); EXPECT_EQ( - "5: EXPECT EQ_BUFFER command cannot compare buffers of different size", + "5: EXPECT EQ_BUFFER command cannot compare buffers of different size: " + "10 vs 99", r.Error()); } @@ -811,7 +812,7 @@ EXPECT buf_1 EQ_BUFFER buf_2 Result r = parser.Parse(in); ASSERT_FALSE(r.IsSuccess()); EXPECT_EQ( - "5: EXPECT EQ_BUFFER command cannot compare buffers of different size", + "5: EXPECT EQ_BUFFER command cannot compare buffers of differing format", r.Error()); } diff --git a/src/buffer.cc b/src/buffer.cc index eaa016355..0495c4c67 100644 --- a/src/buffer.cc +++ b/src/buffer.cc @@ -15,6 +15,7 @@ #include "src/buffer.h" #include +#include namespace amber { namespace { @@ -72,8 +73,8 @@ Result Buffer::CopyTo(Buffer* buffer) const { } Result Buffer::IsEqual(Buffer* buffer) const { - if (buffer->buffer_type_ != buffer_type_) - return Result{"Buffers have a different type"}; + if (!buffer->format_->Equal(format_.get())) + return Result{"Buffers have a different format"}; if (buffer->element_count_ != element_count_) return Result{"Buffers have a different size"}; if (buffer->width_ != width_) @@ -215,4 +216,14 @@ void Buffer::ResizeTo(uint32_t element_count) { bytes_.resize(element_count * format_->SizeInBytes()); } +Result Buffer::SetDataFromBuffer(const Buffer* src, uint32_t offset) { + if (bytes_.size() < offset + src->bytes_.size()) + bytes_.resize(offset + src->bytes_.size()); + + std::memcpy(bytes_.data() + offset, src->bytes_.data(), src->bytes_.size()); + element_count_ = + static_cast(bytes_.size()) / format_->SizeInBytes(); + return {}; +} + } // namespace amber diff --git a/src/buffer.h b/src/buffer.h index f3af69f28..db6907ed4 100644 --- a/src/buffer.h +++ b/src/buffer.h @@ -76,7 +76,7 @@ class Buffer { format_ = std::move(format); } /// Returns the Format describing the buffer data. - Format* GetFormat() { return format_.get(); } + Format* GetFormat() const { return format_.get(); } /// Sets the buffer |name|. void SetName(const std::string& name) { name_ = name; } @@ -155,6 +155,9 @@ class Buffer { /// |size_in_bytes| of data. Result SetDataWithOffset(const std::vector& data, uint32_t offset); + /// Writes |src| data into buffer at |offset|. + Result SetDataFromBuffer(const Buffer* src, uint32_t offset); + /// Returns a pointer to the internal storage of the buffer. std::vector* ValuePtr() { return &bytes_; } /// Returns a pointer to the internal storage of the buffer. diff --git a/src/pipeline.cc b/src/pipeline.cc index 8f2dc053d..e5052690c 100644 --- a/src/pipeline.cc +++ b/src/pipeline.cc @@ -278,6 +278,16 @@ Result Pipeline::AddVertexBuffer(Buffer* buf, uint32_t location) { return {}; } +Result Pipeline::SetPushConstantBuffer(Buffer* buf) { + if (push_constant_buffer_.buffer != nullptr) + return Result("can only bind one push constant buffer in a PIPELINE"); + if (buf->GetBufferType() != BufferType::kPushConstant) + return Result("expected a push constant buffer"); + + push_constant_buffer_.buffer = buf; + return {}; +} + std::unique_ptr Pipeline::GenerateDefaultColorAttachmentBuffer() const { FormatParser fp; diff --git a/src/pipeline.h b/src/pipeline.h index b6fecee76..554494ce4 100644 --- a/src/pipeline.h +++ b/src/pipeline.h @@ -163,6 +163,11 @@ class Pipeline { /// |descriptor_set| and |binding|. Buffer* GetBufferForBinding(uint32_t descriptor_set, uint32_t binding) const; + Result SetPushConstantBuffer(Buffer* buf); + const BufferInfo& GetPushConstantBuffer() const { + return push_constant_buffer_; + } + /// Validates that the pipeline has been created correctly. Result Validate() const; @@ -184,6 +189,7 @@ class Pipeline { std::vector vertex_buffers_; std::vector buffers_; BufferInfo depth_buffer_; + BufferInfo push_constant_buffer_; Buffer* index_buffer_ = nullptr; uint32_t fb_width_ = 250; diff --git a/src/vulkan/engine_vulkan.cc b/src/vulkan/engine_vulkan.cc index f0ccaf2f9..f6757dd35 100644 --- a/src/vulkan/engine_vulkan.cc +++ b/src/vulkan/engine_vulkan.cc @@ -206,6 +206,13 @@ Result EngineVulkan::CreatePipeline(amber::Pipeline* pipeline) { info.vk_pipeline->AsGraphics()->SetIndexBuffer(buf); } + if (pipeline->GetPushConstantBuffer().buffer != nullptr) { + r = info.vk_pipeline->AddPushConstantBuffer( + pipeline->GetPushConstantBuffer().buffer); + if (!r.IsSuccess()) + return r; + } + for (const auto& buf_info : pipeline->GetBuffers()) { auto type = BufferCommand::BufferType::kSSBO; if (buf_info.buffer->GetBufferType() == BufferType::kUniform) { diff --git a/src/vulkan/pipeline.cc b/src/vulkan/pipeline.cc index c7f5e0f98..4a0f98bf8 100644 --- a/src/vulkan/pipeline.cc +++ b/src/vulkan/pipeline.cc @@ -187,7 +187,7 @@ Result Pipeline::CreateVkPipelineLayout(VkPipelineLayout* pipeline_layout) { VkPushConstantRange push_const_range = push_constant_->GetVkPushConstantRange(); - if (push_const_range.size) { + if (push_const_range.size > 0) { pipeline_layout_info.pushConstantRangeCount = 1U; pipeline_layout_info.pPushConstantRanges = &push_const_range; } @@ -241,6 +241,10 @@ Result Pipeline::AddPushConstant(const BufferCommand* command) { return push_constant_->AddBufferData(command); } +Result Pipeline::AddPushConstantBuffer(const Buffer* buf) { + return push_constant_->AddBuffer(buf); +} + Result Pipeline::AddDescriptor(const BufferCommand* cmd) { if (cmd == nullptr) return Result("Pipeline::AddDescriptor BufferCommand is nullptr"); diff --git a/src/vulkan/pipeline.h b/src/vulkan/pipeline.h index 957ff883d..5969a9423 100644 --- a/src/vulkan/pipeline.h +++ b/src/vulkan/pipeline.h @@ -51,6 +51,9 @@ class Pipeline { Result AddDescriptor(const BufferCommand*); + /// Add buffer data to the push constants. + Result AddPushConstantBuffer(const Buffer* buf); + /// Reads back the contents of resources of all descriptors to a /// buffer data object and put it into buffer data queue in host. Result ReadbackDescriptorsToHostDataQueue(); diff --git a/src/vulkan/push_constant.cc b/src/vulkan/push_constant.cc index 8bdc46bdb..3892e9383 100644 --- a/src/vulkan/push_constant.cc +++ b/src/vulkan/push_constant.cc @@ -101,10 +101,20 @@ Result PushConstant::RecordPushConstantVkCommand( return {}; } +Result PushConstant::AddBuffer(const Buffer* buffer) { + push_constant_data_.emplace_back(); + push_constant_data_.back().offset = 0; + push_constant_data_.back().buffer = buffer; + push_constant_data_.back().format = buffer->GetFormat(); + push_constant_data_.back().size_in_bytes = buffer->GetSizeInBytes(); + return {}; +} + Result PushConstant::AddBufferData(const BufferCommand* command) { - if (!command->IsPushConstant()) + if (!command->IsPushConstant()) { return Result( "PushConstant::AddBufferData BufferCommand type is not push constant"); + } auto* fmt = command->GetBuffer()->GetFormat(); push_constant_data_.emplace_back(); @@ -134,7 +144,12 @@ Result PushConstant::UpdateMemoryWithInput(const BufferInput& input) { return Result("Vulkan: push constants must all have the same format"); buffer_->SetFormat(MakeUnique(*input.format)); - buffer_->SetDataWithOffset(input.values, input.offset); + + if (input.buffer) + buffer_->SetDataFromBuffer(input.buffer, input.offset); + else + buffer_->SetDataWithOffset(input.values, input.offset); + return {}; } diff --git a/src/vulkan/push_constant.h b/src/vulkan/push_constant.h index 33ba2c621..2dd7ff4f0 100644 --- a/src/vulkan/push_constant.h +++ b/src/vulkan/push_constant.h @@ -41,17 +41,25 @@ class PushConstant { Result RecordPushConstantVkCommand(CommandBuffer* command, VkPipelineLayout pipeline_layout); + /// Add a set of values from the given |buffer| to the push constants + /// to be used on the next pipeline execution. + Result AddBuffer(const Buffer* buffer); + /// Adds data into the push constant buffer. Result AddBufferData(const BufferCommand* command); private: - // Contain information of filling memory - // [|offset|, |offset| + |size_in_bytes|) with |values| whose data - // type is |type|. This information is given by script. + // Contain information of filling memory. + // If the |buffer| is provided the buffer memory will be copied into the + // result buffer at |offset|. If |buffer| is not provided then + // |size_in_bytes|, |format| and |values| must be provided and will be copied + // into the result buffer at |offset|. struct BufferInput { - uint32_t offset; - uint32_t size_in_bytes; - Format* format; + uint32_t offset = 0; + const Buffer* buffer = nullptr; + + uint32_t size_in_bytes = 0; + Format* format = nullptr; std::vector values; }; diff --git a/tests/cases/compute_push_constant_and_ssbo.amber b/tests/cases/compute_push_constant_and_ssbo.amber new file mode 100644 index 000000000..45fc79178 --- /dev/null +++ b/tests/cases/compute_push_constant_and_ssbo.amber @@ -0,0 +1,71 @@ +#!amber +# Copyright 2019 The Amber 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 +# +# https://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. + +SHADER compute compute_shader GLSL +#version 430 + +layout(set = 0, binding = 0) buffer block0 { + uint data_set0_binding0[]; +}; + +// push_constant compiled as std430 by glslang +layout(push_constant) uniform block1 { + uint constant0[3]; // Offset: 0, array stride: 4 + uint constant1; // 12 + uvec3 constant2[3]; // 16, array stride: 16 + uint constant3; // 64 +}; + +void main() { + int offset = 0; + for (int i = 0; i < 3; ++i) + data_set0_binding0[offset++] = constant0[i]; + + data_set0_binding0[offset++] = constant1; + + for (int i = 0; i < 3; ++i) { + for (int j = 0; j < 3; ++j) + data_set0_binding0[offset++] = constant2[i][j]; + } + + data_set0_binding0[offset++] = constant3; +} +END + +BUFFER buf0 DATA_TYPE uint32 SIZE 14 FILL 0 +BUFFER const_buf DATA_TYPE uint32 DATA + 1 2 3 4 # constant0[3] + constant1 + 5 6 7 0 # constant2[0] + 8 9 10 0 # constant2[1] +11 12 13 0 # constant2[2] +14 # constant3 +END + +PIPELINE compute pipeline + ATTACH compute_shader + + BIND BUFFER buf0 AS storage DESCRIPTOR_SET 0 BINDING 0 + BIND BUFFER const_buf AS push_constant +END + +RUN pipeline 3 1 1 + +BUFFER result DATA_TYPE uint32 DATA + 1 2 3 4 + 5 6 7 8 + 9 10 11 12 +13 14 +END +EXPECT buf0 EQ_BUFFER result