fix(vk): actor motion pairs a model's copies by place, so the Gossip Stones stay whole under frame generation #59
No reviewers
Labels
No labels
area/accessibility
area/audio
area/input
area/install
area/menu
area/renderer
area/saves
area/upscaling
beanstalk: approved @ 1697603
beanstalk: approved @ 2c5a62e
beanstalk: approved @ 4fd82b7
beanstalk: approved @ 64d6d09
beanstalk: approved @ 6f91fae
beanstalk: approved @ 70a78f9
beanstalk: approved @ 8d361dc
beanstalk: approved @ 9d8356e
beanstalk: approved @ abf2227
beanstalk: approved @ d82f202
beanstalk: approved @ e5f1e40
beanstalk: approved @ ee8ab5f
ci: approved
good first issue
gpu/amd
gpu/igpu
gpu/intel
gpu/nvidia
help wanted
kind
bug
kind
chore
kind
docs
kind
feature
kind
performance
kind
question
needs measurements
platform/linux
platform/macos
platform/steam-deck
platform/windows
preview
priority
critical
priority
high
priority
low
regression
status
blocked
status
confirmed
status
duplicate
status
invalid
status
needs-info
status
wontfix
testing wanted
ci: approved
No project
No assignees
5 participants
Notifications
Due date
No due date set.
Dependencies
No dependencies set
Reference
MagicBeanDev/Longshot!59
Loading…
Reference in a new issue
No description provided.
Delete branch "pmwork-personal/Longshot:fix/actor-motion-copies"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Why
Under DLSS Frame Generation the four Gossip Stones by the Temple of Time break apart in the made frames: a stone is drawn part way to its neighbour, doubled or smeared. A player reported it (RTX 5080, 4x); it happens on an RTX 5090 at 3x too.
What changed
The stones are one skinned model drawn four times, and actor motion paired each draw with the frame before's by draw order. The game draws the copies in another order from frame to frame, uses two models for them, and a stone that comes into view has no draw in the frame before. So a stone took its neighbour's pose as its own, and its motion pointed at the neighbour.
temporal_note_skinned_drawingpu/vk/vk_temporal.cpp): each skinned draw's pose (origin at the object's +0x40, then its bones) is read at the draw withub_read, not copied from the game's memory after the frame. The read runs outside the temporal lock, into a per-thread buffer; under the lock the pose is appended to one frame-widepose_bytes, with no allocation per draw.actor_motionstages the read poses into the pose buffer in one copy. A pose that can't be read falls back to the old copy;RX_VK_DLSS_ACTOR_LIVE=1forces the old copy for an A/B.dlss_actor::place_of,dlss_actor::pair_by_placeingpu/vk/dlss_actor.h): the place is the origin plus the bones' mean translation (the copies share one origin and are placed by their bones). All copies are paired at once, the nearest pairs first. Draw order is kept instead when every pair of it is within the reach and it pairs more copies, or as many with no longer a total distance. A copy without a match gets the camera's motion for that frame. A draw whose pose wasn't read pairs by its n-th, as before, unless that frame-before draw was already taken by place.actor_motion):RX_VK_DLSS_ACTOR_REACHworld units a second (default 60, 1 a frame at 60 fps, about ten times a running Link), times the time between the two poses, clamped to 1/60-1/30 s. So the reach is at most 2, under the stones' 2.3 spacing. Past that spacing, draw order (which wins whenever it pairs more) would pair a row of stones each with its neighbour after a stone changes model. The cost: across a gap longer than 1/30 s (a hitch, a base under 30 fps), an actor that moved more than 2 units gets only the camera's motion for that frame.tests/test_dlss_actor.cpp): the stones in and out of order, a stone new in view, a lone copy, two copies passing, two moving fast for their spacing (0.6/0.35 and 0.9/0.5), and a row whose model changed, within a reach of 2 and past the spacing.docs/development.md's switch reference, and rows indocs/tested-configurations.md.Actor motion feeds the whole temporal chain, so FSR 4 and FSR 3.1 get the same motion; with
RX_VK_DLSS_ACTORS=0nothing changes.Flags and settings
RX_VK_DLSS_ACTOR_LIVE=1(new)RX_VK_DLSS_ACTOR_REACH=<units a second>(new)RX_VK_DLSS_ACTORS=0(existing)Measurements
The first run below is the author's A/B of this head against main, in the game. The two after it are of earlier versions; since them, the reach became a speed (it was 1 unit a frame), draw order is kept where it pairs as well, and the poses are read outside the lock.
The author's A/B, 2026-11-14
main:
cf605c85(2026-11-14)this PR: fix/actor-motion-copies @
cc7cb36a, its four commits rebased onto maincf605c85locally (no conflict, the same diff)Hardware: RTX 5090 32 GB, driver 621.24, Ryzen 7 9800X3D, 64 GB, Windows 11 Pro
Scene: the four Gossip Stones in front of the Temple of Time, the camera moved back and forth (main's repro), about a minute a run; fullscreen, 5120x2160 output, DLAA, DLSS Frame Generation 2x (NGX), V-Sync Auto (FIFO), Reflex On + Boost, frame pacing on
Method:
scripts/dev_packs.py: one base of maincf605c85, this PR as a renderer pack, a runtime each from that base (the same caches and settings; only the renderer differs). Four runs, alternated: this PR, main, this PR, main (docs/tested/2026-11-14-gossip-stones-pmwork-personal-run01.mdtorun04.md). The stones judged by eye in play; CPU from the status lines' 600-frame blocks, leaving out each run's first two (the scene loading): the median of each run, then the median of the two runsHow to reproduce: stand by the four Gossip Stones in front of the Temple of Time with DLSS Frame Generation on, and move the camera back and forth
Base: the PR branch is on main
5ad0f503; it was measured rebased onto maincf605c85, and the numbers still hold. Between the two, main has 8 commits, and this PR's own added and removed lines are the same on both. The only one of them that touches a file of this PR is Longshot PR 252 (vk_temporal.cpp): it wraps the pipeline creation (make_pipeline,make_actor_pipeline, once at the start) in an Intel driver guard, not the per-frame actor motion.cf605c85cc7cb36aSo this PR costs about 1.6 us more a skinned draw: about +0.09 ms a frame at the stones (about 50 skinned draws a frame), about +0.25 ms at 150.
mooki's run, of
8a091b34(comment)main:
d4a69456(2026-11-13)this PR: fix/actor-motion-copies @
8a091b34(the first version on Longshot maind4a69456)Hardware: RTX 4090 24 GB, NVIDIA 615.78.08, i9-13900K, Arch Linux (kernel 7.2.15)
Scene: child Link, daylight, the four Gossip Stones outside the Temple of Time; 3840x2160, DLSS Transformer (K) Performance, Reflex On + Boost, frame generation off (DLSS-G wasn't available: its Linux library was missing), windowless
Method: native builds of both; scripted left/right passes of 100 game frames, every present captured at 960x540 with
RX_VK_DLSS_DEBUG=motion, and frames with the stone signature counted by hand. Two interleaved main/PR pairs, then the PR withRX_VK_DLSS_ACTOR_LIVE=1and withRX_VK_DLSS_ACTORS=0. CPU from the status lines; the PC was heavily loaded (load averages 38-217), so the CPU figures are diagnostics, not a speed comparisonHow to reproduce: as mooki's comment describes
d4a694568a091b348a091b34,RX_VK_DLSS_ACTOR_LIVE=1The author's run, of the first version as a Hookshot build (not Longshot main, so its columns name the builds)
Builds: Hookshot main @
ba3d3f73(Longshot's base before the rename) against Hookshot @ 06ad86fe (logv2.1.0-rc.3-281-g06ad86fe): the first version of this change onba3d3f73, with NotRexed/Hookshot PR 2787's layouts and the close-up depth testHardware: RTX 5090 32 GB, driver 621.24, Ryzen 7 9800X3D, Windows 11 Pro
Scene: Castle Town, the four Gossip Stones by the Temple of Time, running past them while the camera turns; 3840x2160, DLAA, DLSS Frame Generation 3x (NGX), Reflex On + Boost, no frame rate cap
Method: about 30 s a run, judged by eye in play; the numbers from the status lines in the log. One run each, not interleaved: the fault is either there or not
How to reproduce: stand by the four Gossip Stones by the Temple of Time with DLSS Frame Generation on, and run past them while turning the camera; compare with
RX_VK_DLSS_ACTORS=0ba3d3f73RX_VK_DLSS_ACTORS=0: whole)The 0.41 ms was the first version, with an allocation and the reads under the lock for each draw. Moving the reads out of the lock brought it down: on the author's PC, per skinned draw, the first version cost about 3.3 us (0.170 ms at 52 draws a frame), this head 2.725 us, and main 1.085 us (the A/B above).
Testing
Tested:
cf605c85; RTX 5090, 621.24, Windows 11 Pro; DLAA, DLSS FG 2x; 2 alternated runs each): in front of the Temple of Time, the camera moved back and forth: on main the stones break apart in made frames, on this head they stay whole. Details under Measurements.8a091b34(main plus this PR only; RTX 5080; DLSS FG 2x, 3x and 4x) (comment): in front of the Temple of Time, the camera moved back and forth (main's repro). No ghosting, smearing or copies of the stones seen.8a091b34against maind4a69456(RTX 4090, 615.78.08, Arch Linux; DLSS K Performance, FG off): both build natively on Linux, and all six runs exited without errors. No stone signature on either side by default. WithRX_VK_DLSS_ACTOR_LIVE=1(the old copy, same head), 2/460 frames show it. The poses counter shows every pose read at the draw. mooki notes that this proves the old path's motion fault, not that the generated-frame fault is gone: FG was off, and plain main showed no signature there.cf605c85,tests/test_dlss_actor.pyand the 10 checks that readvk_temporal.cppordlss_actor.hpass;python -I -B tests/run_all.py: 516 passed, 8 failed, 35 skipped, the same 8 as maincf605c85on the same PC.tests/test_dlss_actor.pypassed by hand on966c69e1, and mooki ran it on8a091b34. The author ranpython -I -B tests/run_all.pyon8a091b34: 488 passed, 4 failed, none from this change. mooki's suite run on8a091b34: 505 passed, 4 failed (environment: a .NET pack, two compiler SIGKILLs, a shader batch that passed serially).10e2b4d1itself: no tests, builds or game runs were made for10e2b4d1on the PC that pushed it (cc7cb36a's commits were run above). Forgejo CI on10e2b4d1skips the Python source checks and the SPIR-V fixtures for this PR; the title check, the workflow lint and the third-party notes pass.Not tested:
Needs other testers:
None.
Screenshots
None: the author chose not to attach a clip. The fault shows only in made frames; the A/B by eye, 2 alternated runs on each side, is under Measurements, and Alimjan's clip on this PR shows the fault on the current release.
Risk and rollback
RX_VK_DLSS_ACTORS=0(actor motion off), orRX_VK_DLSS_ACTOR_LIVE=1(main's pose copy; the pairing by place then isn't used).RX_VK_DLSS_ACTOR_LIVE=1avoids it.Reviewer notes
Start with
dlss_actor::pair_by_placeand its tests, thenactor_motion(the reach and the pairing) andtemporal_note_skinned_draw(the read outside the lock). Whether OoT moves actors by wall time or fixed ticks, which a reach set as a speed assumes, hasn't been checked; the cap at 2 units bounds the reach either way.Issues
None on Longshot. (The same change was proposed to NotRexed/Hookshot as pull 2992; that repository has returned 404 since 2026-11-13.)
Checklist
For everyone
type(scope): description).main. (main @5ad0f503; the commits also rebase ontocf605c85with no conflict)python -I -B tests/run_all.pyrun and passing (any failure that also fails onmainis named above). (On this head's commits rebased ontocf605c85: 516 passed, 8 failed, 35 skipped; maincf605c85on the same PC fails the same 8:test_fpx_sticky,test_framegen_resources,test_module_ids,test_oot_state_tap,test_pad_layout,test_portable_builder_startup,test_spawn_emit,test_tested_configurations. CI skips the Python checks for this PR.)cf605c85and this head's commits, 2 alternated runs each, the medians under Measurements.)third_party/untouched (a new folder with aPROVENANCE.mdinstead).If applicable
Assisted-by:/Co-Authored-By:trailers.docs/tested-configurations.mdfor each measured run. (The 2026-11-14 runs:docs/tested/2026-11-14-gossip-stones-pmwork-personal-run01.mdtorun04.md.)mainand passing here. (tests/test_dlss_actor.pywith this PR's test on maincf605c85fails, as main has nodlss_actor::Placedorpair_by_place; on this head's commits rebased ontocf605c85it passes. Run 2026-11-14.)tests/that reads changed source updated in this PR, not weakened. (tests/test_dlss_actor.cppis extended forpair_by_place; the 10 other checks that readvk_temporal.cppordlss_actor.hpass unchanged on this head.)8a091b34, built there for mooki.)🤖 Generated with Claude Code
Linux result for
8a091b34againstmain @ d4a69456: both native builds succeed. The forced-old-path A/B shows the stone motion-vector signature here, but I have not verified the generated-frame fix.Hardware: RTX 4090 24 GB, NVIDIA 615.78.08, i9-13900K, Arch Linux 7.2.15. Scene: child Link, daylight, the four Gossip Stones outside Temple of Time. Same copied checkpoint; 3840×2160, DLSS Transformer(K), Performance 50%, Reflex On+Boost, frame generation off, windowless. After warming, scripted left/right passes of 100 game frames each; every renderer present captured at 960×540. Two interleaved main/PR pairs, then the PR's LIVE=1 and ACTORS=0 controls.
I used
RX_VK_DLSS_DEBUG=motionand the one-frame saturated-green method described in the issue 2761 follow-up. All candidate pictures were inspected: Link, Navi and broad camera motion also trigger the detector, so those are not counted as stones.d4a694568a091b34On this PR @
8a091b34withRX_VK_DLSS_ACTOR_LIVE=1, 2/460 frames contain white stones: presents 3261 and 3269, with their neighbours inspected. Its selected status report is 0.112 ms/frame, 0 poses read at draw and 28614 copied later. WithRX_VK_DLSS_ACTORS=0: 0/460 confirmed signatures and 0 actor draws.These are diagnostic observations, not a speed comparison. Start/end load averages ranged 38.11–216.51; views and actor counts vary, and the selected status blocks overlap different portions of the passes. The two default-arm counter medians are 0.0955/0.410 ms. Large close-up Link motion obscures part of the first main run; the zero confirmed counts do not prove all motion is correct. Plain main did not give me a confirmed stone signature, so the useful positive result is the author's same-head old-path switch, not a reproduced main→PR visual fix.
Here
1 madein the temporal status is the upscaler feature count, not generated frames. All six motion runs exited normally without fatal/device-loss/kernel-fault lines. I cannot attach pictures to this thread from my account; the before, white and after motion-vector frames of both events are kept and I can post them wherever is convenient.Checks: the actor driver passes on both; glslang 16.6.0 was available for its automatic SPIR-V comparison. Whole head suite: 505 passed, 4 failed, 20 skipped. The missing .NET reference pack also fails on main; two compiler SIGKILLs and a shaderbatch build failure passed on main and on a serial head follow-up. I am not calling the whole suite green.
Initial DLAA/3x Auto startup logged:
The build had DLSS SR but lacked
libnvidia-ngx-dlssg.so.310.9.16; the driver's fallback did not supply FG. The matching official Linux library has since been installed for the test copies, but post-install availability has not been tested. That initial refusal is not a general Linux-FG compatibility verdict.Not tested: generated frames or 3x output, FSR 3.1/4, normal-view paired ghosting/smear, Windows execution, AMD/Intel. The normal DLAA camera capture was on main only. This result establishes Linux buildability, the pose-path counters and a positive old-path motion diagnostic—not that the original generated-frame artifact is fixed.
Tested this in main with only this PR installed:
Tested with in front of the Temple of Time with the 4 Gossip Stones with DLSS FG at FG X2, FG X3, FG X4 on an RTX 5080.
Camera movement was done back and forth (the repro behavior on main) and no additional ghosting, smearing or copies was noticed during camera movement.
Reviewed the code only (no build, no tests run). The two-step fix is sound: reading each draw's pose at the draw with
ub_readinstead of copying it from the game's memory at the upscale, and pairing a model's copies by place with nearest-first and a reach. The fallbacks are careful: a pose that can't be read, a stage that can't be made,RX_VK_DLSS_ACTOR_LIVE=1, and a frame before with no placed copies all lead back to the old nth path. Staging through a host-visible ring and making one copy after the loop keeps the GPU order. Thepair_by_place/place_oftests cover the stone cases well. Without a temporal upscaler nothing changes:temporal_note_skinned_drawreturns on!temporal_wanted()before any read, andactor_motiononly runs from the temporal path.Must change
Not mergeable:
docs/tested-configurations.mdconflicts with main. This branch is ond4a69456, which is 61 commits behindmain, and main has since added 10 rows at the same place (the top of the Runs table). The code files (vk_temporal.cpp,dlss_actor.h,tests/test_dlss_actor.cpp) aren't touched on main and should merge cleanly, but Rebased on currentmainis ticked and isn't true. Rebase, put the row back at the top, and re-run the checks.The PR is out of WIP with a For everyone box unticked (Measurements: "one run each"). The screenshots box is unticked too ("a clip will follow"). AGENTS.md says a PR is ready only when every For everyone box is ticked. There is now more to go on: Desmond27's RTX 5080 run at FG x2/x3/x4 on main + this PR, and mooki's two interleaved main/PR pairs on Linux (FG off). Use those to settle the box, or put the PR back to WIP.
The description no longer matches the PR.
Update the description with
pr update.A bare
#2787in the new docs row, and in the description. It's a GitHub PR, so it needs the full URL: https://github.com/NotRexed/Hookshot/pull/2787.Should change
The reach is absolute, measured per engine frame, and now applies to models drawn only once. On main, a lone Link, Epona, Navi or a boss always paired with its frame before by nth. With this PR, if its place (origin plus the mean of its bone translations) moves more than 1 unit between two engine frames, it gets no actor motion for that frame and only the camera's. That happens on a hitch (the log shows a 55 ms frame), at a low base rate (frame generation is often used from 30-40 fps), or with large fast models whose bone mean sweeps (Volvagia, Ganon, a horse charge). The result is a frame of ghosting on exactly the actors that move most. The lone-stone case (
lone_now/lone_then) shows why a lone copy can't simply skip the reach. Better options: scale the reach by the time between the two engine frames (a speed, not a distance), or relate it to the model's spacing and size.Greedy nearest-first can cross-pair copies that move fast relative to their spacing, where nth was right.
Cost. mooki's counters show actor-motion CPU going from about 0.095 to 0.41 ms a frame on an i9-13900K, about 4x main. Most of it is likely in
temporal_note_skinned_drawunders.mu:ub_readof the bones that, at that size, skips the word cache and walks every one of the stream's uniform updates;memory_read_as_submitted, which takespools_mufor each page.The O(n²) pairing itself is small: at most 512 skins a frame, and the reach prunes the candidates (the worst case is about 65k candidates to sort for 256 copies of one model close together). Either land the promised arena/out-of-lock follow-up, or measure where the time goes and state the cost on a mid-range CPU, since every DLSS and FSR user pays it.
Frame generation off isn't a no-op. With DLSS SR, DLAA or FSR and FG off, the poses read at the draw and the place pairing apply too, so they change the motion vectors the upscaler gets. That's intended (the description says so for FSR), but the only FG-off evidence is mooki's run, which showed 0 stone signatures on both sides. A short look at a moving crowd with FG off would cover the cross-pairing risk above.
Question: can a bone palette be written on the GPU?
ub_readreads the submission's CPU capture plus inline uniform updates. The old path copied from the bound GPU buffer. If any skinned model's palette or object block is written by a buffer copy or compute earlier in the stream, the pose read now would be stale and still count as "read at the draw". Please confirm OoT's palettes are always CPU-written.@ -14,6 +14,7 @@ otherwise.| Date | Build | OS / desktop | GPU / driver | CPU / RAM | Settings | Where | Result ||---|---|---|---|---|---|---|---|| 2026-11-12 | v2.1.0-rc.3-281-g06ad86fe (source 8d4d275a): main ba3d3f73 + this change (before the review's small fixes), #2787's layouts and the close-up depth test; against v2.1.0-rc.3-271-gba3d3f73, main | Windows 11 Pro, 3840x2160 output | RTX 5090 32 GB, 621.24 | Ryzen 7 9800X3D | DLAA, frame generation 3x by DLSS (NGX), Reflex On + Boost, V-Sync Auto (FIFO) | Castle Town, the four Gossip Stones by the Temple of Time: running past them while the camera turns, about 30 s | Main: the stones break apart in made frames (a stone drawn part way to its neighbour); with `RX_VK_DLSS_ACTORS=0` they stay whole. This change: whole. Every pose read at the draw ("poses N read at the draw, 0 copied later"), 51-74 skinned draws a frame, actor motion 0.15-0.28 ms of CPU a frame, DLSS-G made 590 of 600 presents in play. No device lost |Bare
#2787: it's a GitHub PR, so please link it by its full URL (https://github.com/NotRexed/Hookshot/pull/2787). This row also conflicts with the 10 rows main has added at the top of the table, so it needs a rebase.@ -24,0 +59,4 @@std::vector<int> pairs(now.size(), -1);std::vector<char> taken(then.size(), 0);for (const Candidate& c : candidates)if (pairs[c.i] < 0 && !taken[c.j]) pairs[c.i] = c.j, taken[c.j] = 1;Greedy nearest-first can cross-pair two copies that move fast relative to their spacing, where nth would have been right. Example: the frame before has A at 0 and B at 0.6, and both move +0.35. A'-B (0.25) is taken first, then B' falls back to A (0.95, within the reach of 1), so B' gets a motion of -0.95 instead of +0.35. Suggest keeping the same-nth assignment when it is all within the reach and its total distance is no worse than the greedy one (or accepting only mutual-nearest pairs), with a test for this case.
@ -522,0 +544,4 @@// first, never across more than RX_VK_DLSS_ACTOR_REACH world units between two engine frames (default 1: a running Link// moves about 0.1 a frame, the Gossip Stones stand 2.3 apart; after a long frame a fast actor can go further and keeps the// camera's motion for that frame): dlss_actor::pair_by_place. Else the same n-th draw of the model as before)static const float reach = [] {The reach is a distance per engine frame, and it now applies to models drawn only once too (Link, Epona, Navi, bosses), which main always paired by nth. On a hitch, at a low base rate, or for a big model whose bone mean sweeps, a lone actor loses its actor motion for that frame and gets only the camera's. Could the reach be a speed (scaled by the time between the two engine frames), or relative to the model's size?
@ -541,0 +620,4 @@// draws too, else its n-th, as before)auto it = old.end();const auto m = placed ? models.find(k.key >> 8) : models.end();if (m == models.end() || m->second.then.empty()) it = old.find(k.key);Nit: a draw whose pose couldn't be read falls back to old.find(k.key) by nth, and that then-copy may already be paired by place with another draw this frame. Two draws then share one frame-before pose. It's rare, but you could skip then-keys that pair_by_place has already taken.
@ -815,0 +907,4 @@// frame's or another draw's (the place pair_by_place needs, too). RX_VK_DLSS_ACTOR_LIVE=1: as before, for an A/B)static const bool live = env_switch("RX_VK_DLSS_ACTOR_LIVE") == 1;if (!live) {d.pose.resize(16 + (size_t)d.nbones * 48);This runs under s.mu for every skinned draw: an allocation of up to 13 KB, and an ub_read that at this size walks all of the stream's uniform updates and takes pools_mu for each page. mooki's counters show actor-motion CPU going from about 0.095 to 0.41 ms a frame (13900K). The description promises a follow-up commit that moves this out of the lock into one arena a frame, but it isn't in the PR. Also: ub_read reads the CPU capture, so a palette written on the GPU earlier in the stream would come back stale here while the old GPU copy was right. Can you confirm OoT never does that?
8a091b34d7toc2c5fa4364Re-review at
c2c5fa43. I read the code only: no build, no tests, no game run.c2c5fa43deals with most of the code points from the first review, and the move out of the lock is done carefully. The PR still can't merge as is. Nobody has run the head in the game, the Measurements box is unticked, and the description still describes the first version.Earlier points: where they stand
main@origin(2e1e2e8c), and Forgejo reports it mergeable.#2787in the docs row: resolved (it is now a full URL). Still open in the description: "After: the same with this change, #2787's layouts…".RX_VK_DLSS_ACTOR_REACHworld units a second (default 60) times the time between the two poses, clamped to 1/60 s to 1/4 s.poses_timeis set on everyactor_motionthat gets past the early return, anddrop_allresets it.s.poseswas emptied on that early return, so there is nothing to pair with.taken_by_place).ub_readnow runs into athread_localbuffer before the lock is taken.s.pose_bytes, and its offset is taken in the same critical section.actor_motionreadspose_bytesunders.mu, becausetemporal_upscaleholds it.if (s.skins.empty()) s.pose_bytes.clear()), the 512-skin clear anddrop_allall emptyskinsandpose_bytestogether, under the lock. Growth only reallocates, and offsets are indices, not pointers.resizeand the copy.ub_readuses thread-local state and is already called from worker threads withouts.mu, so moving it out of the lock adds no new race.8a091b34.8a091b34(see below).Must change
8a091b34.c2c5fa43changes how the reach works (1 unit a frame became 60 a second), how pairs are chosen and where the poses are read. It needs at least the stones run again with frame generation on, plus the status line's actor-motion CPU against main, as several interleaved runs. That settles the Measurements box and shows the cost the out-of-lock change was for. The PR isn't WIP, but Measurements is unticked, so AGENTS.md says it should be WIP (pr wip 59).8a091b34PR, not this one. Update it withpr update:RX_VK_DLSS_ACTOR_REACHworld units in one frame (default 1…)". It is now 60 a second.#2787. It should be https://github.com/NotRexed/Hookshot/pull/2787.temporal_note_skinned_drawsays "(An assumption, open with the author: …)".Should change
pair_by_placestill prefers greedy when greedy pairs fewer copies (dlss_actor.h,order_n >= nearest_n && order_total <= nearest_total).order_n > nearest_n, or whenorder_n == nearest_nand its total is no longer. Add this case totests/test_dlss_actor.cpp.t0intemporal_note_skinned_drawis taken before the firsts.mu, so time a recording thread spends blocked behindtemporal_upscale(which holdss.muthrough the whole evaluate) counts as actor-motion CPU. That inflates the very number used to compare the cost. Time the unlocked read separately, or startt0after the lock and add the read's own time.main; name the PR. The row records the Hookshot build, not this head, so add a row when the head is measured.Not checked
pose_stagesring outlives every frame in flight with frame generation on. It matches the cameras' ring, which makes the same assumption.@ -14,6 +14,7 @@ otherwise.| Date | Build | OS / desktop | GPU / driver | CPU / RAM | Settings | Where | Result ||---|---|---|---|---|---|---|---|| 2026-11-12 | v2.1.0-rc.3-281-g06ad86fe (source 8d4d275a): main ba3d3f73 + this change (before the review's small fixes), [NotRexed/Hookshot PR 2787](https://github.com/NotRexed/Hookshot/pull/2787)'s layouts and the close-up depth test; against v2.1.0-rc.3-271-gba3d3f73, main | Windows 11 Pro, 3840x2160 output | RTX 5090 32 GB, 621.24 | Ryzen 7 9800X3D | DLAA, frame generation 3x by DLSS (NGX), Reflex On + Boost, V-Sync Auto (FIFO) | Castle Town, the four Gossip Stones by the Temple of Time: running past them while the camera turns, about 30 s | Main: the stones break apart in made frames (a stone drawn part way to its neighbour); with `RX_VK_DLSS_ACTORS=0` they stay whole. This change: whole. Every pose read at the draw ("poses N read at the draw, 0 copied later"), 51-74 skinned draws a frame, actor motion 0.15-0.28 ms of CPU a frame, DLSS-G made 590 of 600 presents in play. No device lost |The table is newest first, and this 2026-11-12 row sits above ten 2026-11-13 rows. "This change" also means nothing once it is in main: name the PR (Longshot PR 59). The row records the Hookshot build before the review fixes. Add a row for the head once it is measured.
@ -24,0 +88,4 @@if (used[j] || !(d <= (double)reach)) return pairs; // (not within the reach: nearest first; NaN fails here too)by_order[i] = j, used[j] = 1, order_n++, order_total += d;}if (order_n >= nearest_n && order_total <= nearest_total + 1e-6) return by_order;When greedy pairs fewer copies, its total is over fewer pairs and isn't comparable. Example, with a reach of 1: the frame before has A at 0 (n-th 0) and B at 0.9 (n-th 1), and both move +0.5. Greedy takes A'-B (0.4), then B'-B is blocked and B'-A (1.4) is out of reach: 1 pair, total 0.4. Draw order gives 2 pairs, total 1.0, all within reach, and it is right, but it loses here. A' then gets -0.4 instead of +0.5, and B' gets only the camera's motion (main had both right). Suggest
order_n > nearest_n || (order_n == nearest_n && order_total <= nearest_total + 1e-6), with this case added to the test.@ -828,28 +945,52 @@ void temporal_note_skinned_draw(const SkinnedDraw& k) {static const bool off = env_switch("RX_VK_DLSS_ACTORS") == 0;if (off || !temporal_wanted() || !k.depth || !k.bones || k.bones_size < 48 || !k.object || k.object_size < 0x4c) return;State& s = st();const std::chrono::steady_clock::time_point t0 = std::chrono::steady_clock::now();t0is taken before the firsts.mu, so a recording thread blocked behindtemporal_upscale(which holdss.muthrough the evaluate) counts its wait as actor-motion CPU. That inflates the cost the status line reports, which is exactly what this change needs to show went down. Time the unlocked read on its own and add it once the lock is held.@ -831,0 +965,4 @@// frame's or another draw's (the place pair_by_place needs, too). RX_VK_DLSS_ACTOR_LIVE=1: as before, for an A/B.// Read without the lock, into this thread's own buffer (its room kept from draw to draw); the lock is taken only to add it// to the frame's poses. ub_read is this recording thread's view of the stream, as the draw's own reads of its uniforms.// (An assumption, open with the author: OoT writes its bone palettes and object blocks on the CPU only. ub_read sees theThis shouldn't merge as an open question. If OoT's palettes and object blocks are CPU-written only, say so as a fact and how it was checked. If that can't be confirmed, keep the GPU copy for a draw whose bones lie in a range the stream wrote on the GPU.
I rebased this onto main @
2e1e2e8cand addressed my first review in one commit of mine on top of your two. It's pushed as966c69e1, and your commits are unchanged apart from the rebase.Rebase. The only conflict was
docs/tested-configurations.md. All of main's rows are kept, and so is yours.My commit, "actor motion's reach is a speed, draw order kept where it pairs as well, poses read outside the lock":
The reach is now a speed.
RX_VK_DLSS_ACTOR_REACHis world units a second, default 60 (1 unit a frame at 60 fps), times the time between the two poses, clamped to 1/60–1/4 s. A model drawn once (Link, Epona, a boss) keeps its motion through a hitch or at a low frame rate. The trade-off is in the comment: below a 26 fps base, a lone stone can take its neighbour's place for a frame.Draw order is kept when it pairs as well.
pair_by_placekeeps the draw-order pairing when every pair is within the reach, and it either pairs more copies, or pairs as many with no longer a total distance. The re-review caught that my first version also compared totals when draw order paired more; that is fixed in this push. New cases intests/test_dlss_actor.cpp:Separately, a draw whose pose wasn't read no longer takes a copy that place pairing already gave to another draw.
Poses are read outside the temporal lock. Each draw reads into a reused per-thread buffer. The lock is taken only to look up the bone count and to append to one frame-wide
pose_bytes, whichSkin::posenow indexes. There's no allocation per draw.The actor-motion CPU counter no longer counts time spent waiting for the lock.
The bone-palette assumption is now a comment at the read: OoT writes the palettes and object blocks on the CPU only. Please confirm it, or say if any are written on the GPU.
#2787in the docs row is now a full URL.tests/test_dlss_actor.pypasses. I didn't run the suite or the game.Still yours, before it's ready:
Measurements. Your earlier runs, Desmond27's and mooki's were all on
8a091b34. This head changes the reach, the pairing and where poses are read, so it needs:Until then, please put it back to WIP (
pr wip 59).Description:
#2787becomes a full URL.The docs row: dated 2026-11-12 but sitting above the 2026-11-13 rows, and it says "this change" rather than naming the PR.
c2c5fa4364966c69e139Hi @HumanBean, this problem was resolved via same PR in the Hookshot repo, but the issue still seems to persist in the latest Longshot release. I think we should look into it further. @Desmond27, sorry to bother you again you're amazing!
Third review, of the PR at
966c69e1. I read the code only: no build, no tests, no game run.Two code points from the second review are fixed:
pair_by_placenow prefers draw order whenever it pairs more copies. The 0.9/0.5 case is tested, and I traced it through.The out-of-lock read still looks right to me. The PR still can't merge, for the reasons below. Several are the second review's, still open.
Must change
It conflicts with main again.
main(514591a3) has added rows at the top ofdocs/tested-configurations.md, the place this PR's row goes, and Forgejo reports the PR not mergeable. So Rebased on currentmainis ticked but isn't true. Rebase, and move the row to where its date puts it (see the inline comment).Nobody has run or measured this version, so it should be WIP. Every game run so far was of
8a091b34or of the Hookshot build. This head changes the reach, the pairing and where poses are read. The Measurements box is unticked.main, as interleaved runs.pr wip 59.The suite hasn't run on this version. The run in Not tested was of
8a091b34. On966c69e1, Forgejo's runs showCI / Python source checks(both parts) and the SPIR-V fixtures as skipped, and onlytests/test_dlss_actor.pywas run by hand. The run_all box needs a run of the rebased head, by hand or by CI.The description is still the
8a091b34PR.pr status 59lists 7 problems:RX_VK_DLSS_ACTOR_LIVEandRX_VK_DLSS_ACTOR_REACH;The text itself is out of date too:
#2787, which should be https://github.com/NotRexed/Hookshot/pull/2787;Fix it with
pr update.The bone-palette assumption is still an open question in the code (
vk_temporal.cpp, "An assumption, open with the author"). @pmwork-personal, please confirm that OoT writes its palettes and object blocks on the CPU only, and say how you checked. Otherwise, keep the GPU copy for bones the stream wrote on the GPU.Should change
Other
@ -14,6 +14,7 @@ otherwise.| Date | Build | OS / desktop | GPU / driver | CPU / RAM | Settings | Where | Result ||---|---|---|---|---|---|---|---|| 2026-11-12 | v2.1.0-rc.3-281-g06ad86fe (source 8d4d275a): main ba3d3f73 + this change (before the review's small fixes), [NotRexed/Hookshot PR 2787](https://github.com/NotRexed/Hookshot/pull/2787)'s layouts and the close-up depth test; against v2.1.0-rc.3-271-gba3d3f73, main | Windows 11 Pro, 3840x2160 output | RTX 5090 32 GB, 621.24 | Ryzen 7 9800X3D | DLAA, frame generation 3x by DLSS (NGX), Reflex On + Boost, V-Sync Auto (FIFO) | Castle Town, the four Gossip Stones by the Temple of Time: running past them while the camera turns, about 30 s | Main: the stones break apart in made frames (a stone drawn part way to its neighbour); with `RX_VK_DLSS_ACTORS=0` they stay whole. This change: whole. Every pose read at the draw ("poses N read at the draw, 0 copied later"), 51-74 skinned draws a frame, actor motion 0.15-0.28 ms of CPU a frame, DLSS-G made 590 of 600 presents in play. No device lost |Still open: this 2026-11-12 row sits above newer rows in a newest-first table, and "this change" should name the PR (Longshot PR 59: #59). It also conflicts with the rows main added at the top since
2e1e2e8c, so it needs a rebase. Add a row for this head once it has been measured.@ -24,0 +90,4 @@if (used[j] || !(d <= (double)reach)) return pairs; // (not within the reach: nearest first; NaN fails here too)by_order[i] = j, used[j] = 1, order_n++, order_total += d;}if (order_n > nearest_n || (order_n == nearest_n && order_total <= nearest_total + 1e-6)) return by_order;With
order_n > nearest_nwinning outright, the reach is now the only guard against draw order cross-pairing a group. Example: the frame before's model X has s0 (n-th 0) and s1 (n-th 1), and this frame's has s1 (n-th 0) and s2 (n-th 1), with stones 2.3 apart and a reach of 3.3 (a 55 ms frame). Nearest first gives 1 pair (s1-s1). Draw order gives 2 pairs (s1-s0, s2-s1) at 2.3 each, so draw order wins, and both stones move toward their neighbour. That is right for the 0.9/0.5 case and wrong here. It's worth a test either way, so the behaviour is pinned down.@ -543,0 +578,4 @@// copy then is the next stone, 2.3 away), so it must not refuse an actor that really moved: as a distance a frame, a// hitch or a low base frame rate (frame generation from 30 fps) took actors past it and they lost their motion for that// frame, ghosting the ones that move most. 60 a second is 1 a frame at 60 fps, and some ten times a running Link (about// 6 a second); it stays under the stones' 2.3 down to a base of 26 fps, and below that a lone stone can take its"Below that a lone stone can take its neighbour" understates the problem. Since draw order wins whenever it pairs more, any gap of 38 ms or more (reach 2.3 or more) lets several stones cross-pair, as in the dlss_actor.h comment, and a single hitch is enough, not only a low base rate. Consider capping the clamp at 1/30 s (reach 2.0), or saying this here.
@ -832,0 +974,4 @@// frame's or another draw's (the place pair_by_place needs, too). RX_VK_DLSS_ACTOR_LIVE=1: as before, for an A/B.// Read without the lock, into this thread's own buffer (its room kept from draw to draw); the lock is taken only to add it// to the frame's poses. ub_read is this recording thread's view of the stream, as the draw's own reads of its uniforms.// (An assumption, open with the author: OoT writes its bone palettes and object blocks on the CPU only. ub_read sees theStill open from the second review: this shouldn't merge as an open question. Either state it as a fact and say how it was checked, or keep the GPU copy for bones in a range the stream wrote on the GPU.
966c69e139f2353ee86cf2353ee86c1168fe3a681168fe3a689938524b1c9938524b1c10e2b4d14eI rebased this onto main @
4bab3915and fixed review 1189's code points in one commit of mine on top:10e2b4d1. The earlier commits are unchanged apart from the rebase. I checked that the code they carry is the same as at966c69e1.Rebase.
docs/tested-configurations.mdconflicted again. Your Hookshot row now sits after the last 2026-11-13 row, where its date puts it in the newest-first table. It names Longshot PR 59 instead of "this change", and links NotRexed/Hookshot PR 2787 in full.My commit:
vk_temporal.cpp). The time between two poses is clamped to 1/60-1/30 s, where the upper bound was 1/4 s. At the default 60 a second, the reach now stays under the stones' 2.3 after any gap. Past 2.3, draw order, which wins whenever it pairs more copies, paired a whole row of stones each with its neighbour after a hitch. The comment states the cost: across a gap longer than 1/30 s, an actor that moved more than 2 units keeps only the camera's motion for that frame.tests/test_dlss_actor.cpp): a row of stones whose model changed. With a reach of 2, the second stone pairs with itself and the third with none. With 3.3, each takes its neighbour, which is why the reach is capped.pair_by_place's comment indlss_actor.hsays that the reach is what guards draw order.RX_VK_DLSS_ACTOR_LIVE=1takes main's copy instead. The code can't confirm what OoT does, so I didn't claim it.RX_VK_DLSS_ACTORS,RX_VK_DLSS_ACTOR_LIVEandRX_VK_DLSS_ACTOR_REACHare now indocs/development.md's switch reference. Desmond27's and mooki's runs of8a091b34have rows indocs/tested-configurations.md.Description. I rewrote it for the PR as it is now:
It now includes the runs from this thread, each with the commit it ran on: yours (Hookshot 06ad86fe), Desmond27's (
8a091b34, RTX 5080, FG 2x-4x, the stones whole) and mooki's (8a091b34, Linux, FG off). My third review wrongly said there had been no testing. There had, and it's in the description now.No tests, builds or game runs were made on this PC. Forgejo's CI on
10e2b4d1skips the Python checks and the SPIR-V fixtures for this PR.Back to WIP. Nobody has run any version since
8a091b34in the game. Since then the reach, the pairing and where the poses are read have all changed, so the earlier runs don't cover this code. Before it's ready, it needs:tests/test_dlss_actor.py(or the suite) on this head;fix(vk): actor motion pairs a model's copies by place, so the Gossip Stones stay whole under frame generationto WIP: fix(vk): actor motion pairs a model's copies by place, so the Gossip Stones stay whole under frame generationDescription only: Measurements now lead with mooki's main @
d4a69456/ this PR @8a091b34runs, and the author's Hookshot run names its builds in its columns, so the format check reads them right. No change to the code.Rebased onto current
main(5ad0f50395). 20 commits had merged since4bab3915, among them #307 and #189. The four commits are rebased with no code conflict, and the first two are still yours.Only
docs/tested-configurations.mdconflicted, becausemainhad added rows at the top of the table. I keptmain's rows and put this PR's after them in date order: the 2026-11-13 runs, then the 2026-11-12 row, worded as in10e2b4d1. Each commit's added and removed lines are the same as before the rebase. No tests or builds were run on this PC.The description's "Rebased on current main" line still names
4bab3915and should name5ad0f50395. Nothing else about the PR changes: it is still WIP and still needs a frame-generation run of this head and the CPU A/B.10e2b4d14ecc7cb36aedRan this head in the game: the stones stay whole in the made frames.
Build: Longshot main
cf605c85with this PR's four commits (cc7cb36a) rebased onto it locally, with no conflict (the same diff ascc7cb36a), built as a renderer dev pack (scripts/dev_packs.py) on a base ofcf605c85.Hardware: RTX 5090 32 GB, driver 621.24, Ryzen 7 9800X3D, 64 GB, Windows 11 Pro.
Scene and settings: the four Gossip Stones in front of the Temple of Time, the camera moved back and forth (main's repro); fullscreen, 5120x2160 output, DLAA, DLSS Frame Generation 2x (NGX), V-Sync Auto (FIFO), Reflex On + Boost, frame pacing on.
8a091b34shows 0.410 ms: moving the reads out of the lock brought it down.cf605c85):tests/test_dlss_actor.pyand the 10 checks that readvk_temporal.cppordlss_actor.hpass.python -I -B tests/run_all.py: 516 passed, 8 failed, 35 skipped; maincf605c85on the same PC fails the same 8 (test_fpx_sticky,test_framegen_resources,test_module_ids,test_oot_state_tap,test_pad_layout,test_portable_builder_startup,test_spawn_emit,test_tested_configurations), so none of them comes from this change.Not done: main was not run on the same PC this time, and this is one run, not interleaved, so the Measured box stays unticked. FG 3x and 4x were not run on this head.
The run is added as
docs/tested/2026-11-14-gossip-stones-pmwork-personal-run01.md, and the description is updated to match.Measured this head against main on the same PC: 2 alternated runs each (this PR, main, this PR, main), from one dev-pack base of main
cf605c85, so both sides have the same caches and settings and only the renderer differs.cf605c85cc7cb36atests/test_dlss_actor.pywith this PR's test fails on main (main has nodlss_actor::Placedorpair_by_place) and passes on this head.Same setup as my last comment: RTX 5090, 621.24; fullscreen 5120x2160, DLAA, DLSS FG 2x (NGX), V-Sync Auto, Reflex On + Boost; in front of the Temple of Time, the camera moved back and forth, about a minute a run. The four runs are
docs/tested/2026-11-14-gossip-stones-pmwork-personal-run01.mdtorun04.md, and the description's Measurements, Testing and checklist are updated (Measured and the test box ticked). Still open: a clip, a Linux build, and the FG 3x/4x and AMD test requests.Description only, no code change:
cf605c85still hold for the PR's base5ad0f503: between the two, main has 8 commits, this PR's own lines are the same on both, and the only one touching a file of this PR (Longshot PR 252) wraps the pipeline creation in an Intel driver guard, not the per-frame actor motion.tests/that reads changed source updated in this PR, not weakened", ticked. "Host code builds on both Windows and Linux" stays unticked: this head built on Windows (llvm-mingw); a Linux build of this head is still to do. "Screenshots" stays unticked: no clip, by the author's choice; the A/B by eye is under Measurements.The PR stays WIP until this head has a Linux build.
WIP: fix(vk): actor motion pairs a model's copies by place, so the Gossip Stones stay whole under frame generationto fix(vk): actor motion pairs a model's copies by place, so the Gossip Stones stay whole under frame generation