fix(vk): tree-shadow flicker without the TAAU network: the shadow mask evened out over frames (rebase of #477) #1581

Merged

rakefire-impala merged 3 commits from fix/tree-shadow-flicker-history into main

2026-11-11 02:30:02 +00:00

rakefire-impala commented

2026-11-10 09:41:43 +00:00

(Migrated from github.com)

Rebase of #477 by @imbahzor onto current main, as asked when it was closed ("please rebase on current main and reopen"). The commit is theirs, cherry-picked with -x; the only changes are the conflict resolution and the push-descriptor mark, which now goes through main's descriptors_disturbed() (see Change).

Problem

Over sunlit ground and along shadow edges near trees, shadows flicker constantly in thin streaks, even with the camera still. This happens on GPUs where the gameplay TAAU network is left out ("a network left out on this card" in the log, for example an RX 5500M), so the bilinear stand-in runs instead.

The tree-shadow pass 3427a1dbefd5e02c is a full-screen draw. It rebuilds a normal from an ImageGather of the neighbouring depths and offsets its shadow-map lookup by that normal. On grass blades and leaf edges, those neighbours flip between the blade and the ground behind it as the camera shifts by a sub-pixel each frame, so the pixels flip between lit and shadowed. On the console, the TAAU network averages that away.

Change

Mask history. Right after that draw, its mask (G and B of an RGBA8 target at the scene's size, MIN blended) is blended with its own history in a small compute pass, shaders/shadow_stab.comp: two history images used in turn, then G and B copied back.
- RX_VK_MASK_BLEND=<w>: the weight, default 0.85; 0 turns it off.
- RX_VK_MASK_RADIUS=<r>: limits the history to a (2r+1)² neighbourhood. Default 0 (no limit), because a 3×3 limit kept nothing.
Push-descriptor cache. The new pass pushes descriptors of its own layout, so it calls descriptors_disturbed() right after the push. Without that, the next dispatch with the cached layout skipped its push and read the pass's bindings, which caused a device lost after a save load in #477's testing. #477 also marked movie_upscale and taau_stand_in; main now does that itself.

Tests

tests/test_tree_shadow_mask.py (new) checks that the blend follows the tree-shadow pass only while RX_VK_MASK_BLEND is above 0, needs an RGBA8 storage mask and push descriptors, runs outside the render pass, and that every push of the stand-ins' or the mask's own layout reaches descriptors_disturbed() before its function returns. It fails with any one of those three calls removed.

Tested

#477 (author): RTX 3060, Windows 11. No flicker standing still, and no visible smearing of tree shadows when turning or walking, in Kokiri Forest and the Lost Woods at several times of day.
First rebase (main 779bdcf): played on an RX 5500M (RDNA1, 4 GB, Windows 11, AMD driver; main 779bdcf with #1505–#1514) in Hyrule Field. The constant shadow flicker is gone. The log shows tree-shadow mask blend on #266 768x432, weight 0.85; the pass runs once a frame there.
GTX 1070 (CiLiBi18, main 88b554e with #1509–#1513 and this PR): no flicker, specks or tree-shadow problems noticed in a 23-minute session; see their comment on #1510.
Rebased on main 9447e1d: one conflict, the renamed skin_check call next to the new guard. Compile check clean. tests/run_all.py: 291 passed; the 6 that fail are the same 6 that fail on plain main on this PC, so no new failures. Not played again: the only code change from the played version is the push-descriptor mark going through descriptors_disturbed().

Rebase of #477 by @imbahzor onto current main, as asked when it was closed ("please rebase on current main and reopen"). The commit is theirs, cherry-picked with `-x`; the only changes are the conflict resolution and the push-descriptor mark, which now goes through main's `descriptors_disturbed()` (see Change). ## Problem Over sunlit ground and along shadow edges near trees, shadows flicker constantly in thin streaks, even with the camera still. This happens on GPUs where the gameplay TAAU network is left out ("a network left out on this card" in the log, for example an RX 5500M), so the bilinear stand-in runs instead. The tree-shadow pass `3427a1dbefd5e02c` is a full-screen draw. It rebuilds a normal from an `ImageGather` of the neighbouring depths and offsets its shadow-map lookup by that normal. On grass blades and leaf edges, those neighbours flip between the blade and the ground behind it as the camera shifts by a sub-pixel each frame, so the pixels flip between lit and shadowed. On the console, the TAAU network averages that away. ## Change - **Mask history.** Right after that draw, its mask (G and B of an RGBA8 target at the scene's size, MIN blended) is blended with its own history in a small compute pass, `shaders/shadow_stab.comp`: two history images used in turn, then G and B copied back. - `RX_VK_MASK_BLEND=<w>`: the weight, default 0.85; `0` turns it off. - `RX_VK_MASK_RADIUS=<r>`: limits the history to a (2r+1)² neighbourhood. Default 0 (no limit), because a 3×3 limit kept nothing. - **Push-descriptor cache.** The new pass pushes descriptors of its own layout, so it calls `descriptors_disturbed()` right after the push. Without that, the next dispatch with the cached layout skipped its push and read the pass's bindings, which caused a device lost after a save load in #477's testing. #477 also marked `movie_upscale` and `taau_stand_in`; main now does that itself. ## Tests `tests/test_tree_shadow_mask.py` (new) checks that the blend follows the tree-shadow pass only while `RX_VK_MASK_BLEND` is above 0, needs an RGBA8 storage mask and push descriptors, runs outside the render pass, and that every push of the stand-ins' or the mask's own layout reaches `descriptors_disturbed()` before its function returns. It fails with any one of those three calls removed. ## Tested - **#477 (author):** RTX 3060, Windows 11. No flicker standing still, and no visible smearing of tree shadows when turning or walking, in Kokiri Forest and the Lost Woods at several times of day. - **First rebase (main `779bdcf`):** played on an RX 5500M (RDNA1, 4 GB, Windows 11, AMD driver; main `779bdcf` with #1505–#1514) in Hyrule Field. The constant shadow flicker is gone. The log shows `tree-shadow mask blend on #266 768x432, weight 0.85`; the pass runs once a frame there. - **GTX 1070 (CiLiBi18, main `88b554e` with #1509–#1513 and this PR):** no flicker, specks or tree-shadow problems noticed in a 23-minute session; see their comment on #1510. - **Rebased on main `9447e1d`:** one conflict, the renamed `skin_check` call next to the new guard. Compile check clean. `tests/run_all.py`: 291 passed; the 6 that fail are the same 6 that fail on plain main on this PC, so no new failures. Not played again: the only code change from the played version is the push-descriptor mark going through `descriptors_disturbed()`.

hckrman101 (Migrated from github.com) approved these changes

2026-11-10 16:39:36 +00:00

hckrman101 (Migrated from github.com) left a comment

Nits, none blocking:

The header comment of shadow_stab.comp still says the history is bounded by the 3x3 neighbourhood. The default is now radius 0 (unbounded), as the later comment in the file and the PR text say.
The static views map in mask_stabilize is keyed by VkImage and never cleared. If the mask texture is destroyed and a new image gets the same handle, a stale view would be reused. Probably rare for a render target at fixed size, but dropping entries on texture destroy (or keying on a texture generation) would be safer.
hist_frame validity doesn't notice the mask texture changing between frames (same size, different image). Harmless apart from one blended frame.

Overlap: #1637/#1776 touch shadow write-back, so check them against this if they merge close together.

Looks good. The tree-shadow mask blend only runs on the 3427a1dbefd5e02c draw and only while RX_VK_MASK_BLEND is above 0, so it stays out of the way otherwise. The render pass is ended and a full barrier is taken before and after the two dispatches. Everything stays in GENERAL like the other compute passes here. The `pushed()[1].cmd` reset after each own-layout push matches how `bind_descriptors` uses it on main. CI is green, and the test is picked up by run_all.py's glob. Nits, none blocking: - The header comment of `shadow_stab.comp` still says the history is bounded by the 3x3 neighbourhood. The default is now radius 0 (unbounded), as the later comment in the file and the PR text say. - The static `views` map in `mask_stabilize` is keyed by VkImage and never cleared. If the mask texture is destroyed and a new image gets the same handle, a stale view would be reused. Probably rare for a render target at fixed size, but dropping entries on texture destroy (or keying on a texture generation) would be safer. - `hist_frame` validity doesn't notice the mask texture changing between frames (same size, different image). Harmless apart from one blended frame. Overlap: #1637/#1776 touch shadow write-back, so check them against this if they merge close together.

hckrman101 commented

2026-11-10 17:49:45 +00:00

(Migrated from github.com)

Heads-up: this conflicts with main now (a lot landed today). Could you merge main into the branch (or rebase) and resolve the conflicts? Thanks!

rakefire-impala commented

2026-11-10 23:00:54 +00:00

(Migrated from github.com)

Rebased on main 9447e1d. The only conflict was the line just before the new guard in draw(): dlss_skin_check is skin_check on main now.

One more change came with the rebase. Main now calls descriptors_disturbed() after movie_upscale and taau_stand_in push their own descriptors, so this PR's marks there are gone, and the new pass uses that helper too. The test checks that all three pushes reach it and fails with any one removed.

I also fixed your first nit: the header comment of shadow_stab.comp now says the history is unbounded by default. It's a comment-only change, so the SPIR-V is unchanged. I left the other two nits alone so the code stays as you reviewed it. #1637 and #1776 are both in main already, and this merges on top of them.

Rebased on main `9447e1d`. The only conflict was the line just before the new guard in `draw()`: `dlss_skin_check` is `skin_check` on main now. One more change came with the rebase. Main now calls `descriptors_disturbed()` after `movie_upscale` and `taau_stand_in` push their own descriptors, so this PR's marks there are gone, and the new pass uses that helper too. The test checks that all three pushes reach it and fails with any one removed. I also fixed your first nit: the header comment of `shadow_stab.comp` now says the history is unbounded by default. It's a comment-only change, so the SPIR-V is unchanged. I left the other two nits alone so the code stays as you reviewed it. #1637 and #1776 are both in main already, and this merges on top of them.

hckrman101 (Migrated from github.com) approved these changes

2026-11-11 02:16:20 +00:00

hckrman101 (Migrated from github.com) left a comment

Re-checked after the rebase onto 9447e1d. Still good.

The first nit is fixed: the header comment of shadow_stab.comp now says the history is unbounded by default. The SPIR-V header is byte-identical to the one I reviewed, so it is a comment-only change.
The push-descriptor mark now goes through main's descriptors_disturbed(), which clears both cached pushes, same effect as the old pushed()[1].cmd reset. The marks this PR used to add in movie_upscale and taau_stand_in are gone because main does them itself now. The test's regex finds exactly those three own-layout pushes, and each one is followed by the call.
The conflict resolution next to skin_check is fine. The rest of the draw() hook is unchanged.
The other two nits (the views map keyed by VkImage, hist_frame not noticing a new mask image) were left as they were on purpose. That's fine, they don't block.

#1637 and #1776 have landed, so the overlap I mentioned is settled. CI: everything passes except test_log_batch.py on Windows, which failed its GetTickCount64() - t0 < 200 timing check. This PR doesn't touch logging, so that looks like a slow runner, not this change. test_tree_shadow_mask.py passes on both platforms. A re-run should clear it.

Re-checked after the rebase onto `9447e1d`. Still good. - The first nit is fixed: the header comment of `shadow_stab.comp` now says the history is unbounded by default. The SPIR-V header is byte-identical to the one I reviewed, so it is a comment-only change. - The push-descriptor mark now goes through main's `descriptors_disturbed()`, which clears both cached pushes, same effect as the old `pushed()[1].cmd` reset. The marks this PR used to add in `movie_upscale` and `taau_stand_in` are gone because main does them itself now. The test's regex finds exactly those three own-layout pushes, and each one is followed by the call. - The conflict resolution next to `skin_check` is fine. The rest of the `draw()` hook is unchanged. - The other two nits (the `views` map keyed by VkImage, `hist_frame` not noticing a new mask image) were left as they were on purpose. That's fine, they don't block. #1637 and #1776 have landed, so the overlap I mentioned is settled. CI: everything passes except `test_log_batch.py` on Windows, which failed its `GetTickCount64() - t0 < 200` timing check. This PR doesn't touch logging, so that looks like a slow runner, not this change. `test_tree_shadow_mask.py` passes on both platforms. A re-run should clear it.