Skip to content

fix(shader): preserve raw fragment exports for integer attachments - #6235

Open
firesand wants to merge 1 commit into
boykopovar:mainfrom
firesand:codex/integer-fragment-outputs
Open

firesand wants to merge 1 commit into
boykopovar:mainfrom
firesand:codex/integer-fragment-outputs

Conversation

@firesand

Copy link
Copy Markdown

What

Raw32 fragment exports currently get a floating-point SPIR-V interface even when the color attachment is UINT or SINT: the interface is selected from the export encoding alone, with only UINT16 mode 7 treated as integer. Carry the attachment's numeric type into shader preparation so modes 1/2/3/9 preserve unsigned words or bitcast signed words through matching output interfaces. Keep packed UNORM on its existing logical-float/packing path.

Include the numeric type in draw and shader cache identities, and serialize it in request format 30. Older captures keep their previous automatic mode-based behavior. Reject compressed raw32 integer exports; signed16 mode 8, integer blending and integer alpha-to-coverage remain unsupported.

The GPU regression covers raw signed/unsigned bits, defined-channel mappings, separate UINT/SINT MRTs and one registered shader switching float→UINT→SINT→float without reusing an incompatible interface. It also covers compressed-raw32 refusal and existing UINT16 preparation. Existing graphics fixtures now distinguish integer attachments from float and packed UNORM attachments.

Semantics: AMD's SI register reference, SPI_SHADER_COL_FORMAT at 0x28714, permits float or signed/unsigned integer raw32 formats. Pinned Mesa export lowering preserves the 32-bit values and defines the R/RG/GFX10+RA placement; format selection selects the corresponding raw formats. RDNA2 ISA §11.1/Table 56 defines compressed exports as 16-bit components. Missing-component defaults have not been verified on native hardware; tests exclude those channels and the limitation is recorded in docs/dev/debt/shader.md. Vulkan readbacks do not constitute a native console oracle.

Related work: #5200 changes reversed-component-order blending and touches shared state/cache/serializer files. It does not implement this integer-output fix. No dependency on it is introduced; merging either may require rebasing the other. Targeted open-PR searches found no direct duplicate.

Tested

Release build and patched libs target passed. The final 1,230-test CTest run completed with 1,217 passed, 12 expected device/display skips, and one allocator failure. All changed shader, cache, serialization and graphics tests passed. guest_allocator_failures reports malloc(300 GiB): succeeded despite RLIMIT_DATA; the same unchanged test fails identically on main on this host. Its cause is outside this patch and remains unresolved. CTest therefore exits 8; this is not an all-green run.

  • Base: d92a8dad390af1807cc91622b924252141816a5a; candidate: 911920f04450f7e063235a988fe79681d210e261 (one commit). Remote main was rechecked after validation.
  • Linux/Gentoo, NVIDIA GeForce RTX 4090, driver 615.78.08; Release GCC build with SPIRV-Tools enabled.
  • Baseline reproduction: main's PreparedExports fixture, changed only to use raw mode 9 and run the integer case, fails with runtime export mapping uses the wrong scalar type before drawing. The candidate's --raw32-integer case passes with GPU readback. The two-line baseline patch is included below.
  • Full suite: ANYPS5_REQUIRE_VULKAN=1 ctest --test-dir build --output-on-failure --timeout 120 -j1; 234.87 seconds. Skip checker accepts all 12 skips: one missing VK_EXT_shader_stencil_export and 11 offscreen display tests.
  • Prepared export GPU tests also pass with APS5_NO_GPL=1. Conventions, test registration (883 executables) and whitespace checks pass. Validation excerpts and the baseline reproduction patch are included below; full JUnit and build logs are retained locally.
  • No performance claim, gameplay-fix claim or game data is included. The earlier Zero Company investigation exercised raw integer targets, but did not establish that this fixes its rendering corruption.
  • Windows, lavapipe and other GPU vendors are not tested locally. Full upstream Linux/Windows PR CI requires a maintainer's manual workflow after submission.

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; silent stubs are unchanged
  • No notes, investigation .md files or images added to the repository
  • New third-party code is a submodule built from source (none added)
  • Generic behaviour, not specific to one title
  • Depends on: none
  • Unblocks: none
  • GPUs tested: NVIDIA GeForce RTX 4090, driver 615.78.08, Gentoo Linux
  • AI-assisted: yes (OpenAI Codex); source review and test evidence are included
Validation excerpts and baseline reproduction

Final candidate 911920f04, GCC 16.2.1, Gentoo Linux, kernel 7.2.9-cachyos-dist, NVIDIA RTX 4090 driver 615.78.08:

request_serializer: PASS (0.00242282 s)
agc_shader_memory: PASS (1.35515 s)
agc_shader_disk_cache: PASS (1.68978 s)
agc_driver_graphics: PASS (3.67467 s)
agc_driver_front_face_all_bits: PASS (0.150003 s)
agc_driver_prepared_exports: PASS (0.356375 s)
Full suite: 1230 total; 1217 passed; 12 skipped; 1 failed; 234.87 s
Only failure: guest_allocator_failures
malloc(300 GiB): succeeded despite RLIMIT_DATA
12 skips accepted by tools/check_skips.py
Conventions and test registration: PASS

Independent build from base d92a8dad3 with unchanged production code and only this fixture patch:

diff --git a/core/libs/prx/libSceAgcDriver/tests/execution/PreparedExports.cpp b/core/libs/prx/libSceAgcDriver/tests/execution/PreparedExports.cpp
index bdbfd1362..43f397b46 100644
--- a/core/libs/prx/libSceAgcDriver/tests/execution/PreparedExports.cpp
+++ b/core/libs/prx/libSceAgcDriver/tests/execution/PreparedExports.cpp
@@ -28,7 +28,7 @@ void Run(AgcDriver::VulkanDevice& device, bool integer) {
     const auto vertex = Recompile(vertexRequest);
     const std::array<std::uint32_t, 4> values = integer ? std::array<std::uint32_t, 4>{11, 22, 33, 44} : std::array<std::uint32_t, 4>{0, 0x3e800000u, 0x3f000000u, 0x3f800000u};
     const std::array<std::uint32_t, 13> code{0x7e0002ffu, values[0], 0x7e0202ffu, values[1], 0x7e0402ffu, values[2], 0x7e0602ffu, values[3], 0xf800100fu, 0x03020100u, 0xf800183fu, 0x03020100u, 0xbf810000u};
-    AgcDriver::Registers context{{0x1b6, 0x8000}, {0x1b3, 0}, {0x1b4, 0}, {0x203, 0}, {0x1c5, integer ? 0x7007u : 0x9009u}};
+    AgcDriver::Registers context{{0x1b6, 0x8000}, {0x1b3, 0}, {0x1b4, 0}, {0x203, 0}, {0x1c5, integer ? 0x9009u : 0x9009u}};
     RecompileRequest request{{ShaderStage::Fragment, 0x20000, code, 0, {}}, {32, 0, {}, {}, AgcDriver::Graphics::DecodePixelStageInfo(context, {}), {}, {}}, device.Target(), PixelPushLayout(0u, device.Target())};
     const auto handle = PrepareShader(request);
     const auto& artifact = GetPreparedArtifact(*handle);
@@ -117,7 +117,6 @@ int main() {
     try {
         auto device = OpenVulkanTestDevice();
         if (!device) return VulkanTestSkipped;
-        Run(*device, false);
         Run(*device, true);
         std::cout << "prepared fragment export mapping tests passed\n";
     } catch (const std::exception& error) {
cmake --build build --target agc_driver_prepared_exports_tests guest_allocator_failures_tests --parallel 6
ANYPS5_REQUIRE_VULKAN=1 ctest --test-dir build --output-on-failure --timeout 120 -j1 -R "^(agc_driver_prepared_exports|guest_allocator_failures)$"

Baseline results:

guest_allocator_failures: FAIL
malloc(300 GiB): succeeded despite RLIMIT_DATA
agc_driver_prepared_exports: FAIL before Draw
AGC graphics: runtime export mapping uses the wrong scalar type

Candidate direct regression:

ANYPS5_REQUIRE_VULKAN=1 ANYPS5_NO_SHADER_CACHE=1 build/tests/agc_driver_prepared_exports_tests --raw32-integer
prepared exports: type=uint mode=9 shared-registration=0 mixed=0
raw32 integer fragment export tests passed

The candidate fixture supplies the new numeric-type field and checks GPU readback. Main's original API lacks that field; its test-only reproduction checks the resulting SPIR-V scalar mismatch before drawing. No game assets or title-specific patches are required.

@github-actions github-actions Bot added shader core/shader docs agc libSceAgc* labels Oct 15, 2026
@github-actions github-actions Bot added the needs rebase Conflicts with main, rebase needed label Oct 15, 2026
This was referenced Oct 15, 2026
@adriadam10 adriadam10 self-assigned this Oct 18, 2026

@adriadam10 adriadam10 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks, the approach (carry the attachment's numeric type into the request and cache keys) looks right. Points to fix before this can merge:

  • Conflicts with main in 9 files (State.cpp, ShaderValidation.cpp, SpirvModuleEmitter.cpp, Recompiler.hpp, RequestSerializer.cpp, DrawRegisterKey.cpp, Graphics.cpp, ShaderMemory.cpp, RequestSerializerTests.cpp). main is already at request serialization version 32 (this PR bumps 29 to 30), so the version, the base64 prefixes and the byte-level fixtures in RequestSerializerTests.cpp/ShaderMemory.cpp need redoing on top of 32, with older versions still replaying.
  • ColorOutputTypeOf (State.cpp:1081) is incomplete. main already decodes VK_FORMAT_R8G8_UINT (State.cpp:800) but it is missing from the Uint cases, so an 8_8 UINT target falls back to Float (also reported by @Amoenus on 911920f04, with a proposed fix). Only R32_SINT is Sint: every other SINT format the decoder can produce would silently get a float interface. Please derive the type from the format family (or list every UINT/SINT format the decoder returns) and add a test row per format.

Welcome to push the rebased version.

@DavidSamir

Copy link
Copy Markdown
Contributor

A title that hits this: Art Swap PPSA99318 v1.2.0 writes a float fragment output into an R8_UINT color attachment.

  • Radeon 680M (Windows 11, AMD driver 32.0.21030.2001), with the mesh fallback, as this iGPU has no mesh shaders: it runs 60 s, with no VUID errors. The validation layer reports only this warning, 6 times:
    Validation Warning: [ Undefined-Value-ShaderFragmentOutputMismatch-DynamicRendering ] | MessageID = 0x1012616b
    vkCmdDraw(): Inside [VK_SHADER_STAGE_FRAGMENT_BIT] [Output variable, Location 0, "out_mrt_0"] with a numeric type of FLOAT but VkRenderingInfo::pColorAttachments[0].imageView is created with VK_FORMAT_R8_UINT (numeric type of UINT) which does not match and the resulting values written will be undefined.
    
    The AMD driver tolerates it, and the values written are undefined.
  • Apple M2 (MoltenVK): Metal rejects the pipeline, and the title dies at 11 s with output of type float4 is not compatible with a MTLPixelFormatR8Uint color attachment.

AI-assisted: yes

@DavidSamir

Copy link
Copy Markdown
Contributor

Another title on Apple M2 (macOS 26, MoltenVK, Rosetta) that stops on this: PS5CEMU-HAR PPSA99360 5.0.0, converted with --trap-sdk-syscall-stubs, stops at 4 s with MoltenVK's output of type float4 is not compatible with a MTLPixelFormatR8Uint color attachment from vkCreateGraphicsPipelines (result -3), the same as Art Swap above. Not run with this PR applied.

@Alexius0815

Copy link
Copy Markdown
Contributor

@firesand I rebased your 911920f04 onto upstream main 329efe1bf and kept you as the commit author: https://github.com/Alexius0815/AnyPS5/tree/contrib/integer-attachment-exports (head 11b18b570). Would you like to use it to update this PR? Two more titles in this thread stop on the same mismatch, one of them on an Apple M2.

The rebase resolves the newer draw-key, blending, sample-state and serializer changes. It preserves reversed-component blending, packed float targets and targetless-output validation, includes the newer R8G8_UINT attachment, and updates an obsolete negative-test expectation.

Main keeps request format 42 for automatic output types and uses a provisional format 43 for explicit numeric types. That number needs coordination with #9356, which also uses 43, before any merge. The shader-cache payload layout is unchanged; the new output type participates in its request identity and changed sources invalidate old artifacts. The actual generated ShaderCacheVersion.cmake command passes, and a missing-review negative control fails as expected.

On Apple M4 / MoltenVK / x86_64 under Rosetta:

  • All 15 changed C++ translation units compile on the main-based branch; its standalone serializer tests and conventions check pass. This is not a full main-based macOS runtime build.
  • The equivalent change is integrated locally on the community Mac stack, head b9c30732e (its private request-format extension is 40; this is not an interchange-compatible main capture format).
  • The same six tests pass before and after, with and without Khronos validation 1.4.357.0: request_serializer, agc_shader_memory, agc_shader_disk_cache, agc_driver_graphics, agc_driver_prepared_exports, agc_driver_front_face_all_bits. The expanded export fixture exercises raw UINT/SINT modes and numeric-type transitions.
  • Validation is not clean: the existing independentBlend VUID VUID-VkPipelineColorBlendStateCreateInfo-pAttachments-00605 occurs six times in the baseline and reaches the ten-message reporting limit with the expanded fixture. No additional VUID identifier appeared, but this does not certify validation-clean GPU readback.
  • ccleste no longer reports the float-to-R8Uint pipeline mismatch in this run, but still fails: Metal reports an invalid resource during submission, Vulkan returns device lost, and the host aborts after 7.1 s with zero captured frames. No gameplay or visual-correctness claim; no further title run followed that failure.

#7746 is a merged test-only correction related to #7703, not a replacement for this production fix. I have not tested the rebase on Windows or Linux. Happy to provide the branch for your review rather than open a competing PR.

AI-assisted: yes

@adriadam10 adriadam10 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Still blocked on a rebase, and it is no longer mechanical. Checked against current main:

  • Conflicts in 12 files (29 hunks), not only the serializer: State.cpp, State.hpp, ShaderValidation.cpp, Recompiler.hpp, CacheKey.hpp, DrawRegisterKey.cpp, SpirvModuleEmitter.cpp, RequestSerializer.cpp and four test files. main is at request serialization version 46 (this PR bumps 29 to 30), so the version, the base64 prefixes and the byte-level fixtures need redoing on top of 46, with older versions still replaying.
  • main now has its own integer-export mechanism: ColorTarget::uintExport (State.hpp:67, set from slotExport == 7 in State.cpp:1492, added in 4554135) and the matching vertex:u32x4 check in ShaderValidation.cpp:728-730. This PR's ColorOutputType overlaps it. Please fold one into the other (for example derive uintExport from the new per-attachment type, or drop it) so there is one place that decides the interface, keeping the "blending into an unsigned integer target is unsupported" check and the UINT16_ABGR tests working.
  • ColorOutputTypeOf (State.cpp:1081) is incomplete (unchanged from our earlier comment): VK_FORMAT_R8G8_UINT falls back to Float, and only R32_SINT is Sint. Derive the type from the format family, or list every UINT/SINT format the decoder returns, and add a test row per format.

Alexius0815 posted a rebase of this commit (https://github.com/Alexius0815/AnyPS5/tree/contrib/integer-attachment-exports) that can be a starting point, but it predates the uintExport interaction and the current serializer version. Welcome to push the rebased version.

@Alexius0815

Copy link
Copy Markdown
Contributor

@adriadam10 @firesand @rooklz before updating the broader #6235 rebase, I checked current main (933c0e4f1d) and #9789 (now at 8d38112287) for overlap.

#9789 covers the raw-UINT attachment mismatch that motivated our M4 work here, using the existing export-packing specialization rather than adding a separate per-attachment output-type field. It also retains the UINT16_ABGR path. Our existing fork rebase (11b18b570) already includes R8G8_UINT and derives uintExport from the attachment type, but predates the current serializer and is not ready for publication on current main.

Two differences need an explicit scope decision:

Would you prefer #9789 to own the raw-UINT fix, with #6235 narrowed to the remaining signed-output/format coverage after it lands? That would avoid two competing interface decisions and an unnecessary second representation for the same UINT output.

#9789's current head narrows its claim: only 32_R into R32_UINT is checked bit for bit on a GPU. The 32_GR, 32_AR and 32_ABGR exports and the raw 32-bit export into R8_UINT (the target ccleste stops on) rest on its decode assertions. I am keeping that separate from the broader signed-output scope and from previously reported title runs.

This is a source/review coordination check only: no new runtime measurements or correctness claims, and no updated rebase has been pushed.

AI-assisted: yes

@Filip722

Copy link
Copy Markdown
Contributor

@firesand Following up on the mapping-hunk coordination proposal: could we agree on #12280 → #6235, with #6235 carrying the downstream integration and combined export validation?

I re-read your current 911920f04450 diff. Your signed/unsigned raw32 interfaces and scalarType must be retained in the nonidentity fallback and both branches must return the same vectorType; #12280 only bypasses identity mapping and does not implement your numeric-type fix. The shared EmitSetAttribute mapping hunk needs an intentional resolution. Please confirm the order/ownership or propose the reverse if it better fits your prerequisites. I am refreshing #12280 against main edf6804cf52c; its existing approval does not qualify this combination.

AI-assisted: yes (Codex).

@fogydes

fogydes commented Oct 30, 2026

Copy link
Copy Markdown
Contributor

stale+rebase

1. Freshness Envelope

  • Head SHA: 911920f04450f7e063235a988fe79681d210e261
  • Merge-base SHA: d92a8dad390af1807cc91622b924252141816a5a
  • Behind origin/main: 14,101 commits
  • Merge-tree status: exit code 1 (conflicts against origin/main across 12 files: DrawRegisterKey.cpp, State.hpp, ShaderValidation.cpp, State.cpp, Graphics.cpp, ShaderMemory.cpp, FrontFaceAllBits.cpp, CacheKey.hpp, RequestSerializer.cpp, Recompiler.hpp, SpirvModuleEmitter.cpp, RequestSerializerTests.cpp)

2. Findings

  1. [verified] Merge conflicts require rebase: git merge-tree --write-tree origin/main 911920f04450f7e063235a988fe79681d210e261 exits with code 1 due to 12 conflicting files across AGC driver state and shader recompiler components.
  2. [verified] Conventions clean against merge-base: python tools/check_conventions.py --base d92a8dad390af1807cc91622b924252141816a5a in an isolated scratch worktree reported 0 findings.
  3. [read-only] Missing checklist line: The PR description omits the required Fixes: line from .github/pull_request_template.md.
  4. [read-only] Missing Vulkan validation layer comparison: AGENTS.md requires AGC and shader recompiler changes to report test runs with ANYPS5_REQUIRE_VULKAN=1, ANYPS5_NO_SHADER_CACHE=1, and VK_INSTANCE_LAYERS=VK_LAYER_KHRONOS_validation, confirming no new VUID- validation errors are introduced.
  5. [author-reported] Reported test run on Linux Gentoo / NVIDIA RTX 4090 (1,217 passed, 12 expected device skips, 1 host allocator failure matching baseline).

3. Cross-PR Ledger

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agc libSceAgc* docs needs rebase Conflicts with main, rebase needed shader core/shader

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants