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); });