feat(vk): an opt-in linear filter switch to narrow down the blocky light halos (#1117) #1366
Open
attarwei-design wants to merge 1 commit from
attarwei-design/fix/vk-nearest-sampler-linear-switch into main
pull from: attarwei-design/fix/vk-nearest-sampler-linear-switch
merge into: MagicBeanDev:main
MagicBeanDev:main
MagicBeanDev:github/pr/2893
MagicBeanDev:github/pr/2890
MagicBeanDev:github/pr/2887
MagicBeanDev:github/pr/2878
MagicBeanDev:github/pr/2875
MagicBeanDev:github/pr/2874
MagicBeanDev:github/pr/2873
MagicBeanDev:github/pr/2871
MagicBeanDev:github/pr/2756
MagicBeanDev:github/pr/2897
MagicBeanDev:github/pr/2896
MagicBeanDev:github/pr/2718
MagicBeanDev:github/pr/2842
MagicBeanDev:github/pr/2691
MagicBeanDev:github/pr/2895
MagicBeanDev:github/pr/2743
MagicBeanDev:github/pr/2765
MagicBeanDev:github/pr/2708
MagicBeanDev:github/pr/2732
MagicBeanDev:github/pr/2755
MagicBeanDev:github/pr/2817
MagicBeanDev:github/pr/2876
MagicBeanDev:release-please--branches--main
MagicBeanDev:release-please--branches--main--release-notes
MagicBeanDev:github/pr/2778
MagicBeanDev:github/pr/2888
MagicBeanDev:github/pr/2866
MagicBeanDev:github/pr/2877
MagicBeanDev:github/pr/2800
MagicBeanDev:github/pr/2829
MagicBeanDev:github/pr/2850
MagicBeanDev:github/pr/2889
MagicBeanDev:github/pr/2882
MagicBeanDev:github/pr/2860
MagicBeanDev:github/pr/2849
MagicBeanDev:github/pr/2886
MagicBeanDev:github/pr/2794
MagicBeanDev:github/pr/2840
MagicBeanDev:github/pr/2830
MagicBeanDev:github/pr/2834
MagicBeanDev:github/pr/2719
MagicBeanDev:github/pr/2742
MagicBeanDev:github/pr/2741
MagicBeanDev:github/pr/2820
MagicBeanDev:github/pr/2771
MagicBeanDev:github/pr/2859
MagicBeanDev:github/pr/2858
MagicBeanDev:github/pr/2823
MagicBeanDev:github/pr/2757
MagicBeanDev:github/pr/2769
MagicBeanDev:github/pr/2844
MagicBeanDev:github/pr/2843
MagicBeanDev:github/pr/2673
MagicBeanDev:github/pr/2855
MagicBeanDev:github/pr/2739
MagicBeanDev:github/pr/2807
MagicBeanDev:github/pr/2851
MagicBeanDev:github/pr/2789
MagicBeanDev:github/pr/2814
MagicBeanDev:github/pr/2818
MagicBeanDev:github/pr/2774
MagicBeanDev:github/pr/2772
MagicBeanDev:github/pr/2750
MagicBeanDev:github/pr/2786
MagicBeanDev:github/pr/2785
MagicBeanDev:github/pr/2782
MagicBeanDev:github/pr/2779
MagicBeanDev:github/pr/2775
MagicBeanDev:github/pr/2706
MagicBeanDev:github/pr/2766
MagicBeanDev:github/pr/2747
MagicBeanDev:github/pr/2759
MagicBeanDev:github/pr/2745
MagicBeanDev:github/pr/2740
MagicBeanDev:github/pr/2733
MagicBeanDev:github/pr/2725
MagicBeanDev:github/pr/2727
MagicBeanDev:github/pr/2728
MagicBeanDev:github/pr/2729
MagicBeanDev:github/pr/2721
MagicBeanDev:github/pr/2722
MagicBeanDev:github/pr/2715
MagicBeanDev:github/pr/2714
MagicBeanDev:github/pr/2707
MagicBeanDev:github/pr/2703
MagicBeanDev:github/pr/2705
MagicBeanDev:github/pr/2697
MagicBeanDev:github/pr/2679
MagicBeanDev:github/pr/2676
MagicBeanDev:github/pr/2669
MagicBeanDev:docs/what-hookshot-connects-to
MagicBeanDev:fix/hdr-encode-test
MagicBeanDev:fix/main-tests
MagicBeanDev:fix/play-cache-files
MagicBeanDev:perf/game-barrier-order
MagicBeanDev:feat/heap-lazy
MagicBeanDev:feat/texture-detail
MagicBeanDev:perf/astc-opaque-bc1
MagicBeanDev:fix/fs-dir-lock
MagicBeanDev:fix/framegen-native-kept
MagicBeanDev:fix/update-no-downgrade
MagicBeanDev:port/k10-gcn-f16-mirrors
MagicBeanDev:fix/framegen-mult-cap
MagicBeanDev:chore/release-as-2.1.1
MagicBeanDev:perf/pcache-load-switch
MagicBeanDev:fix/prepare-progress-log
MagicBeanDev:fix/update-closes-stale-game
MagicBeanDev:fix/hdr-scaled-sampled
MagicBeanDev:perf/first-start-preset
MagicBeanDev:perf/zero-upload
MagicBeanDev:fix/vcore-escape-2s
MagicBeanDev:test/fake-gh-atomic
MagicBeanDev:perf/batch-idle-flush
MagicBeanDev:test/finish-stub-a
MagicBeanDev:chore/record-2.1.0
MagicBeanDev:fix/main-red-tests
MagicBeanDev:fix/test-display-startup-driver-cache
MagicBeanDev:feat/mesh-shader-mode
MagicBeanDev:fix/menu-probes-at-startup
MagicBeanDev:fix/fps-counter-classic
MagicBeanDev:fix/test-log-batch-flake
MagicBeanDev:fix/narrow-screen-hud
MagicBeanDev:ci/prereleases
MagicBeanDev:release/v2.0.15
MagicBeanDev:hotfix/v2.0.15
MagicBeanDev:fix/present-ahead-replay
MagicBeanDev:release/v2.0.13
MagicBeanDev:fix/bracket-paths-2013
MagicBeanDev:fix/hdr-overlays-2013
MagicBeanDev:fix/fps-counter-scale-2013
MagicBeanDev:fix/stick-seen-at-rest
MagicBeanDev:hotfix/menu-fits-screen
MagicBeanDev:fix/f1-always-opens-menu
MagicBeanDev:fix/ini-keep-lines
MagicBeanDev:claude/cranky-pike-7cf247
MagicBeanDev:perf/texture-answer-flat
MagicBeanDev:fix/stamp-test-modules
MagicBeanDev:fix/appimage-fonts
MagicBeanDev:fix/display-awake
MagicBeanDev:release/v2.0.12
MagicBeanDev:fix/discord-windows
MagicBeanDev:rc/v2.0.12-tests
MagicBeanDev:fix/dlss-keep-feature
MagicBeanDev:rc/v2.0.12
MagicBeanDev:button-icons
No reviewers
Labels
Clear labels
accessibility
Barrier affecting people with disabilities
autorelease: pending
autorelease: tagged
bug
Something isn't working
dependencies
Pull requests that update a dependency file
documentation
Improvements or additions to documentation
duplicate
This issue or pull request already exists
enhancement
New feature or request
github_actions
Pull requests that update GitHub Actions code
good first issue
Good for newcomers
help wanted
Extra attention is needed
invalid
This doesn't seem right
question
Further information is requested
testing wanted
wontfix
This will not be worked on
No labels
accessibility
autorelease: pending
autorelease: tagged
bug
dependencies
documentation
duplicate
enhancement
github_actions
good first issue
help wanted
invalid
question
testing wanted
wontfix
Milestone
Clear milestone
No items
No milestone
Assignees
Clear assignees
No assignees
1 participant
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".
No due date set.
Dependencies
No dependencies set
Reference
MagicBeanDev/Hookshot!1366
Loading…
Reference in a new issue
No description provided.
Delete branch "attarwei-design/fix/vk-nearest-sampler-linear-switch"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
What this is
An opt-in switch to help narrow down #1117 (blocky/pixelated light halos and checkerboard fog), not a fix for it. The root cause is still unknown: a handful of samplers filter as
NEARESTwhere the picture has smooth light, and a low-resolution bloom or light buffer read through them shows its texels as square blocks. #1117's reporter verified that making those few samplers linear removes both artefacts, and an independent AMD reporter confirmed it in the same thread, but both had to patch and rebuild their own copy to try it. This makes that experiment available fromhookshot_env.txt, and makes it possible to isolate which samplers are responsible.Related: #1006, #1004, #606, #1172.
What changed
RX_VK_FORCE_LINEAR=<mode>ingpu/vk/vk_draw.cpp:0/ unset1NEARESTsampler becomesLINEAR2/34RX_VK_FORCE_LINEAR_IDS=<id,id,...>5NEARESTsampler except those idsDetails that matter for the replay rules and for testing:
NEARESTminification filter maps to theLINEARfilter with the same mip mode;mipmapModeis untouched, matching #1117's own patch. Comparison (depth-reference) samplers are skipped.disk_stamp()hashes, so switching modes does not rebuild the multi-GB program cache.readme.mddocuments both switches beside the other test switches.tests/test_sampler_force_linear.pyis a game-free source check covering the off-by-default path, the mip-mode mappings, the cache-key ordering and whole-token id matching; it is picked up bytests/run_all.pyautomatically, so no workflow change is needed.Why it is a diagnostic switch and not a fix
Forcing all
NEARESTsamplers to linear also interpolates the low-resolution volumetric buffers into a continuous haze, which on some builds shows up as a washed-out, over-exposed picture. I would rather not guess which samplers should be linear: modes 4 and 5 let anyone bisect the id list from the log and report the exact ids that remove the squares without touching the fog. #1117's reporter did not see the haze on their setup, so this is likely driver/build dependent.Testing
0x9a4a8000, v2.0.11 (also reproduced on v2.0.10), 2560x1440, Kokiri Forest and the Great Deku Tree, 60 fps with the game's own frame generation x3.NEAREST. With it off, the squares are back immediately. No other texture looked blurred in the areas played.python -I -B tests/run_all.py sampler_force_linear: 12 tests pass. The full suite needs the CI toolchain (compilers, SPIR-V/Vulkan headers), so I left it to the workflows here.One topic, no game files, dumps, generated files or build output are included. Happy to rebase onto #1330 if that lands first - it also edits
sampler_make, but a different part of it.Assisted-by: Tencent Hunyuan Hy (via WorkBuddy)
Thanks, this is a useful thing to have for #1117: an env switch is a much better way to bisect the sampler ids than asking people to patch and rebuild. The filter mapping looks right to me (NEAREST -> LINEAR keeps the "no mipmap" maxLod clamp, the two MIPMAP variants keep their mip mode), comparison samplers are skipped, and changing
pbefore the key is mixed does keep the sampler caches honest. Neither name collides with adisk_stamp()prefix either. No settings option ids are added, so nothing to clash with the other open PRs there.Two things before it can go in:
It conflicts with main (GitHub shows it as not mergeable; the branch is 66 commits behind).
sampler_makechanged on main: theif (!sampler_of(id, &p)) ...line is now a per-thread parameters cache (seen,sampler_slot_version) followed byif (!known) p.minf = .... On the rebase, theforce_linear(id, compare, p);call should go right after thatif (!known)line, i.e. afterseen[id] = {v, p}has stored the game's own parameters, so the cached copy stays unmodified."Logged once" isn't what happens (inline). The log fires every time
sampler_makeruns and changes something, which is once per thread per sampler-registration generation bump, and withRX_VK_SAMPLER_CACHE=0it's every call (the comment abovesampler_forputs that at ~48000 a frame). A smallstatic std::mutex+std::unordered_seton(id, was_mag, was_min)with a cap, the way the anisotropy and depth-compare logs just below already do it, would make it actually once.Nits:
6or-1) quietly behaves like mode 5, and something likeonreads as 0. Clamping/ignoring unknown values (or logging the mode once at startup) would make a typo inhookshot_env.txteasier to notice.IdListTestschecks a Python re-implementation of the id matcher rather than the C++ one, so it won't catch a bug in the actual loop. Fine for a diagnostic switch, just worth knowing.CI hasn't run on this branch (no checks reported), so it'd be good to see the workflows go green after the rebase.
@ -1614,0 +1646,4 @@else if (p.minf == NVN_MIN_FILTER_NEAREST_MIPMAP_LINEAR) p.minf = NVN_MIN_FILTER_LINEAR_MIPMAP_LINEAR;}if (p.magf != was_mag || p.minf != was_min)logf("vk: RX_VK_FORCE_LINEAR: mode %d sampler %d was mag %d min %d", mode, id, was_mag, was_min);This runs on every
sampler_makecall that changes the filters, not once per sampler:sampler_for's table is per thread and is dropped on every sampler-registration generation bump, and withRX_VK_SAMPLER_CACHE=0there's no table at all, so this would log on every texture of every draw. A cappedstatic std::unordered_setof(id, was_mag, was_min)under a mutex, like the anisotropy log further down insampler_make, would match the comment and the readme.This line conflicts with main:
sampler_makenow has the per-threadseenparameters cache and anif (!known) ...fallback here. On the rebase, put the call after theif (!known)line so the game's own parameters are what gets cached inseen, and the forced filters only go into the key and the VkSampler.@ -0,0 +33,4 @@BODY = function_body("force_linear")class IdListTests(unittest.TestCase):Nit: these test a Python copy of the matcher (
listedhere), not the C++ loop inforce_linear, so they'd still pass if the C++ had an off-by-one. Not a blocker for a diagnostic switch.Heads-up: this conflicts with main now. Could you rebase onto main and resolve the conflicts? Thanks!
Thanks for the heads-up, and sorry for the delay. I checked current
mainbefore replying, because this turned out to be more than a textual conflict. Two parts: what we actually run locally, and how I'd like to resolve this PR.1. What we do on our own build (context only — nothing here is proposed for merge)
We have been hitting #1117's blocky halos / checkerboard fog since v2.0.11 (also present on v2.0.10) on an RTX 4080, Windows 11, Vulkan, 2560x1440. That build carried a small local
gpu/vk/vk_draw.cpppatch that changed only the handful ofNEARESTsamplers (9–11 per session) toLINEAR, plus a ~32-line tweak invk_shader.cpp'soot_visual()that lowers the fog coefficient (0.08 → 0.02) behindRX_VK_FOG_ADJUST=1/RX_VK_FOG_SCALE=0.02. The fog tweak exists because making every nearest sampler linear also turns the low-resolution volumetric buffers into a continuous white haze on our setup; a broader exposure workaround was dropped early on because it rewrote ~198 programs and wrecked black levels.After upstream added its own
RX_VK_FORCE_LINEARA/B switch in #1485 (2026-11-10), we deleted our sampler patch completely. On the v2.1.0 release (8ef846110, 2026-11-11 01:32 UTC) our build is stockvk_draw.cppplus that one fog tweak, enabled fromhookshot_env.txt:oot_visual()in v2.1.0 is byte-for-byte identical to v2.0.14's original, so the tweak ports unchanged; we rebuild with the official Builder (--update <game> --tag v2.1.0 --fetched) after each upgrade, sincebin\and the patched sources get replaced. Result: the square grids around the fairy glow, the lamps and the doorway light shafts, and the checkerboard fog are gone; no washed-out haze, black levels and materials intact; headless startup logs show the fog tweak active, ~46k program keys kept and no errors.Two honest caveats: we kept the workaround as-is and did not run an A/B on v2.1.0 with it disabled, so "still reproduces" for that release rests on our earlier v2.0.11 A/B (the squares returned immediately with the switch off) and on reports in #1117 (e.g. dmajchrowski on v2.1.0 / RTX 5080). Also, #2289 landed on
mainat 17:14 UTC today, after the v2.1.0 tag; we have not built that revision yet. If it holds up as the root-cause fix, we will re-test with our workaround off and retire it locally. All of the above is a downstream-only workaround, not a candidate for upstream.2. The conflict, and how I'll resolve it
State right now: this PR is one commit (
e7176315) based on581afafb;mainis2ed98c46, 787 commits ahead, and GitHub reportsmergeable: false / dirty / rebaseable: false. The three files aregpu/vk/vk_draw.cpp(+38),readme.md(+13) and a newtests/test_sampler_force_linear.py(+128).The substantive problem:
mainlanded #1330 (sampler cache / descriptor slotgen) and #1485, and #1485 added its ownRX_VK_FORCE_LINEARswitch insidesampler_make()at theVkSamplerCreateInfolayer (env_on("RX_VK_FORCE_LINEAR"), logged asmade linear: RX_VK_FORCE_LINEAR). Our old call-site anchor (if (!sampler_of(id, &p)) ...) no longer exists, and the two switches share a name but not a meaning: ours readsatoias modes 1–5 plusRX_VK_FORCE_LINEAR_IDS, upstream's is a boolean A/B (0/false/off/nomean off). A mechanical rebase that just relocates our hunk would makeRX_VK_FORCE_LINEAR=4or=5silently force all samplers — actively misleading. Upstream's version also affects comparison samplers, which mine deliberately skipped, and it leavesmipmapModealone; those are real behavioural differences, not just whitespace. With #2289 potentially fixing the root cause, a second diagnostic switch may be churn.So rather than force the old diff, I would propose one of these (happy to take your preference):
mainand reduce the PR to a no-behavior-change contribution: drop ourforce_linear()helper and its call; rewrite thereadme.mdaddition as documentation of the existing upstream switch (what it does, that it is an A/B diagnostic for #1117 and not a fix, that it can add haze on some drivers); retarget the 12 game-free tests at whatmainactually does (off by default, onlyNEAREST→LINEAR,mipmapModeuntouched, the log line present).tests/run_all.pydiscoverstests/test_*.pyautomatically, so no workflow change.vk_draw.cppstays untouched → clean merge.RX_VK_FORCE_LINEAR_MODE) and implement it as an extension of upstream's switch at the NVN parameter layer, before the sampler cache key is mixed, with updated tests. No name collision; more code to review.Mechanically: local git transport here is unreliable (proxy / schannel), so I rebase through the GitHub Git Data API — take
main's tree, create blobs for the rebased files, build a tree withmainHEAD as parent, thenPATCHthe branch ref (the same route used for the earlier rebase). I use function-level anchors with assertions rather than line numbers, confirmmergeableturns true, and run the Python test locally before pushing. I will proceed with A by default; if you would rather have B, or consider this obsolete after #2289 (C), just say so and I will adjust or close instead. One small note: my workflows still show 0 check-runs (first-time contributor approval), so CI green will need a maintainer's approval on your side.This reply is AI-assisted and for reference only.
I'd go with your option C and close this. Main already has
RX_VK_FORCE_LINEAR, an env_on boolean added in #1485 (vk_draw.cpp:2391). As you said, rebasing this mode-based version under the same name would make=4or=5silently force every sampler.#2289 has also merged. It fixes the cause behind #1117: registrations made before the renderer started reached the back end as zeros, which turned sampler 258 into a nearest sampler. #1117 itself was closed today and folded into #1004. So there's no need left for a per-id bisection switch.
Option A wouldn't add much on its own, since main's switch already has a test (sampler_admission_test.cpp). If the haze still shows on a build with #2289, please report it in #1004 with a log, and that would be the case for reopening this under a separate name such as
RX_VK_FORCE_LINEAR_MODE.This now conflicts with main in
gpu/vk/vk_draw.cpp,readme.md. Could you merge main into the branch (or rebase) when you get a chance? I'll take another look once it's up to date.Thanks both — for the nudge, and for laying out the reasoning so clearly. I checked
mainagain before answering.Yes, I can bring the branch up to date — a merge of
maininto the branch or a rebase, whichever you prefer. But before I spend another round of your time on it, I want to lay out why I think that's no longer the right move.What I verified on current
main:main(gpu/vk/vk_bank.cppnow reads the early registrations from the host's copies). The explanation fits what I saw — 258 was the first id my v2.0.11 log kept naming, and a zeroed registration is exactly what turned it into a nearest sampler.RX_VK_FORCE_LINEARis already there as anenv_onboolean from #1485 (now aroundvk_draw.cpp:2407), andtests/sampler_admission_test.cppdoes have aforce-linearcase. So the A/B switch and a test for it both exist upstream.sampler_make()gained the per-threadseencache, the boolean switch and #2289's fix. Those review notes — put the call afterif (!known), make the log actually once, clamp unknown modes, test the C++ matcher rather than the Python copy — would all have to be re-implemented against code that no longer needs the diagnostic. And keeping my mode-based switch under the same name would make=4/=5silently force every sampler, as you both pointed out. The honest result of a "successful" rebase is an empty diff.So I'm with option C and happy to close this now. If you'd still like an up-to-date branch for the record, or want one more look before I do, just say the word and I'll do the merge/rebase first — otherwise I'll close it rather than leave it hanging. One small gap I did notice while checking:
readme.mdonmainstill doesn't document the existing switch; if that's worth having, I can send it as a tiny docs-only PR instead.On the haze: the one I saw came from forcing every nearest sampler to linear on a v2.0.x / v2.1.0 build plus a local fog-scale tweak to compensate, and that build predates #2289, so it isn't evidence against the fix. The first thing I'll try on the next release is a clean, unpatched build. If haze or overexposure is still there, I'll report it in #1004 with a log — and that would be the point to reopen a per-id switch under a separate name such as
RX_VK_FORCE_LINEAR_MODE, as suggested.I drafted this with a little help from an AI assistant.