Skip to content

fix(shader): keep cube image gradients out of the face coordinate bias - #4209

Merged
adriadam10 merged 2 commits into
boykopovar:mainfrom
karanvora2599:fix/shader-cube-gradients
Oct 13, 2026
Merged

adriadam10 merged 2 commits into
boykopovar:mainfrom
karanvora2599:fix/shader-cube-gradients

Conversation

@karanvora2599

Copy link
Copy Markdown
Contributor

What

image_sample_d and image_sample_cd on a cube image now pass their gradients to the host sample unchanged. Before, the gradients were run through the coordinate conversion and lost 1.0 on each axis, so the sample read the smallest mip whatever the gradient.

Background. Cubes are bound as 2D arrays. CoordF32 maps AMD's cube face coordinates from their [1, 2] range to [0, 1] by subtracting 1.0 from x and y (CubeAxis). The derivative path builds the gradient operands with the same function (SpirvImageEmitter.cpp, the RdnaImageSampleFlagDerivative branch of the sample emitter), so a cube's gradients were biased too. A gradient is a difference of coordinates, so the bias doesn't belong there. A gradient of one texel, 1/16 on a 16×16 face, became about -0.94, and the LOD came out near the last level.

Source. Mesa's AMD texture lowering, src/amd/common/nir/ac_nir_lower_tex_coords.c, prepare_cube_coords:

  • It converts cube derivatives to 2D face derivatives: deriv_sc * invma - deriv_ma * sc, and the same for tc.
  • It adds 1.5 only to the coordinates (sc + 1.5, tc + 1.5), never to the derivatives.

So the hardware takes unbiased face-space gradients next to [1, 2] coordinates.

How. CoordF32 takes a gradient flag, and the two gradient operands pass it, which skips CubeAxis. Coordinates, image_get_lod and every non-cube image are unchanged.

Tested

Windows 11, MinGW-w64 GCC 15.2.0, Release. Devices:

  • RTX 4060 Laptop (Ada, driver 591.86)
  • Intel UHD Graphics (Raptor Lake, driver 101.6790)
  • lavapipe from Mesa 25.2.8 (llvmpipe, LLVM 21.1.7): the version CI installs; Windows build with GALLIUM_OVERRIDE_CPU_CAPS=avx.

This round's four changes were built and tested together on main at bca7a29 (cube image gradients, early depth tests, bindless table search, LDS fence; one pull request each, in unrelated code). The branch is rebased onto 34ba918. Its two new commits touch only GuestMemory.cpp and the PreparedShaders.cpp fixture.

  • New agc_driver_image_sample_cube_derivatives:
    • The image is a 16×16 R8G8B8A8 cube with five levels, each texel holding its level and face.
    • 32 lanes sample all six faces with image_sample_d. One gradient is set to 2^(level - 0.25) texels to select levels 0 to 4, the others are 0.
    • The test checks the level and face each lane read.
    • Results:
      • It passes on all three devices.
      • With the fix reverted it fails on the RTX 4060: image_sample_d cube: thread 0 (face 0, level 0) component 0 is 4.000000, expected 0. Every level-0 to level-3 sample read the last level.
  • agc_driver_image_sample_derivatives, agc_driver_emulated_color_compare and agc_driver_image_sample_lod_clamp pass. The last is skipped on the Intel UHD, which has no shaderResourceMinLod.
  • Shader and driver tests (agc_driver_*, agc_shader_*, agc_spirv_*, vector_*, agc_masked_select, agc_validate_program, agc_denormal*, agc_float16_*; 338 tests):

Limits.

  • I haven't run it on AMD hardware. The handling follows Mesa's AMD lowering, and TechnicalDebt.md records that.
  • The test covers image_sample_d. image_sample_cd takes the same gradient path.

Checklist

  • Based on current main; no other open PR implements the same functions
  • One topic per PR; follow-ups go in a new PR
  • No comments in code except technical debt
  • Unimplemented paths throw (NotImplemented_nid_no_patch); silent stubs are listed in TechnicalDebt
  • No notes, investigation .md files or images added to the repository (attach them to this PR)
  • New third-party code is a submodule built from source
  • Generic behaviour, not specific to one title
  • Depends on: none
  • AI-assisted: yes

@karanvora2599

Copy link
Copy Markdown
Contributor Author

For the record, from @fma-rc's census of the shaders nine titles submit (https://github.com/fma-rc/AnyPS5/tree/evidence/shader-census: 749 shaders, 301,896 instructions; see #4061): there is no image_sample_d or image_sample_cd at all, so none of those titles takes this path.

It stays a correctness fix for titles that sample cubes with explicit gradients. The new test shows what they get on main: the last mip instead of the one the gradient selects.

karanvora2599 and others added 2 commits October 13, 2026 15:26
image_sample_d and image_sample_cd on a cube pass their gradients through the
same conversion as the face coordinates, which subtracts 1.0 to move AMD's
[1, 2] face range to [0, 1]. A gradient is a difference of coordinates, so the
bias turned a one-texel gradient into one of about -1 and the sample read the
smallest mip. Gradients now skip the bias.

agc_driver_image_sample_cube_derivatives samples every face of a 16x16 cube at
five levels through image_sample_d and checks the level and face it read.

TechnicalDebt records that the gradient handling follows Mesa and was not
measured on hardware.
…radients

Co-Authored-By: Claude <noreply@anthropic.com>
@adriadam10

Copy link
Copy Markdown
Collaborator

Measured on RDNA2 (gfx1035) with the hardware oracle: on a 16x16, five-level cube, the level image_sample_d reads matches unbiased face-space gradients (round(log2(16 * max(|ddx|, |ddy|)))) on all 1032 rows (every component, both signs, mixed, six faces, two coordinates); main's biased gradients miss 816 of them. The new test fails without the fix, and the scoped build and ctest pass.

What we added on your branch:

  • Rebased onto main. TechnicalDebt.md has been split into docs/dev/debt/, so the entry moved to debt/shader.md.
  • The entry now records the measurement instead of "Not measured on hardware". It also notes that image_sample_cd takes one LOD per quad, from the quad's first lane, while the host computes it per lane. Main already has that gap and this PR doesn't change it.

Merging once the full build is green.

@adriadam10
adriadam10 force-pushed the fix/shader-cube-gradients branch from d1dc1aa to 3973e5a Compare October 13, 2026 13:34
@github-actions github-actions Bot removed the needs rebase Conflicts with main, rebase needed label Oct 13, 2026
@adriadam10
adriadam10 merged commit e152ba8 into boykopovar:main Oct 13, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agc libSceAgc* docs shader core/shader

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants