diff options
author | asuonpaa <34128694+asuonpaa@users.noreply.github.com> | 2020-11-10 12:41:12 +0200 |
---|---|---|
committer | GitHub <noreply@github.com> | 2020-11-10 10:41:12 +0000 |
commit | 4d6fcf03895737bea3aacffde22cd983f1c8c492 (patch) | |
tree | 661d50ceb1959445157dca5e454b400b2979c03a | |
parent | 9ef2248b535fafabf6bc18e01e2e8808e2d6e6db (diff) | |
download | amber-4d6fcf03895737bea3aacffde22cd983f1c8c492.tar.gz |
Skip data readback for read-only descriptors (#921)
This works as an optimization but also fixes an issue where binding a single buffer multiple times (to read-only descriptors) in a pipeline would fail because Amber would try to copy back to the Amber buffer multiple times.
-rw-r--r-- | src/command.h | 4 | ||||
-rw-r--r-- | src/pipeline.cc | 1 | ||||
-rw-r--r-- | src/pipeline.h | 1 | ||||
-rw-r--r-- | src/vulkan/buffer_backed_descriptor.cc | 48 | ||||
-rw-r--r-- | src/vulkan/buffer_backed_descriptor.h | 5 | ||||
-rw-r--r-- | src/vulkan/buffer_descriptor.cc | 1 | ||||
-rw-r--r-- | src/vulkan/engine_vulkan.cc | 1 | ||||
-rw-r--r-- | src/vulkan/image_descriptor.cc | 13 | ||||
-rw-r--r-- | src/vulkan/pipeline.cc | 2 | ||||
-rw-r--r-- | tests/cases/multiple_samplers.amber | 126 |
10 files changed, 185 insertions, 17 deletions
diff --git a/src/command.h b/src/command.h index c6c7e08..207c336 100644 --- a/src/command.h +++ b/src/command.h @@ -552,10 +552,14 @@ class BufferCommand : public BindableResourceCommand { void SetBuffer(Buffer* buffer) { buffer_ = buffer; } Buffer* GetBuffer() const { return buffer_; } + void SetSampler(Sampler* sampler) { sampler_ = sampler; } + Sampler* GetSampler() const { return sampler_; } + std::string ToString() const override { return "BufferCommand"; } private: Buffer* buffer_ = nullptr; + Sampler* sampler_ = nullptr; BufferType buffer_type_; bool is_subdata_ = false; uint32_t offset_ = 0; diff --git a/src/pipeline.cc b/src/pipeline.cc index 3e5353a..beb09b1 100644 --- a/src/pipeline.cc +++ b/src/pipeline.cc @@ -521,6 +521,7 @@ void Pipeline::AddBuffer(Buffer* buf, info.type = type; info.base_mip_level = base_mip_level; info.dynamic_offset = dynamic_offset; + info.sampler = buf->GetSampler(); } void Pipeline::AddBuffer(Buffer* buf, diff --git a/src/pipeline.h b/src/pipeline.h index ddeb2a4..bdb1679 100644 --- a/src/pipeline.h +++ b/src/pipeline.h @@ -199,6 +199,7 @@ class Pipeline { uint32_t arg_no = 0; BufferType type = BufferType::kUnknown; InputRate input_rate = InputRate::kVertex; + Sampler* sampler; }; /// Information on a sampler attached to the pipeline. diff --git a/src/vulkan/buffer_backed_descriptor.cc b/src/vulkan/buffer_backed_descriptor.cc index 746195a..2f7aa98 100644 --- a/src/vulkan/buffer_backed_descriptor.cc +++ b/src/vulkan/buffer_backed_descriptor.cc @@ -45,7 +45,11 @@ Result BufferBackedDescriptor::RecordCopyDataToResourceIfNeeded( for (size_t i = 0; i < resources.size(); i++) { if (!buffers[i]->ValuePtr()->empty()) { resources[i]->UpdateMemoryWithRawData(*buffers[i]->ValuePtr()); - buffers[i]->ValuePtr()->clear(); + // If the resource is read-only, keep the buffer data; Amber won't copy + // read-only resources back into the host buffers, so it makes sense to + // leave the buffer intact. + if (!IsReadOnly()) + buffers[i]->ValuePtr()->clear(); } resources[i]->CopyToDevice(command); @@ -55,19 +59,25 @@ Result BufferBackedDescriptor::RecordCopyDataToResourceIfNeeded( } Result BufferBackedDescriptor::RecordCopyDataToHost(CommandBuffer* command) { - if (GetResources().empty()) { - return Result( - "Vulkan: BufferBackedDescriptor::RecordCopyDataToHost() no transfer " - "resources"); - } + if (!IsReadOnly()) { + if (GetResources().empty()) { + return Result( + "Vulkan: BufferBackedDescriptor::RecordCopyDataToHost() no transfer " + "resources"); + } - for (const auto& r : GetResources()) - r->CopyToHost(command); + for (const auto& r : GetResources()) + r->CopyToHost(command); + } return {}; } Result BufferBackedDescriptor::MoveResourceToBufferOutput() { + // No need to copy results of read only resources. + if (IsReadOnly()) + return {}; + auto resources = GetResources(); auto buffers = GetAmberBuffers(); if (resources.size() != buffers.size()) @@ -78,8 +88,7 @@ Result BufferBackedDescriptor::MoveResourceToBufferOutput() { if (resources.empty()) { return Result( "Vulkan: BufferBackedDescriptor::MoveResourceToBufferOutput() no " - "transfer" - " resource"); + "transfer resource"); } for (size_t i = 0; i < resources.size(); i++) { @@ -107,5 +116,24 @@ Result BufferBackedDescriptor::MoveResourceToBufferOutput() { return {}; } +bool BufferBackedDescriptor::IsReadOnly() const { + switch (type_) { + case DescriptorType::kUniformBuffer: + case DescriptorType::kUniformBufferDynamic: + case DescriptorType::kUniformTexelBuffer: + case DescriptorType::kSampledImage: + case DescriptorType::kCombinedImageSampler: + return true; + case DescriptorType::kStorageBuffer: + case DescriptorType::kStorageBufferDynamic: + case DescriptorType::kStorageTexelBuffer: + case DescriptorType::kStorageImage: + return false; + default: + assert(false && "Unexpected descriptor type"); + return false; + } +} + } // namespace vulkan } // namespace amber diff --git a/src/vulkan/buffer_backed_descriptor.h b/src/vulkan/buffer_backed_descriptor.h index 788bf4e..c626dd8 100644 --- a/src/vulkan/buffer_backed_descriptor.h +++ b/src/vulkan/buffer_backed_descriptor.h @@ -42,13 +42,16 @@ class BufferBackedDescriptor : public Descriptor { Result RecordCopyDataToResourceIfNeeded(CommandBuffer* command) override; Result RecordCopyDataToHost(CommandBuffer* command) override; Result MoveResourceToBufferOutput() override; - virtual std::vector<Resource*> GetResources() = 0; uint32_t GetDescriptorCount() override { return static_cast<uint32_t>(amber_buffers_.size()); } const std::vector<Buffer*>& GetAmberBuffers() const { return amber_buffers_; } void AddAmberBuffer(Buffer* buffer) { amber_buffers_.push_back(buffer); } BufferBackedDescriptor* AsBufferBackedDescriptor() override { return this; } + bool IsReadOnly() const; + + protected: + virtual std::vector<Resource*> GetResources() = 0; private: std::vector<Buffer*> amber_buffers_; diff --git a/src/vulkan/buffer_descriptor.cc b/src/vulkan/buffer_descriptor.cc index 001c45e..510565d 100644 --- a/src/vulkan/buffer_descriptor.cc +++ b/src/vulkan/buffer_descriptor.cc @@ -75,6 +75,7 @@ Result BufferDescriptor::CreateResourceIfNeeded() { Result BufferDescriptor::MoveResourceToBufferOutput() { Result r = BufferBackedDescriptor::MoveResourceToBufferOutput(); + transfer_buffers_.clear(); return r; diff --git a/src/vulkan/engine_vulkan.cc b/src/vulkan/engine_vulkan.cc index 0e65298..035c269 100644 --- a/src/vulkan/engine_vulkan.cc +++ b/src/vulkan/engine_vulkan.cc @@ -253,6 +253,7 @@ Result EngineVulkan::CreatePipeline(amber::Pipeline* pipeline) { cmd->SetBaseMipLevel(buf_info.base_mip_level); cmd->SetDynamicOffset(buf_info.dynamic_offset); cmd->SetBuffer(buf_info.buffer); + cmd->SetSampler(buf_info.sampler); if (cmd->GetValues().empty()) { cmd->GetBuffer()->SetSizeInElements(cmd->GetBuffer()->ElementCount()); diff --git a/src/vulkan/image_descriptor.cc b/src/vulkan/image_descriptor.cc index e1e9535..1305a52 100644 --- a/src/vulkan/image_descriptor.cc +++ b/src/vulkan/image_descriptor.cc @@ -125,18 +125,21 @@ Result ImageDescriptor::CreateResourceIfNeeded() { } Result ImageDescriptor::RecordCopyDataToHost(CommandBuffer* command) { - for (auto& image : transfer_images_) { - image->ImageBarrier(command, VK_IMAGE_LAYOUT_TRANSFER_SRC_OPTIMAL, - VK_PIPELINE_STAGE_TRANSFER_BIT); - } + if (!IsReadOnly()) { + for (auto& image : transfer_images_) { + image->ImageBarrier(command, VK_IMAGE_LAYOUT_TRANSFER_SRC_OPTIMAL, + VK_PIPELINE_STAGE_TRANSFER_BIT); + } - BufferBackedDescriptor::RecordCopyDataToHost(command); + BufferBackedDescriptor::RecordCopyDataToHost(command); + } return {}; } Result ImageDescriptor::MoveResourceToBufferOutput() { Result r = BufferBackedDescriptor::MoveResourceToBufferOutput(); + transfer_images_.clear(); return r; diff --git a/src/vulkan/pipeline.cc b/src/vulkan/pipeline.cc index 0bd3e36..4f4075b 100644 --- a/src/vulkan/pipeline.cc +++ b/src/vulkan/pipeline.cc @@ -321,7 +321,7 @@ Result Pipeline::AddBufferDescriptor(const BufferCommand* cmd) { cmd->GetBuffer(), desc_type, device_, cmd->GetBaseMipLevel(), cmd->GetDescriptorSet(), cmd->GetBinding()); if (cmd->IsSampledImage() || cmd->IsCombinedImageSampler()) - image_desc->SetAmberSampler(cmd->GetBuffer()->GetSampler()); + image_desc->SetAmberSampler(cmd->GetSampler()); descriptors.push_back(std::move(image_desc)); } else { auto buffer_desc = MakeUnique<BufferDescriptor>( diff --git a/tests/cases/multiple_samplers.amber b/tests/cases/multiple_samplers.amber new file mode 100644 index 0000000..dd37e8e --- /dev/null +++ b/tests/cases/multiple_samplers.amber @@ -0,0 +1,126 @@ +#!amber +# Copyright 2020 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 vertex vert_shader PASSTHROUGH + +SHADER fragment frag_shader_red GLSL +#version 430 +layout(location = 0) out vec4 color_out; +void main() { + color_out = vec4(1.0, 0.0, 0.0, 1.0); +} +END + +SHADER vertex vert_shader_tex GLSL +#version 430 +layout(location = 0) in vec4 position; +layout(location = 1) in vec2 texcoords_in; +layout(location = 0) out vec2 texcoords_out; +void main() { + gl_Position = position; + texcoords_out = texcoords_in; +} +END + +SHADER fragment frag_shader_tex GLSL +#version 430 +layout(location = 0) in vec2 texcoords_in; +layout(location = 0) out vec4 color_out; +uniform layout(set=0, binding=0) sampler2D tex_sampler_white; +uniform layout(set=0, binding=1) sampler2D tex_sampler_black; +void main() { + // Use a sampler with a white border color for the right + // side of the color buffer and a sampler with a black + // border color for the left side. + if (gl_FragCoord.x > 128.0) + color_out = texture(tex_sampler_white, texcoords_in); + else + color_out = texture(tex_sampler_black, texcoords_in); +} +END + +BUFFER texture FORMAT R8G8B8A8_UNORM +BUFFER framebuffer FORMAT B8G8R8A8_UNORM + +# A sampler that generates white when texture coordinates +# go out of range [0..1]. +SAMPLER sampler_white \ + ADDRESS_MODE_U clamp_to_border \ + ADDRESS_MODE_V clamp_to_border \ + BORDER_COLOR float_opaque_white + +# A sampler that generates black when texture coordinates +# go out of range [0..1]. +SAMPLER sampler_black \ + ADDRESS_MODE_U clamp_to_border \ + ADDRESS_MODE_V clamp_to_border \ + BORDER_COLOR float_opaque_black + +BUFFER position DATA_TYPE vec2<float> DATA +-0.75 -0.75 + 0.75 -0.75 + 0.75 0.75 +-0.75 0.75 +END +BUFFER texcoords DATA_TYPE vec2<float> DATA +-0.5 -0.5 + 1.5 -0.5 + 1.5 1.5 +-0.5 1.5 +END + +# A pipeline for generating a texture. Renders pure red. +PIPELINE graphics texgen + ATTACH vert_shader + ATTACH frag_shader_red + FRAMEBUFFER_SIZE 256 256 + BIND BUFFER texture AS color LOCATION 0 +END + +# A pipeline for drawing a textured quad using two samplers. +PIPELINE graphics pipeline + ATTACH vert_shader_tex + ATTACH frag_shader_tex + BIND BUFFER texture AS combined_image_sampler SAMPLER sampler_white DESCRIPTOR_SET 0 BINDING 0 + BIND BUFFER texture AS combined_image_sampler SAMPLER sampler_black DESCRIPTOR_SET 0 BINDING 1 + VERTEX_DATA position LOCATION 0 + VERTEX_DATA texcoords LOCATION 1 + FRAMEBUFFER_SIZE 256 256 + BIND BUFFER framebuffer AS color LOCATION 0 +END + +# Generate a texture with a blue background and red quad at the lower right corner. +CLEAR_COLOR texgen 0 0 255 255 +CLEAR texgen +RUN texgen DRAW_RECT POS 128 128 SIZE 128 128 + +# Clear to green and draw the generated texture in the center of the screen leaving +# the background color to the edges. Use texture coordinates that go past the [0..1] +# range to show border colors. The left side of the texture is using a sampler with +# a black border, and the right side uses a sampler with a white border. +CLEAR_COLOR pipeline 0 255 0 255 +CLEAR pipeline +RUN pipeline DRAW_ARRAY AS TRIANGLE_FAN START_IDX 0 COUNT 4 + +# Check for the green background. +EXPECT framebuffer IDX 1 1 SIZE 1 1 EQ_RGBA 0 255 0 255 +# Check for the black border color. +EXPECT framebuffer IDX 55 55 SIZE 1 1 EQ_RGBA 0 0 0 255 +# Check for the blue part of the texture. +EXPECT framebuffer IDX 105 105 SIZE 1 1 EQ_RGBA 0 0 255 255 +# Check for the red part of the texture. +EXPECT framebuffer IDX 150 150 SIZE 1 1 EQ_RGBA 255 0 0 255 +# Check for the white border color. +EXPECT framebuffer IDX 200 200 SIZE 1 1 EQ_RGBA 255 255 255 255 |