From ed0b3cf15389ff2f9fc84e0d218ae2555b53d8a1 Mon Sep 17 00:00:00 2001 From: Corentin Wallez Date: Thu, 24 Sep 2020 11:27:47 +0000 Subject: [PATCH] Refactor PipelineLayoutBase::CreateDefault This function was bit long and was difficult to read. Refactor it to use a single double keyed map and helper functions. Bug: dawn:527 Change-Id: I8c1173fd0e06256c7e7060a850996e1e90187d50 Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/28640 Reviewed-by: Stephen White Reviewed-by: Corentin Wallez Reviewed-by: Austin Eng Commit-Queue: Corentin Wallez --- src/dawn_native/PipelineLayout.cpp | 182 ++++++++++++++--------------- 1 file changed, 91 insertions(+), 91 deletions(-) diff --git a/src/dawn_native/PipelineLayout.cpp b/src/dawn_native/PipelineLayout.cpp index 6b0c5986bf..77df277458 100644 --- a/src/dawn_native/PipelineLayout.cpp +++ b/src/dawn_native/PipelineLayout.cpp @@ -24,20 +24,6 @@ namespace dawn_native { - namespace { - - bool InferredBindGroupLayoutEntriesCompatible(const BindGroupLayoutEntry& lhs, - const BindGroupLayoutEntry& rhs) { - // Minimum buffer binding size excluded because we take the maximum seen across stages. - // Visibility is excluded because we take the OR across stages. - return lhs.binding == rhs.binding && lhs.type == rhs.type && - lhs.hasDynamicOffset == rhs.hasDynamicOffset && - lhs.multisampled == rhs.multisampled && lhs.viewDimension == rhs.viewDimension && - lhs.textureComponentType == rhs.textureComponentType; - } - - } // anonymous namespace - MaybeError ValidatePipelineLayoutDescriptor(DeviceBase* device, const PipelineLayoutDescriptor* descriptor) { if (descriptor->nextInChain != nullptr) { @@ -92,113 +78,127 @@ namespace dawn_native { ResultOrError PipelineLayoutBase::CreateDefault( DeviceBase* device, std::vector stages) { + using EntryMap = std::map; + + // Merges two entries at the same location, if they are allowed to be merged. + auto MergeEntries = [](BindGroupLayoutEntry* modifiedEntry, + const BindGroupLayoutEntry& mergedEntry) -> MaybeError { + // Minimum buffer binding size excluded because we take the maximum seen across stages. + // Visibility is excluded because we take the OR across stages. + bool compatible = + modifiedEntry->binding == mergedEntry.binding && // + modifiedEntry->type == mergedEntry.type && // + modifiedEntry->hasDynamicOffset == mergedEntry.hasDynamicOffset && // + modifiedEntry->multisampled == mergedEntry.multisampled && // + modifiedEntry->viewDimension == mergedEntry.viewDimension && // + modifiedEntry->textureComponentType == mergedEntry.textureComponentType; + + // Check if any properties are incompatible with existing entry + // If compatible, we will merge some properties + if (!compatible) { + return DAWN_VALIDATION_ERROR( + "Duplicate binding in default pipeline layout initialization " + "not compatible with previous declaration"); + } + + // Use the max |minBufferBindingSize| we find. + modifiedEntry->minBufferBindingSize = + std::max(modifiedEntry->minBufferBindingSize, mergedEntry.minBufferBindingSize); + + // Use the OR of all the stages at which we find this binding. + modifiedEntry->visibility |= mergedEntry.visibility; + + return {}; + }; + + // Does the trivial conversions from a ShaderBindingInfo to a BindGroupLayoutEntry + auto ConvertMetadataToEntry = + [](const EntryPointMetadata::ShaderBindingInfo& shaderBinding) -> BindGroupLayoutEntry { + BindGroupLayoutEntry entry = {}; + entry.type = shaderBinding.type; + entry.hasDynamicOffset = false; + entry.multisampled = shaderBinding.multisampled; + entry.viewDimension = shaderBinding.viewDimension; + entry.textureComponentType = + Format::FormatTypeToTextureComponentType(shaderBinding.textureComponentType); + entry.storageTextureFormat = shaderBinding.storageTextureFormat; + entry.minBufferBindingSize = shaderBinding.minBufferBindingSize; + return entry; + }; + + // Creates the BGL from the entries for a stage, checking it is valid. + auto CreateBGL = [](DeviceBase* device, + const EntryMap& entries) -> ResultOrError> { + std::vector entryVec; + entryVec.reserve(entries.size()); + for (auto& it : entries) { + entryVec.push_back(it.second); + } + + BindGroupLayoutDescriptor desc = {}; + desc.entries = entryVec.data(); + desc.entryCount = entryVec.size(); + + DAWN_TRY(ValidateBindGroupLayoutDescriptor(device, &desc)); + return device->GetOrCreateBindGroupLayout(&desc); + }; + ASSERT(!stages.empty()); // Data which BindGroupLayoutDescriptor will point to for creation - ityp::array< - BindGroupIndex, - ityp::stack_vec, - kMaxBindGroups> + ityp::array, kMaxBindGroups> entryData = {}; - // A map of bindings to the index in |entryData| - ityp::array, kMaxBindGroups> - usedBindingsMap = {}; - - // A counter of how many bindings we've populated in |entryData| - ityp::array entryCounts = {}; - - BindingCounts bindingCounts = {}; - BindGroupIndex bindGroupLayoutCount(0); + // Loops over all the reflected BindGroupLayoutEntries from shaders. for (const StageAndDescriptor& stage : stages) { - // Extract argument for this stage. SingleShaderStage shaderStage = stage.first; const EntryPointMetadata::BindingInfo& info = stage.second->module->GetEntryPoint(stage.second->entryPoint, shaderStage).bindings; for (BindGroupIndex group(0); group < info.size(); ++group) { - for (const auto& it : info[group]) { - BindingNumber bindingNumber = it.first; - const EntryPointMetadata::ShaderBindingInfo& bindingInfo = it.second; + for (const auto& bindingIt : info[group]) { + BindingNumber bindingNumber = bindingIt.first; + const EntryPointMetadata::ShaderBindingInfo& shaderBinding = bindingIt.second; - BindGroupLayoutEntry bindingSlot; - bindingSlot.binding = static_cast(bindingNumber); - bindingSlot.visibility = StageBit(shaderStage); - bindingSlot.type = bindingInfo.type; - bindingSlot.hasDynamicOffset = false; - bindingSlot.multisampled = bindingInfo.multisampled; - bindingSlot.viewDimension = bindingInfo.viewDimension; - bindingSlot.textureComponentType = - Format::FormatTypeToTextureComponentType(bindingInfo.textureComponentType); - bindingSlot.storageTextureFormat = bindingInfo.storageTextureFormat; - bindingSlot.minBufferBindingSize = bindingInfo.minBufferBindingSize; + // Create the BindGroupLayoutEntry + BindGroupLayoutEntry entry = ConvertMetadataToEntry(shaderBinding); + entry.binding = static_cast(bindingNumber); + entry.visibility = StageBit(shaderStage); - { - const auto& it = usedBindingsMap[group].find(bindingNumber); - if (it != usedBindingsMap[group].end()) { - BindGroupLayoutEntry* existingEntry = &entryData[group][it->second]; - - // Check if any properties are incompatible with existing entry - // If compatible, we will merge some properties - if (!InferredBindGroupLayoutEntriesCompatible(*existingEntry, - bindingSlot)) { - return DAWN_VALIDATION_ERROR( - "Duplicate binding in default pipeline layout initialization " - "not compatible with previous declaration"); - } - - // Use the max |minBufferBindingSize| we find. - existingEntry->minBufferBindingSize = - std::max(existingEntry->minBufferBindingSize, - bindingSlot.minBufferBindingSize); - - // Use the OR of all the stages at which we find this binding. - existingEntry->visibility |= bindingSlot.visibility; - - // Already used slot, continue - continue; - } + // Add it to our map of all entries, if there is an existing entry, then we + // need to merge, if we can. + const auto& insertion = entryData[group].insert({bindingNumber, entry}); + if (!insertion.second) { + DAWN_TRY(MergeEntries(&insertion.first->second, entry)); } - - IncrementBindingCounts(&bindingCounts, bindingSlot); - BindingIndex currentBindingCount = entryCounts[group]; - entryData[group].resize(currentBindingCount + BindingIndex(1)); - entryData[group][currentBindingCount] = bindingSlot; - - usedBindingsMap[group][bindingNumber] = currentBindingCount; - - entryCounts[group]++; - - bindGroupLayoutCount = - std::max(bindGroupLayoutCount, group + BindGroupIndex(1)); } } } - // Create the deduced BGLs, validating if they are valid. + // Create the bind group layouts. We need to keep track of the last non-empty BGL because + // Dawn doesn't yet know that an empty BGL and a null BGL are the same thing. + // TODO(cwallez@chromium.org): remove this when Dawn knows that empty and null BGL are the + // same. + BindGroupIndex pipelineBGLCount = BindGroupIndex(0); ityp::array, kMaxBindGroups> bindGroupLayouts = {}; - for (BindGroupIndex group(0); group < bindGroupLayoutCount; ++group) { - BindGroupLayoutDescriptor desc = {}; - desc.entries = entryData[group].data(); - desc.entryCount = static_cast(entryCounts[group]); - - DAWN_TRY(ValidateBindGroupLayoutDescriptor(device, &desc)); - DAWN_TRY_ASSIGN(bindGroupLayouts[group], device->GetOrCreateBindGroupLayout(&desc)); - - ASSERT(!bindGroupLayouts[group]->IsError()); + for (BindGroupIndex group(0); group < kMaxBindGroupsTyped; ++group) { + DAWN_TRY_ASSIGN(bindGroupLayouts[group], CreateBGL(device, entryData[group])); + if (entryData[group].size() != 0) { + pipelineBGLCount = group + BindGroupIndex(1); + } } // Create the deduced pipeline layout, validating if it is valid. PipelineLayoutBase* pipelineLayout = nullptr; { ityp::array bgls = {}; - for (BindGroupIndex group(0); group < bindGroupLayoutCount; ++group) { + for (BindGroupIndex group(0); group < pipelineBGLCount; ++group) { bgls[group] = bindGroupLayouts[group].Get(); } PipelineLayoutDescriptor desc = {}; desc.bindGroupLayouts = bgls.data(); - desc.bindGroupLayoutCount = static_cast(bindGroupLayoutCount); + desc.bindGroupLayoutCount = static_cast(pipelineBGLCount); DAWN_TRY(ValidatePipelineLayoutDescriptor(device, &desc)); DAWN_TRY_ASSIGN(pipelineLayout, device->GetOrCreatePipelineLayout(&desc));