Merge BindGroupLayout and ShaderModule BindingInfos

This moves BindGroupLayoutBase::BindingInfo into the dawn_native
namespace and changes ShaderModule::BindingInfo to extend it with
SPIR-V ids.

Bug: dawn:354
Change-Id: I6a2187e94c0200bee729cf8290f74e4f8c648334
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/17920
Commit-Queue: Austin Eng <enga@chromium.org>
Reviewed-by: Kai Ninomiya <kainino@chromium.org>

BUILD.gn

@@ -175,6 +175,7 @@ source_set("libdawn_native_sources") {
     "src/dawn_native/BindGroupLayout.cpp",
     "src/dawn_native/BindGroupLayout.h",
     "src/dawn_native/BindGroupTracker.h",
+    "src/dawn_native/BindingInfo.h",
     "src/dawn_native/BuddyAllocator.cpp",
     "src/dawn_native/BuddyAllocator.h",
     "src/dawn_native/BuddyMemoryAllocator.cpp",

src/dawn_native/BindGroup.cpp

@@ -64,7 +64,7 @@ namespace dawn_native {
         MaybeError ValidateTextureBinding(const DeviceBase* device,
                                           const BindGroupBinding& binding,
                                           wgpu::TextureUsage requiredUsage,
-                                          const BindGroupLayoutBase::BindingInfo& bindingInfo) {
+                                          const BindingInfo& bindingInfo) {
             if (binding.textureView == nullptr || binding.sampler != nullptr ||
                 binding.buffer != nullptr) {
                 return DAWN_VALIDATION_ERROR("expected texture binding");
@@ -146,8 +146,7 @@ namespace dawn_native {
             }
             bindingsSet.set(bindingIndex);
 
-            const BindGroupLayoutBase::BindingInfo& bindingInfo =
+            const BindingInfo& bindingInfo = descriptor->layout->GetBindingInfo(bindingIndex);
-                descriptor->layout->GetBindingInfo(bindingIndex);
 
             // Perform binding-type specific validation.
             switch (bindingInfo.type) {

src/dawn_native/BindGroupAndStorageBarrierTracker.h

@@ -41,8 +41,7 @@ namespace dawn_native {
 
                 for (BindingIndex bindingIndex = 0; bindingIndex < layout->GetBindingCount();
                      ++bindingIndex) {
-                    const BindGroupLayoutBase::BindingInfo& bindingInfo =
+                    const BindingInfo& bindingInfo = layout->GetBindingInfo(bindingIndex);
-                        layout->GetBindingInfo(bindingIndex);
 
                     if ((bindingInfo.visibility & wgpu::ShaderStage::Compute) == 0) {
                         continue;

src/dawn_native/BindGroupLayout.cpp

@@ -171,14 +171,13 @@ namespace dawn_native {
 
     namespace {
 
-        void HashCombineBindingInfo(size_t* hash, const BindGroupLayoutBase::BindingInfo& info) {
+        void HashCombineBindingInfo(size_t* hash, const BindingInfo& info) {
             HashCombine(hash, info.hasDynamicOffset, info.multisampled, info.visibility, info.type,
                         info.textureComponentType, info.textureDimension,
                         info.storageTextureFormat);
         }
 
-        bool operator!=(const BindGroupLayoutBase::BindingInfo& a,
+        bool operator!=(const BindingInfo& a, const BindingInfo& b) {
-                        const BindGroupLayoutBase::BindingInfo& b) {
             return a.hasDynamicOffset != b.hasDynamicOffset ||          //
                    a.multisampled != b.multisampled ||                  //
                    a.visibility != b.visibility ||                      //
@@ -219,8 +218,7 @@ namespace dawn_native {
 
         // This is a utility function to help ASSERT that the BGL-binding comparator places buffers
         // first.
-        bool CheckBufferBindingsFirst(const BindGroupLayoutBase::BindingInfo* bindings,
+        bool CheckBufferBindingsFirst(const BindingInfo* bindings, BindingIndex count) {
-                                      BindingIndex count) {
             ASSERT(count <= kMaxBindingsPerGroup);
 
             BindingIndex lastBufferIndex = 0;
@@ -266,7 +264,8 @@ namespace dawn_native {
             const BindGroupLayoutBinding& binding = sortedBindings[i];
             mBindingInfo[i].type = binding.type;
             mBindingInfo[i].visibility = binding.visibility;
-            mBindingInfo[i].textureComponentType = binding.textureComponentType;
+            mBindingInfo[i].textureComponentType =
+                Format::TextureComponentTypeToFormatType(binding.textureComponentType);
             mBindingInfo[i].storageTextureFormat = binding.storageTextureFormat;
 
             switch (binding.type) {

src/dawn_native/BindGroupLayout.h

@@ -18,10 +18,10 @@
 #include "common/Constants.h"
 #include "common/Math.h"
 #include "common/SlabAllocator.h"
+#include "dawn_native/BindingInfo.h"
 #include "dawn_native/CachedObject.h"
 #include "dawn_native/Error.h"
 #include "dawn_native/Forward.h"
-#include "dawn_native/IntegerTypes.h"
 
 #include "dawn_native/dawn_platform.h"
 
@@ -51,15 +51,6 @@ namespace dawn_native {
         ~BindGroupLayoutBase() override;
 
         static BindGroupLayoutBase* MakeError(DeviceBase* device);
-        struct BindingInfo {
-            wgpu::ShaderStage visibility;
-            wgpu::BindingType type;
-            wgpu::TextureComponentType textureComponentType;
-            wgpu::TextureViewDimension textureDimension;
-            wgpu::TextureFormat storageTextureFormat;
-            bool hasDynamicOffset;
-            bool multisampled;
-        };
 
         // A map from the BindingNumber to its packed BindingIndex.
         using BindingMap = std::map<BindingNumber, BindingIndex>;

src/dawn_native/BindingInfo.h

@@ -12,8 +12,11 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-#ifndef DAWNNATIVE_INTEGERTYPES_H_
+#ifndef DAWNNATIVE_BINDINGINFO_H_
-#define DAWNNATIVE_INTEGERTYPES_H_
+#define DAWNNATIVE_BINDINGINFO_H_
+
+#include "dawn_native/Format.h"
+#include "dawn_native/dawn_platform.h"
 
 #include <cstdint>
 
@@ -28,6 +31,16 @@ namespace dawn_native {
     // Binding numbers get mapped to a packed range of indices
     using BindingIndex = uint32_t;
 
+    struct BindingInfo {
+        wgpu::ShaderStage visibility;
+        wgpu::BindingType type;
+        Format::Type textureComponentType = Format::Type::Float;
+        wgpu::TextureViewDimension textureDimension = wgpu::TextureViewDimension::Undefined;
+        wgpu::TextureFormat storageTextureFormat = wgpu::TextureFormat::Undefined;
+        bool hasDynamicOffset = false;
+        bool multisampled = false;
+    };
+
 }  // namespace dawn_native
 
-#endif  // DAWNNATIVE_INTEGERTYPES_H_
+#endif  // DAWNNATIVE_BINDINGINFO_H_

src/dawn_native/CMakeLists.txt

@@ -35,6 +35,7 @@ target_sources(dawn_native PRIVATE
     "BindGroupLayout.cpp"
     "BindGroupLayout.h"
     "BindGroupTracker.h"
+    "BindingInfo.h"
     "BuddyAllocator.cpp"
     "BuddyAllocator.h"
     "BuddyMemoryAllocator.cpp"

src/dawn_native/Format.cpp

@@ -73,13 +73,13 @@ namespace dawn_native {
         return aspect != Color;
     }
 
-    bool Format::HasComponentType(wgpu::TextureComponentType componentType) const {
+    bool Format::HasComponentType(Type componentType) const {
         // Depth stencil textures need to be special cased but we don't support sampling them yet.
         if (aspect != Color) {
             return false;
         }
 
-        return TextureComponentTypeToFormatType(componentType) == type;
+        return componentType == type;
     }
 
     size_t Format::GetIndex() const {

src/dawn_native/Format.h

@@ -65,7 +65,7 @@ namespace dawn_native {
         bool HasDepth() const;
         bool HasStencil() const;
         bool HasDepthOrStencil() const;
-        bool HasComponentType(wgpu::TextureComponentType componentType) const;
+        bool HasComponentType(Type componentType) const;
 
         // The index of the format in the list of all known formats: a unique number for each format
         // in [0, kKnownFormatCount)

src/dawn_native/PipelineLayout.cpp

@@ -144,7 +144,7 @@ namespace dawn_native {
             for (uint32_t group = 0; group < info.size(); ++group) {
                 for (const auto& it : info[group]) {
                     BindingNumber bindingNumber = it.first;
-                    const ShaderModuleBase::BindingInfo& bindingInfo = it.second;
+                    const ShaderModuleBase::ShaderBindingInfo& bindingInfo = it.second;
 
                     if (bindingInfo.multisampled) {
                         return DAWN_VALIDATION_ERROR("Multisampled textures not supported (yet)");

src/dawn_native/ProgrammablePassEncoder.cpp

@@ -132,7 +132,7 @@ namespace dawn_native {
                 }
 
                 for (BindingIndex i = 0; i < dynamicOffsetCount; ++i) {
-                    const BindGroupLayoutBase::BindingInfo& bindingInfo = layout->GetBindingInfo(i);
+                    const BindingInfo& bindingInfo = layout->GetBindingInfo(i);
 
                     // BGL creation sorts bindings such that the dynamic buffer bindings are first.
                     // ASSERT that this true.

src/dawn_native/ShaderModule.cpp

@@ -379,12 +379,12 @@ namespace dawn_native {
                 }
 
                 const auto& it = mBindingInfo[binding.set].emplace(BindingNumber(binding.binding),
-                                                                   BindingInfo{});
+                                                                   ShaderBindingInfo{});
                 if (!it.second) {
                     return DAWN_VALIDATION_ERROR("Shader has duplicate bindings");
                 }
 
-                BindingInfo* info = &it.first->second;
+                ShaderBindingInfo* info = &it.first->second;
                 info->id = binding.id;
                 info->base_type_id = binding.base_type_id;
                 info->type = ToWGPUBindingType(binding.binding_type);
@@ -558,14 +558,15 @@ namespace dawn_native {
                     return DAWN_VALIDATION_ERROR("Bind group index over limits in the SPIRV");
                 }
 
-                const auto& it = mBindingInfo[set].emplace(bindingNumber, BindingInfo{});
+                const auto& it = mBindingInfo[set].emplace(bindingNumber, ShaderBindingInfo{});
                 if (!it.second) {
                     return DAWN_VALIDATION_ERROR("Shader has duplicate bindings");
                 }
 
-                BindingInfo* info = &it.first->second;
+                ShaderBindingInfo* info = &it.first->second;
                 info->id = resource.id;
                 info->base_type_id = resource.base_type_id;
+
                 switch (bindingType) {
                     case wgpu::BindingType::SampledTexture: {
                         spirv_cross::SPIRType::ImageType imageType =
@@ -747,7 +748,7 @@ namespace dawn_native {
         // corresponding binding in the BindGroupLayout, if it exists.
         for (const auto& it : mBindingInfo[group]) {
             BindingNumber bindingNumber = it.first;
-            const auto& moduleInfo = it.second;
+            const ShaderBindingInfo& moduleInfo = it.second;
 
             const auto& bindingIt = bindingMap.find(bindingNumber);
             if (bindingIt == bindingMap.end()) {
@@ -755,17 +756,15 @@ namespace dawn_native {
             }
             BindingIndex bindingIndex(bindingIt->second);
 
-            const BindGroupLayoutBase::BindingInfo& bindingInfo =
+            const BindingInfo& bindingInfo = layout->GetBindingInfo(bindingIndex);
-                layout->GetBindingInfo(bindingIndex);
-            const auto& layoutBindingType = bindingInfo.type;
 
-            if (layoutBindingType != moduleInfo.type) {
+            if (bindingInfo.type != moduleInfo.type) {
                 // Binding mismatch between shader and bind group is invalid. For example, a
                 // writable binding in the shader with a readonly storage buffer in the bind group
                 // layout is invalid. However, a readonly binding in the shader with a writable
                 // storage buffer in the bind group layout is valid.
                 bool validBindingConversion =
-                    layoutBindingType == wgpu::BindingType::StorageBuffer &&
+                    bindingInfo.type == wgpu::BindingType::StorageBuffer &&
                     moduleInfo.type == wgpu::BindingType::ReadonlyStorageBuffer;
                 if (!validBindingConversion) {
                     return false;
@@ -776,11 +775,9 @@ namespace dawn_native {
                 return false;
             }
 
-            switch (layoutBindingType) {
+            switch (bindingInfo.type) {
                 case wgpu::BindingType::SampledTexture: {
-                    Format::Type layoutTextureComponentType =
+                    if (bindingInfo.textureComponentType != moduleInfo.textureComponentType) {
-                        Format::TextureComponentTypeToFormatType(bindingInfo.textureComponentType);
-                    if (layoutTextureComponentType != moduleInfo.textureComponentType) {
                         return false;
                     }
 

src/dawn_native/ShaderModule.h

@@ -16,11 +16,11 @@
 #define DAWNNATIVE_SHADERMODULE_H_
 
 #include "common/Constants.h"
+#include "dawn_native/BindingInfo.h"
 #include "dawn_native/CachedObject.h"
 #include "dawn_native/Error.h"
 #include "dawn_native/Format.h"
 #include "dawn_native/Forward.h"
-#include "dawn_native/IntegerTypes.h"
 #include "dawn_native/PerStage.h"
 
 #include "dawn_native/dawn_platform.h"
@@ -50,18 +50,19 @@ namespace dawn_native {
 
         MaybeError ExtractSpirvInfo(const spirv_cross::Compiler& compiler);
 
-        struct BindingInfo {
+        struct ShaderBindingInfo : BindingInfo {
             // The SPIRV ID of the resource.
             uint32_t id;
             uint32_t base_type_id;
-            wgpu::BindingType type;
+
-            // Match the defaults in BindGroupLayoutDescriptor
+          private:
-            wgpu::TextureViewDimension textureDimension = wgpu::TextureViewDimension::Undefined;
+            // Disallow access to unused members.
-            Format::Type textureComponentType = Format::Type::Float;
+            using BindingInfo::hasDynamicOffset;
-            bool multisampled = false;
+            using BindingInfo::visibility;
-            wgpu::TextureFormat storageTextureFormat = wgpu::TextureFormat::Undefined;
         };
-        using ModuleBindingInfo = std::array<std::map<BindingNumber, BindingInfo>, kMaxBindGroups>;
+
+        using ModuleBindingInfo =
+            std::array<std::map<BindingNumber, ShaderBindingInfo>, kMaxBindGroups>;
 
         const ModuleBindingInfo& GetBindingInfo() const;
         const std::bitset<kMaxVertexAttributes>& GetUsedVertexAttributes() const;

src/dawn_native/d3d12/BindGroupD3D12.cpp

@@ -86,7 +86,7 @@ namespace dawn_native { namespace d3d12 {
         ID3D12Device* d3d12Device = device->GetD3D12Device().Get();
 
         for (BindingIndex bindingIndex = 0; bindingIndex < bgl->GetBindingCount(); ++bindingIndex) {
-            const BindGroupLayoutBase::BindingInfo& bindingInfo = bgl->GetBindingInfo(bindingIndex);
+            const BindingInfo& bindingInfo = bgl->GetBindingInfo(bindingIndex);
 
             // It's not necessary to create descriptors in descriptor heap for dynamic
             // resources. So skip allocating descriptors in descriptor heaps for dynamic

src/dawn_native/d3d12/BindGroupLayoutD3D12.cpp

@@ -47,7 +47,7 @@ namespace dawn_native { namespace d3d12 {
           mBindGroupAllocator(MakeFrontendBindGroupAllocator<BindGroup>(4096)) {
         for (BindingIndex bindingIndex = GetDynamicBufferCount(); bindingIndex < GetBindingCount();
              ++bindingIndex) {
-            const BindGroupLayoutBase::BindingInfo& bindingInfo = GetBindingInfo(bindingIndex);
+            const BindingInfo& bindingInfo = GetBindingInfo(bindingIndex);
 
             // For dynamic resources, Dawn uses root descriptor in D3D12 backend.
             // So there is no need to allocate the descriptor from descriptor heap.
@@ -101,7 +101,7 @@ namespace dawn_native { namespace d3d12 {
         descriptorOffsets[Sampler] = 0;
 
         for (BindingIndex bindingIndex = 0; bindingIndex < GetBindingCount(); ++bindingIndex) {
-            const BindGroupLayoutBase::BindingInfo& bindingInfo = GetBindingInfo(bindingIndex);
+            const BindingInfo& bindingInfo = GetBindingInfo(bindingIndex);
 
             if (bindingInfo.hasDynamicOffset) {
                 // Dawn is using values in mBindingOffsets to decide register number in HLSL.

src/dawn_native/d3d12/PipelineLayoutD3D12.cpp

@@ -133,7 +133,7 @@ namespace dawn_native { namespace d3d12 {
             for (BindingIndex dynamicBindingIndex = 0;
                  dynamicBindingIndex < bindGroupLayout->GetDynamicBufferCount();
                  ++dynamicBindingIndex) {
-                const BindGroupLayoutBase::BindingInfo& bindingInfo =
+                const BindingInfo& bindingInfo =
                     bindGroupLayout->GetBindingInfo(dynamicBindingIndex);
 
                 D3D12_ROOT_PARAMETER* rootParameter = &rootParameters[parameterIndex];

src/dawn_native/d3d12/PipelineLayoutD3D12.h

@@ -15,7 +15,7 @@
 #ifndef DAWNNATIVE_D3D12_PIPELINELAYOUTD3D12_H_
 #define DAWNNATIVE_D3D12_PIPELINELAYOUTD3D12_H_
 
-#include "dawn_native/IntegerTypes.h"
+#include "dawn_native/BindingInfo.h"
 #include "dawn_native/PipelineLayout.h"
 #include "dawn_native/d3d12/d3d12_platform.h"
 

src/dawn_native/d3d12/ShaderModuleD3D12.cpp

@@ -97,7 +97,7 @@ namespace dawn_native { namespace d3d12 {
             const auto& bindingOffsets = bgl->GetBindingOffsets();
             const auto& groupBindingInfo = moduleBindingInfo[group];
             for (const auto& it : groupBindingInfo) {
-                const BindingInfo& bindingInfo = it.second;
+                const ShaderBindingInfo& bindingInfo = it.second;
                 BindingNumber bindingNumber = it.first;
                 BindingIndex bindingIndex = bgl->GetBindingIndex(bindingNumber);
 

src/dawn_native/metal/CommandBufferMTL.mm

@@ -500,7 +500,7 @@ namespace dawn_native { namespace metal {
                 // call here.
                 for (BindingIndex bindingIndex = 0;
                      bindingIndex < group->GetLayout()->GetBindingCount(); ++bindingIndex) {
-                    const BindGroupLayoutBase::BindingInfo& bindingInfo =
+                    const BindingInfo& bindingInfo =
                         group->GetLayout()->GetBindingInfo(bindingIndex);
 
                     bool hasVertStage =

src/dawn_native/metal/PipelineLayoutMTL.mm

@@ -31,7 +31,7 @@ namespace dawn_native { namespace metal {
             for (uint32_t group : IterateBitSet(GetBindGroupLayoutsMask())) {
                 for (BindingIndex bindingIndex = 0;
                      bindingIndex < GetBindGroupLayout(group)->GetBindingCount(); ++bindingIndex) {
-                    const BindGroupLayoutBase::BindingInfo& bindingInfo =
+                    const BindingInfo& bindingInfo =
                         GetBindGroupLayout(group)->GetBindingInfo(bindingIndex);
                     if (!(bindingInfo.visibility & StageBit(stage))) {
                         continue;

src/dawn_native/metal/ShaderModuleMTL.mm

@@ -138,7 +138,7 @@ namespace dawn_native { namespace metal {
                 BindingNumber bindingNumber = it.first;
                 BindingIndex bindingIndex = it.second;
 
-                const BindGroupLayoutBase::BindingInfo& bindingInfo =
+                const BindingInfo& bindingInfo =
                     layout->GetBindGroupLayout(group)->GetBindingInfo(bindingIndex);
 
                 for (auto stage : IterateStages(bindingInfo.visibility)) {

src/dawn_native/opengl/CommandBufferGL.cpp

@@ -243,7 +243,7 @@ namespace dawn_native { namespace opengl {
 
                 for (BindingIndex bindingIndex = 0;
                      bindingIndex < group->GetLayout()->GetBindingCount(); ++bindingIndex) {
-                    const BindGroupLayoutBase::BindingInfo& bindingInfo =
+                    const BindingInfo& bindingInfo =
                         group->GetLayout()->GetBindingInfo(bindingIndex);
 
                     switch (bindingInfo.type) {

src/dawn_native/opengl/PipelineGL.cpp

@@ -179,10 +179,10 @@ namespace dawn_native { namespace opengl {
 
                 const BindGroupLayoutBase* bgl =
                     layout->GetBindGroupLayout(combined.textureLocation.group);
-                wgpu::TextureComponentType componentType =
+                Format::Type componentType =
                     bgl->GetBindingInfo(bgl->GetBindingIndex(combined.textureLocation.binding))
                         .textureComponentType;
-                bool shouldUseFiltering = componentType == wgpu::TextureComponentType::Float;
+                bool shouldUseFiltering = componentType == Format::Type::Float;
 
                 GLuint samplerIndex =
                     indices[combined.samplerLocation.group][combined.samplerLocation.binding];

src/dawn_native/vulkan/BindGroupVk.cpp

@@ -46,8 +46,7 @@ namespace dawn_native { namespace vulkan {
         for (const auto& it : GetLayout()->GetBindingMap()) {
             BindingNumber bindingNumber = it.first;
             BindingIndex bindingIndex = it.second;
-            const BindGroupLayoutBase::BindingInfo& bindingInfo =
+            const BindingInfo& bindingInfo = GetLayout()->GetBindingInfo(bindingIndex);
-                GetLayout()->GetBindingInfo(bindingIndex);
 
             auto& write = writes[numWrites];
             write.sType = VK_STRUCTURE_TYPE_WRITE_DESCRIPTOR_SET;