From 2b620e65fdb71ddcc5d9bb758fab8bbf542345d6 Mon Sep 17 00:00:00 2001 From: Mathias Agopian Date: Fri, 30 Aug 2024 14:48:47 -0700 Subject: [PATCH] fix potential framegraph textures use-after free make sure to unset all textures in the per-view sampler group after they are used, because the resource could be destroyed after the pass is finished - unset the fog and ibl_specular after the color pass - move that cleanup a bit earlier - in the case of screen-space reflection the structure pass is set, but might not be used in the color pass, so we also need to unset it after the SSR pass and before any other passes. --- filament/src/PerViewUniforms.cpp | 4 +++- filament/src/details/Renderer.cpp | 19 ++++++++++++------- 2 files changed, 15 insertions(+), 8 deletions(-) diff --git a/filament/src/PerViewUniforms.cpp b/filament/src/PerViewUniforms.cpp index eaf8df8cb4..35867ea855 100644 --- a/filament/src/PerViewUniforms.cpp +++ b/filament/src/PerViewUniforms.cpp @@ -459,10 +459,12 @@ void PerViewUniforms::bind(backend::DriverApi& driver) noexcept { void PerViewUniforms::unbindSamplers() noexcept { auto& samplerGroup = mSamplers; + samplerGroup.clearSampler(PerViewSib::SHADOW_MAP); + samplerGroup.clearSampler(PerViewSib::IBL_SPECULAR); samplerGroup.clearSampler(PerViewSib::SSAO); samplerGroup.clearSampler(PerViewSib::SSR); samplerGroup.clearSampler(PerViewSib::STRUCTURE); - samplerGroup.clearSampler(PerViewSib::SHADOW_MAP); + samplerGroup.clearSampler(PerViewSib::FOG); } } // namespace filament diff --git a/filament/src/details/Renderer.cpp b/filament/src/details/Renderer.cpp index 9348ed9e0c..a8e71b48dd 100644 --- a/filament/src/details/Renderer.cpp +++ b/filament/src/details/Renderer.cpp @@ -1019,6 +1019,10 @@ void FRenderer::renderJob(RootArenaScope& rootArenaScope, FView& view) { { .width = svp.width, .height = svp.height }); if (UTILS_LIKELY(reflections)) { + fg.addTrivialSideEffectPass("SSR Cleanup", [&view](DriverApi& driver) { + view.getPerViewUniforms().prepareStructure({}); + view.commitUniforms(driver); + }); // generate the mipchain PostProcessManager::generateMipmapSSR(ppm, fg, reflections, ssrConfig.reflection, false, ssrConfig); @@ -1134,6 +1138,12 @@ void FRenderer::renderJob(RootArenaScope& rootArenaScope, FView& view) { } } + fg.addTrivialSideEffectPass("Finish Color Passes", [&view](DriverApi& driver) { + // Unbind SSAO sampler, b/c the FrameGraph will delete the texture at the end of the pass. + view.cleanupRenderPasses(); + view.commitUniforms(driver); + }); + if (colorGradingConfig.customResolve) { assert_invariant(fg.getDescriptor(colorPassOutput.linearColor).samples <= 1); // TODO: we have to "uncompress" (i.e. detonemap) the color buffer here because it's used @@ -1172,16 +1182,11 @@ void FRenderer::renderJob(RootArenaScope& rootArenaScope, FView& view) { // this is the output of the color pass / input to post processing, // this is only used later for comparing it with the output after post-processing FrameGraphId const postProcessInput = colorGradingConfig.asSubpass ? - colorPassOutput.tonemappedColor : - colorPassOutput.linearColor; + colorPassOutput.tonemappedColor : + colorPassOutput.linearColor; // input can change below FrameGraphId input = postProcessInput; - fg.addTrivialSideEffectPass("Finish Color Passes", [&view](DriverApi& driver) { - // Unbind SSAO sampler, b/c the FrameGraph will delete the texture at the end of the pass. - view.cleanupRenderPasses(); - view.commitUniforms(driver); - }); // Resolve depth -- which might be needed because of TAA or DoF. This pass will be culled // if the depth is not used below or if the depth is not MS (e.g. it could have been