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
attarwei-design commented 2026-11-09 23:22:03 +00:00 (Migrated from github.com)

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 #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)

### What this is An opt-in switch to help narrow down [#1117](https://git.magicbean.dev/MagicBeanDev/Hookshot/issues/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](https://git.magicbean.dev/MagicBeanDev/Hookshot/issues/1006), [#1004](https://git.magicbean.dev/MagicBeanDev/Hookshot/issues/1004), [#606](https://git.magicbean.dev/MagicBeanDev/Hookshot/issues/606), [#1172](https://git.magicbean.dev/MagicBeanDev/Hookshot/issues/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 #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)
hckrman101 (Migrated from github.com) reviewed 2026-11-10 04:14:42 +00:00
hckrman101 (Migrated from github.com) left a comment

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 p before the key is mixed does keep the sampler caches honest. Neither name collides with a disk_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:

  1. It conflicts with main (GitHub shows it as not mergeable; the branch is 66 commits behind). sampler_make changed on main: the if (!sampler_of(id, &p)) ... line is now a per-thread parameters cache (seen, sampler_slot_version) followed by if (!known) p.minf = .... On the rebase, the force_linear(id, compare, p); call should go right after that if (!known) line, i.e. after seen[id] = {v, p} has stored the game's own parameters, so the cached copy stays unmodified.

  2. "Logged once" isn't what happens (inline). The log fires every time sampler_make runs and changes something, which is once per thread per sampler-registration generation bump, and with RX_VK_SAMPLER_CACHE=0 it's every call (the comment above sampler_for puts that at ~48000 a frame). A small static std::mutex + std::unordered_set on (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:

  • Any mode other than 0-5 (say 6 or -1) quietly behaves like mode 5, and something like on reads as 0. Clamping/ignoring unknown values (or logging the mode once at startup) would make a typo in hookshot_env.txt easier to notice.
  • Most of the test is string matching on the source, and IdListTests checks 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.

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 `p` before the key is mixed does keep the sampler caches honest. Neither name collides with a `disk_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: 1. **It conflicts with main** (GitHub shows it as not mergeable; the branch is 66 commits behind). `sampler_make` changed on main: the `if (!sampler_of(id, &p)) ...` line is now a per-thread parameters cache (`seen`, `sampler_slot_version`) followed by `if (!known) p.minf = ...`. On the rebase, the `force_linear(id, compare, p);` call should go right after that `if (!known)` line, i.e. after `seen[id] = {v, p}` has stored the game's own parameters, so the cached copy stays unmodified. 2. **"Logged once" isn't what happens** (inline). The log fires every time `sampler_make` runs and changes something, which is once per thread per sampler-registration generation bump, and with `RX_VK_SAMPLER_CACHE=0` it's every call (the comment above `sampler_for` puts that at ~48000 a frame). A small `static std::mutex` + `std::unordered_set` on `(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: - Any mode other than 0-5 (say `6` or `-1`) quietly behaves like mode 5, and something like `on` reads as 0. Clamping/ignoring unknown values (or logging the mode once at startup) would make a typo in `hookshot_env.txt` easier to notice. - Most of the test is string matching on the source, and `IdListTests` checks 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);
hckrman101 (Migrated from github.com) commented 2026-11-10 04:14:42 +00:00

This runs on every sampler_make call 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 with RX_VK_SAMPLER_CACHE=0 there's no table at all, so this would log on every texture of every draw. A capped static std::unordered_set of (id, was_mag, was_min) under a mutex, like the anisotropy log further down in sampler_make, would match the comment and the readme.

This runs on every `sampler_make` call 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 with `RX_VK_SAMPLER_CACHE=0` there's no table at all, so this would log on every texture of every draw. A capped `static std::unordered_set` of `(id, was_mag, was_min)` under a mutex, like the anisotropy log further down in `sampler_make`, would match the comment and the readme.
hckrman101 (Migrated from github.com) commented 2026-11-10 04:14:42 +00:00

This line conflicts with main: sampler_make now has the per-thread seen parameters cache and an if (!known) ... fallback here. On the rebase, put the call after the if (!known) line so the game's own parameters are what gets cached in seen, and the forced filters only go into the key and the VkSampler.

This line conflicts with main: `sampler_make` now has the per-thread `seen` parameters cache and an `if (!known) ...` fallback here. On the rebase, put the call after the `if (!known)` line so the game's own parameters are what gets cached in `seen`, 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):
hckrman101 (Migrated from github.com) commented 2026-11-10 04:14:42 +00:00

Nit: these test a Python copy of the matcher (listed here), not the C++ loop in force_linear, so they'd still pass if the C++ had an off-by-one. Not a blocker for a diagnostic switch.

Nit: these test a Python copy of the matcher (`listed` here), not the C++ loop in `force_linear`, so they'd still pass if the C++ had an off-by-one. Not a blocker for a diagnostic switch.
hckrman101 commented 2026-11-11 15:51:07 +00:00 (Migrated from github.com)

Heads-up: this conflicts with main now. Could you rebase onto main and resolve the conflicts? Thanks!

Heads-up: this conflicts with main now. Could you rebase onto main and resolve the conflicts? Thanks!
attarwei-design commented 2026-11-11 18:55:57 +00:00 (Migrated from github.com)

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):

  • A (preferred) — rebase onto current main and reduce the PR to a no-behavior-change contribution: drop our force_linear() helper and its call; rewrite the readme.md addition 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 what main actually does (off by default, only NEAREST → LINEAR, mipmapMode untouched, the log line present). tests/run_all.py discovers tests/test_*.py automatically, so no workflow change. vk_draw.cpp stays untouched → clean merge.
  • B — if you want the mode / id bisection capability, keep it but rename the variable (e.g. 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.
  • C — if #2289 is confirmed and the A/B switch is going away, I am equally happy to close #1366 as obsolete rather than add churn.

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.

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): - **A (preferred)** — rebase onto current `main` and reduce the PR to a no-behavior-change contribution: drop our `force_linear()` helper and its call; rewrite the `readme.md` addition 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 what `main` actually does (off by default, only `NEAREST` → `LINEAR`, `mipmapMode` untouched, the log line present). `tests/run_all.py` discovers `tests/test_*.py` automatically, so no workflow change. `vk_draw.cpp` stays untouched → clean merge. - **B** — if you want the mode / id bisection capability, keep it but **rename** the variable (e.g. `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. - **C** — if #2289 is confirmed and the A/B switch is going away, I am equally happy to close #1366 as obsolete rather than add churn. 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 (Migrated from github.com) reviewed 2026-11-11 21:45:51 +00:00
Joey-Magnus (Migrated from github.com) left a comment

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.

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`.
hckrman101 commented 2026-11-11 22:55:31 +00:00 (Migrated from github.com)

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.

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.
attarwei-design commented 2026-11-12 01:02:07 +00:00 (Migrated from github.com)

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:

  • #1117 was closed at 21:22 UTC today and folded into #1004; #2289 is in main (gpu/vk/vk_bank.cpp now 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_LINEAR is already there as an env_on boolean from #1485 (now around vk_draw.cpp:2407), and tests/sampler_admission_test.cpp does have a force-linear case. So the A/B switch and a test for it both exist upstream.
  • Resolving the conflict isn't just moving a hunk. Since the Nov 10 review, sampler_make() gained the per-thread seen cache, the boolean switch and #2289's fix. Those review notes — put the call after if (!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 / =5 silently 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.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.

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`: - #1117 was closed at 21:22 UTC today and folded into #1004; #2289 is in `main` (`gpu/vk/vk_bank.cpp` now 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_LINEAR` is already there as an `env_on` boolean from #1485 (now around `vk_draw.cpp:2407`), and `tests/sampler_admission_test.cpp` does have a `force-linear` case. So the A/B switch and a test for it both exist upstream. - Resolving the conflict isn't just moving a hunk. Since the Nov 10 review, `sampler_make()` gained the per-thread `seen` cache, the boolean switch and #2289's fix. Those review notes — put the call after `if (!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` / `=5` silently 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.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.
Commenting is not possible because the repository is archived.
No description provided.