From f192b89ef917d01ba32f06f2b097c5fd3f045f31 Mon Sep 17 00:00:00 2001 From: Mathias Agopian Date: Wed, 28 Aug 2019 19:06:06 -0700 Subject: [PATCH] sample count for rendertargets doesn't need to be specified when creating a rendertarget, if the sample count is not specified, any existing target with the same attachment will match. this simplifies both the user and implementation. getRenderTargetDescriptor() now takes a RenderTargetHandle instead of a texture handle. --- filament/src/PostProcessManager.cpp | 6 +- filament/src/fg/FrameGraph.cpp | 84 +++++++++------------------ filament/src/fg/FrameGraph.h | 10 +--- filament/src/fg/FrameGraphHandle.h | 2 +- filament/src/fg/ResourceAllocator.cpp | 2 +- filament/src/fg/fg/ResourceNode.h | 4 -- 6 files changed, 35 insertions(+), 73 deletions(-) diff --git a/filament/src/PostProcessManager.cpp b/filament/src/PostProcessManager.cpp index b93cf2cc6d..ad3eba05c2 100644 --- a/filament/src/PostProcessManager.cpp +++ b/filament/src/PostProcessManager.cpp @@ -272,8 +272,7 @@ FrameGraphId PostProcessManager::resolve( data.input = builder.read(input, true); data.srt = builder.createRenderTarget(builder.getName(data.input), - { .attachments.color = { data.input }, - .samples = builder.getSamples(data.input) + { .attachments.color = { data.input } }); data.output = builder.createTexture("resolve output", { @@ -312,8 +311,7 @@ FrameGraphId PostProcessManager::dynamicScaling(FrameGraph& f data.input = builder.read(input, true); data.srt = builder.createRenderTarget(builder.getName(data.input), - { .attachments.color = { data.input }, - .samples = builder.getSamples(data.input) + { .attachments.color = { data.input } }); data.output = builder.createTexture("scale output", { diff --git a/filament/src/fg/FrameGraph.cpp b/filament/src/fg/FrameGraph.cpp index d2e9f3425a..2659567bf0 100644 --- a/filament/src/fg/FrameGraph.cpp +++ b/filament/src/fg/FrameGraph.cpp @@ -73,17 +73,10 @@ bool FrameGraph::Builder::isAttachment(FrameGraphId r) const } FrameGraphRenderTarget::Descriptor const& -FrameGraph::Builder::getRenderTargetDescriptor(FrameGraphId attachment) const { +FrameGraph::Builder::getRenderTargetDescriptor(FrameGraphRenderTargetHandle handle) const { FrameGraph& fg = mFrameGraph; - ResourceNode& node = fg.getResourceNodeUnchecked(attachment); - ASSERT_POSTCONDITION(node.renderTargetIndex != ResourceNode::UNINITIALIZED, - "Resource \"%s\" isn't a render target attachment", node.resource->name); - assert(node.renderTargetIndex < fg.mRenderTargets.size()); - return fg.mRenderTargets[node.renderTargetIndex].desc; -} - -uint8_t FrameGraph::Builder::getSamples(FrameGraphId r) const noexcept { - return isAttachment(r) ? getRenderTargetDescriptor(r).samples : 1; + assert(handle < fg.mRenderTargets.size()); + return fg.mRenderTargets[handle].desc; } FrameGraphRenderTargetHandle FrameGraph::Builder::createRenderTarget(const char* name, @@ -112,10 +105,6 @@ FrameGraphRenderTargetHandle FrameGraph::Builder::createRenderTarget(const char* // figure out the attachment flags fg::ResourceEntry& entry = fg.getResourceEntry(attachmentInfo.getHandle()); entry.descriptor.usage |= usages[i]; - - // renderTargetIndex is used to retrieve the Descriptor - ResourceNode& node = fg.getResourceNode(attachmentInfo.getHandle()); - node.renderTargetIndex = renderTarget.index; } } return FrameGraphRenderTargetHandle(renderTarget.index); @@ -125,8 +114,7 @@ FrameGraphRenderTargetHandle FrameGraph::Builder::createRenderTarget(FrameGraphI TargetBufferFlags clearFlags) noexcept { texture = this->write(texture); return createRenderTarget(getName(texture), { - .attachments.color = texture, - .samples = getSamples(texture) + .attachments.color = texture }, clearFlags); } @@ -170,45 +158,24 @@ fg::ResourceEntryBase const& FrameGraphPassResources::getResourceEntryBase(Frame FrameGraphPassResources::RenderTargetInfo FrameGraphPassResources::getRenderTarget(FrameGraphRenderTargetHandle handle, uint8_t level) const noexcept { - FrameGraphPassResources::RenderTargetInfo info{}; FrameGraph& fg = mFrameGraph; - fg::RenderTarget* const renderTarget = &fg.mRenderTargets[handle]; + fg::RenderTarget& renderTarget = fg.mRenderTargets[handle]; + assert(renderTarget.cache); + + info = renderTarget.cache->targetInfo; + 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.dependencies = renderTarget.targetFlags.dependencies; -// auto const& resourceNodes = fg.mResourceNodes; -// auto const& renderTargets = fg.mRenderTargets; -// ResourceEntryBase const* const pResource = resourceNodes[r.index].resource; -// -// // find a rendertarget in this pass that has this resource has attachment -// -// // TODO: for cubemaps/arrays, we'll need to be able to specify the face/index -// -// for (auto i : mPass.renderTargets) { -// fg::RenderTarget const* const renderTarget = &renderTargets[i]; -// auto const& desc = renderTarget->desc; -// auto pos = std::find_if( -// desc.attachments.textures.begin(), -// desc.attachments.textures.end(), -// [pResource, &resourceNodes, level](FrameGraphRenderTarget::Attachments::AttachmentInfo const& info) { -// return info.isValid() && resourceNodes[info.getHandle().index].resource == pResource && info.getLevel() == level; -// }); -// if (pos != std::end(desc.attachments.textures)) { - assert(renderTarget->cache); - info = renderTarget->cache->targetInfo; - // 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.dependencies = renderTarget->targetFlags.dependencies; - assert(info.target); -// break; -// } -// } - // 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", - mPass.name, renderTarget->name); + mPass.name, renderTarget.name); // slog.d << mPass.name << ": resource = \"" << renderTarget.name << "\", flags = " // << io::hex @@ -227,10 +194,8 @@ FrameGraph::FrameGraph(fg::ResourceAllocator& resourceAllocator) mResourceNodes(mArena), mRenderTargets(mArena), mAliases(mArena), -// mResourceRegistry(mArena), mResourceEntries(mArena), - mRenderTargetCache(mArena) - { + mRenderTargetCache(mArena) { // slog.d << "PassNode: " << sizeof(PassNode) << io::endl; // slog.d << "ResourceNode: " << sizeof(ResourceNode) << io::endl; // slog.d << "Resource: " << sizeof(Resource) << io::endl; @@ -329,12 +294,19 @@ fg::ResourceEntryBase& FrameGraph::getResourceEntryBaseUnchecked(FrameGraphHandl return *node.resource; } -bool FrameGraph::equals(FrameGraphRenderTarget::Descriptor const& lhs, - FrameGraphRenderTarget::Descriptor const& rhs) const noexcept { +bool FrameGraph::equals(FrameGraphRenderTarget::Descriptor const& cacheEntry, + FrameGraphRenderTarget::Descriptor const& rt) const noexcept { const Vector& resourceNodes = mResourceNodes; + + // if the rendertarget we're looking up doesn't has the sample field set to 0, it means the + // user didn't specify it, and it's okay to match it to any sample count. + // Otherwise, sample count must match, with the caveat that 0 or 1 are treated the same. + const bool samplesMatch = (!rt.samples) || + (std::max(1u, cacheEntry.samples) == std::max(1u, rt.samples)); + return std::equal( - lhs.attachments.textures.begin(), lhs.attachments.textures.end(), - rhs.attachments.textures.begin(), rhs.attachments.textures.end(), + cacheEntry.attachments.textures.begin(), cacheEntry.attachments.textures.end(), + rt.attachments.textures.begin(), rt.attachments.textures.end(), [&resourceNodes]( FrameGraphRenderTarget::Attachments::AttachmentInfo lhs, FrameGraphRenderTarget::Attachments::AttachmentInfo rhs) { @@ -359,7 +331,7 @@ bool FrameGraph::equals(FrameGraphRenderTarget::Descriptor const& lhs, return true; } return false; - }) && lhs.samples == rhs.samples; + }) && samplesMatch; } FrameGraphId FrameGraph::importResource(const char* name, diff --git a/filament/src/fg/FrameGraph.h b/filament/src/fg/FrameGraph.h index 33b5e765c5..96077ad562 100644 --- a/filament/src/fg/FrameGraph.h +++ b/filament/src/fg/FrameGraph.h @@ -122,16 +122,12 @@ public: return mFrameGraph.getDescriptor(r); } - // return a render target's attachment sample count. Returns 1 if the resource - // is not an attachement to some rendertarget - uint8_t getSamples(FrameGraphId r) const noexcept; - // returns whether this texture resource is an attachment to some rendertarget bool isAttachment(FrameGraphId r) const noexcept; // returns the descriptor of the render target this attachment belongs to FrameGraphRenderTarget::Descriptor const& getRenderTargetDescriptor( - FrameGraphId attachment) const; + FrameGraphRenderTargetHandle handle) const; private: friend class FrameGraph; @@ -263,8 +259,8 @@ private: fg::PassNode const* curr, fg::PassNode const* first, fg::RenderTarget const& renderTarget); - bool equals(FrameGraphRenderTarget::Descriptor const& lhs, - FrameGraphRenderTarget::Descriptor const& rhs) const noexcept; + bool equals(FrameGraphRenderTarget::Descriptor const& cacheEntry, + FrameGraphRenderTarget::Descriptor const& rt) const noexcept; void executeInternal(fg::PassNode const& node, backend::DriverApi& driver) noexcept; diff --git a/filament/src/fg/FrameGraphHandle.h b/filament/src/fg/FrameGraphHandle.h index efc180a4be..0b629474e3 100644 --- a/filament/src/fg/FrameGraphHandle.h +++ b/filament/src/fg/FrameGraphHandle.h @@ -150,7 +150,7 @@ struct Attachments { struct Descriptor { Attachments attachments; Viewport viewport; - uint8_t samples = 1; // # of samples + uint8_t samples = 0; // # of samples (0 = unset, default) }; } // namespace FrameGraphRenderTarget diff --git a/filament/src/fg/ResourceAllocator.cpp b/filament/src/fg/ResourceAllocator.cpp index cf60faea22..d7e2b4ce3f 100644 --- a/filament/src/fg/ResourceAllocator.cpp +++ b/filament/src/fg/ResourceAllocator.cpp @@ -102,7 +102,7 @@ RenderTargetHandle ResourceAllocator::createRenderTarget( uint8_t samples, TargetBufferInfo color, TargetBufferInfo depth, TargetBufferInfo stencil) noexcept { return mBackend.createRenderTarget(targetBufferFlags, - width, height, samples, color, depth, stencil); + width, height, samples ? samples : 1u, color, depth, stencil); } void ResourceAllocator::destroyRenderTarget(RenderTargetHandle h) noexcept { diff --git a/filament/src/fg/fg/ResourceNode.h b/filament/src/fg/fg/ResourceNode.h index 286bc2123e..724e8dd2b8 100644 --- a/filament/src/fg/fg/ResourceNode.h +++ b/filament/src/fg/fg/ResourceNode.h @@ -39,10 +39,6 @@ struct ResourceNode { // 24 PassNode* writer = nullptr; // writer to this node uint32_t readerCount = 0; // # of passes reading from this resource - // updated by builder - static constexpr uint16_t UNINITIALIZED = std::numeric_limits::max(); - uint16_t renderTargetIndex = UNINITIALIZED; // used to retrieve the RT infos - // constants const uint8_t version; // version of the resource when the node was created };