From 311e2a44b973709cffa53d62959ae461797989c6 Mon Sep 17 00:00:00 2001 From: Kai Ninomiya Date: Fri, 29 Jun 2018 18:22:02 -0700 Subject: [PATCH] Allow bind group layouts to be sparse in pipeline layout Instead of initializing all of the bind group layouts in the pipeline layout to default values, let them be nullptr and allow this elsewhere. Follow-up on some changes in #206 --- src/backend/PipelineLayout.cpp | 11 +---------- src/backend/PipelineLayout.h | 1 + src/backend/d3d12/PipelineLayoutD3D12.cpp | 3 ++- src/backend/metal/PipelineLayoutMTL.mm | 3 ++- src/backend/opengl/PipelineGL.cpp | 3 ++- src/backend/opengl/PipelineLayoutGL.cpp | 3 ++- 6 files changed, 10 insertions(+), 14 deletions(-) diff --git a/src/backend/PipelineLayout.cpp b/src/backend/PipelineLayout.cpp index d25c2acce0..e47855791c 100644 --- a/src/backend/PipelineLayout.cpp +++ b/src/backend/PipelineLayout.cpp @@ -34,7 +34,7 @@ namespace backend { // PipelineLayoutBase - PipelineLayoutBase::PipelineLayoutBase(DeviceBase* device, + PipelineLayoutBase::PipelineLayoutBase(DeviceBase*, const nxt::PipelineLayoutDescriptor* descriptor) { ASSERT(descriptor->numBindGroupLayouts <= kMaxBindGroups); for (uint32_t group = 0; group < descriptor->numBindGroupLayouts; ++group) { @@ -42,15 +42,6 @@ namespace backend { reinterpret_cast(descriptor->bindGroupLayouts[group].Get()); mMask.set(group); } - // TODO(kainino@chromium.org): It shouldn't be necessary to construct default bind - // group layouts here. Remove these and fix things so that they are not needed. - for (uint32_t group = descriptor->numBindGroupLayouts; group < kMaxBindGroups; ++group) { - auto builder = device->CreateBindGroupLayoutBuilder(); - mBindGroupLayouts[group] = builder->GetResult(); - // Remove the external ref objects are created with - mBindGroupLayouts[group]->Release(); - builder->Release(); - } } const BindGroupLayoutBase* PipelineLayoutBase::GetBindGroupLayout(size_t group) const { diff --git a/src/backend/PipelineLayout.h b/src/backend/PipelineLayout.h index 262dafb502..e11629614e 100644 --- a/src/backend/PipelineLayout.h +++ b/src/backend/PipelineLayout.h @@ -37,6 +37,7 @@ namespace backend { PipelineLayoutBase(DeviceBase* device, const nxt::PipelineLayoutDescriptor* descriptor); const BindGroupLayoutBase* GetBindGroupLayout(size_t group) const; + // XXX: rename to GetBindGroupLayoutsMask? const std::bitset GetBindGroupsLayoutMask() const; // Utility functions to compute inherited bind groups. diff --git a/src/backend/d3d12/PipelineLayoutD3D12.cpp b/src/backend/d3d12/PipelineLayoutD3D12.cpp index 2b6674f972..3ee78ed2b0 100644 --- a/src/backend/d3d12/PipelineLayoutD3D12.cpp +++ b/src/backend/d3d12/PipelineLayoutD3D12.cpp @@ -17,6 +17,7 @@ #include "backend/d3d12/BindGroupLayoutD3D12.h" #include "backend/d3d12/DeviceD3D12.h" #include "common/Assert.h" +#include "common/BitSetIterator.h" using Microsoft::WRL::ComPtr; @@ -42,7 +43,7 @@ namespace backend { namespace d3d12 { uint32_t parameterIndex = 0; uint32_t rangeIndex = 0; - for (uint32_t group = 0; group < kMaxBindGroups; ++group) { + for (uint32_t group : IterateBitSet(GetBindGroupsLayoutMask())) { const BindGroupLayout* bindGroupLayout = ToBackend(GetBindGroupLayout(group)); // Set the root descriptor table parameter and copy ranges. Ranges are offset by the diff --git a/src/backend/metal/PipelineLayoutMTL.mm b/src/backend/metal/PipelineLayoutMTL.mm index 0a85d4f381..49be220ba2 100644 --- a/src/backend/metal/PipelineLayoutMTL.mm +++ b/src/backend/metal/PipelineLayoutMTL.mm @@ -16,6 +16,7 @@ #include "backend/BindGroupLayout.h" #include "backend/metal/DeviceMTL.h" +#include "common/BitSetIterator.h" namespace backend { namespace metal { @@ -28,7 +29,7 @@ namespace backend { namespace metal { uint32_t samplerIndex = 0; uint32_t textureIndex = 0; - for (size_t group = 0; group < kMaxBindGroups; ++group) { + for (uint32_t group : IterateBitSet(GetBindGroupsLayoutMask())) { const auto& groupInfo = GetBindGroupLayout(group)->GetBindingInfo(); for (size_t binding = 0; binding < kMaxBindingsPerGroup; ++binding) { if (!(groupInfo.visibilities[binding] & StageBit(stage))) { diff --git a/src/backend/opengl/PipelineGL.cpp b/src/backend/opengl/PipelineGL.cpp index a71702b7dd..491e1b3521 100644 --- a/src/backend/opengl/PipelineGL.cpp +++ b/src/backend/opengl/PipelineGL.cpp @@ -19,6 +19,7 @@ #include "backend/opengl/PersistentPipelineStateGL.h" #include "backend/opengl/PipelineLayoutGL.h" #include "backend/opengl/ShaderModuleGL.h" +#include "common/BitSetIterator.h" #include #include @@ -125,7 +126,7 @@ namespace backend { namespace opengl { const auto& layout = ToBackend(parent->GetLayout()); const auto& indices = layout->GetBindingIndexInfo(); - for (uint32_t group = 0; group < kMaxBindGroups; ++group) { + for (uint32_t group : IterateBitSet(layout->GetBindGroupsLayoutMask())) { const auto& groupInfo = layout->GetBindGroupLayout(group)->GetBindingInfo(); for (uint32_t binding = 0; binding < kMaxBindingsPerGroup; ++binding) { diff --git a/src/backend/opengl/PipelineLayoutGL.cpp b/src/backend/opengl/PipelineLayoutGL.cpp index 44a08d89b0..77942d6690 100644 --- a/src/backend/opengl/PipelineLayoutGL.cpp +++ b/src/backend/opengl/PipelineLayoutGL.cpp @@ -16,6 +16,7 @@ #include "backend/BindGroupLayout.h" #include "backend/opengl/DeviceGL.h" +#include "common/BitSetIterator.h" namespace backend { namespace opengl { @@ -26,7 +27,7 @@ namespace backend { namespace opengl { GLuint sampledTextureIndex = 0; GLuint ssboIndex = 0; - for (size_t group = 0; group < kMaxBindGroups; ++group) { + for (uint32_t group : IterateBitSet(GetBindGroupsLayoutMask())) { const auto& groupInfo = GetBindGroupLayout(group)->GetBindingInfo(); for (size_t binding = 0; binding < kMaxBindingsPerGroup; ++binding) {