Skip to content

fix(recompiler): take point-filtered sampler copies' descriptors from their source sampler - #1402

Merged
adriadam10 merged 2 commits into
boykopovar:mainfrom
gjonn:claude/sleepy-engelbart-7b6379
Oct 8, 2026
Merged

adriadam10 merged 2 commits into
boykopovar:mainfrom
gjonn:claude/sleepy-engelbart-7b6379

Conversation

@gjonn

@gjonn gjonn commented Oct 8, 2026

Copy link
Copy Markdown
Contributor

What

When a shader uses one guest sampler with an image that needs point filtering (sint, depth-bits, or a converted format without in-shader filtering) and also with an ordinary image, ResourceMaterializer::Apply adds a point-filtered copy of that sampler after the original samplers. The ResourceSnapshot only holds the original samplers, so DescriptorBindingBuilder::Populate threw guest sampler index is out of range on the copy. It threw in both the sampler binding and ProveUnnormalized, which reads every sampler's descriptor.

  • SamplerResource::copyOf records the sampler a copy was made from (NoCopy otherwise). Apply sets it on the copy, next to forcePointFiltering.
  • GuestSamplersDescriptor takes the ShaderInfo and reads a copy's descriptor from its source sampler. The point-filter rewrite of word 2 still applies to the copy.
  • The field is part of the shader disk cache encoding: the size assert and the per-element minimum are updated, and the cache format version goes from 7 to 8.
  • New execution test ImageSampleSharedSampler: a compute shader samples a 32_SINT texture and an 8_8_8_8_UNORM texture through one bilinear sampler, at a coordinate between two texel columns. It checks that the SINT result is the exact point-sampled texel and that the UNORM result is the bilinear blend (0.25), so the copy is point-filtered and the original still filters.
  • The disk cache round-trip test sets copyOf.

Tested

Windows 11, MinGW, Release, NVIDIA Vulkan device.

  • The new test throws DescriptorBindingBuilder::Populate guest sampler index is out of range on unmodified main and passes with this change.
  • All 171 agc_driver_*, agc_shader_memory, agc_masked_select, agc_shader_disk_cache and image_sample_gather tests pass. This includes every Vulkan sampler execution test (sample reduction, unnormalized, 8_SINT, emulated color compare, gathers, LOD clamp, derivatives).
  • agc_driver_mesh was not built: tests/Mesh.cpp does not compile under MinGW (setenv, std::aligned_alloc), and this change does not touch it.
  • tools/check_conventions.py --base origin/main passes.

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

🤖 Generated with Claude Code

@github-actions

github-actions Bot commented Oct 8, 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
#1292 core/libs/prx/libSceAgcDriver/tests/ShaderDiskCache.cpp L175-176
core/shader/recompiler/Optimization/src/DescriptorBindingBuilder.cpp L155, L159, L205, L348
core/shader/recompiler/ShaderDiskCache.cpp L73-75, L522
#1487 core/libs/prx/libSceAgcDriver/tests/ShaderDiskCache.cpp L175-176
core/shader/recompiler/Optimization/src/DescriptorBindingBuilder.cpp L155, L159, L205, L348
core/shader/recompiler/ShaderDiskCache.cpp L73-75, L522

gjonn and others added 2 commits October 8, 2026 14:55
… their source sampler

When one guest sampler is paired both with an image that needs point
filtering (sint, depth-bits or converted format) and with an ordinary
image, ResourceMaterializer::Apply appends a point-filtered copy of the
sampler after the original samplers. The resource snapshot only holds
the original samplers, so DescriptorBindingBuilder::Populate threw
"guest sampler index is out of range" for the copy, both in the sampler
binding and in the unnormalized-coordinate check.

SamplerResource::copyOf records the sampler a copy was made from, and
the guest sampler descriptor of a copy is read from that sampler; the
point-filter rewrite of word 2 still applies to the copy. The field is
part of the disk cache encoding: cache format version 8.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…mpler copy source

Co-Authored-By: Claude <noreply@anthropic.com>
@adriadam10

Copy link
Copy Markdown
Collaborator

Checked the diff, built and ran the full ctest on top of current main (455/455), and confirmed the new test needs copyOf to compile and pass. CI is green on all jobs.

One fix added as our own commit on your branch:

  • ShaderDiskCache.hpp: main is already at FormatVersion = 8, so this change (new copyOf field in the sampler encoding) bumps it to 9. Otherwise a cache written by main@8 would be decoded with the wrong layout. The description's "7 to 8" is stale because of this.

Note: #1292 touches the same lines in DescriptorBindingBuilder.cpp and ShaderDiskCache.cpp; whichever merges second will need a rebase.

Merging once CI is green on the pushed commit.

@adriadam10
adriadam10 force-pushed the claude/sleepy-engelbart-7b6379 branch from c521cb8 to 87323a5 Compare October 8, 2026 13:02
@adriadam10
adriadam10 merged commit 588c7b7 into boykopovar:main Oct 8, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agc libSceAgc* shader core/shader

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants