Skip to content

fix(shader): return 0 for an 8-bit sRGB bilinear cube comparison - #3540

Merged
adriadam10 merged 1 commit into
boykopovar:mainfrom
ivica00:fix/srgb-bilinear-cube-zero
Oct 11, 2026
Merged

adriadam10 merged 1 commit into
boykopovar:mainfrom
ivica00:fix/srgb-bilinear-cube-zero

Conversation

@ivica00

@ivica00 ivica00 commented Oct 11, 2026

Copy link
Copy Markdown
Contributor

What

PPSA02456 (Subnautica Below Zero) stops on current main at:

presentation failed: bilinear cube comparison requires DISABLE_CUBE_WRAP to avoid seamless face filtering

The texture is a 1x1 cube, six faces, format 130 (Format8_8_8_8Srgb). An 8-bit sRGB compare (Format8Srgb, Format8_8Srgb, Format8_8_8_8Srgb) is already 0 for every texel and every compare function, measured with image_sample_c_lz on gfx1035, so the face selected by a bilinear cube without DISABLE_CUBE_WRAP cannot change the result. Float, unorm and snorm bilinear cubes still throw.

#3526 is a different stop: a device that cannot sample 8 or 8_8 sRGB refuses the compare in decodeImageDescriptor before that 0 is reached. #3532 skips the throw for a depth compare, which then fails mode matching (the descriptor is srgb 1 and every emulated mode is srgb 0). This pull request does not change that path.

Tested

Windows 11, AMD Radeon RX 9070 XT. agc_driver_emulated_color_compare passes: a bilinear unorm cube without DISABLE_CUBE_WRAP still throws, and a bilinear 8_8_8_8 sRGB cube returns 0 for every compare function. The same run covers the 8 and 8_8 sRGB cubes on this GPU, which can sample those formats. PPSA02456 got past the cube throw; the title session was confirmed by hand.

On lavapipe and on an RTX 4080 the existing 8 / 8_8 sRGB rows still stop with the #3526 refusal. The new 8 and 8_8 cube rows run after those, so they do not move that failure. The format 130 rows are not on the shader-decode 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

Made with Cursor

@github-actions github-actions Bot added shader core/shader docs agc libSceAgc* labels Oct 11, 2026
@MarcZX10

Copy link
Copy Markdown
Contributor

I reviewed the code, regression coverage, and debt note. The change is narrowly scoped to the measured 8-bit sRGB constant-zero case, while non-sRGB bilinear cubes retain the existing rejection; I did not find a code issue in this diff. One CI item is actionable: the conventions job fails because commit bc70de0ed is named Revert "test(shader): run narrow sRGB cube compares after the 2D rows", which is not a Conventional Commit subject. Please rename/squash that revert commit and rerun conventions.

@ivica00
ivica00 force-pushed the fix/srgb-bilinear-cube-zero branch from bc70de0 to 49ecb37 Compare October 11, 2026 12:50
@MarcZX10

Copy link
Copy Markdown
Contributor

Update: current head 49ecb37e has a Conventional Commit subject, and the conventions check now passes. My earlier note applied to the prior bc70de0 head; thanks for fixing it.

@ivica00

ivica00 commented Oct 11, 2026

Copy link
Copy Markdown
Contributor Author

This does not build on #3532, so Depends on stays none.

Both edits are in EmulatedColorCompare.cpp. This pull request adds the 8-bit sRGB bilinear cube rows, which return 0. #3532 changes RunSrgbZero for the 2D rows, and its new cube rows still expect the DISABLE_CUBE_WRAP throw. Whichever merges second has to rebase that test file. If #3532 lands first, those cube rows need to expect 0.

@karanvora2599

Copy link
Copy Markdown
Contributor

Tested 49ecb37 (on main a11c8f7) on Windows 11, MinGW-w64 GCC 15.2.0, Release: an RTX 4060 Laptop (driver 591.86), an Intel UHD Graphics (Raptor Lake, driver 101.6790), and lavapipe from Mesa 25.2.8 (the version CI installs) and 26.2.4, both with GALLIUM_OVERRIDE_CPU_CAPS=avx.

  • The eight new bilinear 8_8_8_8 sRGB cube rows return 0 on all four. The new 8 sRGB cube row also passes on the RTX 4060, which can sample R8_SRGB.
  • With the !eightBitSrgb term removed, the first 8_8_8_8 cube row throws bilinear cube comparison requires DISABLE_CUBE_WRAP on the RTX 4060 and on lavapipe, so the new rows cover the change.
  • With the six rows that need 8 or 8_8 sRGB sampling removed locally (the two new cube rows and the four 2D rows), agc_driver_emulated_color_compare passes on all four devices.

One correction to the description: the cube rows come before the 2D sRGB rows in main() (L362-L363, L377-L380). So on a device that can't sample those formats, the #3526 refusal now first stops the test at the new cube rows: the RTX 4060 (no R8G8_SRGB) stops at the new 8_8 cube row, and the Intel UHD and both lavapipes at the new 8 cube row. It's the same refusal as on main, just earlier, so whether the test passes on those devices doesn't change. The sentence saying the new rows run after the 2D rows matches the reverted commit, not this head.

@github-actions github-actions Bot added the needs rebase Conflicts with main, rebase needed label Oct 11, 2026
An 8-bit sRGB compare is already 0 for every texel, so the selected face cannot change the result.
@adriadam10

Copy link
Copy Markdown
Collaborator

Checked on the HW oracle (gfx1035): a bilinear cube without DISABLE_CUBE_WRAP on 8, 8_8 and 8_8_8_8 sRGB returns 0 for all 8 compare functions over edge and inner rows (0 of 6048 non-zero), so the check can't change the result. float/unorm/snorm still throw.

main now has #3532's test, which expects the cube throw for those three formats. I rebased and replaced that expectation with this PR's rows that expect 0 (single commit, your authorship kept). Build and ctest pass; reverting the ResourceMaterializer.cpp change makes agc_driver_emulated_color_compare fail. Merging once CI is green.

@adriadam10
adriadam10 force-pushed the fix/srgb-bilinear-cube-zero branch from 49ecb37 to cd814b8 Compare October 11, 2026 21:37
@github-actions github-actions Bot removed the needs rebase Conflicts with main, rebase needed label Oct 11, 2026
@adriadam10
adriadam10 merged commit cbd98bb into boykopovar:main Oct 11, 2026
12 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.

4 participants