fix a discard flags issue

An attachment would be wrongly discarded if used as read-only: because
it didn't have a dependency to it, the discard flag was set.
This is fixed by setting the discard flag only if a resource is written
and has dependencies to it.

Should fix: #5005
This commit is contained in:
Mathias Agopian
2023-03-06 22:39:11 -08:00
committed by Mathias Agopian
parent 6f25e8ae5a
commit efd9e875db
2 changed files with 59 additions and 2 deletions
+7 -2
View File
@@ -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<FrameGraphTexture>* pTextureResource = static_cast<Resource<FrameGraphTexture>*>(pResource);
@@ -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<FrameGraphTexture> input;
};
auto& writePass = fg.addPass<PassData>("Write Pass", [&](FrameGraph::Builder& builder, auto& data) {
data.input = builder.create<FrameGraphTexture>("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<PassData>("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<PassData>("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);
}