[TwoToneEddy]

…rent draws (RX_VK_REACTIVE=1)

The temporal upscalers follow a pixel by its depth and motion. What the scene blends over itself without writing depth (water, smoke, fire, glows) has neither, so it is taken for the surface behind and smears or ghosts.

With RX_VK_REACTIVE=1 the scene's colour is copied before its transparent draws (blended, depth tested, depth not written: reactive_policy.h), and at the upscale the scene's target is compared with that copy (shaders/reactive_mask.comp). The difference goes to FSR 3.1's reactivity pass as its reactive mask and to DLSS as its bias-current-colour mask.

Off by default: which draws are transparent is told from their state alone and has not been checked in the game. RX_VK_REACTIVE_DEBUG=1 shows the mask.

[hckrman101]

Thanks, the idea is sound and the policy/copy/compare split is easy to follow. The reactive_policy.h unit test is a good touch. I read the change through; I haven't run it, and it's off by default so the risk is contained. One real blocker:

This is based on a pre-#1008 main and no longer merges (mergeable_state: dirty). The upscaler code the hooks attach to has moved:

• dlss_jitter_size() no longer exists on main (the DLSS side now lives in the TemporalEngine: temporal_jitter_size in vk_temporal.h, and temporal_scene_size() / temporal_note_scene_draw() in vk_draw.cpp). reactive_scene_draw() calls dlss_jitter_size(0/1), so it won't compile after a rebase as is. It probably wants temporal_scene_size(at.width, at.height) (or temporal_jitter_size) for the DLSS/FSR 4 side, keeping fsr3_inputs::jitter_size for FSR 3.1.
• DLSS evaluation is now evaluate(cmd, const TemporalFrame& f) in vk_dlss.cpp (ep.InReset = f.reset...); the dlss_upscale(... s.color.view, s.feature_rw ...) hunk where you set pInBiasCurrentColorMask has nothing to apply to. The mask call needs to go into that function using the TemporalFrame's colour view and size.
• vk_fsr3.cpp and the in.reactive swap into P_PREPARE_REACTIVITY look like they still apply (main still passes tex(g.default_reactive) there).

Also worth deciding while you rebase: with FSR 4 now going through the same TemporalEngine, does it have a reactive input you'd want to feed too, or should the doc say explicitly DLSS + FSR 3.1 only? (#1654 is open in the same area, so a quick look at how it lands is worthwhile.)

Things I checked that look fine: copy/mask recorded outside a pass (end_pass before the copy), full_barrier around the copy and the dispatch, descriptors_disturbed() after the push-descriptor pass, destroy-generation check before touching the scene texture, deferred freeing of replaced images, constant-initialised thread_locals (no llvm-mingw TLS-init cost), tests picked up by the run_all glob, R8 storage-image check in wanted().

Nit: the mask is the whole difference between the copy and the final scene target, so a blended no-depth-test draw that comes after the last transparent run (a full-screen fade, say) will also light the mask. You say it's unverified in-game, so fine for an opt-in switch, just worth a line in the doc once you've looked at a few scenes with RX_VK_REACTIVE_DEBUG=1.

CI: no checks reported on the branch, so a rebase will also give it a run.

[hckrman101]

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!

[TwoToneEddy]

Done: main is merged in again and the conflicts are resolved. Each point from your review is answered below.

dlss_jitter_size() no longer exists on main […] reactive_scene_draw() calls dlss_jitter_size(0/1), so it won't compile after a rebase as is.

Fixed. reactive_scene_draw() now uses temporal_jitter_size for the chain, with fsr3_inputs::jitter_size kept for FSR 3.1.

DLSS evaluation is now evaluate(cmd, const TemporalFrame& f) in vk_dlss.cpp […] The mask call needs to go into that function using the TemporalFrame's colour view and size.

Done. The mask is made in evaluate from the frame's colour view and sizes, and set as pInBiasCurrentColorMask as before.

vk_fsr3.cpp and the in.reactive swap into P_PREPARE_REACTIVITY look like they still apply.

They did; those hunks are unchanged.

does [FSR 4] have a reactive input you'd want to feed too, or should the doc say explicitly DLSS + FSR 3.1 only?

DLSS + FSR 3.1 only for now, and the doc says so. FSR 4 is given no mask, and no scene copy is made while it draws. I'd add its reactive input as a follow-up once this has been looked at in-game.

a blended no-depth-test draw that comes after the last transparent run (a full-screen fade, say) will also light the mask.

Added a line to the doc saying so.

Also

• The test declares RUN_ALL_ARGS = ["--cxx", "{cxx}"] itself, since the central ARGS table is retired on main.

Checked

• tests/test_reactive_mask.py passes, and the changed files pass a g++ syntax pass (vk_dlss.cpp against NVIDIA's NGX headers too).
• Merges cleanly together with feat(vk): the upscalers' texture LOD bias at Auto, from the size drawn #1654.
• Still not run in-game; off by default.

[hckrman101]

Thanks for the quick turnaround. All the points from my last review are covered:

• dlss_jitter_size is gone: reactive_scene_draw() uses temporal_jitter_size for the frame chain and keeps fsr3_inputs::jitter_size for FSR 3.1. Fixed.
• The DLSS mask is now made in evaluate(cmd, const TemporalFrame&) from the frame's colour view and sizes. Fixed.
• FSR 4 gets no mask and no scene copy while it draws (is_fsr4(temporal_mode()), and that enum matches upscaler_rows). The doc says DLSS + FSR 3.1 only. Answered; a follow-up for FSR 4 sounds right.
• The doc now mentions the full-screen-fade case. Done.
• The test declares RUN_ALL_ARGS itself, which matches main since #1822.

Still blocking: it conflicts with main again (mergeable_state: dirty). A lot landed after your 18:15 merge, and the overlaps with this PR are gpu/vk/vk_dlss.cpp, gpu/vk/vk_draw.cpp, gpu/vk/vk_fsr3.cpp and docs/development.md. One of them is more than a textual conflict:

• The recorder (#1473, RX_VK_RECORDER) put ::mkwvk::rec::boundary(::mkwvk::rec::B_DIRECT); in evaluate() right before NGX_VULKAN_EVALUATE_DLSS_EXT, at exactly the line where your mask block goes. When you resolve it, the reactive::mask(...) call has to come before that boundary. mask() records its barrier, bind, push and dispatch through vk_work.h, so with RX_VK_RECORDER=defer/thread those packets are only flushed into cmd by the boundary. If the mask ends up after it, NGX would read the mask before the dispatch that writes it reaches the command buffer. See the inline comment.
• vk_reactive.cpp already includes vk_work.h last, so its own commands go through the recorder correctly. Nothing to change there.
• docs/development.md: main added the RX_VK_COMPACT row between RX_VK_VRAM_MB and RX_VK_NO_WRITEBACK, right where your row goes. Keep both.
• vk_draw.cpp / vk_fsr3.cpp: everything your hunks use (barrier_every, clear_after_draws, default_reactive in P_PREPARE_REACTIVITY, ai.reset = f.reset) is still on main, so those should resolve mechanically.

CI: still no checks reported on the branch. A clean merge should give it a run.

Once it's merged and the mask sits before the B_DIRECT boundary, I'm happy with this as an opt-in switch.

[hckrman101]

Heads-up for the merge: main now has ::mkwvk::rec::boundary(::mkwvk::rec::B_DIRECT); just before NGX_VULKAN_EVALUATE_DLSS_EXT here (the recorder, #1473). This block needs to stay above that line. reactive::mask records through vk_work.h, and under RX_VK_RECORDER=defer/thread it only reaches cmd at the boundary, so NGX has to come after it.

[TwoToneEddy]

Thanks. Main is merged in again and the conflicts are resolved.

• The mask block in evaluate() now sits directly above rec::boundary(B_DIRECT), so its dispatch reaches cmd before NGX reads the mask.
• docs/development.md keeps both the RX_VK_COMPACT row and this one.
• vk_draw.cpp and vk_fsr3.cpp merged without a conflict, as you expected.

tests/run_all.py fails only where main does on my machine. I couldn't compile vk_dlss.cpp this time (no NGX headers here); the resolution there only keeps the existing block above main's new line. Still not run in-game.

[hckrman101]

Confirmed: the mask block is right above the B_DIRECT boundary on the new head. Thanks!

[hckrman101]

Thanks, the merge looks right. I compared the PR's patches before and after it file by file: apart from the merge context nothing in your hunks changed.

• The mask block in evaluate() now sits directly above ::mkwvk::rec::boundary(::mkwvk::rec::B_DIRECT), so under RX_VK_RECORDER=defer/thread the mask's dispatch reaches cmd before NGX reads it. Fixed.
• docs/development.md keeps both the RX_VK_COMPACT row and yours. Fixed.
• vk_draw.cpp / vk_fsr3.cpp: unchanged hunks, and fsr3_upscale on main still builds ai from the same fsr3_inputs::Frame (f.color, f.width/height) after #2147. With the new Upscaler Off choice (#2097) both jitter sizes are 0, so reactive_scene_draw() copies nothing there. Fine.
• It merges cleanly now.

Heads-up for later, not for this PR: #2305 moves FSR 3.1 onto the temporal chain (chain_evaluate in vk_fsr3.cpp). If it lands, reactive_scene_draw() will see temporal_jitter_size != 0 with a non-FSR 4 mode and make the copy, but the chain's FSR 3.1 path doesn't call reactive::mask, so the copy goes unused. Whichever of the two merges second should add the reactive::mask(...) → ai.reactive line to chain_evaluate. #1654 touches the same files but you already checked that the two merge cleanly together.

CI: the build and test workflows on this head are waiting for a maintainer to approve them (action_required), so nothing has actually built yet, vk_dlss.cpp included. Only the PR title check ran. It's off by default and the change since my last review is a placement-only move, so I'm approving. Someone with access should still approve the runs and look at them before merging.