Repository navigation
fix(render): publish coherent temporal jitter samples - #1691
Conversation
73a476d to
ec10792
Compare
8684181 to
9dfa218
Compare
hckrman101
left a comment
There was a problem hiding this comment.
The approach is sound: publishing width, height and phases together under a small mutex, one callback query per host frame, and a copy of the cache under the host lock. The API bump to 13 is handled correctly, and CI is green at this head.
The problem is that the PR no longer applies to main. It shows as conflicting, and the cause is more than a textual merge. #1008 (AMD FSR 4) moved the DLSS jitter state out of vk_dlss.cpp into the shared engine in gpu/vk/vk_temporal.cpp. That file now holds State::jitter_w/h/phases and last_frame, and temporal_jitter_size(axis) reads them. The replay side calls temporal_jitter_size followed by fsr3_inputs::jitter_size. vk_dlss.cpp and dlss_jitter_size no longer exist in that form, so the DLSS half of this change has nothing to attach to.
Please rebase and port it:
- Publish the temporal engine's width, height and phases as one sample, for both DLSS and FSR 4 (mode via
ur::is_fsr4), and keep the 30-frame expiry and thefailed/GAMEmode checks thattemporal_jitter_sizedoes today. - Keep FSR 3.1's own publisher in
vk_fsr3_inputs.cpp. - In
jitter_sample, prefer the temporal engine's sample and fall back to FSR 3.1's, asjitter_optiondoes now. - The test fixture and its marker substitutions (
DLSS_PUBLISHand so on) will need to follow the new code. Also check thattests/run_all.pyandvk_api.hmerge cleanly, since the API version should still end up at 13 on top of whatever main has.
Once it's rebased and the fixture runs against the new code, this looks good to me.
|
The shared-chain port is now pushed at The mixed-sample baseline fails causally; the port passes publication stress and concurrent host callers. The FSR 4 failure gate has an executed mutation control. Exact-head Windows, AppImage and DLSS + FSR 4 compile/link workflows passed. Source checks remain red on the existing base failures, with fixes in upstream #1793; the PR description records the exact results. The private D04+D05 composition A further main integration is prepared locally, preserving every upstream test registration through the new additive |
9dfa218 to
1ce75c6
Compare
|
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! |
11be9bf to
4f92dfa
Compare
|
Refreshed on main |
Assisted-by: OpenAI Codex Co-Authored-By: OpenAI Codex <noreply@openai.com>
Assisted-by: OpenAI Codex Co-Authored-By: OpenAI Codex <noreply@openai.com>
4f92dfa to
1f32a16
Compare
hckrman101
left a comment
There was a problem hiding this comment.
Thanks for the port. My earlier point is fixed: the shared sample now lives in vk_temporal.cpp and covers DLSS and FSR 4, with the engine's failed gate, the GAME/off checks and the 30-frame expiry kept. FSR 3.1 keeps its own publisher, and jitter_sample in vk_replay.cpp prefers the temporal sample and falls back to FSR 3.1, as jitter_option does. The host now asks once per frame under a lock, and the API bump to 14 sits on top of main's 13. The fixture extracts the new code, and run_all.py discovers it, so no registration is needed. It is mergeable now. The Windows, AppImage and DLSS/FSR 4 link checks pass. The two red Python jobs are the failures you say the base has too.
One nit, inline: a caller that reads the frame counter just before a publish can get an empty sample for that frame. It isn't a blocker.
|
|
||
| MkwVkJitter read(uint64_t now) { | ||
| std::lock_guard lock(mu); | ||
| return frame && now - frame <= 30 ? sample : MkwVkJitter{}; |
There was a problem hiding this comment.
Nit: now - frame is unsigned. A reader that took texture_frame_no() just before a replay-side publish has now < frame, so the subtraction wraps and that caller gets an empty sample, i.e. one frame without jitter. It is tiny and self-healing. frame && now >= frame && now - frame <= 30 would avoid it, if you ever touch this again.
Summary
During resize or render-scale changes, separate jitter queries can mix dimensions and phases from different completed upscales. Publish one sample and latch it across the host frame.
Technical changes
Validation
Actual producers, readers and host transport pass concurrency and failure-gate cases. The delayed-caller regression fails on the old source and passes here; engine availability is stubbed.
Head
1f32a16, main8c92fe3: full local suite 301 passed / 3 failed / 21 skipped, versus 299 / 4 / 21 on the exact base. Three shared failures: Windows symlink privilege, missingzstdCLI in the new bake fixture, and the local Windows-header harness. Baseline also timed out in frame-wait; its identical production block and fixtures passed all18 cases in focused reruns on both base and candidate. Original counts are preserved.Matching NGX host/backend build passed with officially regenerated runtime. Windows builder, Linux AppImage and NGX CIs passed. Source CI failed: Windows cannot load libzstd and hits a logging timing assertion; Linux rejects the bake compiler's C++23 probe. Exact upstream baseline CI has those failures too. This run additionally timed out in Windows bake_skeleton (600s). No temporal check failed; the source workflow remains red. Bounded native smoke on RTX5070Ti passed: copied Kokiri checkpoint, official DLSS310.9.16/preset K loaded and evaluated, window/fullscreen, pause/resume and exit0; eight3440x1440 engine captures. Internal upscale dimensions stayed fixed. SDK recreation, render-scale transitions, quality and performance comparisons remain untested.