Skip to content

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 into
NotRexed:mainfrom
attarwei-design:fix/vk-nearest-sampler-linear-switch
Open

attarwei-design wants to merge 1 commit into
NotRexed:mainfrom
attarwei-design:fix/vk-nearest-sampler-linear-switch

Conversation

@attarwei-design

Copy link
Copy Markdown

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 NEAREST where 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 from hookshot_env.txt, and makes it possible to isolate which samplers are responsible.

Related: #1006, #1004, #606, #1172.

What changed

RX_VK_FORCE_LINEAR=<mode> in gpu/vk/vk_draw.cpp:

mode effect
0 / unset off (default)
1 every non-comparison NEAREST sampler becomes LINEAR
2 / 3 only magnification / only minification
4 only the ids in RX_VK_FORCE_LINEAR_IDS=<id,id,...>
5 every NEAREST sampler except those ids

Details that matter for the replay rules and for testing:

  • The filter is changed on the NVN parameters before they are mixed into the sampler cache key. An id whitelist therefore cannot hand a sampler made for one id to a different id with the same original parameters.
  • Each NEAREST minification filter maps to the LINEAR filter with the same mip mode; mipmapMode is untouched, matching Blocky light halos and checkerboard smoke go away when the few NEAREST samplers are made linear #1117's own patch. Comparison (depth-reference) samplers are skipped.
  • Read once into function-local statics, so it costs nothing while off. Neither name is among the prefixes disk_stamp() hashes, so switching modes does not rebuild the multi-GB program cache.
  • Every changed sampler is logged once with the filters it had, so a short id list can be built from a real run.

readme.md documents both switches beside the other test switches. tests/test_sampler_force_linear.py is 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 by tests/run_all.py automatically, so no workflow change is needed.

Why it is a diagnostic switch and not a fix

Forcing all NEAREST samplers 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

  • Windows 11, RTX 4080, Vulkan 1.4, driver 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.
  • With the switch on, the square grids around the fairy's glow, the lamps and the doorway light shafts are gone; 9-11 samplers per session are reported (for example ids 258, 268, 318, 405, 406, 419, 430, 434, 437, 270, 378), each originally 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)

@attarwei-design
attarwei-design force-pushed the fix/vk-nearest-sampler-linear-switch branch from c8f750d to 7d726c5 Compare November 9, 2026 23:23
…otRexed#1117)

A few samplers filter as nearest where the picture has smooth light: a
low-resolution bloom or light buffer read through them shows its texels as
square blocks around Navi, the Kokiri lamps and light shafts, and as a
checkerboard in the fog. NotRexed#1117 found the cause and NotRexed#1006 collects the
reports, but which sampler registrations should be linear is still unknown.

RX_VK_FORCE_LINEAR adds a diagnostic switch to narrow that down. Mode 1
filters every non-comparison nearest sampler linearly, 2 only magnification,
3 only minification, 4 only the ids in RX_VK_FORCE_LINEAR_IDS and 5 every
nearest sampler except those ids. It is off unless set, reads the environment
once into a function-local static, and its names are not among the prefixes
disk_stamp() hashes, so trying a mode does not rebuild the program cache.

The filter change is made to the NVN parameters before they are mixed into
the sampler cache key, so an id whitelist cannot hand one sampler to another
id with the same original parameters. Each nearest minification filter maps
to the linear filter with the same mip mode; comparison samplers are left
alone. Every changed sampler is logged once with the filters it had.

This is a diagnostic switch, not the fix. Forcing all nearest samplers also
smoothes the low-resolution volumetric buffers into a continuous haze on
some drivers, so mode 4 with a short id list is the way to isolate them.
README documents the switch beside the other test switches, and a game-free
source test covers the off-by-default path, the mip modes, the cache key and
the id list.

Assisted-by: Tencent Hunyuan Hy (via WorkBuddy)
Co-Authored-By: Tencent Hunyuan Hy <noreply@tencent.com>
@attarwei-design

Copy link
Copy Markdown
Author

Thanks for the heads-up, and sorry for the delay. I checked current main before 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.cpp patch that changed only the handful of NEAREST samplers (9–11 per session) to LINEAR, plus a ~32-line tweak in vk_shader.cpp's oot_visual() that lowers the fog coefficient (0.08 → 0.02) behind RX_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_LINEAR A/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 stock vk_draw.cpp plus that one fog tweak, enabled from hookshot_env.txt:

RX_VK_FORCE_LINEAR=1
RX_VK_FOG_ADJUST=1
RX_VK_FOG_SCALE=0.02

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, since bin\ 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 main at 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 on 581afafb; main is 2ed98c46, 787 commits ahead, and GitHub reports mergeable: false / dirty / rebaseable: false. The three files are gpu/vk/vk_draw.cpp (+38), readme.md (+13) and a new tests/test_sampler_force_linear.py (+128).

The substantive problem: main landed #1330 (sampler cache / descriptor slotgen) and #1485, and #1485 added its own RX_VK_FORCE_LINEAR switch inside sampler_make() at the VkSamplerCreateInfo layer (env_on("RX_VK_FORCE_LINEAR"), logged as made 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 reads atoi as modes 1–5 plus RX_VK_FORCE_LINEAR_IDS, upstream's is a boolean A/B (0 / false / off / no mean off). A mechanical rebase that just relocates our hunk would make RX_VK_FORCE_LINEAR=4 or =5 silently force all samplers — actively misleading. Upstream's version also affects comparison samplers, which mine deliberately skipped, and it leaves mipmapMode alone; 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):

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 with main HEAD as parent, then PATCH the branch ref (the same route used for the earlier rebase). I use function-level anchors with assertions rather than line numbers, confirm mergeable turns 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.

@Joey-Magnus Joey-Magnus left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 =4 or =5 silently 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.

@attarwei-design

Copy link
Copy Markdown
Author

Thanks both — for the nudge, and for laying out the reasoning so clearly. I checked main again before answering.

Yes, I can bring the branch up to date — a merge of main into 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:

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.md on main still 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.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants