Skip to content

fix(agc): accept a min or max reduction sampler with anisotropic filtering - #5821

Merged
adriadam10 merged 2 commits into
boykopovar:mainfrom
SandstromPL:fix/agc-sampler-aniso-minmax-reduction
Oct 15, 2026
Merged

adriadam10 merged 2 commits into
boykopovar:mainfrom
SandstromPL:fix/agc-sampler-aniso-minmax-reduction

Conversation

@SandstromPL

@SandstromPL SandstromPL commented Oct 14, 2026 •

Copy link
Copy Markdown
Contributor

What

GuestSamplerResource.cpp refused an S# that combines a min or max reduction with anisotropic filtering:

guest sampler descriptor combines a min or max reduction with anisotropic filtering, which is not implemented

Tamalero first listed it on #5156 for High on Life (PPSA15388). The S# captured there later turned out to be a descriptor read before it was valid: word 0 changes from capture to capture, and one capture has ratio field 6 (Tamalero on this PR). So no title is known to reach this path.

The decoder now accepts an S# that has FILTER_MODE min or max together with an anisotropic XY filter (2 or 3). It becomes a Vulkan sampler with both anisotropyEnable and the reduction mode, as a weighted-average anisotropic S# already does. RADV (Mesa 25.0.7's radv_init_sampler, as mirrored in OpenBSD's xenocara) programs exactly this S# for such a Vulkan sampler:

  • FILTER_MODE comes from the reduction mode;
  • the XY filters are ANISO_POINT or ANISO_BILINEAR whenever the anisotropy is above 1;
  • MAX_ANISO_RATIO comes from maxAnisotropy;
  • nothing keeps the two apart.

So on AMD this is the hardware filter.

These still throw:

  • min or max with a linear mip filter;
  • min or max with a depth compare;
  • bilinear or anisotropic filtering through a min or max sampler of an image the recompiler point-samples (sint, converted or depth-bits). PointFilteredSamplerWord refuses it with "needs point filtering".

Measured on RDNA2, not on the console:

  • Tamalero on #5156 (gfx1030, tools/hw-oracle): one sample without derivatives through a 2x anisotropic S# over a single-level texture. Min or max with anisotropic bilinear returns exactly the bilinear minimum or maximum, and min with anisotropic point the point result.
  • @adriadam10 on this PR (gfx1035, tools/hw-oracle):
    • With image_sample_lz, min and max with anisotropic point and anisotropic bilinear at ratio fields 0-4 return the point result or the bilinear minimum or maximum.
    • With image_sample_d and a 4:1 footprint, anisotropic bilinear at ratio fields 2 and 4 reduces over all the anisotropic taps, which is what the Vulkan sampler gets on RADV.

The debt entry records these results.

Vulkan also leaves nearest filtering with anisotropy to the implementation ("Texel Anisotropic Filtering"), so on other host GPUs an anisotropic point S# follows the host driver.

The existing min/max format check still applies at bind time. VUID-vkCmdDraw-magFilter-09598 and VUID-vkCmdDispatch-magFilter-09598 require SAMPLED_IMAGE_FILTER_MINMAX when the filter is linear. So an anisotropic bilinear sampler needs it, like any bilinear min/max sampler, and an anisotropic point sampler doesn't.

Tested

Ubuntu 24.04, GCC 13.3, Release with BUILD_TESTING=ON, on main e736e5b, which includes #5156. Locally, the libusb lines were removed from CMakeLists.txt, because the libusb submodule isn't checked out here.

agc_driver_graphics (tests/GuestSamplerResource.cpp) replaced its expected refusal with decode checks. They cover min and max, each with anisotropic point (2) and anisotropic bilinear (3), at ratio field 2. Each must:

  • decode to anisotropyEnable, maxAnisotropy 4 and its reduction mode;
  • reach vkCreateSampler with the reduction mode chained, anisotropy enabled and maxAnisotropy 4;
  • need the min/max format feature only when bilinear.

It also checks two existing refusals, which pass on main too:

  • an anisotropic min sampler with a linear mip filter;
  • an anisotropic point min sampler through PointFilteredSamplerWord.

On main it fails with the message above.

agc_driver_image_sample_reduction (tests/execution/ImageSampleReduction.cpp) gains min and max samplers with 2x anisotropic bilinear filtering and a point mip filter. They use the existing explicit-LOD image_sample_l rows. With a Lod operand Vulkan sets the derivatives to zero, so the sample is isotropic. Every thread must read exactly the bilinear minimum or maximum the existing bilinear rows expect. Anisotropic point has no execution row, since its result is up to the driver.

On main the test fails at the first anisotropic sampler with the message above. With the change it passes 5 runs out of 5 on each GPU below, as does agc_driver_graphics. Under the Khronos validation layer, neither GPU reports anything for the samplers or dispatches. The one message is an existing vkCreateDevice warning about a structure newer than the layer.

Other checks:

  • Full ctest on the RTX 3050 Ti: 1,138 of 1,140 pass, 7 of them skipping themselves, including agc_driver_stencil_ref_export on this GPU. guest_usbd and patched_libraries are not built here, because the libusb submodule isn't checked out.
  • @adriadam10 also checked on RADV that both tests fail with GuestSamplerResource.cpp reverted and pass with it, and that the libSceAgcDriver suite is green.
  • feat(libSceAgcDriver): support secondary LOD bias and relax unnormalized coordinate filters #5156 changed other lines of GuestSamplerResource.cpp, its test and the debt file; this is rebased onto it. 35 open PRs touch these four files, 20 of which already conflict with main; none of them gets a new conflict.
  • tools/check_conventions.py from main passes with this title and body.

@Tamalero, thanks for the measurement and for the captures. @adriadam10, thanks for the gfx1035 run and the debt entry.

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 known; the High on Life capture was a descriptor read before it was valid (see above)
  • GPUs tested: NVIDIA GeForce RTX 3050 Ti Laptop GPU, driver 580.178.04, Ubuntu 24.04; lavapipe (Mesa 25.2.8, LLVM 20.1.2), Ubuntu 24.04; RDNA2 gfx1035 (RADV, by @adriadam10)
  • AI-assisted: yes (Claude Code); I reviewed and tested every line

🤖 Generated with Claude Code

@github-actions github-actions Bot added docs agc libSceAgc* labels Oct 14, 2026
@Tamalero

Copy link
Copy Markdown
Contributor

Thanks for picking this up. I ran your change (applied on my local High on Life stack, with one log line per min/max+anisotropic S# decoded) through the home base, the house, a portal trip and Blim City: about 15 minutes, warm caches. No min/max+anisotropic S# was decoded at all.

Looking back, the stop I listed on #5156 happened exactly once: on 2026-10-12, in a run with an experimental "store pending results at labels" switch on an older main, which corrupted other guest data in that run (it is the experiment I later reported on #5324 as harmful). So that S# may have been garbage rather than something the title really uses, and I can't give you its words or FORCE_UNNORMALIZED. The crash didn't log them, and the sampler hasn't appeared since.

So High on Life doesn't need this PR as far as I can tell, and it shouldn't be counted as evidence for it. The change still stands on its own: RADV programs the same S#, and the gfx1030 measurement on #5156 covers the result. Sorry for the false lead.

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

@SandstromPL

Copy link
Copy Markdown
Contributor Author

Thanks for checking, and for being upfront about where that stop came from. With no title known to reach this sampler, I'm closing this under the current queue rule; the branch stays, so it can be reopened with a title ID and log line if one turns up.

@Tamalero

Copy link
Copy Markdown
Contributor

Correction to my comment above: on another run from the same save, the title does decode one, during the loading screen into the home base:

S# 20db0400 c0100000 0087c0ef 90000204
  • FORCE_UNNORMALIZED is clear (and the S# isn't bound as unnormalized).
  • FILTER_MODE min, XY mag point, XY min aniso point, mip point, max aniso ratio field 2.

With your change it was accepted and the load continued. That run then stopped later in the same load on an unrelated check ("buffer descriptor uses an unsupported type"), so I can't confirm the full home base on that run yet. But this is a real High on Life S#, and it needs this PR. The earlier run that never decoded it simply didn't hit it. Sorry for the back and forth.

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

@SandstromPL SandstromPL reopened this Oct 14, 2026
@SandstromPL

Copy link
Copy Markdown
Contributor Author

Reopening, since the S# turned up after all. Thanks for the second run, @Tamalero. I've pinned your exact words (20db0400 c0100000 0087c0ef 90000204) as a decode test in a second commit: on main they throw this PR's message, and with the change they decode to a min reduction with 4x anisotropy and point filters. The description now leads with that S# and with the stop your run reached next.

@Tamalero

Copy link
Copy Markdown
Contributor

Follow-up, and I'd now discount the High on Life case. More runs with your change logged the same S# three times:

Capture word 0 words 1–3 ratio field
loading into the home base 20db0400 c0100000 0087c0ef 90000204 2
home base, later run 214b1400 same 2
near the portal, same run 20d27c00 same 6

Words 1–3 never change, but word 0 is different every time and always looks like the low dword of a guest address (0x2…). FILTER_MODE (word 0 bits 29–30) reads "min" because bit 29 is set in each of those values. The third capture's ratio field 6 then stops the title with "unknown anisotropy ratio 6".

In the same kind of pixel shader, my local "bind invalid T#s as null" workaround also had to null an invalid T# just before one of the captures. So in High on Life this looks like a descriptor read before it is valid, or from the wrong source, rather than a real min/max+anisotropic sampler. I wouldn't use it as evidence for this PR. The change still stands on RADV's behaviour and the gfx1030 measurement, and I'll look into where that word 0 comes from separately. Apologies for the earlier back-and-forth.

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

@github-actions

github-actions Bot commented Oct 15, 2026 •

Copy link
Copy Markdown

Some comparisons could not be completed because a pull request conflicts with the base branch. Review these shared files manually before concluding that there is no overlap.

Pull request Shared files not compared
PR 4448 docs/dev/debt/libSceAgcDriver.md
PR 4650 docs/dev/debt/libSceAgcDriver.md
PR 5058 docs/dev/debt/libSceAgcDriver.md
PR 5372 docs/dev/debt/libSceAgcDriver.md
PR 5499 docs/dev/debt/libSceAgcDriver.md
PR 5761 docs/dev/debt/libSceAgcDriver.md
PR 5925 docs/dev/debt/libSceAgcDriver.md
PR 6222 docs/dev/debt/libSceAgcDriver.md
PR 6454 docs/dev/debt/libSceAgcDriver.md

SandstromPL and others added 2 commits October 15, 2026 15:16
…ering

A guest S# with FILTER_MODE min or max and an anisotropic XY filter
threw "combines a min or max reduction with anisotropic filtering",
which stops High on Life (PPSA15388) in its home base. It now decodes
to a Vulkan sampler with both anisotropy and the reduction mode, as a
weighted-average anisotropic S# already does. RADV programs the same
S# for such a Vulkan sampler.

Tamalero's gfx1030 measurement on boykopovar#5156 covers a sample without
derivatives through a 2x anisotropic S# over a single-level texture:
min or max with anisotropic bilinear returns the bilinear minimum or
maximum, and min with anisotropic point the point result. The
execution test checks the bilinear case on the host GPU; the debt
entry records what is not measured.

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

Copy link
Copy Markdown
Collaborator

Checked on RDNA2 (gfx1035, tools/hw-oracle) and merging with two changes of ours:

  • Measured every form the decoder now accepts: with image_sample_lz, min and max with anisotropic point (2) and anisotropic bilinear (3) at ratio fields 0-4 return exactly the point result or the bilinear minimum/maximum (the only differing rows are the sub-1/512-texel ones, which the plain bilinear control shows too). With image_sample_d and a 4:1 footprint, anisotropic bilinear at ratio fields 2 and 4 reduces over all the anisotropic taps, which is what the Vulkan sampler gets on RADV. Max with anisotropic point and the real footprint are no longer unmeasured; the debt line now says so.
  • Dropped the second commit (highOnLifeLoading): as @Tamalero found, that S# is a descriptor read before it is valid (word 0 changes per capture, one capture has ratio field 6), so it shouldn't be pinned as a real sampler, and test names stay generic. The decode loop over min/max × filters 2/3 already covers the form. For the record, no title is known to reach this path, so the Unblocks: PPSA15388 line doesn't hold.
  • agc_driver_graphics and agc_driver_image_sample_reduction fail with GuestSamplerResource.cpp reverted and pass with it; the libSceAgcDriver suite is green locally on RADV.

@adriadam10
adriadam10 force-pushed the fix/agc-sampler-aniso-minmax-reduction branch from b63087f to 17ed121 Compare October 15, 2026 13:21
@SandstromPL

Copy link
Copy Markdown
Contributor Author

@adriadam10 Thanks for the gfx1035 measurements and the debt entry! I've updated the description to match: no title claim, and your results credited.

@adriadam10
adriadam10 merged commit c10aa03 into boykopovar:main Oct 15, 2026
9 checks passed
@frankea

frankea commented Oct 15, 2026

Copy link
Copy Markdown
Contributor

On an RTX 4090 (NVIDIA 616.92, Windows 11 26200, MinGW-w64 GCC 15.2.0 r7, Release) agc_driver_image_sample_reduction fails on current main 31cbb55, at the first of the two anisotropic cases this PR added:

AGC graphics: min, 2x anisotropic bilinear, point mip: thread 0 sampled 5.000000, expected 17.000000

2 of 2 runs, same value. The other 47 agc_driver_image* tests pass except agc_driver_image_plane_address_on_line (the NVIDIA mip-blend question on #6221). The full main run on this box on 10/14 (fa539dd, 1233 tests) passed, so the failure arrived with this PR or with something merged since. I have not looked at what the min reduction with 2x anisotropy returns on this driver beyond the one value; happy to run a variant or print more if useful.

AI-assisted: yes (Claude Code).

@SandstromPL

Copy link
Copy Markdown
Contributor Author

@frankea Thanks for running it on the 4090! That row is a test problem, not a decoder one. The two anisotropic rows sample a two-level texture at explicit LOD 0.25 and 0.75 through a point mip filter, and expect only the nearest level. At thread 0 (LOD 0.25), level 0's bilinear footprint is 17, 17, 20 and 20, and level 1 there is 5, so the 4090 reads level 1 with anisotropy on. Vulkan leaves the anisotropic scheme to the implementation and only says it should consider mipmapMode, minLod and maxLod (textures.adoc L2777-L2783).

This change samples those rows without a mip filter. The decoder then gives the host sampler minLod = maxLod = 0 (the S#'s MAX_LOD stays 0xfff; the missing mip filter is what clamps it), so the rows read level 0 only. That matches the RDNA2 measurements without derivatives: no mip filter over a single-level texture at LOD 0 on gfx1030, and image_sample_lz on gfx1035:

-        Run(*device, {ReductionMin, FilterAnisoBilinear, MipPoint, Aniso2x}, "min, 2x anisotropic bilinear, point mip");
-        Run(*device, {ReductionMax, FilterAnisoBilinear, MipPoint, Aniso2x}, "max, 2x anisotropic bilinear, point mip");
+        Run(*device, {ReductionMin, FilterAnisoBilinear, MipNone, Aniso2x}, "min, 2x anisotropic bilinear, no mip");
+        Run(*device, {ReductionMax, FilterAnisoBilinear, MipNone, Aniso2x}, "max, 2x anisotropic bilinear, no mip");

It passes 5 of 5 on an RTX 3050 Ti (580.178.04) and on lavapipe, with and without a local change that gives the host sampler a linear mip filter under anisotropy. On the 3050 Ti that local change reproduces your exact 5.000000 on main, and with this change the maxLod 0 clamp still keeps it on level 0. I can't run a 4090. Could you check agc_driver_image_sample_reduction with it there? If it passes, I'll open it as a fix(test) PR.

AI Assisted

@frankea

frankea commented Oct 18, 2026

Copy link
Copy Markdown
Contributor

@SandstromPL it passes. RTX 4090 (NVIDIA 616.92), Windows 11 26200, MinGW-w64 GCC 15.2.0 r7, Release, main 922dc89 in a fresh build directory:

  • with the two rows changed to MipNone / "no mip" exactly as in your diff: agc_driver_image_sample_reduction passes 3 of 3 runs there, and 5 of 5 in a second build of the same tree;
  • unchanged rows, same build, before and after, with the shader cache kept and with it cleared: fails every time with the same value as on 10/15, thread 0 sampled 5.000000, expected 17.000000;
  • main 31cbb55 (the 10/15 tree) rebuilt from scratch today: fails 3 of 3, so nothing merged since changed the result; it is the row, as you said.

Go ahead with the fix(test) PR; happy to run it again once it's up.

AI-assisted: yes (Claude Code).

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

Labels

agc libSceAgc* docs

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants