Skip to content

feat(agc): accept texel offsets with unnormalized sampler coordinates - #822

Merged
adriadam10 merged 1 commit into
boykopovar:mainfrom
Celegans12:feat/agc-unnormalized-sampler-offsets
Oct 11, 2026
Merged

adriadam10 merged 1 commit into
boykopovar:mainfrom
Celegans12:feat/agc-unnormalized-sampler-offsets

Conversation

@Celegans12

@Celegans12 Celegans12 commented Oct 6, 2026 •

Copy link
Copy Markdown
Contributor

What

An image_sample_*_o through an S# with FORCE_UNNORMALIZED throws in Prepare: Vulkan forbids offset operands with unnormalized coordinates.

For supported explicit-LOD samples, the registered artifact adds signed texel offsets to f32 coordinates with OpFAdd/NoContraction, selected from the S# bit. Normalized sampling retains native offsets. Without maintenance8, nonconstant offsets require the folded form; normalized samplers throw.

Offset folding excludes gathers, comparison, derivatives, implicit LOD and _a. Preserve #1417's direct constant-selector modes, including 3D; ordinary selectors cannot match them, and indirect images remain rejected. The rebase retains main's flatVolumeCompatible and flatLineCompatible beside foldTexelOffsetsOnly, with matching codec fields and fixtures. Disk format is 34; main's runtime ABI 11 and request 15 remain unchanged.

PPSA26344's compute shader 0x8000265d00 uses image_sample_lz_o. adriadam10 measured 3840 bit-identical offset-versus-coordinate-shift cells on RDNA2 gfx1035 with point/linear filtering and clamp edge cases.

Tested

Exact source 93d9b5dd, based on main 57f96fd2: Windows 11 / MinGW-w64 GCC 15.2.0 / NVIDIA and Linux / GCC 13.3 / lavapipe, SPIRV-Tools ON. Focused builds, patched libraries and registration passed. Both platforms passed 4/4 CTests with zero failures or whole-case skips; raw targeted skip checks pass.

Cases: agc_driver, agc_shader_memory, agc_shader_disk_cache, agc_driver_image_sample_unnormalized.

Default Recorder also passed on both platforms, including main's swept-buffer staging and image memoization cases. It omitted watched-import refresh, draw-snapshot reuse, unit-shadow and snapshot-patching cases on Windows; Linux also omitted other write-watch cases and the unavailable minimum-LOD extension. These are internal omissions, not passing coverage.

Historical full suites at published 14399efb (main c72d5ce): Windows 570 passed/2 skipped/1 unexplained video_out_flip_lifetime heap-corruption failure (0xc0000374); unchanged-binary retry 570/3/0. The strict skip gate failed; both VideoOut cases passed native SDL, third skip used a missing ICD. Linux 558/29/0; raw skip checker rejected WSL asynchronous-userfaultfd write tracking, accepted locally, not an upstream pass. Both independent constant-mode controls failed as expected and restored shader-memory passed on that earlier head.

Not rerun on this rebase: full suites, earlier constant-mode negative controls, PPSA26344 or console behaviour. Linux uses memory comparisons when write protection is unavailable.

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

@adriadam10

Copy link
Copy Markdown
Collaborator

Checked the last commit:

  • Build + ctest green on RADV (385/385, merged onto current main); the execution test fails on main with the texel-offset refusal, as it should.
  • Measured on hardware (RDNA2, gfx1035): image_sample_lz_o through a FORCE_UNNORMALIZED S# against image_sample_lz at (u + Δu, v + Δv), point and linear, last-texel and border clamp, offsets -32..31, coordinates outside the image and off the 1/256 grid: 3840 cells, all bit-identical. The offset is applied before clamping, so folding it into the coordinate is exact. The TechnicalDebt line can say it was measured on RDNA2 instead of "unmeasured".

@boykopovar @oneandonlydean one design question before merging: this adds foldTexelOffsets to ResourceSpecialization, i.e. a new compile-time variant chosen from an S# bit (and a new disk-cache key field). #639 replaces resource specialization with runtime descriptors and samplers are still pending there. Is it fine to land this now and let #639 absorb it, or should offset folding wait for the sampler side of #639? Otherwise it's ready from our side.

@Celegans12

Copy link
Copy Markdown
Contributor Author

Thanks for measuring it. 3840 bit-identical cells on gfx1035 is much better evidence than I had. I'll change the TechnicalDebt line to say the folding was measured on RDNA2 (offset applied before the clamp) and push it together with the rebase onto main, now that #799 is in, so the branch drops #799's commit.

The ResourceSpecialization question is the owner's call. If #639 should absorb it, I'm happy to hold this one or port the fold into #639's sampler path when that lands.

@Celegans12
Celegans12 force-pushed the feat/agc-unnormalized-sampler-offsets branch from 759c741 to dfcf72c Compare October 7, 2026 09:59
@Celegans12
Celegans12 force-pushed the feat/agc-unnormalized-sampler-offsets branch from dfcf72c to c6cc8ed Compare October 7, 2026 12:11
@github-actions github-actions Bot added the needs rebase Conflicts with main, rebase needed label Oct 8, 2026
@Celegans12
Celegans12 force-pushed the feat/agc-unnormalized-sampler-offsets branch from c6cc8ed to d84d4ce Compare October 8, 2026 14:04
@github-actions github-actions Bot removed the needs rebase Conflicts with main, rebase needed label Oct 8, 2026
@EdouardSence

Copy link
Copy Markdown
Contributor

A title that needs this: Ghost of Yōtei (PPSA26344 v01.008.000, retail). Linux, RX 9070 XT, Mesa 26.2.4 RADV.

On current main the title stops earlier, on an image descriptor read from a buffer (see #759). With #759 applied (older base, main 6b57386 + #759 c838c16), the run stops here:

compute shader 0x8000265d00: ShaderRecompiler::Recompile: DescriptorBindingBuilder::Populate unnormalized guest sampler is used with a texel offset, which is not implemented

That is the message this PR replaces. I couldn't run both together: #759 and this PR merged on current main give 8 recompiler conflicts. So I have no in-game result with this PR yet, only that it is the next blocker for this title.

Tested and written by Claude (AI assistant) on a contributor's machine.

@Celegans12

Copy link
Copy Markdown
Contributor Author

Thanks for running Yotei on the 9070 XT. Seeing #759 stop on the texel-offset error in the compute shader 0x8000265d00 on RADV is really useful, because it confirms that #822 is the next stop after #759 on AMD as well.

The conflicts come from both PRs editing the same sampler and specialization structs, the disk cache encoder and the sample emitter, and from both moving the cache format from 9 to 10. I've pushed the merge of the two current heads (#759 at 6fb7f68, #822 at d84d4ce, both on main b3fcd6d) to https://github.com/Celegans12/AnyPS5/tree/test/yotei-759-822. Both behaviours are kept, and the cache format goes to 11 because the two encodings differ. Upstream's Linux job passes on it with lavapipe (464/464), and the image table, null pointer, unnormalized sampler and disk cache tests also pass on Windows.

I haven't run that exact branch in the game, so I can't tell you where it stops next. In our integration branch, which has these two plus a lot of other open PRs, Yotei compiles that shader and gets into the opening cinematic, though only at a few frames a minute.

If you get a chance to run it on RADV, I'd like to know where it stops.

Details
  • Merge commit 477f2b3 (parents 6fb7f68 and d84d4ce). The conflicts were in tests/ShaderDiskCache.cpp, IrMetadata/Resources.hpp, ResourceMaterializer.hpp/.cpp, ShaderDiskCache.cpp and SpirvImageEmitter.cpp. SamplerResource keeps foldTexelOffsets before tableView (20 bytes), ResourceSpecialization carries tableViewClasses, srtPoison and foldTexelOffsets (128 bytes), the info encoder/decoder and BuildKey write both sides' fields, and SampleValue folds the offset into the coordinate before AccessSampledImage. FormatVersion is 11.
  • Linux, WSL Ubuntu 24.04 (GCC 13.3, Mesa 25.2.8 llvmpipe), upstream's job config on the branch tip: 464/464. guest_audio_in_capture aborted once in the parallel pass and passed when rerun serially, the way the job runs it.
  • Windows 11, MinGW-w64 GCC 15.2.0: agc_shader_disk_cache, agc_shader_memory, agc_driver_image_table, agc_driver_image_table_cache, agc_driver_guarded_null_pointer, agc_driver_image_sample_unnormalized and agc_driver_graphics pass on an RTX 3080 Laptop and on lavapipe (Mesa 26.2.4).

@github-actions github-actions Bot added the needs rebase Conflicts with main, rebase needed label Oct 8, 2026
@Celegans12
Celegans12 force-pushed the feat/agc-unnormalized-sampler-offsets branch from d84d4ce to 9e50278 Compare October 8, 2026 18:58
@github-actions github-actions Bot added needs rebase Conflicts with main, rebase needed and removed needs rebase Conflicts with main, rebase needed labels Oct 8, 2026
@EdouardSence

Copy link
Copy Markdown
Contributor

Ran test/yotei-759-822 (477f2b3) in the game: Ghost of Yōtei PPSA26344 v01.008.000, Linux, RX 9070 XT, Mesa 26.2.4 RADV, headless, 240 s.

Both earlier stops are gone. Neither 0x80003aa700 nor 0x8000265d00 fails. It stops before the first present, after 95 resource builds, on a buffer descriptor this time:

what():  compute shader 0x8000289300: GetBufferResource dword 0 is not a valid runtime value; chain: ReadConstBuffer@pc=0x0000054c(GetBufferResource(ReadConst(GetSrtResource, Immediate), ReadConst(GetSrtResource, Immediate), ReadConst(GetSrtResource, Immediate), ReadConst(GetSrtResource, Immediate)), IMul32(Phi(Immediate, IAdd32(Phi(Immediate, IAdd32(Phi(Immediate, IAdd32(Phi, Immediate)), Immediate)), Immediate)), Immediate))

The V# is read from a buffer at an offset that a loop counter scales (IMul32(Phi(...), Immediate)). That looks like the runtime-selected V# case. #1183 extends that path, but I haven't tried it, so I can't say whether it covers this one. No amdgpu faults in the kernel log.

Tested and written by Claude (AI assistant) on a contributor's machine.

@EdouardSence

Copy link
Copy Markdown
Contributor

I tested #1183 on top of test/yotei-759-822 (477f2b3 + a8dda42, clean merge). There is no change: same stop on compute shader 0x8000289300, same GetBufferResource dword 0 is not a valid runtime value, after the same 95 builds.

In the chain, the offset is a scalar loop counter (Phi(Immediate, IAdd32(Phi, Immediate)) times an immediate), not a per-lane value. So this looks like a V# read at an offset that changes per loop iteration, rather than the waterfall case #1183 extends.

Tested and written by Claude (AI assistant) on a contributor's machine.

@Celegans12

Copy link
Copy Markdown
Contributor Author

Thanks for running it, and for trying #1183 on top. We hit the same 0x8000289300 stop in our runs: with our probe tolerance on, it shows up as skipped dispatches instead of ending the run, so we hadn't looked at it closely yet. Your read of the chain matches what I'd expect: a scalar loop over V#s, not the waterfall case. It's on our list next.

@github-actions github-actions Bot added the needs rebase Conflicts with main, rebase needed label Oct 9, 2026
@Celegans12
Celegans12 force-pushed the feat/agc-unnormalized-sampler-offsets branch from 88fb2ca to 2a35792 Compare October 9, 2026 22:12
@github-actions github-actions Bot removed the needs rebase Conflicts with main, rebase needed label Oct 9, 2026
@github-actions github-actions Bot added the needs rebase Conflicts with main, rebase needed label Oct 10, 2026
@Celegans12
Celegans12 force-pushed the feat/agc-unnormalized-sampler-offsets branch from 2a35792 to 14399ef Compare October 10, 2026 05:24
@github-actions github-actions Bot added needs rebase Conflicts with main, rebase needed and removed needs rebase Conflicts with main, rebase needed labels Oct 10, 2026
@oneandonlydean

Copy link
Copy Markdown
Collaborator

#2133 added flatVolumeCompatible next to your foldTexelOffsetsOnly, so this conflicts in Resources.hpp, ShaderDiskCache.cpp/.hpp and tests/ShaderDiskCache.cpp. Please rebase, keep both fields and take the next free FormatVersion. It tested fine at 14399ef, so it can go in after that.

@Celegans12
Celegans12 force-pushed the feat/agc-unnormalized-sampler-offsets branch from 14399ef to dbc0609 Compare October 10, 2026 08:57
@github-actions github-actions Bot removed the needs rebase Conflicts with main, rebase needed label Oct 10, 2026
@github-actions github-actions Bot added the needs rebase Conflicts with main, rebase needed label Oct 10, 2026
@Celegans12
Celegans12 force-pushed the feat/agc-unnormalized-sampler-offsets branch from dbc0609 to 93d9b5d Compare October 10, 2026 16:39
@github-actions github-actions Bot removed the needs rebase Conflicts with main, rebase needed label Oct 10, 2026
@github-actions github-actions Bot added the needs rebase Conflicts with main, rebase needed label Oct 10, 2026
@oneandonlydean

Copy link
Copy Markdown
Collaborator

#2920 landed and this conflicts with it in ResourceMaterializer.cpp; please rebase, keeping #2920's message and your foldTexelOffsetsOnly throw before it.

Explicit-LOD offset samples through FORCE_UNNORMALIZED samplers now add
sign-extended texel offsets to f32 coordinates before clamping, using
OpFAdd with NoContraction and a sampler specialization selector.

Without native nonconstant-offset support, retain only runtime image
modes valid for unnormalized sampling and reject normalized samplers.
Preserve constant-swizzle handling, indirect-image rejection and main's
flat-volume and flat-line compatibility rules when selecting those modes.

Serialize image compatibility and folding flags and the sampler folding
flag in cache format 34, with roundtrip, preparation and execution coverage.
Retain main's request format 15 and runtime ABI 11.
@adriadam10

Copy link
Copy Markdown
Collaborator

Rebased onto current main (two mechanical conflicts):

Checked on the rebased tree: build + ctest 388/388 (libSceAgc, libSceAgcDriver); the three PR tests no longer compile without the change; the offset-vs-coordinate-shift fold was measured bit-identical on RDNA2 (3840 cells). Merging once CI is green.

@adriadam10
adriadam10 force-pushed the feat/agc-unnormalized-sampler-offsets branch from 93d9b5d to d0d127b Compare October 11, 2026 08:19
@adriadam10
adriadam10 merged commit bbd7157 into boykopovar:main Oct 11, 2026
14 checks passed
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