diff --git a/filament/src/fg/PassNode.cpp b/filament/src/fg/PassNode.cpp index a28c2352a0..8515c53c97 100644 --- a/filament/src/fg/PassNode.cpp +++ b/filament/src/fg/PassNode.cpp @@ -151,7 +151,12 @@ void RenderPassNode::resolve() noexcept { rt.targetBufferFlags |= target; - if (!rt.outgoing[i] || !rt.outgoing[i]->hasActiveReaders()) { + // Discard at the end only if we are writing to this attachment AND later reading + // from it. (in particular, don't discard if we're not writing at all, because this + // attachment might have other readers after us). + // TODO: we could set the discard flag if we are the last reader, i.e. + // if rt->incoming[i] last reader is us. + if (rt.outgoing[i] && !rt.outgoing[i]->hasActiveReaders()) { rt.backend.params.flags.discardEnd |= target; } if (!rt.outgoing[i] || !rt.outgoing[i]->hasWriterPass()) { @@ -161,10 +166,10 @@ void RenderPassNode::resolve() noexcept { rt.backend.params.readOnlyDepthStencil |= RenderPassParams::READONLY_STENCIL; } } + // Discard at the start if this attachment has no prior writer if (!rt.incoming[i] || !rt.incoming[i]->hasActiveWriters()) { rt.backend.params.flags.discardStart |= target; } - VirtualResource* pResource = mFrameGraph.getResource(rt.descriptor.attachments.array[i]); Resource* pTextureResource = static_cast*>(pResource); diff --git a/filament/test/filament_framegraph_test.cpp b/filament/test/filament_framegraph_test.cpp index d5463a9041..32eb3a6ed7 100644 --- a/filament/test/filament_framegraph_test.cpp +++ b/filament/test/filament_framegraph_test.cpp @@ -890,3 +890,55 @@ TEST_F(FrameGraphTest, SubResourcesWriteRead) { fg.execute(driverApi); } + +TEST_F(FrameGraphTest, WriteResourceReadAsAttachment) { + + // this check s that using a resources as a read-only attachment doesn't set the + // discard flag on that resource. i.e. that the mere fact of reading a resource as an + // attachment doesn't make it invalid. + // see: https://github.com/google/filament/issues/5005 + + struct PassData { + FrameGraphId input; + }; + auto& writePass = fg.addPass("Write Pass", [&](FrameGraph::Builder& builder, auto& data) { + data.input = builder.create("Output buffer", {.width=16, .height=32}); + data.input = builder.write(data.input, FrameGraphTexture::Usage::COLOR_ATTACHMENT); + builder.declareRenderPass("Output target", { .attachments = { + .color = { data.input } + }}); + builder.sideEffect(); + }, + [=](FrameGraphResources const& resources, auto const&, backend::DriverApi&) { + auto rt = resources.getRenderPassInfo(); + EXPECT_EQ(rt.params.flags.discardStart, TargetBufferFlags::COLOR0); + EXPECT_EQ(rt.params.flags.discardEnd, TargetBufferFlags::NONE); + }); + + fg.addPass("Read Pass", [&](FrameGraph::Builder& builder, auto& data) { + data.input = builder.read(writePass->input, FrameGraphTexture::Usage::COLOR_ATTACHMENT); + builder.declareRenderPass("Input target", { .attachments = { + .color = { data.input } + }}); + builder.sideEffect(); + }, + [=](FrameGraphResources const& resources, auto const&, backend::DriverApi&) { + auto rt = resources.getRenderPassInfo(); + EXPECT_EQ(rt.params.flags.discardStart, TargetBufferFlags::NONE); + EXPECT_EQ(rt.params.flags.discardEnd, TargetBufferFlags::NONE); + }); + + fg.addPass("Sample Pass", [&](FrameGraph::Builder& builder, auto& data) { + data.input = builder.sample(writePass->input); + builder.sideEffect(); + }, + [=](FrameGraphResources const& resources, auto const&, backend::DriverApi&) { + }); + + + EXPECT_TRUE(fg.isAcyclic()); + + fg.compile(); + + fg.execute(driverApi); +}