Skip to content

fix(shader): propagate color view slice to pixel ancillary input - #5500

Open
ivica00 wants to merge 3 commits into
boykopovar:mainfrom
ivica00:fix/agc-pixel-layer-base
Open

ivica00 wants to merge 3 commits into
boykopovar:mainfrom
ivica00:fix/agc-pixel-layer-base

Conversation

@ivica00

@ivica00 ivica00 commented Oct 13, 2026 •

Copy link
Copy Markdown
Contributor

What

When a draw does not export Layer, the pixel ancillary render-target index was 0 even if CB_COLOR_VIEW started at a later slice. PixelLayerBase uses the 3D depth slice, or the 2D array slice, in that case, and 0 when a layer is exported or no color target is bound. DrawDecode stores it on the pixel stage and sameDecode compares it. The recompiler adds that value to the exported layer in the packed ancillary word.

PipelineSpecialization::LayerBase is 8236 (8235 is StencilReferenceOffset on main). Recompile requests are version 28; versions through 27 leave the base at 0.

The color-view warning suppression and the 2D-array color.layers edit are not part of this PR. The slice addend is this driver's model of SLICE_START. It is not a console measurement; the ancillary bit positions follow the RDNA 3 / GCN guides already cited in docs/dev/debt/shader.md.

Tested

pixelLayerBaseTests in agc graphics tests checks the 2D slice, the 3D depth slice, an exported layer, and a draw with no color target. pixel_ancillary and agc_shader_memory were not re-run after the merge onto current main.

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 under Silent stubs in their library's technical debt file
  • 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
  • Unblocks: none
  • GPUs tested: n/a for this push
  • AI-assisted: yes

When a draw targets a specific 3D or 2D array slice without vertex layer exports, PA_CL_VS_OUT_CNTL does not set bit 18, so layerExported is 0. With a single-layer view, Vulkan's gl_Layer evaluates to 0, which previously caused the packed pixel ancillary word (bits 16..28) to read layer index 0 across all slices.

Unreal Engine 4's PostProcessCombineLUTs draws each slice of a 3D volume LUT with a separate quad setting CB_COLOR_VIEW.SLICE_START. Because InLayerIndex read 0 for every slice, the neutral LUT's blue channel was collapsed to 0, severely altering scene color grading and producing an unnatural green tint in outdoor environments.

Add pipeline specialization constant PipelineSpecialization::LayerBase (8235u) to carry the base slice (depthSlice or arraySlice) into the fragment shader when layerExported is 0. The effective layer index is evaluated as (gl_Layer * LayerExported) + LayerBase. Also compute the layer count for 2D array targets in State::DecodeColorBuffer when multiple slices are bound. Bump recompile request serialization to version 24.
@MarcZX10

Copy link
Copy Markdown
Contributor

The conventions check is failing because the required ### What section is missing or empty. The body currently starts with the implementation description but uses a BOM before the heading, so the template parser does not recognize it. Please remove the leading BOM and use a plain ### What heading so the check can parse this section.

@xXPBSXx

xXPBSXx commented Oct 14, 2026

Copy link
Copy Markdown
Contributor

Windows build of #5500: Windows 10 (19045), MinGW-w64 GCC 15.2, Release, ninja -j 4. Tree: main feddf7f4c + merge of PR head 1fa0de855fc10d8058265b548f22a39cfcbe7355 (the merge itself had no conflict). Targets: agc_driver_pixel_ancillary_word_tests, agc_driver_relative_ancillary_tests, agc_shader_memory_tests, pixel_ancillary_tests. Log: rtx5500_result.txt.

Result: the build stops (ninja exit code 1) at shader/recompiler/Recompiler.cpp:184:22: error: 'struct ShaderRecompiler::TranslateOptions' has no member named 'integerDot4x8'. The line translateOptions.integerDot4x8 = ...(spv::CapabilityDotProductInput4x8BitPacked) is added by this PR's commit (git diff 1fa0de855~1 1fa0de855 -- core/shader/recompiler/Recompiler.cpp, merge base cf5f5b53b); no member integerDot4x8 exists in TranslateOptions on that base or on main (git grep integerDot4x8 upstream/main -- core finds nothing). Open #5337 (perf: v_dot4 with packed integer dot products) mentions the same capability; I did not check whether it defines the member.
Because of the compile error none of the PR's tests could be built or run (0 tests run). The PR's Windows/Linux CI jobs show as skipped, so no CI build covers this.

Not tested: everything that needs a build of this PR, whether the line comes from a stacked branch, Linux, a title run.

AI-assisted (Claude Code).

@github-actions github-actions Bot added the needs rebase Conflicts with main, rebase needed label Oct 14, 2026
@MarcZX10

Copy link
Copy Markdown
Contributor

I checked the current TranslateOptions declaration at core/shader/recompiler/Translation/include/Translation/InstructionTranslator.hpp: it has no integerDot4x8 member. The same assignment appears in #5337, but that PR also does not add the member, so stacking on #5337 alone would not resolve this compile failure. Please coordinate the option/API change with #5337 (or adjust this patch to the current translator API) and ensure the supported-capability path is covered by a build before merging.

RINNECODER added a commit to RINNECODER/AnyPS5 that referenced this pull request Oct 14, 2026
Ported from upstream PR boykopovar#5500, adapted to main: the draw decode sets the
color view slice as layerBase (the layer-export decode this PR also
touches does not exist on main, so its diagnostic and layers hunks are
skipped). layerBase is added to the pixel info, request key, shader info,
disk cache and pipeline specialization constants. The request
serialization stays at version 24, bumped once by the front-face commit.
The shader disk cache format is bumped to 36.

Not ported: unrelated hunks the upstream commit carried in Recompiler.cpp
(dot-product capability option) and its ShaderMemory serializer tests.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DgW4n3qYgi5jAAohEmXRQj
RINNECODER added a commit to RINNECODER/AnyPS5 that referenced this pull request Oct 14, 2026
* fix(agc): accept integer tessellation partitioning

Ported from upstream PR boykopovar#5234, adapted to our triangle/fractional-odd
only tessellation path: VGT_TF_PARAM.PARTITIONING 0 decodes, emits equal
spacing and bounds factors at 64. Metal still refuses guest tessellation
draws in MetalRenderState.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DgW4n3qYgi5jAAohEmXRQj

* fix(agc): accept fractional-even tessellation partitioning

Ported from upstream PR boykopovar#5402 (its integer-partitioning base commit is
the previous commit), adapted to our triangle-only tessellation path:
VGT_TF_PARAM.PARTITIONING 3 decodes, emits fractional-even spacing and
bounds factors at 64. Metal still refuses guest tessellation draws.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DgW4n3qYgi5jAAohEmXRQj

* fix(shader): place hull input components by their offset in the control-point slot

Ported from upstream PR boykopovar#4723 (shader hunk; its Vulkan execution test is
not ported).

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DgW4n3qYgi5jAAohEmXRQj

* fix(shader): lower hulls with one input or one output control point (partial)

Ported from upstream PR boykopovar#5457, commits 1, 4 and 5 of 5 (hull exec masks
restored through phis, input stride from the patch term, one output
control point per patch). Vulkan execution-test hunks are not ported.

Skipped from boykopovar#5457, with reasons:
- relative patch index rebuilt as data: needs GE_CNTL.PRIM_GRP_SIZE and
  BREAK_WAVE_AT_EOI in TessellationConfiguration, VGT_LS_HS_CONFIG
  patchesPerGroup and the request key/serializer additions. None of
  these exist on main, which only lowers triangle fractional-odd hulls.
- recording BDA faults in hull programs: drops a refusal whose safety
  rests on upstream hull work we lack; the refusal stays.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DgW4n3qYgi5jAAohEmXRQj

* fix(shader): honour SPI_BARYC_CNTL.FRONT_FACE_ALL_BITS for the front-face input

Ported from upstream PR boykopovar#5314. Tracks SPI_BARYC_CNTL in the draw key and
shader request key, and bumps the request serialization to version 24
(shared with the following slice-offset port, which adds no second bump).

Not ported: the ShaderMemory serializer-version assertions of the PR,
which build on upstream test scaffolding absent from main.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DgW4n3qYgi5jAAohEmXRQj

* fix(shader): propagate color view slice to pixel ancillary input

Ported from upstream PR boykopovar#5500, adapted to main: the draw decode sets the
color view slice as layerBase (the layer-export decode this PR also
touches does not exist on main, so its diagnostic and layers hunks are
skipped). layerBase is added to the pixel info, request key, shader info,
disk cache and pipeline specialization constants. The request
serialization stays at version 24, bumped once by the front-face commit.
The shader disk cache format is bumped to 36.

Not ported: unrelated hunks the upstream commit carried in Recompiler.cpp
(dot-product capability option) and its ShaderMemory serializer tests.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DgW4n3qYgi5jAAohEmXRQj

* test(shader): cover tessellation partitioning, hull slot placement, layerBase and serializer v24

Fix the stale serialization version expectation in the shader memory test,
add pixel ancillary rows with a non-zero layerBase, extract PixelLayerBase
for a derivation test, and point the FrontFace note at TechnicalDebt.md.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DgW4n3qYgi5jAAohEmXRQj

---------

Co-authored-by: Claude Sonnet 5.5 <noreply@anthropic.com>
Co-authored-by: XairRrR <dev910382@gmail.com>
…-base

Co-authored-by: Cursor <cursoragent@cursor.com>

# Conflicts:
#	core/shader/recompiler/ShaderDiskCache.cpp
@ivica00

ivica00 commented Oct 14, 2026

Copy link
Copy Markdown
Contributor Author

Removed the translateOptions.integerDot4x8 assignment. TranslateOptions has no such member, and #5337 does not add one, so that line did not compile on this branch or on main.

Also merged current main. The ShaderDiskCache conflict keeps layerBase on the shader-info encode and decode, inside the split encodeInfo / decodeInfo helpers.

Pushed in 720127c.

@github-actions github-actions Bot added needs rebase Conflicts with main, rebase needed and removed needs rebase Conflicts with main, rebase needed labels Oct 14, 2026
@adriadam10 adriadam10 self-assigned this Oct 14, 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 for tracking this down. Points before this can go in:

  • Source for the semantics. The description says the rasterizer feeds CB_COLOR_VIEW.SLICE_START into ancillary bits 16..28 when no layer is exported, but cites nothing. As far as I know the render target index there comes from the layer exported by the previous stage, and the CB applies the view slice itself when addressing the target. Please cite a source (ISA guide, Mesa/LLPC, a measurement) or a test that shows the slice offset reaches the pixel shader. The Hello Neighbor 2 screenshot only shows the symptom; it does not show where the value comes from.
  • Unrelated/undescribed changes. State.cpp:873 now suppresses the "layer/viewport index vertex exports are ignored" message for non-zero slices, and State.cpp:1119 changes color.layers for 2D arrays. Neither is in the description and neither has a test. Please justify them or drop them.
  • No test for the new wiring. DrawDecode.cpp:120-122 (layerBase from depthSlice/arraySlice) and sameDecode are not exercised; the tests only cover the IR and the serializer.
  • Conflicts with main. main is now at request serialization version 27 and PipelineSpecialization id 8235u is taken by StencilReferenceOffset; StageIO.hpp, Recompiler.hpp, DrawDecode.cpp, DrawRegisterKey.cpp and ShaderMemory.cpp conflict too. Please rebase and renumber the version and the constant, and rerun pixel_ancillary and agc_shader_memory.

LayerBase moves to specialization id 8236 and request version 28. The unrelated color-view warning and 2D array layer-count edits are dropped. PixelLayerBase covers the slice chosen for a draw that does not export Layer.

Co-authored-by: Cursor <cursoragent@cursor.com>
@ivica00

ivica00 commented Oct 14, 2026

Copy link
Copy Markdown
Contributor Author

Merged current main. LayerBase is specialization id 8236 and recompile requests are version 28, so versions through 27 still read a base of 0. The warning suppression and the 2D-array color.layers edit are removed. PixelLayerBase is what DrawDecode stores, and pixelLayerBaseTests covers a 2D slice, a 3D depth slice, an exported layer, and a draw with no color target. sameDecode compares layerBase together with layerExported and dithered coverage. The slice addend is this driver's model of SLICE_START; I do not have a console measurement that the rasterizer itself inserts that register into the ancillary word, and docs/dev/debt/shader.md says so. pixel_ancillary and agc_shader_memory were not re-run after this merge.

@ivica00
ivica00 requested a review from adriadam10 October 14, 2026 23:27
@github-actions github-actions Bot removed the needs rebase Conflicts with main, rebase needed label Oct 14, 2026
This was referenced Oct 15, 2026
@ivica00

ivica00 commented Oct 17, 2026

Copy link
Copy Markdown
Contributor Author

The two open points are still open, and I did not invent a source for them.

There is still no ISA, Mesa or LLPC reference that feeds CB_COLOR_VIEW.SLICE_START into ancillary bits 16..28, and no measurement or capture of a pixel shader reading a non-zero layer with a non-zero slice and no exported layer. The debt line already says the addend is this driver's model, not a console measurement. Adding it in the shader would apply the slice twice if the color block already adds SLICE_START when it addresses the target. I will not rebase or renumber until that evidence is there, as the last review asked.

DrawDecode storing layerBase, and sameDecode comparing it, are still not covered by a test. pixelLayerBaseTests only calls PixelLayerBase.

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.

4 participants