Repository navigation
feat(agc): implement unnormalized S# clamp modes 4 and 5 (half border) - #14964
shahulmubeen-a wants to merge 4 commits into
Conversation
|
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.
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.
|
✅ NVIDIA GPU test: passedPR head
Unstable on main (2, these also failed in recent main runs on this machine; not counted against this PR)Failing on main too (3, not counted against this PR)Validation IDs already present on main (5 IDs in 484 tests)Skipped (45)ReproduceAutomated build and test on a contributor's NVIDIA machine; it does not replace upstream CI. A new comment is posted for each new push. Full logs. |
|
AMD check of this stack, since it was tested on NVIDIA only: Radeon RX 6700 XT (RDNA 2), AMD proprietary driver 26.10.1, Windows 11 Enterprise 10.0.22621, WinLibs GCC 15.2.0 UCRT r7, Release. Exact head
This covers the AMD proprietary driver on RDNA 2. RADV and a PS5 were not run. 🤖 Generated with Claude Code |
BunnyFl
left a comment
There was a problem hiding this comment.
An RDNA 4 run, to go with JJenkx's RDNA 2 one. Windows 11, AMD Radeon RX 9070 XT (RDNA 4), AMD proprietary driver 32.0.32015.7029, WinLibs GCC, BUILD_TESTING=ON, with VK_INSTANCE_LAYERS=VK_LAYER_KHRONOS_validation, ANYPS5_REQUIRE_VULKAN=1 and ANYPS5_NO_SHADER_CACHE=1.
Head 51aaebd7d6 (which carries #14963's c1baa5b783), merged into current main:
agc_driver_image_sample_unnormalizedandagc_driver_graphicspass, with 0 VUIDs.- With
main'sGuestSamplerResource.cppand the four recompiler files restored, both fail, with 0 VUIDs.image_sample_unnormalizedstops onguest sampler descriptor uses unnormalized coordinates with clamp mode 7 on X.
So the emitted modes 4, 5 and 7 give the measured gfx1036 values on RDNA 4 as well. The half-border bilinear result and the per-tap mirroring hold on this generation too.
RDNA2 (gfx1036) mirrors texel index i < 0 to -i - 1 under clamp mode 7, as under modes 1 and 3, and reads the border colour past the edge. Bind clamp-to-border for a mode 7 axis and fold its coordinate with the existing mirror specialization. Raise a folded coordinate to at least 0.5: each bilinear tap is mirrored on its own, so the tap at index -1 reads texel 0 again where clamp-to-border would read the border. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
RDNA2 (gfx1036) samples clamp mode 4 like clamp-to-last-texel and mode 5 like mirror-once with a point filter. With a bilinear filter both clamp the coordinate, mode 5 after mirroring, to [0, size of the level] and read the border colour past the edge, so an edge sample is half texel and half border. The magnification filter decides, as for the other unnormalized samplers. Bind clamp-to-border for a mode 4 or 5 axis when the magnification filter is bilinear and clamp-to-edge otherwise, set the mirror axis bit for mode 5, and carry the mode 4 and 5 axes in a new specialization constant (HalfBorderUnnormalizedBase, 57728). The shader clamps those axes to the size of level 0 of the bound view, the level an unnormalized sample reads. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
51aaebd to
9b4cca8
Compare
|
RADV/RX 9060 XT: Head Sensitivity check: on the merged tree, forcing the half-border axis mask to zero at the shader-emission call made the test fail 3/3 for the bilinear X/Y clamp-half-border sample: component 0 was 255 instead of 127.5. Restoring the shader emitter passed 3/3. This is a targeted mutation, not an unmodified-main comparison for the new cases. Host: CachyOS Linux ReproductionBuild export ANYPS5_REQUIRE_VULKAN=1 ANYPS5_NO_SHADER_CACHE=1
export VK_INSTANCE_LAYERS=VK_LAYER_KHRONOS_validation VK_LOADER_DEBUG=layer
export VK_LAYER_PATH="$HOME/.local/share/anyps5-tester/vk-layers/usr/share/vulkan/explicit_layer.d"
ctest --test-dir build-main -V -R '^agc_driver_image_sample_unnormalized$'Limits: one focused synthetic sampling test, no title, performance measurement or PS5 hardware. The mutation demonstrates sensitivity for the stated bilinear case, not independent confirmation of console behavior. AI-assisted independent hardware test run (GPT-6.1 Terra). |
✅ NVIDIA GPU test: passedPR head
Unstable on main (2, these also failed in recent main runs on this machine; not counted against this PR)Failing on main too (3, not counted against this PR)Validation IDs already present on main (8 IDs in 531 tests)Skipped (50)ReproduceAutomated build and test on a contributor's NVIDIA machine; it does not replace upstream CI. A new comment is posted for each new push. Full logs. |
✅ AMD GPU test: passedPR head
Unstable on main (1, these also failed in recent main runs on this machine; not counted against this PR)Failing on main too (7, not counted against this PR)Validation IDs already present on main (8 IDs in 531 tests)Skipped (61)ReproduceAutomated build and test on a contributor's AMD machine; it does not replace upstream CI. A new comment is posted for each new push. Full logs. |
|
@shahulmubeen-a @danielhorngt a heads-up: this PR and #14621 now take the same specialization constant base.
On |
BunnyFl
left a comment
There was a problem hiding this comment.
@shahulmubeen-a re-checked at 9b4cca8cb6. git range-diff of my approved 51aaebd7d6 against this head shows one change to this PR's own patch: HalfBorderUnnormalizedBase moves from 57600 to 57728 (core/shader/recompiler/PipelineSpecialization.hpp:54), because main now uses 57600 for MinLodBase (:52). #14963's commit underneath is unchanged.
What I ran: head 9b4cca8cb6 merged onto main b11c7bf84f (merge bd34bba607), Windows 11, WinLibs GCC, Release, AMD Radeon RX 9070 XT, driver 32.0.32015.12012, with ANYPS5_REQUIRE_VULKAN=1, ANYPS5_NO_SHADER_CACHE=1 and VK_INSTANCE_LAYERS=VK_LAYER_KHRONOS_validation:
agc_driver_image_sample_unnormalized: passes, including the mode 4 and 5 half-border cases. The log has noVUID-,SYNC-HAZARDor missing-layer line.check_conventions.py --base upstream/mainpasses.pr_overlap.py --branch: 63 open PRs touch the same files, none the same exports. The 57728 clash with #14621 is in my note above. Whichever of you lands second takes the next free base.
|
@BunnyFl thanks for catching this. @danielhorngt, a proposal for the order: #14964 first, then #14621 on top of it.
If that works for you, #14621 would move Written by Claude Code, verified by me. |
|
@shahulmubeen-a a heads-up on numbering, no change asked of this PR as it stands: AI-assisted: yes (Claude Code) |
|
Thanks man |
✅ NVIDIA GPU test: passedPR head
Unstable on main (1, these also failed in recent main runs on this machine; not counted against this PR)Failing on main too (3, not counted against this PR)Validation IDs already present on main (9 IDs in 595 tests)Skipped (56)ReproduceAutomated build and test on a contributor's NVIDIA machine; it does not replace upstream CI. A new comment is posted for each new push. Full logs. |
What
A FORCE_UNNORMALIZED S# with clamp mode 4 (clamp-half-border) or 5 (mirror-once-half-border) on X or Y is refused today (after #14963):
These are the last two clamp modes an unnormalized S# refuses. The measurement is the one from #14963 (RDNA2 gfx1036 through
tools/hw-oracle,image_sample_lzandimage_sample_lz_o, 64x4 linear 32_FLOAT with texel100 + x + 1000 * y, transparent black and opaque white borders, point and bilinear, a run with unequal filters and a run on a view from mip 1).What the hardware does:
Measured rows (excerpt)
X, 64x4, v = 1.5 (row 1, texel = 1100 + x), transparent black border:
With the white border: 550.5, 825.25, 872.5 and 582 in place of 550, 825, 872.25 and 581.5.
Y (u = 10.5, texel = 110 + 1000 * y), linear, transparent black: clamp 4
-9.5:55 0.0:55 0.5:110 3.75:2332.5 4.0:1555 8.5:1555; clamp 5-9.5:1555 -3.5:3110 -1.0:610 0.0:110 4.5:1555.Offsets (
image_sample_lz_o, linear, transparent black): clamp 41.5 + (-3):550 -1.5 + 3:1101 62.5 + 3:581.5; clamp 52.5 + (-5):1102 63.5 + 1:581.5.Unequal filters, clamp 4, transparent black,
-1.0 / 0.25 / 64.0: magnification point, minification bilinear1100 / 1100 / 1163; magnification bilinear, minification point550 / 825 / 581.5.View from mip 1 (32x2, v = 0.5, texel 5000 + x), linear, transparent black: clamp 4
-0.25:2500 0.25:3750 31.75:3773.25 32.0:2515.5 64.0:2515.5; clamp 5-32.5:2515.5 -31.5:5031 0.0:5000 32.5:2515.5.How the change implements this:
DecodeSamplerResourceaccepts modes 4 and 5 with unnormalized coordinates. It binds clamp-to-border for that axis when the magnification filter is bilinear and clamp-to-edge when it is point (the host sampler already uses the magnification filter for both).ProveUnnormalizedsets the mirror axis bit for mode 5, as for 1, 3 and 7, and a new per-sampler constantHalfBorderUnnormalizedBase(57792, the next free 64-aligned base after fix(agc): bind a T# minimum LOD through an S# that holds the LOD at 0 #12980'sMinLodBase57600 andVolumeGatherBase57664 and fix(agc): apply a LOD bias beyond the device limit in the shader #14621'sSamplerLodBiasBase57728) carries the axes in mode 4 or 5.OpImageQuerySizeLodat level 0 of the bound view, the level an unnormalized sample reads. With a point filter the clamp changes nothing, since clamp-to-edge reads the last texel at size anyway.The model check in #14963's first comment (fold to the float below -c, at least 0.5, this clamp, then the host sampler with 8-bit bilinear weights) matches all 1660 measured values, including every mode 4 and 5 row and the mip-1 view.
The
debt/libSceAgcDriver.mdentry now covers modes 4 and 5; with this change no clamp mode throws for unnormalized coordinates. Anisotropy and MC_COORD_TRUNC still do.No title I know of uses these modes, so
Unblocksis none.Tested
Windows 11 Pro 26300, NVIDIA GeForce RTX 4070 Ti SUPER (610.88), WinLibs GCC 15.2.0, Release. Tested head: 51aaebd on c1baa5b (#14963) on #11126's head 1c9c1fa.
agc_driver_image_sample_unnormalizedgains six mode 4 and 5 samplers, checked like the others (all sample instructions, 1-level and 4-level views, and the view from mip 1, whose 6x3 size the clamp has to use):The reference clamps a bilinear coordinate on a mode 4 axis to [0, size] and on a mode 5 axis to [-size, size] before the footprint, mirrors index i < 0 on mode 5, and returns the border colour past the edge for a bilinear mode 4 or 5.
guest sampler descriptor uses unnormalized coordinates with clamp mode 4 on X; only wrap, mirror, clamp-to-last-texel, mirror-once, clamp-to-border and mirror-once-border are implemented.SpirvImageEmitter.cppfrom c1baa5b (no clamp in the shader), it fails withimage_sample_lz, bilinear clamp-half-border on X and Y, opaque white, 1-level image: (-1.000000, 2.500000) offset (0, 0) component 0 is 255.000000, expected 127.500000.agc_driver_graphics(the sampler decode test) expects modes 4 and 5 to decode to clamp-to-border with a bilinear magnification filter and clamp-to-edge with a point one, each with the other minification filter. With the sources of c1baa5b it fails with the same mode 4 refusal.I ran 84 sampling and shader tests with
ctest -V,VK_INSTANCE_LAYERS=VK_LAYER_KHRONOS_validation,ANYPS5_NO_SHADER_CACHE=1andANYPS5_REQUIRE_VULKAN=1onmain926f7dd and on c1baa5b and 51aaebd. The regex wassample|gather|sampler|shader_memory|offset|unnormalized|shader_disk_cache|on_line|on_plane|on_volume|lod|^agc_driver_graphics$.was not found but was requested, and each log has the same 9 validation warnings (Undefined-Value-ShaderOutputNotConsumed-DynamicRendering).VUID-error.agc_driver_sample_order_grid_reference, which skips itself on this device (it does not keep depth at custom sample locations).Retested on the head with main merged in (05700f8, main ef0d852): the same regex now matches 104 tests. Run with the validation layer, which
VK_LOADER_DEBUG=layershows inserted as an instance and a device layer: 103 pass andagc_driver_sample_order_grid_referenceskips itself. NoVUID-lines.tools/check_conventions.py --base origin/mainpasses.Checklist
main; no other open PR implements the same functionsNotImplemented_nid_no_patch); silent stubs are listed under Silent stubs in their library's technical debt file.mdfiles or images added to the repository (attach them to this PR)🤖 Generated with Claude Code