Skip to content

fix: read by direction when resampling the prefiltered radiance highest level - #3341

Open
sasvdw wants to merge 3 commits into
stride3d:masterfrom
LazyWorksZA:fix/ggx-prefilter-mip0-per-face-rescale
Open

fix: read by direction when resampling the prefiltered radiance highest level#3341
sasvdw wants to merge 3 commits into
stride3d:masterfrom
LazyWorksZA:fix/ggx-prefilter-mip0-per-face-rescale

Conversation

@sasvdw

@sasvdw sasvdw commented Aug 8, 2026

Copy link
Copy Markdown
Contributor

PR Details

Summary

Mip 0 of the prefiltered cubemap lost continuity around every face edge whenever the copy shortcut did not
apply, because the fallback rescaled each face on its own.

Description

RadiancePrefilteringGGXNoCompute fills mip 0 from the source when DoNotFilterHighestLevel is set. The
copy at :103 serves that only while the sizes, the mip count and the format agree. Everything else went
through ImageScaler over a single face view, which clamps at the face border, so the border texels never
saw the neighboring face.

This replaces that fallback with CubemapFaceResampleShader, which reads the source by direction. A
border texel then crosses the face edge the way the hardware samples a cubemap.

Falling through to the existing filter would look like the tidier fix, and it is what the compute path
appears to do, but it does not work. At roughness 0 the GGX distribution returns a zero probability
density, the solid angle goes to infinity, and the sample mip clamps to MipmapCount
(RadiancePrefilteringGGXNoComputeShader.sdsl:41-44). Every texel reads the smallest mipmap and the level
becomes one flat color. That is why mip 0 carries a special case at all.

Measured over the 12 shared face edges, against the step between neighboring texels inside a face:

within-face seam ratio
before, ImageScaler 4.03 0.86
falling through to the filter 0.00 0.00
after, resample by direction 5.16 0.11

The middle row is the trap. A flat level reports no seam, so a continuity check alone would call it fixed.
The test asserts the level still carries an image for that reason.

Motivation and context — see #3334.

Related Issue

Fixes #3334.

Types of changes

  • Docs change / refactoring / dependency upgrade
  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist

  • My change requires a change to the documentation.
  • I have added tests to cover my changes.
  • All new and existing tests passed.
  • I have built and run the editor to try this change out.

Validation status

  • Tests: TestRadiancePrefilteringGgxFaceContinuity drives the filter with an output format that differs
    from the source, which is the case SkyboxGenerator creates for a low dynamic range skybox. It measures
    the step across the 12 shared face edges and asserts the level still carries an image. Red at 0.86 before
    the change, green at 0.11 after, verified both ways.
  • No gold image moves. Every cubemap in the tree takes the copy path, so nothing that is captured today
    reaches the changed branch. Stride.Graphics.Tests.11_0 passes 5 of 5, including
    TestRadiancePrefilteringGgx. The skybox cases in Stride.Graphics.Tests.10_0 pass 3 of 3.
  • Not covered: the same roughness 0 problem sits in the compute path if its size check ever fails, and
    that path also skips the format check the no-compute path makes. Both are described in the issue and left
    alone here.
  • Editor: not exercised.

Comment on lines +127 to +130
[SkippableFact]
public void HighestLevelIsContinuousAcrossFaces()
{
SkipTestForGraphicPlatform(GraphicsPlatform.Vulkan);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this skipped for Vulkan?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was a bad copy on my end. Thought I was still working with the code from another branch where I had the fixes living, for skipping some related tests and ended up duplicating the test suite without a proper look.

sasvdw and others added 3 commits August 9, 2026 21:38
Fails today. RadiancePrefilteringGGXNoCompute fills mip 0 with a per-face
ImageScaler rescale when it cannot take its copy shortcut. That rescale reads a
single face view and clamps at the face border, so the border texels never see
the neighboring face.

The test drives the filter with an output format that differs from the source,
which is the condition SkyboxGenerator creates for a low dynamic range skybox. It
then measures the step across the 12 shared face edges against the step between
neighboring texels inside a face. Mip 0 measures 0.86, while the levels the
filter produces stay near 0.32.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…st level

RadiancePrefilteringGGXNoCompute fills mip 0 from the source when
DoNotFilterHighestLevel is set. A straight copy serves that only while the sizes,
the mip count and the format all line up. Everything else went through
ImageScaler over a single face view, which clamps at the face border, so the
border texels never saw the neighboring face. SkyboxGenerator creates exactly
that case for a low dynamic range skybox, because it fixes the filtering format
at R8G8B8A8_UNorm while the source compiles to a block format.

The filter cannot serve this level either. At roughness 0 the GGX distribution
gives a zero probability density, the solid angle goes to infinity and the sample
mip clamps to the smallest level, so every texel returns one averaged color.

CubemapFaceResampleShader reads the source by direction instead, which crosses
face boundaries the way the hardware samples a cubemap. Measured over the 12
shared face edges, against the step between neighboring texels inside a face, the
level goes from 0.86 to 0.11 and keeps its detail.

No gold image moves. Every cubemap in the tree takes the copy path.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The test carried SkipTestForGraphicPlatform(Vulkan), copied from the neighboring
test without a reason of its own. stride3d#3311 removed the Vulkan skips from this suite,
including the one on TestRadiancePrefilteringGgx, so keeping this one puts back
the pattern that work removed.

Nothing here is backend specific. The test drives the pixel shader path, writes a
render target cubemap array, reads it back and asserts on numbers. It takes no
screenshot, so no gold image is involved.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@sasvdw
sasvdw force-pushed the fix/ggx-prefilter-mip0-per-face-rescale branch from 5ceebdc to 8267444 Compare August 9, 2026 19:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Prefiltered radiance mip 0 rescales each cube face on its own, so face edges lose their neighbors

2 participants