Skip to content

Commit

Permalink
fix a use-after-free of texture handles
Browse files Browse the repository at this point in the history
Because ColorPassDescriptorSet is used for both the color pass and the
picking/ssr/structure passes, the texture it holds can be stale.

In this CL we fix this by reseting the offending textures when preparing
the picking/ssr/structure passes. This has a side effect of recreating
the descriptor-set internally. A better fix would be to use separate
descriptor-sets for these two cases and rely on the texture cache
to avoid recreation from a frame to the next.

BUGS=[376705346]
  • Loading branch information
pixelflinger committed Dec 7, 2024
1 parent 6bb7f89 commit 6b7080e
Show file tree
Hide file tree
Showing 5 changed files with 23 additions and 0 deletions.
5 changes: 5 additions & 0 deletions filament/src/details/Renderer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -963,6 +963,11 @@ void FRenderer::renderJob(RootArenaScope& rootArenaScope, FView& view) {
uint32_t(float(xvp.width ) * aoOptions.resolution),
uint32_t(float(xvp.height) * aoOptions.resolution)});

// this needs to reset the sampler that are only set in RendererUtils::colorPass(), because
// this descriptor-set is also used for ssr/picking/structure and these could be stale
// it would be better to use a separate desriptor-set for those two cases so that we don't
// have to do this
view.unbindSamplers(driver);
view.commitUniformsAndSamplers(driver);
});

Expand Down
4 changes: 4 additions & 0 deletions filament/src/details/View.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -912,6 +912,10 @@ void FView::commitUniformsAndSamplers(DriverApi& driver) const noexcept {
mColorPassDescriptorSet.commit(driver);
}

void FView::unbindSamplers(DriverApi& driver) noexcept {
mColorPassDescriptorSet.unbindSamplers(driver);
}

void FView::commitFroxels(DriverApi& driverApi) const noexcept {
if (mHasDynamicLighting) {
mFroxelizer.commit(driverApi);
Expand Down
1 change: 1 addition & 0 deletions filament/src/details/View.h
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,7 @@ class FView : public View {

void commitFroxels(backend::DriverApi& driverApi) const noexcept;
void commitUniformsAndSamplers(backend::DriverApi& driver) const noexcept;
void unbindSamplers(backend::DriverApi& driver) noexcept;

utils::JobSystem::Job* getFroxelizerSync() const noexcept { return mFroxelizerSync; }
void setFroxelizerSync(utils::JobSystem::Job* sync) noexcept { mFroxelizerSync = sync; }
Expand Down
11 changes: 11 additions & 0 deletions filament/src/ds/ColorPassDescriptorSet.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -541,6 +541,17 @@ void ColorPassDescriptorSet::commit(backend::DriverApi& driver) noexcept {
}
}

void ColorPassDescriptorSet::unbindSamplers(DriverApi&) noexcept {
// this needs to reset the sampler that are only set in RendererUtils::colorPass(), because
// this descriptor-set is also used for ssr/picking/structure and these could be stale
// it would be better to use a separate descriptor-set for those two cases so that we don't
// have to do this
setSampler(+PerViewBindingPoints::STRUCTURE, {}, {});
setSampler(+PerViewBindingPoints::SHADOW_MAP, {}, {});
setSampler(+PerViewBindingPoints::SSAO, {}, {});
setSampler(+PerViewBindingPoints::SSR, {}, {});
}

void ColorPassDescriptorSet::setSampler(backend::descriptor_binding_t binding,
TextureHandle th, SamplerParams params) noexcept {
for (size_t i = 0; i < DESCRIPTOR_LAYOUT_COUNT; i++) {
Expand Down
2 changes: 2 additions & 0 deletions filament/src/ds/ColorPassDescriptorSet.h
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,8 @@ class ColorPassDescriptorSet {
// update local data into GPU UBO
void commit(backend::DriverApi& driver) noexcept;

void unbindSamplers(backend::DriverApi& driver) noexcept;

// bind this UBO
void bind(backend::DriverApi& driver, uint8_t index) const noexcept {
mDescriptorSet[index].bind(driver, DescriptorSetBindingPoints::PER_VIEW);
Expand Down

0 comments on commit 6b7080e

Please sign in to comment.