From fcd7e198c29a8634e12033119092ff3bda03fe1f Mon Sep 17 00:00:00 2001 From: Pixelflinger Date: Tue, 25 Feb 2020 23:53:56 -0800 Subject: [PATCH] First step in reworking how rendertargets work in the framegraph The ultimate goal is to get rid of the concept of render targets in the framegraph, and this is the first step. Here RenderTarget doesn't store the discard flags anymore. These flags are now calculated per-resource, per-pass. A side effect of this change is that the discard flags are now slightly better, as they don't contain attachments that don't exist. i.e. we won't see the STENCIL flag set for instance. --- filament/src/fg/FrameGraph.cpp | 112 ++++++++------------ filament/src/fg/FrameGraph.h | 6 +- filament/src/fg/ResourceAllocator.cpp | 2 +- filament/src/fg/fg/RenderTarget.h | 2 +- filament/src/fg/fg/RenderTargetResource.cpp | 4 +- filament/src/fg/fg/RenderTargetResource.h | 7 +- filament/src/fg/fg/ResourceEntry.h | 16 ++- filament/src/fg/fg/VirtualResource.h | 6 +- filament/test/filament_framegraph_test.cpp | 32 +++--- 9 files changed, 90 insertions(+), 97 deletions(-) diff --git a/filament/src/fg/FrameGraph.cpp b/filament/src/fg/FrameGraph.cpp index f17d391777..b66bccdc0e 100644 --- a/filament/src/fg/FrameGraph.cpp +++ b/filament/src/fg/FrameGraph.cpp @@ -152,10 +152,36 @@ FrameGraphPassResources::getRenderTarget(FrameGraphRenderTargetHandle handle, ui assert(info.target); // overwrite discard flags with the per-rendertarget (per-pass) computed value - info.params.flags.discardStart = renderTarget.targetFlags.discardStart; - info.params.flags.discardEnd = renderTarget.targetFlags.discardEnd; + info.params.flags.discardStart = TargetBufferFlags::NONE; + info.params.flags.discardEnd = TargetBufferFlags::NONE; info.params.flags.clear = renderTarget.userClearFlags; + static constexpr TargetBufferFlags flags[] = { + TargetBufferFlags::COLOR, + TargetBufferFlags::DEPTH, + TargetBufferFlags::STENCIL }; + + auto& resourceNodes = fg.mResourceNodes; + for (size_t i = 0; i discardStart) { + info.params.flags.discardStart |= flags[i]; + } + if (resourceNodes[attachment.index].resource->discardEnd) { + info.params.flags.discardEnd |= flags[i]; + } + } + } + + // clear implies discarding the content of the buffer + info.params.flags.discardStart |= (renderTarget.userClearFlags & TargetBufferFlags::ALL); + if (renderTarget.cache->imported) { + // we never discard more than the user flags + info.params.flags.discardStart &= renderTarget.cache->discardStart; + info.params.flags.discardEnd &= renderTarget.cache->discardEnd; + } + // check that this FrameGraphRenderTarget is indeed declared by this pass ASSERT_POSTCONDITION_NON_FATAL(info.target, "Pass \"%s\" doesn't declare rendertarget \"%s\" -- expect graphic corruptions", @@ -369,57 +395,25 @@ FrameGraphRenderTargetHandle FrameGraph::importRenderTarget(const char* name, return FrameGraphRenderTargetHandle(renderTarget.index); } -TargetBufferFlags FrameGraph::computeDiscardFlags(DiscardPhase phase, - PassNode const* curr, PassNode const* first, fg::RenderTarget const& renderTarget) { +bool FrameGraph::computeDiscard(const Vector fg::PassNode::* list, + PassNode const* curr, PassNode const* first, FrameGraphHandle attachment) { auto& resourceNodes = mResourceNodes; - TargetBufferFlags discardFlags = TargetBufferFlags::ALL; - - static constexpr TargetBufferFlags flags[] = { - TargetBufferFlags::COLOR, - TargetBufferFlags::DEPTH, - TargetBufferFlags::STENCIL }; - - auto const& desc = renderTarget.cache->desc; - - // for each pass... - while (any(discardFlags) && curr != first) { + while (curr != first) { PassNode const& pass = *curr++; // TODO: maybe find a more efficient way of figuring this out // for each resource written or read... - for (FrameGraphHandle cur : ((phase == DiscardPhase::START) ? pass.writes : pass.reads)) { + for (FrameGraphHandle cur : pass.*list) { // for all possible attachments of our renderTarget... ResourceEntryBase const* const pResource = resourceNodes[cur.index].resource; - for (size_t i = 0; i < desc.attachments.textures.size(); i++) { - FrameGraphHandle attachment = desc.attachments.textures[i]; - if (attachment.isValid() && resourceNodes[attachment.index].resource == pResource) { - // we can't discard this attachment since it's read/written - discardFlags &= ~flags[i]; - } - } - if (!discardFlags) { - break; + if (attachment.isValid() && resourceNodes[attachment.index].resource == pResource) { + return false; } } } - - if (phase == DiscardPhase::START) { - // clear implies discarding the content of the buffer - discardFlags |= (renderTarget.userClearFlags & TargetBufferFlags::ALL); - } - - if (renderTarget.cache->imported) { - // we never discard more than the user flags - if (phase == DiscardPhase::START) { - discardFlags &= renderTarget.cache->discardStart; - } - if (phase == DiscardPhase::END) { - discardFlags &= renderTarget.cache->discardEnd; - } - } - - return discardFlags; + return true; } + FrameGraph& FrameGraph::compile() noexcept { Vector& passNodes = mPassNodes; Vector& renderTargets = mRenderTargets; @@ -586,8 +580,6 @@ FrameGraph& FrameGraph::compile() noexcept { * compute first/last users for active passes */ - auto const first = passNodes.data(); - auto const last = passNodes.data() + passNodes.size(); for (PassNode& pass : passNodes) { if (!pass.refCount) { assert(!pass.hasSideEffect); @@ -608,30 +600,11 @@ FrameGraph& FrameGraph::compile() noexcept { pResource->last = &pass; } for (auto i : pass.renderTargets) { - fg::RenderTarget* const pRenderTarget = &renderTargets[i]; VirtualResource* const pResource = renderTargets[i].cache; // figure out which is the first pass to need this resource pResource->first = pResource->first ? pResource->first : &pass; // figure out which is the last pass to need this resource pResource->last = &pass; - - // compute this resource discard flag for this pass for this resource - - // does anyone writes to this resource before us -- if so, don't discard those buffers on enter - // (i.e. if nobody wrote, no need to load from memory) - TargetBufferFlags discardStart = computeDiscardFlags( - DiscardPhase::START, first, &pass, *pRenderTarget); - - // does anyone reads this resource after us -- if so, don't discard those buffers on exit - // (i.e. if nobody is going to read, no need to write back to memory) - TargetBufferFlags discardEnd = computeDiscardFlags( - DiscardPhase::END, &pass + 1, last, *pRenderTarget); - - pRenderTarget->targetFlags = { - .clear = {}, // this is eventually set by the user - .discardStart = discardStart, - .discardEnd = discardEnd - }; } } @@ -662,16 +635,22 @@ void FrameGraph::executeInternal(PassNode const& node, DriverApi& driver) noexce assert(node.base); // create concrete resources and rendertargets for (VirtualResource* resource : node.devirtualize) { - resource->create(*this); + resource->preExecuteDevirtualize(*this); + } + for (VirtualResource* resource : node.destroy) { + resource->preExecuteDestroy(*this); } // execute the pass FrameGraphPassResources resources(*this, node); node.base->execute(resources, driver); + for (VirtualResource* resource : node.devirtualize) { + resource->postExecuteDevirtualize(*this); + } // destroy concrete resources for (VirtualResource* resource : node.destroy) { - resource->destroy(*this); + resource->postExecuteDestroy(*this); } } @@ -741,7 +720,8 @@ void FrameGraph::export_graphviz(utils::io::ostream& out) { ResourceEntryBase const* subresource = node.resource; out << "\"R" << node.resource->id << "_" << +node.version << "\"" - "[label=\"" << node.resource->name << "\\n(version: " << +node.version << ")" "\\nid:" << node.resource->id << + "[label=\"" << node.resource->name << "\\n(version: " << +node.version << ")" + "\\nid:" << node.resource->id << "\\nrefs:" << node.resource->refs; #if UTILS_HAS_RTTI diff --git a/filament/src/fg/FrameGraph.h b/filament/src/fg/FrameGraph.h index 0b59c718f1..ade56303fb 100644 --- a/filament/src/fg/FrameGraph.h +++ b/filament/src/fg/FrameGraph.h @@ -288,10 +288,8 @@ private: FrameGraphHandle createResourceNode(fg::ResourceEntryBase* resource) noexcept; - enum class DiscardPhase { START, END }; - backend::TargetBufferFlags computeDiscardFlags(DiscardPhase phase, - fg::PassNode const* curr, fg::PassNode const* first, - fg::RenderTarget const& renderTarget); + bool computeDiscard(const Vector fg::PassNode::* list, + fg::PassNode const* curr, fg::PassNode const* first, FrameGraphHandle resource); bool equals(FrameGraphRenderTarget::Descriptor const& cacheEntry, FrameGraphRenderTarget::Descriptor const& rt) const noexcept; diff --git a/filament/src/fg/ResourceAllocator.cpp b/filament/src/fg/ResourceAllocator.cpp index d331e130c1..23fae183ab 100644 --- a/filament/src/fg/ResourceAllocator.cpp +++ b/filament/src/fg/ResourceAllocator.cpp @@ -107,7 +107,7 @@ RenderTargetHandle ResourceAllocator::createRenderTarget(const char* name, } void ResourceAllocator::destroyRenderTarget(RenderTargetHandle h) noexcept { - return mBackend.destroyRenderTarget(h); + mBackend.destroyRenderTarget(h); } backend::TextureHandle ResourceAllocator::createTexture(const char* name, diff --git a/filament/src/fg/fg/RenderTarget.h b/filament/src/fg/fg/RenderTarget.h index f91889dc2a..a73a23d324 100644 --- a/filament/src/fg/fg/RenderTarget.h +++ b/filament/src/fg/fg/RenderTarget.h @@ -49,7 +49,7 @@ struct RenderTarget { // 32 backend::TargetBufferFlags userClearFlags{}; // set in compile - backend::RenderPassFlags targetFlags{}; + backend::TargetBufferFlags clear = {}; // this is eventually set by the user RenderTargetResource* cache = nullptr; void resolve(FrameGraph& fg) noexcept; diff --git a/filament/src/fg/fg/RenderTargetResource.cpp b/filament/src/fg/fg/RenderTargetResource.cpp index d05a9b3e62..1478f07f9e 100644 --- a/filament/src/fg/fg/RenderTargetResource.cpp +++ b/filament/src/fg/fg/RenderTargetResource.cpp @@ -22,7 +22,7 @@ using namespace backend; namespace fg { -void RenderTargetResource::create(FrameGraph& fg) noexcept { +void RenderTargetResource::preExecuteDevirtualize(FrameGraph& fg) noexcept { if (!imported) { if (any(attachments)) { // devirtualize our texture handles. By this point these handles have been @@ -61,7 +61,7 @@ void RenderTargetResource::create(FrameGraph& fg) noexcept { } } -void RenderTargetResource::destroy(FrameGraph& fg) noexcept { +void RenderTargetResource::postExecuteDestroy(FrameGraph& fg) noexcept { if (!imported) { if (targetInfo.target) { fg.getResourceAllocator().destroyRenderTarget(targetInfo.target); diff --git a/filament/src/fg/fg/RenderTargetResource.h b/filament/src/fg/fg/RenderTargetResource.h index 97b2caffa3..9e8d914135 100644 --- a/filament/src/fg/fg/RenderTargetResource.h +++ b/filament/src/fg/fg/RenderTargetResource.h @@ -69,9 +69,10 @@ struct RenderTargetResource final : public VirtualResource { // 104 // updated during execute with the current pass' discard flags FrameGraphPassResources::RenderTargetInfo targetInfo; - void create(FrameGraph& fg) noexcept override; - - void destroy(FrameGraph& fg) noexcept override; + void preExecuteDevirtualize(FrameGraph& fg) noexcept override; + void postExecuteDestroy(FrameGraph& fg) noexcept override; + void preExecuteDestroy(FrameGraph& fg) noexcept override {} + void postExecuteDevirtualize(FrameGraph& fg) noexcept override {} }; } // namespace fg diff --git a/filament/src/fg/fg/ResourceEntry.h b/filament/src/fg/fg/ResourceEntry.h index 0773b9d9fb..21878a5dd7 100644 --- a/filament/src/fg/fg/ResourceEntry.h +++ b/filament/src/fg/fg/ResourceEntry.h @@ -35,6 +35,14 @@ public: ResourceEntryBase(ResourceEntryBase const&) = default; ~ResourceEntryBase() override; + void preExecuteDestroy(FrameGraph& fg) noexcept override { + discardEnd = true; + } + + void postExecuteDevirtualize(FrameGraph& fg) noexcept override { + discardStart = false; + } + // constants const char* const name; const uint16_t id; // for debugging and graphing @@ -45,6 +53,10 @@ public: // computed during compile() uint32_t refs = 0; // final reference count + + // updated during execute() + bool discardStart = true; + bool discardEnd = false; }; @@ -68,13 +80,13 @@ public: T& getResource() noexcept { return resource; } - void create(FrameGraph& fg) noexcept override { + void preExecuteDevirtualize(FrameGraph& fg) noexcept override { if (!imported) { resource.create(fg, name, descriptor); } } - void destroy(FrameGraph& fg) noexcept override { + void postExecuteDestroy(FrameGraph& fg) noexcept override { if (!imported) { resource.destroy(fg); } diff --git a/filament/src/fg/fg/VirtualResource.h b/filament/src/fg/fg/VirtualResource.h index cfa5ff37ce..4b4285c626 100644 --- a/filament/src/fg/fg/VirtualResource.h +++ b/filament/src/fg/fg/VirtualResource.h @@ -28,8 +28,10 @@ struct PassNode; struct VirtualResource { VirtualResource() noexcept = default; VirtualResource(VirtualResource const&) = default; - virtual void create(FrameGraph& fg) noexcept = 0; - virtual void destroy(FrameGraph& fg) noexcept = 0; + virtual void preExecuteDevirtualize(FrameGraph& fg) noexcept = 0; + virtual void preExecuteDestroy(FrameGraph& fg) noexcept = 0; + virtual void postExecuteDestroy(FrameGraph& fg) noexcept = 0; + virtual void postExecuteDevirtualize(FrameGraph& fg) noexcept = 0; virtual ~VirtualResource(); // computed during compile() diff --git a/filament/test/filament_framegraph_test.cpp b/filament/test/filament_framegraph_test.cpp index 170efc815e..1425e8a5f8 100644 --- a/filament/test/filament_framegraph_test.cpp +++ b/filament/test/filament_framegraph_test.cpp @@ -60,8 +60,8 @@ TEST(FrameGraphTest, SimpleRenderPass) { renderPassExecuted = true; auto const& rt = resources.getRenderTarget(data.rt); EXPECT_TRUE(rt.target); - EXPECT_EQ(TargetBufferFlags::ALL, rt.params.flags.discardStart); - EXPECT_EQ(TargetBufferFlags::DEPTH_AND_STENCIL, rt.params.flags.discardEnd); + EXPECT_EQ(TargetBufferFlags::COLOR, rt.params.flags.discardStart); + EXPECT_EQ(TargetBufferFlags::NONE, rt.params.flags.discardEnd); }); @@ -113,8 +113,8 @@ TEST(FrameGraphTest, SimpleRenderPass2) { renderPassExecuted = true; auto const& rt = resources.getRenderTarget(data.rt); EXPECT_TRUE(rt.target); - EXPECT_EQ(TargetBufferFlags::ALL, rt.params.flags.discardStart); - EXPECT_EQ(TargetBufferFlags::STENCIL, rt.params.flags.discardEnd); + EXPECT_EQ(TargetBufferFlags::COLOR_AND_DEPTH, rt.params.flags.discardStart); + EXPECT_EQ(TargetBufferFlags::NONE, rt.params.flags.discardEnd); }); fg.present(renderPass.getData().outColor); @@ -158,8 +158,8 @@ TEST(FrameGraphTest, ScenarioDepthPrePass) { depthPrepassExecuted = true; auto const& rt = resources.getRenderTarget(data.rt); EXPECT_TRUE(rt.target); - EXPECT_EQ(TargetBufferFlags::ALL, rt.params.flags.discardStart); - EXPECT_EQ(TargetBufferFlags::COLOR_AND_STENCIL, rt.params.flags.discardEnd); + EXPECT_EQ(TargetBufferFlags::DEPTH, rt.params.flags.discardStart); + EXPECT_EQ(TargetBufferFlags::NONE, rt.params.flags.discardEnd); }); struct ColorPassData { @@ -195,8 +195,8 @@ TEST(FrameGraphTest, ScenarioDepthPrePass) { colorPassExecuted = true; auto const& rt = resources.getRenderTarget(data.rt); EXPECT_TRUE(rt.target); - EXPECT_EQ(TargetBufferFlags::COLOR_AND_STENCIL, rt.params.flags.discardStart); - EXPECT_EQ(TargetBufferFlags::DEPTH_AND_STENCIL, rt.params.flags.discardEnd); + EXPECT_EQ(TargetBufferFlags::COLOR, rt.params.flags.discardStart); + EXPECT_EQ(TargetBufferFlags::DEPTH, rt.params.flags.discardEnd); }); fg.present(colorPass.getData().outColor); @@ -235,8 +235,8 @@ TEST(FrameGraphTest, SimplePassCulling) { renderPassExecuted = true; auto const& rt = resources.getRenderTarget(data.rt); EXPECT_TRUE(rt.target); - EXPECT_EQ(TargetBufferFlags::ALL, rt.params.flags.discardStart); - EXPECT_EQ(TargetBufferFlags::DEPTH_AND_STENCIL, rt.params.flags.discardEnd); + EXPECT_EQ(TargetBufferFlags::COLOR, rt.params.flags.discardStart); + EXPECT_EQ(TargetBufferFlags::NONE, rt.params.flags.discardEnd); }); @@ -259,8 +259,8 @@ TEST(FrameGraphTest, SimplePassCulling) { postProcessPassExecuted = true; auto const& rt = resources.getRenderTarget(data.rt); EXPECT_TRUE(rt.target); - EXPECT_EQ(TargetBufferFlags::ALL, rt.params.flags.discardStart); - EXPECT_EQ(TargetBufferFlags::DEPTH_AND_STENCIL, rt.params.flags.discardEnd); + EXPECT_EQ(TargetBufferFlags::COLOR, rt.params.flags.discardStart); + EXPECT_EQ(TargetBufferFlags::NONE, rt.params.flags.discardEnd); }); @@ -362,8 +362,8 @@ TEST(FrameGraphTest, RenderTargetLifetime) { rt1 = rt.target; EXPECT_TRUE(rt.target); EXPECT_EQ(TargetBufferFlags::COLOR, rt.params.flags.clear); - EXPECT_EQ(TargetBufferFlags::ALL, rt.params.flags.discardStart); - EXPECT_EQ(TargetBufferFlags::DEPTH_AND_STENCIL, rt.params.flags.discardEnd); + EXPECT_EQ(TargetBufferFlags::COLOR, rt.params.flags.discardStart); + EXPECT_EQ(TargetBufferFlags::NONE, rt.params.flags.discardEnd); }); auto& renderPass2 = fg.addPass("Render2", @@ -381,8 +381,8 @@ TEST(FrameGraphTest, RenderTargetLifetime) { EXPECT_TRUE(rt.target); EXPECT_EQ(TargetBufferFlags::NONE, rt.params.flags.clear); EXPECT_EQ(rt1.getId(), rt.target.getId()); - EXPECT_EQ(TargetBufferFlags::DEPTH_AND_STENCIL, rt.params.flags.discardStart); - EXPECT_EQ(TargetBufferFlags::DEPTH_AND_STENCIL, rt.params.flags.discardEnd); + EXPECT_EQ(TargetBufferFlags::NONE, rt.params.flags.discardStart); + EXPECT_EQ(TargetBufferFlags::NONE, rt.params.flags.discardEnd); });