Skip to content

fix(shader): allow constant-zero sRGB comparisons - #3532

Merged
oneandonlydean merged 3 commits into
boykopovar:mainfrom
MarcZX10:codex/srgb-compare-zero-3526
Oct 11, 2026
Merged

oneandonlydean merged 3 commits into
boykopovar:mainfrom
MarcZX10:codex/srgb-compare-zero-3526

Conversation

@MarcZX10

@MarcZX10 MarcZX10 commented Oct 11, 2026 •

Copy link
Copy Markdown
Contributor

What

Fixes #3526. On devices that cannot sample an 8-bit sRGB view, a comparison-only sRGB descriptor was rejected during decodeImageDescriptor before the emulated comparison could produce its measured constant-zero result.

For comparison resources, the decoded descriptor now keeps srgbDecode disabled so its UNORM view matches the emulated comparison runtime mode. Ordinary unsupported sRGB sampling and gathering still throw, including when a descriptor is used for both comparison and ordinary sampling. The patch preserves the existing cube-wrap restriction; #3540 covers the separate 8-bit sRGB bilinear cube case.

The regression coverage and implementation from jweidenh's tested commit 5377951c are included with the original commit authorship preserved.

Tested

  • Linux x64, GCC 12.2, Release: built agc_shader_memory_tests, agc_driver_graphics_tests, agc_driver_emulated_color_compare_tests, agc_driver_image_srgb_load_tests, and agc_driver_runtime_images_tests.
  • Six focused CTests passed, with no CTest-level skips: agc_shader_memory, agc_driver_graphics, agc_driver_emulated_color_compare, agc_driver_image_srgb_load, agc_driver_runtime_images, and agc_driver_emulated_color_compare_srgb_decode (forced sRGB shader decode).
  • python3 tools/check_conventions.py --base upstream/main and git diff --check upstream/main...HEAD passed.
  • Hosted CI is rerunning on the current head. No Windows, physical-PS5, or title-startup validation is claimed.

Checklist

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

@github-actions github-actions Bot added the shader core/shader label Oct 11, 2026
@MarcZX10
MarcZX10 force-pushed the codex/srgb-compare-zero-3526 branch from d74f660 to 12400d5 Compare October 11, 2026 12:48
@MarcZX10
MarcZX10 force-pushed the codex/srgb-compare-zero-3526 branch from 12400d5 to 9e8b5a2 Compare October 11, 2026 12:49
@MarcZX10

Copy link
Copy Markdown
Contributor Author

I followed up on Ivica’s mode-matching observation from #3540. The comparison-only sRGB fallback now also adds matching srgbDecode runtime modes, so the UNORM view can pass RuntimeImageMode and reach the emulated compare shader. Ordinary unsupported sRGB sampling remains rejected. The amended commit is pushed; thanks for catching the missing second half of the path.

@jweidenh

Copy link
Copy Markdown
Contributor

Update before posting: this PR has advanced to 9e8b5a21bcc597e44ac5be5fa940ee765f3a923f, adding sRGB-decode runtime modes for comparisons. The mode-matching diagnosis below describes the earlier d74f6605 head. I am offering the tested patch below as an alternative implementation and regression coverage; these results do not validate the new PR head, and the patch is based on d74f6605 rather than the updated branch. The runtime-mode assertion in our tests checks the alternative implementation's non-decode mode, so it would need adapting if reusing the tests with the new approach.

I prepared a tested correction on top of this PR's d74f6605178265bf3c7c7476c13b5709935b080b: commit 5377951c (patch).

The remaining problem in d74f6605, from code inspection, is that moving the exception guard alone leaves decoded.srgbDecode = true. RuntimeImageMode then compares that against emulated comparison modes whose srgbDecode is false. The correction excludes comparison resources when computing decoded.srgbDecode, and keeps the original ordinary-sampling rejection:

decoded.srgbDecode = !storage && !base.depthCompare && (srgbDecodeFormats & SrgbDecodeBit(format)) != 0u;
if (decoded.srgbDecode && !base.srgbDecodeCompatible) {
    throw std::runtime_error("samples or gathers an sRGB image the device cannot sample, which is not implemented");
}

This preserves #3438's existing zero-result semantics. ResourceTracker separates comparison and ordinary sampling resources, including when they use the same descriptor.

The commit includes regression coverage for all 8 compare functions × point/bilinear × R8/RG8/RGBA8 sRGB, uncached and cached; cache-hit/variant assertions across UNORM → sRGB → UNORM; all four decode masks in runtime mode selection; mixed compare/sample and compare/gather refusals; and a forced-decode CTest. It preserves the current cube-wrap restriction. #3540 changes sRGB cube behavior separately, so its tests would need reconciling if that lands first.

Validated on a locally managed Linux x64 runner: GCC 13.3, Release, Mesa 25.2.8 lavapipe (LLVM 20.1.2), SPIR-V Tools enabled. Seven focused executions passed, with zero CTest skips:

Test Seconds
agc_shader_memory 3.22
agc_driver_graphics 8.57
agc_driver_emulated_color_compare 5.11
agc_driver_image_srgb_load 1.75
agc_driver_runtime_images 0.31
agc_driver_emulated_color_compare_srgb_decode 3.91
agc_driver_runtime_images, forced decode 0.30

The run tested 107bfac08e0186fff51baab44e7daa17f49eb5e9 on base 1e8fff68. The offered commit 5377951c is rebased onto your PR and has the identical Git tree, f5e4c102232e578393c440db22b94169420bcaa1; no source difference is being presented as tested. Removing only the production fix reproduced the original decode refusal in 0.39 s. Restoring it passed forced comparison (4.01 s) and image-load/refusal tests (1.45 s), with clean source restoration verified. This control reproduced main's original guard, not your PR's later mode-selection failure.

Focused tests can be reproduced after a Release build with:

ctest --test-dir build --output-on-failure -R '^(agc_driver_(emulated_color_compare(_srgb_decode)?|image_srgb_load|runtime_images|graphics)|agc_shader_memory)$'
APS5_SRGB_SHADER_DECODE=1 ctest --test-dir build --output-on-failure -R '^agc_driver_runtime_images$'

The logs are in private validation run 38139866682, workflow b961f70174e688da1415ae70cc2abb7323221dc3; the results above do not rely on a publicly accessible CI link. All 11 declared submodules were initialized without modifying source; the orphan ImGui gitlink on this base was left uninitialized (now fixed upstream by #3511). agc_driver_graphics internally skipped two heap-mirror subcases because the runner lacks the required guest-arena/write-watch/import support. No Windows, physical-GPU, full-suite, or title-startup validation is claimed. Conventions and whitespace checks passed.

AI-assisted: yes (Codex).

@MarcZX10

Copy link
Copy Markdown
Contributor Author

Thanks, jweidenh. I carried your tested commit 5377951c into #3532 as 82c04e402, preserving your authorship, and added a0e3d2098 to remove the runtime-mode expansion that the comparison-only decode change no longer needs. The ordinary sRGB sample/gather rejection remains in place.

I rebuilt the Release targets on current main (a11c8f7f) with GCC 12.2 and ran the focused CTest set:

Test Result
agc_shader_memory Passed
agc_driver_graphics Passed
agc_driver_emulated_color_compare Passed
agc_driver_image_srgb_load Passed
agc_driver_runtime_images Passed
agc_driver_emulated_color_compare_srgb_decode Passed

CTest reported 6/6 passed with no test-level skips. Conventions and diff checks also pass. Hosted checks are rerunning on the updated head.

@MarcZX10

Copy link
Copy Markdown
Contributor Author

Update: hosted checks finished on a0e3d209. Conventions, eligibility/report, Ubuntu and Intel macOS register-context/write-watch, and all three relinker jobs pass. Full Linux/Windows jobs are skipped by the contributor workflow.

@github-actions

github-actions Bot commented Oct 11, 2026 •

Copy link
Copy Markdown

Other open pull requests touch the same code. Merging one will break the other, or both implement the same thing. If this one builds on them, list them in Depends on.

Pull request Conflicting files (lines on main) Same exports Same new files
#3540 core/libs/prx/libSceAgcDriver/tests/execution/EmulatedColorCompare.cpp L308-309
#3741 core/libs/prx/libSceAgcDriver/CMakeLists.txt L1016
#3763 core/libs/prx/libSceAgcDriver/CMakeLists.txt L1016
#3783 core/libs/prx/libSceAgcDriver/CMakeLists.txt L1016

@Zurisen

Zurisen commented Oct 11, 2026

Copy link
Copy Markdown
Contributor

Native Windows + NVIDIA run, the configuration #3526 was reported on: a0e3d209 merged into main 4be9c255 (clean), Windows 11 Home (26200), RTX 4070 Laptop (driver 596.36), WinLibs GCC 15.2.0 r7, Release, full build, ANYPS5_REQUIRE_VULKAN=1. Like the RTX 4080 in #3526, this GPU samples VK_FORMAT_R8_SRGB and reports no format features for VK_FORMAT_R8G8_SRGB (vulkaninfo).

Test main main + this PR
agc_driver_emulated_color_compare fails 3/3 passes 3/3
agc_driver_emulated_color_compare_srgb_decode (not on main) passes 3/3
agc_shader_memory 3/3 3/3
agc_driver_graphics 3/3 3/3
agc_driver_image_srgb_load 3/3 3/3
agc_driver_runtime_images 3/3 3/3

With the PR the test creates all three 8-bit sRGB textures (format=128 vk=15, format=129 vk=16 through the UNORM view, format=130 vk=43) and prints emulated color compare tests passed.

@oneandonlydean
oneandonlydean merged commit f7d6235 into boykopovar:main Oct 11, 2026
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agc libSceAgc* build shader core/shader

Projects

None yet

Development

Successfully merging this pull request may close these issues.

linux CI + NVIDIA: agc_driver_emulated_color_compare fails on main since #3438 (8-bit sRGB compare refused on the decode path)

4 participants