From 1ea205fb12406a71c0a1dcef8e13ad5a9cbd8940 Mon Sep 17 00:00:00 2001 From: Corentin Wallez Date: Wed, 18 Jul 2018 18:23:08 +0200 Subject: [PATCH] CommandBuffer: have a state tracker per-pass Also perform small code simplifications of the CommandBufferStateTracker now that it only tracks aspects. --- src/backend/CommandBuffer.cpp | 27 ++- src/backend/CommandBuffer.h | 2 - src/backend/CommandBufferStateTracker.cpp | 219 ++++++++++------------ src/backend/CommandBufferStateTracker.h | 32 +--- 4 files changed, 124 insertions(+), 156 deletions(-) diff --git a/src/backend/CommandBuffer.cpp b/src/backend/CommandBuffer.cpp index 9b74c3f512..beb31ca027 100644 --- a/src/backend/CommandBuffer.cpp +++ b/src/backend/CommandBuffer.cpp @@ -278,8 +278,7 @@ namespace backend { // CommandBufferBuilder - CommandBufferBuilder::CommandBufferBuilder(DeviceBase* device) - : Builder(device), mState(std::make_unique()) { + CommandBufferBuilder::CommandBufferBuilder(DeviceBase* device) : Builder(device) { } CommandBufferBuilder::~CommandBufferBuilder() { @@ -392,6 +391,7 @@ namespace backend { MaybeError CommandBufferBuilder::ValidateComputePass() { PassResourceUsageTracker usageTracker; + CommandBufferStateTracker persistentState; Command type; while (mIterator.NextCommandId(&type)) { @@ -401,20 +401,18 @@ namespace backend { DAWN_TRY(usageTracker.ValidateUsages(PassType::Compute)); mPassResourceUsages.push_back(usageTracker.AcquireResourceUsage()); - - mState->EndPass(); return {}; } break; case Command::Dispatch: { mIterator.NextCommand(); - DAWN_TRY(mState->ValidateCanDispatch()); + DAWN_TRY(persistentState.ValidateCanDispatch()); } break; case Command::SetComputePipeline: { SetComputePipelineCmd* cmd = mIterator.NextCommand(); ComputePipelineBase* pipeline = cmd->pipeline.Get(); - mState->SetComputePipeline(pipeline); + persistentState.SetComputePipeline(pipeline); } break; case Command::SetPushConstants: { @@ -433,7 +431,7 @@ namespace backend { SetBindGroupCmd* cmd = mIterator.NextCommand(); TrackBindGroupResourceUsage(cmd->group.Get(), &usageTracker); - mState->SetBindGroup(cmd->index, cmd->group.Get()); + persistentState.SetBindGroup(cmd->index, cmd->group.Get()); } break; default: @@ -446,6 +444,7 @@ namespace backend { MaybeError CommandBufferBuilder::ValidateRenderPass(RenderPassDescriptorBase* renderPass) { PassResourceUsageTracker usageTracker; + CommandBufferStateTracker persistentState; // Track usage of the render pass attachments for (uint32_t i : IterateBitSet(renderPass->GetColorAttachmentMask())) { @@ -466,19 +465,17 @@ namespace backend { DAWN_TRY(usageTracker.ValidateUsages(PassType::Render)); mPassResourceUsages.push_back(usageTracker.AcquireResourceUsage()); - - mState->EndPass(); return {}; } break; case Command::DrawArrays: { mIterator.NextCommand(); - DAWN_TRY(mState->ValidateCanDrawArrays()); + DAWN_TRY(persistentState.ValidateCanDrawArrays()); } break; case Command::DrawElements: { mIterator.NextCommand(); - DAWN_TRY(mState->ValidateCanDrawElements()); + DAWN_TRY(persistentState.ValidateCanDrawElements()); } break; case Command::SetRenderPipeline: { @@ -489,7 +486,7 @@ namespace backend { DAWN_RETURN_ERROR("Pipeline is incompatible with this render pass"); } - mState->SetRenderPipeline(pipeline); + persistentState.SetRenderPipeline(pipeline); } break; case Command::SetPushConstants: { @@ -522,14 +519,14 @@ namespace backend { SetBindGroupCmd* cmd = mIterator.NextCommand(); TrackBindGroupResourceUsage(cmd->group.Get(), &usageTracker); - mState->SetBindGroup(cmd->index, cmd->group.Get()); + persistentState.SetBindGroup(cmd->index, cmd->group.Get()); } break; case Command::SetIndexBuffer: { SetIndexBufferCmd* cmd = mIterator.NextCommand(); usageTracker.BufferUsedAs(cmd->buffer.Get(), dawn::BufferUsageBit::Index); - DAWN_TRY(mState->SetIndexBuffer()); + persistentState.SetIndexBuffer(); } break; case Command::SetVertexBuffers: { @@ -539,8 +536,8 @@ namespace backend { for (uint32_t i = 0; i < cmd->count; ++i) { usageTracker.BufferUsedAs(buffers[i].Get(), dawn::BufferUsageBit::Vertex); - DAWN_TRY(mState->SetVertexBuffer(cmd->startSlot + i)); } + persistentState.SetVertexBuffer(cmd->startSlot, cmd->count); } break; default: diff --git a/src/backend/CommandBuffer.h b/src/backend/CommandBuffer.h index 4efd772eca..ce156231f1 100644 --- a/src/backend/CommandBuffer.h +++ b/src/backend/CommandBuffer.h @@ -31,7 +31,6 @@ namespace backend { class BindGroupBase; class BufferBase; - class CommandBufferStateTracker; class FramebufferBase; class DeviceBase; class PipelineBase; @@ -138,7 +137,6 @@ namespace backend { MaybeError ValidateComputePass(); MaybeError ValidateRenderPass(RenderPassDescriptorBase* renderPass); - std::unique_ptr mState; CommandAllocator mAllocator; CommandIterator mIterator; bool mWasMovedToIterator = false; diff --git a/src/backend/CommandBufferStateTracker.cpp b/src/backend/CommandBufferStateTracker.cpp index 943cf8ff92..37b04c37be 100644 --- a/src/backend/CommandBufferStateTracker.cpp +++ b/src/backend/CommandBufferStateTracker.cpp @@ -15,69 +15,124 @@ #include "backend/CommandBufferStateTracker.h" #include "backend/BindGroup.h" -#include "backend/BindGroupLayout.h" -#include "backend/Buffer.h" #include "backend/ComputePipeline.h" #include "backend/Forward.h" #include "backend/InputState.h" #include "backend/PipelineLayout.h" -#include "backend/RenderPassDescriptor.h" #include "backend/RenderPipeline.h" -#include "backend/Texture.h" #include "common/Assert.h" #include "common/BitSetIterator.h" namespace backend { - MaybeError CommandBufferStateTracker::ValidateCanDispatch() { - constexpr ValidationAspects requiredAspects = - 1 << VALIDATION_ASPECT_PIPELINE | 1 << VALIDATION_ASPECT_BIND_GROUPS; - if ((requiredAspects & ~mAspects).none()) { - // Fast return-true path if everything is good - return {}; - } + enum ValidationAspect { + VALIDATION_ASPECT_PIPELINE, + VALIDATION_ASPECT_BIND_GROUPS, + VALIDATION_ASPECT_VERTEX_BUFFERS, + VALIDATION_ASPECT_INDEX_BUFFER, - if (!mAspects[VALIDATION_ASPECT_PIPELINE]) { - DAWN_RETURN_ERROR("No active compute pipeline"); - } - // Compute the lazily computed mAspects - if (!RecomputeHaveAspectBindGroups()) { - DAWN_RETURN_ERROR("Bind group state not valid"); - } - return {}; + VALIDATION_ASPECT_COUNT + }; + static_assert(VALIDATION_ASPECT_COUNT == CommandBufferStateTracker::kNumAspects, ""); + + static constexpr CommandBufferStateTracker::ValidationAspects kDispatchAspects = + 1 << VALIDATION_ASPECT_PIPELINE | 1 << VALIDATION_ASPECT_BIND_GROUPS; + + static constexpr CommandBufferStateTracker::ValidationAspects kDrawArraysAspects = + 1 << VALIDATION_ASPECT_PIPELINE | 1 << VALIDATION_ASPECT_BIND_GROUPS | + 1 << VALIDATION_ASPECT_VERTEX_BUFFERS; + + static constexpr CommandBufferStateTracker::ValidationAspects kDrawElementsAspects = + 1 << VALIDATION_ASPECT_PIPELINE | 1 << VALIDATION_ASPECT_BIND_GROUPS | + 1 << VALIDATION_ASPECT_VERTEX_BUFFERS | 1 << VALIDATION_ASPECT_INDEX_BUFFER; + + static constexpr CommandBufferStateTracker::ValidationAspects kLazyAspects = + 1 << VALIDATION_ASPECT_BIND_GROUPS | 1 << VALIDATION_ASPECT_VERTEX_BUFFERS; + + MaybeError CommandBufferStateTracker::ValidateCanDispatch() { + return ValidateOperation(kDispatchAspects); } MaybeError CommandBufferStateTracker::ValidateCanDrawArrays() { - constexpr ValidationAspects requiredAspects = 1 << VALIDATION_ASPECT_PIPELINE | - 1 << VALIDATION_ASPECT_BIND_GROUPS | - 1 << VALIDATION_ASPECT_VERTEX_BUFFERS; - if ((requiredAspects & ~mAspects).none()) { - // Fast return-true path if everything is good - return {}; - } - - return RevalidateCanDraw(); + return ValidateOperation(kDrawArraysAspects); } MaybeError CommandBufferStateTracker::ValidateCanDrawElements() { - constexpr ValidationAspects requiredAspects = - 1 << VALIDATION_ASPECT_PIPELINE | 1 << VALIDATION_ASPECT_BIND_GROUPS | - 1 << VALIDATION_ASPECT_VERTEX_BUFFERS | 1 << VALIDATION_ASPECT_INDEX_BUFFER; - if ((requiredAspects & ~mAspects).none()) { - // Fast return-true path if everything is good + return ValidateOperation(kDrawElementsAspects); + } + + MaybeError CommandBufferStateTracker::ValidateOperation(ValidationAspects requiredAspects) { + // Fast return-true path if everything is good + ValidationAspects missingAspects = requiredAspects & ~mAspects; + if (missingAspects.none()) { return {}; } - if (!mAspects[VALIDATION_ASPECT_INDEX_BUFFER]) { - DAWN_RETURN_ERROR("Cannot DrawElements without index buffer set"); + // Generate an error immediately if a non-lazy aspect is missing as computing lazy aspects + // requires the pipeline to be set. + if ((missingAspects & ~kLazyAspects).any()) { + return GenerateAspectError(missingAspects); } - return RevalidateCanDraw(); + + RecomputeLazyAspects(missingAspects); + + missingAspects = requiredAspects & ~mAspects; + if (missingAspects.any()) { + return GenerateAspectError(missingAspects); + } + + return {}; } - void CommandBufferStateTracker::EndPass() { - mInputsSet.reset(); - mAspects = 0; - mBindgroups.fill(nullptr); + void CommandBufferStateTracker::RecomputeLazyAspects(ValidationAspects aspects) { + ASSERT(mAspects[VALIDATION_ASPECT_PIPELINE]); + ASSERT((aspects & ~kLazyAspects).none()); + + if (aspects[VALIDATION_ASPECT_BIND_GROUPS]) { + bool matches = true; + + for (uint32_t i : IterateBitSet(mLastPipelineLayout->GetBindGroupLayoutsMask())) { + if (mLastPipelineLayout->GetBindGroupLayout(i) != mBindgroups[i]->GetLayout()) { + matches = false; + break; + } + } + + if (matches) { + mAspects.set(VALIDATION_ASPECT_BIND_GROUPS); + } + } + + if (aspects[VALIDATION_ASPECT_VERTEX_BUFFERS]) { + ASSERT(mLastRenderPipeline != nullptr); + + auto requiredInputs = mLastRenderPipeline->GetInputState()->GetInputsSetMask(); + if ((mInputsSet & requiredInputs) == requiredInputs) { + mAspects.set(VALIDATION_ASPECT_VERTEX_BUFFERS); + } + } + } + + MaybeError CommandBufferStateTracker::GenerateAspectError(ValidationAspects aspects) { + ASSERT(aspects.any()); + + if (aspects[VALIDATION_ASPECT_INDEX_BUFFER]) { + DAWN_RETURN_ERROR("Missing index buffer"); + } + + if (aspects[VALIDATION_ASPECT_VERTEX_BUFFERS]) { + DAWN_RETURN_ERROR("Missing vertex buffer"); + } + + if (aspects[VALIDATION_ASPECT_BIND_GROUPS]) { + DAWN_RETURN_ERROR("Missing bind group"); + } + + if (aspects[VALIDATION_ASPECT_PIPELINE]) { + DAWN_RETURN_ERROR("Missing pipeline"); + } + + UNREACHABLE(); } void CommandBufferStateTracker::SetComputePipeline(ComputePipelineBase* pipeline) { @@ -90,96 +145,26 @@ namespace backend { } void CommandBufferStateTracker::SetBindGroup(uint32_t index, BindGroupBase* bindgroup) { - mBindgroupsSet.set(index); mBindgroups[index] = bindgroup; } - MaybeError CommandBufferStateTracker::SetIndexBuffer() { - if (!HavePipeline()) { - DAWN_RETURN_ERROR("Can't set the index buffer without a pipeline"); - } - + void CommandBufferStateTracker::SetIndexBuffer() { mAspects.set(VALIDATION_ASPECT_INDEX_BUFFER); - return {}; } - MaybeError CommandBufferStateTracker::SetVertexBuffer(uint32_t index) { - if (!HavePipeline()) { - DAWN_RETURN_ERROR("Can't set vertex buffers without a pipeline"); + void CommandBufferStateTracker::SetVertexBuffer(uint32_t start, uint32_t count) { + for (uint32_t i = 0; i < count; ++i) { + mInputsSet.set(start + i); } - - mInputsSet.set(index); - return {}; - } - - bool CommandBufferStateTracker::RecomputeHaveAspectBindGroups() { - if (mAspects[VALIDATION_ASPECT_BIND_GROUPS]) { - return true; - } - // Assumes we have a pipeline already - if (!mBindgroupsSet.all()) { - return false; - } - for (size_t i = 0; i < mBindgroups.size(); ++i) { - if (auto* bindgroup = mBindgroups[i]) { - // TODO(kainino@chromium.org): bind group compatibility - auto* pipelineBGL = mLastPipeline->GetLayout()->GetBindGroupLayout(i); - if (pipelineBGL && bindgroup->GetLayout() != pipelineBGL) { - return false; - } - } - } - mAspects.set(VALIDATION_ASPECT_BIND_GROUPS); - return true; - } - - bool CommandBufferStateTracker::RecomputeHaveAspectVertexBuffers() { - if (mAspects[VALIDATION_ASPECT_VERTEX_BUFFERS]) { - return true; - } - // Assumes we have a pipeline already - auto requiredInputs = mLastRenderPipeline->GetInputState()->GetInputsSetMask(); - if ((mInputsSet & requiredInputs) == requiredInputs) { - mAspects.set(VALIDATION_ASPECT_VERTEX_BUFFERS); - return true; - } - return false; - } - - bool CommandBufferStateTracker::HavePipeline() const { - return mAspects[VALIDATION_ASPECT_PIPELINE]; - } - - MaybeError CommandBufferStateTracker::RevalidateCanDraw() { - if (!mAspects[VALIDATION_ASPECT_PIPELINE]) { - DAWN_RETURN_ERROR("No active render pipeline"); - } - // Compute the lazily computed mAspects - if (!RecomputeHaveAspectBindGroups()) { - DAWN_RETURN_ERROR("Bind group state not valid"); - } - if (!RecomputeHaveAspectVertexBuffers()) { - DAWN_RETURN_ERROR("Some vertex buffers are not set"); - } - return {}; } void CommandBufferStateTracker::SetPipelineCommon(PipelineBase* pipeline) { - PipelineLayoutBase* layout = pipeline->GetLayout(); + mLastPipelineLayout = pipeline->GetLayout(); mAspects.set(VALIDATION_ASPECT_PIPELINE); - mAspects.reset(VALIDATION_ASPECT_BIND_GROUPS); - mAspects.reset(VALIDATION_ASPECT_VERTEX_BUFFERS); - // Reset bindgroups but mark unused bindgroups as valid - mBindgroupsSet = ~layout->GetBindGroupLayoutsMask(); - - // Only bindgroups that were not the same layout in the last pipeline need to be set again. - if (mLastPipeline) { - mBindgroupsSet |= layout->InheritedGroupsMask(mLastPipeline->GetLayout()); - } - - mLastPipeline = pipeline; + // Reset lazy aspects so they get recomputed on the next operation. + mAspects &= ~kLazyAspects; } } // namespace backend diff --git a/src/backend/CommandBufferStateTracker.h b/src/backend/CommandBufferStateTracker.h index 45079e24ae..7b4454be05 100644 --- a/src/backend/CommandBufferStateTracker.h +++ b/src/backend/CommandBufferStateTracker.h @@ -28,45 +28,33 @@ namespace backend { class CommandBufferStateTracker { public: // Non-state-modifying validation functions - MaybeError ValidateCanCopy() const; MaybeError ValidateCanDispatch(); MaybeError ValidateCanDrawArrays(); MaybeError ValidateCanDrawElements(); // State-modifying methods - void EndPass(); void SetComputePipeline(ComputePipelineBase* pipeline); void SetRenderPipeline(RenderPipelineBase* pipeline); void SetBindGroup(uint32_t index, BindGroupBase* bindgroup); - MaybeError SetIndexBuffer(); - MaybeError SetVertexBuffer(uint32_t index); + void SetIndexBuffer(); + void SetVertexBuffer(uint32_t start, uint32_t count); + + static constexpr size_t kNumAspects = 4; + using ValidationAspects = std::bitset; private: - enum ValidationAspect { - VALIDATION_ASPECT_PIPELINE, - VALIDATION_ASPECT_BIND_GROUPS, - VALIDATION_ASPECT_VERTEX_BUFFERS, - VALIDATION_ASPECT_INDEX_BUFFER, - - VALIDATION_ASPECT_COUNT - }; - using ValidationAspects = std::bitset; - - // Queries for lazily evaluated aspects - bool RecomputeHaveAspectBindGroups(); - bool RecomputeHaveAspectVertexBuffers(); - - bool HavePipeline() const; - MaybeError RevalidateCanDraw(); + MaybeError ValidateOperation(ValidationAspects requiredAspects); + void RecomputeLazyAspects(ValidationAspects aspects); + MaybeError GenerateAspectError(ValidationAspects aspects); void SetPipelineCommon(PipelineBase* pipeline); ValidationAspects mAspects; - std::bitset mBindgroupsSet; std::array mBindgroups = {}; std::bitset mInputsSet; - PipelineBase* mLastPipeline = nullptr; + + PipelineLayoutBase* mLastPipelineLayout = nullptr; RenderPipelineBase* mLastRenderPipeline = nullptr; };