Skip to content

fix(shader): read cube T#s through DIM 2D image_sample and image_load as face 0 - #5509

Merged
adriadam10 merged 3 commits into
boykopovar:mainfrom
billythemenace2:fix/shader-cube-tsharp-2d-access
Oct 15, 2026
Merged

adriadam10 merged 3 commits into
boykopovar:mainfrom
billythemenace2:fix/shader-cube-tsharp-2d-access

Conversation

@billythemenace2

@billythemenace2 billythemenace2 commented Oct 13, 2026 •

Copy link
Copy Markdown

What

Before: a DIM 2D image_sample or image_load bound to a cube T# had no runtime image mode. The draw threw "image descriptor is incompatible with the static runtime image interface ... {class 1, dim 5, ..., cube 1, ...}". This is shape 2 of #4139: High on Life (PPSA15388) samples a 32x32 cube placeholder through dim:SQ_RSRC_IMG_2D.

After: these accesses get a cube mode, the one DIM CUBE accesses already use, with face coordinate 0. Sampling sees the BASE_ARRAY face of the cube's 2D-array view at (u - 1, v - 1). A load reads texel (x, y) of that face. The mode is offered only when every access is image_sample_lz, plain image_sample, image_sample_l or a mip-less image_load; other DIM 2D accesses on a cube T# still throw. Shape 1 of #4139 (a non-image descriptor in an image slot) is not addressed here.

Hardware semantics, measured on RDNA1 (gfx1010, AMD Radeon RX 5700 XT) with the tools/hw-oracle template. The kernels were assembled with the Windows driver's amd_comgr_2.dll and launched with its HIP runtime. The cubes were 8x8 and 32x32 32_FLOAT, with DEPTH 5 and 11 and BASE_ARRAY 0 and 6, and every texel holds its own word index.

  • Samples: DIM 2D image_sample_lz equals DIM CUBE image_sample_lz with face coordinate 0 on every row. That covers point and bilinear filters, wrap, clamp and mirror addressing, DISABLE_CUBE_WRAP 0 and 1, and coordinates from -1 to 4. The same holds for plain image_sample (one level, and six levels in 2x2 compute quads with steps of 1-32 texels) and image_sample_l (the third address VGPR is the LOD, integer and fractional). Explicit LOD matched 24/24 rows and implicit LOD 24/24.
  • Face selection: the VGPR after the two coordinates is not read. Face coordinates 0, 1 and 3 in it all give the BASE_ARRAY face.
  • Coordinate range: the face coordinates are cube coordinates in [1, 2], and the texel is floor((u - 1) * width).
  • Loads: DIM 2D image_load reads texel (x, y) of the BASE_ARRAY face, the same word as a DIM 2D load on a 2D T# for base 0.

The measurement write-up is attached.
measurement-results-cube-2d-gfx1010.md

Outside the face the translation follows the DIM CUBE path and samples the 2D-array view with the S# addressing. That matches the hardware only inside [1, 2). On the hardware:

  • a coordinate below 1 clamps to the first texel for every filter, wrap mode and DISABLE_CUBE_WRAP setting measured;
  • with DISABLE_CUBE_WRAP 0, a coordinate of 2 or more clamps to the last texel (point filter), and bilinear filtering blends texels of the neighbouring faces at the edges;
  • with DISABLE_CUBE_WRAP 1, a coordinate of 2 or more follows the S# addressing.

This is recorded in technical debt, together with the accesses that still throw.

Tested

  • Windows 11, AMD Radeon RX 5700 XT, Adrenalin 32.0.21045.5002, MinGW GCC 15.2.0. The full build is clean.
  • New test agc_driver_image_plane_address_on_cube runs DIM 2D image_sample_lz, image_sample_l, plain image_sample and image_load on an 8x8 32_FLOAT cube (six faces) over 32 lanes. The same kernel semantics ran on the RX 5700 XT through the oracle, and its expected values matched all 32 lanes. The test passes. Without the change it fails with the error above.
  • ctest -R "image_|shader_memory|disk_cache|runtime_images|cube|sampl|emulated_color_compare": no test fails that does not also fail on unchanged main (dacd785) on this machine.
  • python3 tools/check_conventions.py --base origin/main passes.
  • Not tested: High on Life itself (not my dump).

Checklist

@billythemenace2 billythemenace2 changed the title fix(shader): read cube T#s through DIM 2D image_sample and image_load… fix(shader): read cube T#s through DIM 2D image_sample and image_load as face 0 Oct 13, 2026
@github-actions github-actions Bot added shader core/shader docs agc libSceAgc* labels Oct 13, 2026
@xXPBSXx

xXPBSXx commented Oct 13, 2026

Copy link
Copy Markdown
Contributor

Windows test of #5509: Windows 10 (19045), MinGW-w64 GCC 15.2, Release, ninja -j 4, ctest called directly. Tree: main d1670009f + merge of da58289f0ea7e540caf3c2baced8d8f7bd586ae6 (local merge 799a8b817).
Six ctest targets, 3 runs each, exit code 0 in every run (18 of 18): agc_driver_image_plane_address_on_cube (the new test of this PR), agc_driver_image_plane_address_on_line, agc_driver_image_plane_address_on_line_implicit_lod, agc_driver_image_sample_cube_derivatives, agc_driver_vop3_cube_clamp, agc_shader_disk_cache.
Additional run, same main d1670009f + the same PR head (local merge 27519610d): agc_shader_bindless and agc_shader_memory (both cover recompiler files this PR changes), exit code 0, 3 of 3 each.
Not tested: that the new test fails without the change (no baseline run), other ctest targets that touch ResourceMaterializer.cpp, which GPU the execution tests used (not recorded in the ctest output; the machine has an NVIDIA GeForce RTX 4080 SUPER and an AMD Radeon RX 6400), Linux, a title run (PPSA15388 not available here).

AI-assisted (Claude Code).

@xXPBSXx

xXPBSXx commented Oct 14, 2026

Copy link
Copy Markdown
Contributor

Windows build check of #5509 (head da58289f0ea7e540caf3c2baced8d8f7bd586ae6) on main 52d134c02: Windows 10 (19045), MinGW-w64 GCC 15.2, Release. The merge has no textual conflict, but compiling core/shader/recompiler/ShaderDiskCache.cpp fails: static assertion failed: ImageResource changed: update the info encoder. With this PR merged the assertion on ImageResource fires: the shader disk cache pins the field counts of the structures it encodes (assertions from 664ad7703, 13 October), so the PR needs the info encoder and the count updated. Only this one object file was built (ninja ...ShaderDiskCache.cpp.obj, log dc_check_result.txt); a full build of the merge was not run. My earlier test of this PR (all tests exit 0, 18 of 18 + agc_shader_bindless/agc_shader_memory) was on main d1670009f.

AI-assisted (Claude Code).

@oneandonlydean

Copy link
Copy Markdown
Collaborator

This no longer builds on main: ShaderDiskCache.cpp asserts fieldCount<ImageResource>() == 37 (664ad77), and cubeFaceCompatible makes it 38. Please update that count and bump FormatVersion in ShaderDiskCache.hpp (30 on main), since the encoded info changed.

RINNECODER added a commit to RINNECODER/AnyPS5 that referenced this pull request Oct 14, 2026
* Metal semantics: run upstream image execution tests through the Metal driver

The existing semantics shim binds only guest buffers. A second shim
(tests/metal_semantics/driver_shim) runs the upstream execution tests that
bind images, samplers and color targets through the native Metal driver:
dispatches bind their descriptors with MetalShaderResources and draws go
through MetalDraw::DrawSynchronously, as in a title. The tests' host arrays
are the guest memory; the device borrows every range the shader's buffer
and image descriptors and the color targets name.

ImagePlaneAddressOnLine and ImageVolumeAddressOnPlane (already on main)
now run on Metal and pass.

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

* fix(shader): capture image descriptors read from constant buffers at uniform offsets

Removes the tracker check that refused a T# whose dwords come from a
ReadConstBuffer, so a T# loaded with s_buffer_load from a table record that
user data selects is captured like any other runtime descriptor.

Metal: tests/metal_semantics/ImageTableRecord.cpp runs the upstream
program through the Metal driver (MetalShaderResources image binding) and
checks that the record user data selects is the texture sampled. It fails
on main with the removed "is not a valid runtime value" error.

Ported from upstream PR boykopovar#5188

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

* fix(shader): accept TYPE=0 images during materialization

A T# with TYPE=0 is null on AMD hardware even when its base and other
words are nonzero; the materializer now accepts it as a null image
instead of refusing it as invalid (reserved TYPE=1 is still refused).
Upstream's frontend test is ported to tests/ShaderMemory.cpp.

Metal: the Metal image binding already treats TYPE=0 as null
(IsNullTextureResource), so such an image binds no texture. The Metal
ImageTableRecord test now also samples a TYPE=0 record with a nonzero
base and reads zero; on main it fails with "runtime image descriptor is
invalid". Debt note added to docs/dev/TechnicalDebt.md.

Ported from upstream PR boykopovar#5473

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

* fix(shader): read 1D T#s through 3D image_sample and image_load accesses

A DIM 3D image_sample_lz, plain image_sample or mip-less image_load
whose T# is 1D gets a runtime image mode that binds the 1D texture: the
sample reads the texel u selects, and the load reads the x texel when z
is 0 and zero otherwise, as measured on RDNA1. Other 3D accesses of such
an image still throw.

ImageResource gains flatVolumeLineCompatible, so the shader disk cache
FormatVersion goes from 35 to 36. This is the only bump in this series:
the later image-mode fields reuse it.

Metal: the 1D mode binds through the existing Image1D shape (a 2D
texture of height 1) with no new view. Upstream's ImageVolumeAddressOnLine
execution test runs unchanged through the Metal driver
(anyps5_metal_driver_semantics_imagevolumeaddressonline). It fails
without the materializer hunk ("incompatible with the static runtime
image interface"). Debt note added to docs/dev/TechnicalDebt.md.

Ported from upstream PR boykopovar#5375

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

* fix(shader): sample single-level volumes through biased 2D accesses

A pixel-stage, direct, plain 32-bit image_sample_b with DIM 2D whose T#
is a one-slice, single-level 3D image gets the 3D runtime image mode:
the sample supplies a zero third coordinate and keeps the bias, as
measured on RDNA2. The binding requires a normalized point sampler and
the descriptor one slice and mip zero; other cases throw.

ImageResource gains singleMipPlaneBiasCompatible; it is encoded by the
shader disk cache under the FormatVersion 36 bump of this series
(upstream's own bump to 31 is not taken).

Metal: the 3D mode binds the volume as the Image3D shape it already has,
with no new view. Upstream's ImagePlaneBiasOnSingleLevelVolume execution
test (a pixel draw) runs unchanged through MetalDraw
(anyps5_metal_driver_semantics_imageplanebiasonsinglelevelvolume). It
fails without the tracker hunk ("incompatible with the static runtime
image interface"). Debt note added to docs/dev/TechnicalDebt.md.

Ported from upstream PR boykopovar#5461

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

* fix(shader): decode forced degamma on biased RGBA8_UINT samples

A direct RGBA8_UINT image sampled only by plain biased pixel accesses
(the one-slice volume case of boykopovar#5461) through samplers that all set
FORCE_DEGAMMA is materialized as RGBA8_SRGB, which returns the same
decoded floats as the hardware. Mixed sampler states, indirect images,
other accesses and formats still throw.

Metal: MetalSampler refused every FORCE_DEGAMMA S#, so this (and the
existing sRGB case of ImageSampleForceDegamma) could not bind. Metal now
does what the Vulkan driver does: MetalShaderResources::Bindings creates
the sampler with forceDegammaPaired and refuses a sampled image of a
non-sRGB format paired with a forcing sampler (the same message as
RequireDegammaFormat). The rewritten descriptor binds through the
existing sRGB pixel-format view. Upstream's extended
ImagePlaneBiasOnSingleLevelVolume and the existing
ImageSampleForceDegamma tests now pass on Metal; both failed with "forces
sRGB decoding which is not implemented". Debt note added to
docs/dev/TechnicalDebt.md.

Ported from upstream PR boykopovar#5472

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

* fix(shader): read cube T#s through DIM 2D image_sample and image_load as face 0

A DIM 2D image_sample_lz, plain image_sample, image_sample_l or mip-less
image_load whose T# is a cube gets the cube runtime image mode and reads
the T#'s BASE_ARRAY face with face coordinate 0, as measured on RDNA1.
Other DIM 2D accesses through a cube T# still throw.

ImageResource gains cubeFaceCompatible; encoded under the series'
FormatVersion 36 bump.

Metal: a cube T# is already a 2D-array texture of faces and the cube mode
binds the Image2DArray shape, so no new view is needed. Upstream's
ImagePlaneAddressOnCube execution test runs unchanged through the Metal
driver (anyps5_metal_driver_semantics_imageplaneaddressoncube) and fails
without the materializer hunk. Debt note added to
docs/dev/TechnicalDebt.md.

Ported from upstream PR boykopovar#5509

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

* fix(shader): compare through 2D image_sample_c on 1D T#s

The flat-line (2D access through a 1D T#) compatibility now ignores the
compare flag, so image_sample_c and image_sample_c_lz with DIM 2D on a
1D T# take the 1D mode like their non-compare forms.

Metal: the 1D mode binds through the existing Image1D shape (a 2D texture
of height 1), with no new view. Upstream's ImagePlaneCompareOnLine execution test runs unchanged
through the Metal driver
(anyps5_metal_driver_semantics_imageplanecompareonline) and fails without
the tracker hunk. The debt note in docs/dev/TechnicalDebt.md is updated
as upstream's.

Ported from upstream PR boykopovar#5456

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

* fix(shader): apply the V# DST_SEL to formatted buffer stores

buffer_store_format_* (D16 included) wrote memory component i from data
VGPR i whatever the V#'s DST_SEL. A formatted store now writes each
format component from the slot its selector names, only for the
DST_SELs measured on RDNA2 or covered by conformance tests: the identity,
Z,Y,X,W on 8_8_8_8_UNORM and 16_16_16_16_UINT, and 0,0,0,X on 8_UNORM
and 8_UINT. Every other DST_SEL on a formatted store is refused: a host V#
throws in DescriptorBindingBuilder::Prepare, and a GPU-selected V#
records an invalid-descriptor BDA fault and skips the store.

BufferResource gains formattedWritten; encoded under the series'
FormatVersion 36 bump.

Metal: the store is lowered in the shared SPIR-V (SpirvRuntimeBuffer), so
no Metal binding change. Upstream's BufferStoreFormatDstSel execution test
runs unchanged through the Metal driver
(anyps5_metal_driver_semantics_bufferstoreformatdstsel). Without the
SpirvRuntimeBuffer hunk it fails (Z,Y,X,W on 8_8_8_8_UNORM writes
0x4033bfff, expected 0x40ffbf33). Debt note added to
docs/dev/TechnicalDebt.md.

Ported from upstream PR boykopovar#5172

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

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
@billythemenace2
billythemenace2 force-pushed the fix/shader-cube-tsharp-2d-access branch from da58289 to ab8f319 Compare October 14, 2026 12:32
@billythemenace2

Copy link
Copy Markdown
Author

Rebased on main. fieldCount<ImageResource>() is now 38, the minimum image record is 76 bytes and FormatVersion is 33. The new cube test, the cube derivative and clamp tests, agc_shader_disk_cache and agc_shader_memory pass on an RX 5700 XT. image_plane_address_on_line and its implicit-LOD variant fail on this GPU on plain main too.

@adriadam10
adriadam10 force-pushed the fix/shader-cube-tsharp-2d-access branch from 37c8994 to 1df5ae9 Compare October 14, 2026 21:46
@adriadam10

Copy link
Copy Markdown
Collaborator

Checked on the HW oracle (gfx1035, IEEE off): on 8x8 32_FLOAT cubes (DEPTH 5 with BASE_ARRAY 0, DEPTH 11 with BASE_ARRAY 6), DIM 2D image_sample_lz, image_sample_l and plain image_sample equal DIM CUBE with face 0 bit for bit over 484 rows per mode (wrap/mirror/clamp, point/bilinear, DISABLE_CUBE_WRAP 0 and 1, u and v from -1 to 4), and image_load reads texel (x, y) of the BASE_ARRAY face, matching your gfx1010 measurements. The new test fails without the change and passes here on RADV and on CI's lavapipe.

What we added on your branch:

  • Rebased on main. Main's shader disk cache now pins 37 fields and sizeof(ImageResource) == 88, so with cubeFaceCompatible they are 38 and 92, the minimum image record is 77 bytes and FormatVersion is 36.
  • One correction to the technical debt line: inside the face, a bilinear sample with DISABLE_CUBE_WRAP 0 within half a texel of the face edge already blends texels of the neighbouring faces on hardware (e.g. u = 1.0625, v = 1.03125 on face 0), so the translation matches only away from those edges. The line now says so.

Merging once the full Build is green.

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

github-actions Bot commented Oct 14, 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
#5841 core/shader/recompiler/Optimization/src/ResourceMaterializer.cpp L1240-1241

@adriadam10

Copy link
Copy Markdown
Collaborator

Checked on the HW oracle (gfx1035, IEEE off): on 8x8 32_FLOAT cubes (DEPTH 5 with BASE_ARRAY 0, DEPTH 11 with BASE_ARRAY 6), DIM 2D image_sample_lz, image_sample_l and plain image_sample equal DIM CUBE with face 0 bit for bit over 484 rows per mode (wrap/mirror/clamp, point/bilinear, DISABLE_CUBE_WRAP 0 and 1, u and v from -1 to 4), and image_load reads texel (x, y) of the BASE_ARRAY face, matching your gfx1010 measurements. The new test fails without the change and passes here on RADV and on CI's lavapipe.

What we added on your branch:

  • Rebased on main. Main's shader disk cache now pins 37 fields and sizeof(ImageResource) == 88, so with cubeFaceCompatible they are 38 and 92, the minimum image record is 77 bytes and FormatVersion is 36.
  • One correction to the technical debt line: inside the face, a bilinear sample with DISABLE_CUBE_WRAP 0 within half a texel of the face edge already blends texels of the neighbouring faces on hardware (e.g. u = 1.0625, v = 1.03125 on face 0), so the translation matches only away from those edges. The line now says so.

Merging once the full Build is green.

@adriadam10
adriadam10 merged commit 821c69e into boykopovar:main Oct 15, 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.

4 participants