Repository navigation
fix(shader): keep mesh register windows out of local memory on NVIDIA - #12233
Conversation
github-actions
Bot
added
the
unblocks title
MarcZX10
left a comment
MarcZX10
left a comment
There was a problem hiding this comment.
I found one initialization lifetime issue in the host pool setup that should be resolved before merge. PrepareMeshWindow records the header fill into a recorder command buffer and immediately sets pool.cleared = true. Recorder batches can be discarded if vkEndCommandBuffer or vkQueueSubmit fails; after that discard, later draws skip the fill even though no submitted command initialized the ticket counter and per-slot locks. The first mesh draw could then spin claiming a slot or reuse stale lock state. Please tie the initialized state to a successfully submitted initialization batch, or use an initialization path whose completion is guaranteed before later mesh draws can use the pool. Also include a focused check for the failed/discarded-first-batch case if feasible. I reviewed the shader indexing, liveness/initialization analysis, cache serialization, descriptor layout and release ordering statically. I could not reproduce the proprietary NVIDIA driver failure or run the new GPU regression test on this host, so I am not approving the driver-specific behavior without the author’s hardware evidence and this lifecycle case being addressed.
|
@beardsciences thanks, this reads well. I checked the slot lock (claim and release are well-structured, because the release sits in the mesh entry point after the guest call, so early returns still reach it), the pool size (header included) and the gating (NVIDIA proprietary + mesh stage + non-empty window + fits a slot, so other drivers are unchanged). One thing blocks CI: conventions fails with "Fill in the pull request template: add the '### What' heading of the template". The description starts straight with the text, so adding a |
|
Other open pull requests touch the same code. Merging one will break the other, or both implement the same thing. If this one builds on them, list them in Depends on.
Some comparisons could not be completed because a pull request conflicts with the base branch. Review these shared files manually before concluding that there is no overlap.
|
|
@beardsciences thanks, the window buffer path reads correctly to me: the slot claim and release, the bound that falls back to the arrays when a window doesn't fit a slot, and the stale-load and live-store planning all check out, and the cache key and disk cache carry the new flag. One change before merge: the new entry in |
|
@MarcZX10 thanks, you're right: Fixed in 076cec9. The header's initialization no longer goes into a draw's command buffer:
Two other approaches I tried and dropped:
I did not add a test for the discarded-batch case: no recorded initialization is left to discard, and I did not find a way in the test harness to make Rerun on 076cec9 (RTX 3060 Ti, 621.01, Windows 11, current
|
|
@oneandonlydean done in 7779011: the debt entry now names only PPSA07550. Thanks. |
|
verdict: should-fix
Freshness envelope: head 7779011; merge-base cccdaa2; behind main: 180; git merge-tree --write-tree origin/main refs/remotes/pr-head/12233: exit 0, clean. |
|
I rechecked head
Conventions passes, but only conventions/labels are reported for this head; I found no current focused GPU validation run. I am keeping changes requested pending the above evidence and rebase. |
MarcZX10
left a comment
MarcZX10
left a comment
There was a problem hiding this comment.
I rechecked head 77790116601ca240390208a280ad51481acbb5fa. The pool initialization race from my earlier review appears addressed: header initialization now runs in a separate submitted-and-waited batch, and pool readiness is published afterward. The report still does not satisfy the current review requirements:
- The branch is diverged from current
main(316615d042c087942e3fa11f258669abd92c3f27) by 276 commits behind and 3 ahead. Please rebase/update it to currentmain, resolve the newly exposed overlap, and rerun the focused tests. - The new mesh shader path needs the required paired validation run for this shader change:
agc_driver_mesh_vector_window_waveson current main and the updated head withANYPS5_REQUIRE_VULKAN=1,ANYPS5_NO_SHADER_CACHE=1,VK_INSTANCE_LAYERS=VK_LAYER_KHRONOS_validation; provide logs confirming the layer loaded and compare newVUID-findings. The currently reported RTX 3060 Ti runs andAPS5_NO_MESH_WINDOW_BUFFERnegative control are useful, but do not document that required pair or validation result. - The description makes frame-rate and compilation-time comparisons but has no paired current-main/change
aps5-summary.jsonllines, attached traces andtrace_digest.pyoutput. Please provide those for the speed claim or remove the unsupported speed comparison. - The checklist says
Unblocks: PPSA07550 (no Game bring-up discussion yet). CONTRIBUTING requires the title ID and its Game bring-up discussion, ornone; please add the discussion link or change the line tonone.
Conventions passes, but only conventions/labels are reported for this head; I found no current focused GPU validation run. I am keeping changes requested pending the above evidence and rebase.
7779011 to
1f9b309
Compare
github-actions
Bot
removed
the
unblocks title
|
@MarcZX10 @fogydes thanks. Updated:
Validation logs: Traces (
|
MarcZX10
left a comment
MarcZX10
left a comment
There was a problem hiding this comment.
Reviewing exact head 1f9b309c584049a6c6bec2836dc1a969cae4b62c against current main 319621ba2e578bd053b0baba2877fe70520e0737.
What I ran
git diff --checkagainst currentmain: passed.git merge-tree --write-treeagainst currentmain: clean merge tree (e7369b6c37aa7841c323a1ae0362c44c9aa9b0e5).- Inspected the PR body, current diff, review history, and hosted checks. Conventions, labels, report, eligibility, and Windows passed; Linux and specialized jobs were skipped.
- I did not build the project or run CTest/GPU tests locally. The validation and trace evidence below is author-reported, not independently reproduced.
Results
| Check | Result | Evidence |
|---|---|---|
| Patch whitespace | Pass | Local git diff --check against current main |
| Mergeability | Pass | Local merge-tree against current main |
| Hosted Windows | Pass | GitHub Actions for this head |
| Hosted conventions, labels, report, eligibility | Pass | GitHub Actions |
| Hosted Linux / specialized jobs | Skipped | GitHub Actions |
| Focused GPU validation | Stale baseline | Author posted a same-machine RTX 3060 Ti pair for 750cf6cf48 and this PR head, with Vulkan validation enabled and logs attached. Current main is 319621ba2e578bd053b0baba2877fe70520e0737, so that report does not cover the current base. |
| Performance comparison | Stale baseline | The paired traces and trace_digest.py outputs compare 750cf6cf48 plus #7893 with this PR plus #7893. They document a substantial GPU-time regression, but do not compare current main with the change. |
Findings
Blocking
- The branch and its validation evidence are behind current
main. The PR's merge base is750cf6cf487f948017295b1f3717af6f886f0f85; currentmainis319621ba2e578bd053b0baba2877fe70520e0737(282 commits ahead of the PR branch). The merge-tree is clean, but the PR template requires a current-main basis, and this branch has not been integrated with the latest base. Please update/rebase it onto currentmain, resolve any newly exposed overlaps, and rerun the focused checks on the updated head. - The required shader/AGC validation pair must cover current
mainand the updated head.AGENTS.mdrequires the same focused tests on both builds, on the same machine and GPU, withANYPS5_REQUIRE_VULKAN=1,ANYPS5_NO_SHADER_CACHE=1, andVK_INSTANCE_LAYERS=VK_LAYER_KHRONOS_validation; confirm the layer loaded and compare allVUID-lines. The posted pair uses750cf6cf48, not currentmain. Keep the PR Draft until this updated pair is available, or post the pair and update the report. - The performance claim also needs a current-main pair. The attached summary lines, traces, and digests use
750cf6cf48as “main”; they do not establish the comparison against currentmain. Please rerun the same scene and machine against the updated base and head and attach the matching summary lines, traces, and digest output, or remove the stale comparison from the PR description.
The previous pool-initialization concern appears addressed by publishing the descriptor set only after the separately submitted initialization batch completes. I did not independently execute the GPU regression test on this head.
Sources
- Repository review and GPU validation rules
- PR's validation and performance evidence
- Current pull request
Not verified
- I did not run a build, CTest, or GPU test locally.
- I did not independently inspect the attached GPU logs or trace files.
- No current-
mainvalidation or performance comparison is included in the evidence I checked.
To get this to a good state
Rebase/update onto current main 319621ba2e578bd053b0baba2877fe70520e0737, then post fresh same-machine GPU validation logs and trace comparison for that base and the new head. The validation logs should confirm actual layer loading and show no new VUID- errors; the performance evidence should include the paired summary lines, traces, and trace_digest.py output. Keep the PR Draft until the required GPU pair is available.
|
Correction to the Sources section of my formal review: the author’s current-base GPU and trace evidence is in this comment. The earlier link in my review used the wrong comment ID; the review findings are unchanged. |
1f9b309 to
7f0e19d
Compare
|
@MarcZX10 thanks. Updated on current
aps5-trace-a6abcf9de1ec-8436.json.gz |
MarcZX10
left a comment
MarcZX10
left a comment
There was a problem hiding this comment.
Re-checked — changes still requested
I reviewed the updated head 7f0e19dd6d15017f8f281c1b3a98d1325f8da3d6 against current main (3a2b91981651569163768854a1fcdc7193b6667b). The implementation review and updated test evidence address the earlier technical questions. I am keeping this review at Changes requested because the four performance summary lines still record APS5_TRACE_DIR; the repository's performance report form instructs reporters not to set other APS5_TRACE_* or APS5_PROFILE_* switches.
What I ran
On Linux x86_64 (GCC), I checked the exact PR head and built the focused mesh test in the isolated review build:
python3 tools/check_conventions.py --base upstream/main --head refs/remotes/review/12233git diff --check upstream/main...refs/remotes/review/12233git merge-tree --write-tree upstream/main refs/remotes/review/12233ninja -C /tmp/anyps5-pr12664-build-full agc_driver_mesh_vector_window_waves_tests- Native CTest for
agc_driver_mesh_vector_window_waves - The same executable under
qemu-x86_64 -cpu Haswell
I also downloaded the four attached traces and matching digest outputs and recomputed each digest using tools/trace_digest.py with its attached summary. I read both attached Vulkan validation logs; I did not run the Windows/NVIDIA workload locally.
Results
| Check | Result |
|---|---|
| Conventions | Passed |
| Diff whitespace | Passed |
Merge preview with current main |
Clean (17122221904ecf0f155886fb2f15a7f22fd048c1) |
| Focused mesh test build | Passed (167/167 Ninja actions) |
| Native CTest | Could not reach assertions: host CPU feature guard aborts on this Xeon E5-1620 v2 |
| QEMU Haswell run | Skipped with exit 77 because llvmpipe does not provide the required mesh-shader compiler |
| Four attached trace digests | All reproduced exactly (after normalizing CRLF) |
| Attached Vulkan validation logs | Same ten WARNING-VkImageSubresourceRange-layerCount-compatibility warnings in each; no VUID- errors |
Raw local check output
Convention checker: passed
Diff check: clean
Merge preview tree: 17122221904ecf0f155886fb2f15a7f22fd048c1
Ninja: 167/167 actions; agc_driver_mesh_vector_window_waves_tests linked
Native CTest: aborted by CPU feature guard before test assertions
QEMU Haswell: test skipped (exit 77); llvmpipe lacks required mesh-shader compiler
Trace digest recomputation: 4/4 matched attached digest output
Negative control and hardware evidence
The author reports that the exact current PR head passes the new mesh test twice on an RTX 3060 Ti, while setting APS5_NO_MESH_WINDOW_BUFFER=1 reproduces wrong results in waves 1–3 twice. Their current-head Windows validation run reports 27/27 tests on the change and 26/26 on its matching main baseline, with no VUID- errors. The author also supplied paired baseline/change traces from the same title, scene, and machine. I independently recomputed all four attached digests successfully. These are author-run hardware results, not locally reproduced runs. Details and attachments are in the author's current-head evidence comment.
Findings
Blocking
- The four
aps5-summary.jsonllines includeAPS5_TRACE_DIR. The current performance report form says not to set any otherAPS5_TRACE_*orAPS5_PROFILE_*switch. The current report therefore does not meet the repository's stated capture procedure, even though its traces are attached and their digests reproduce. Please recapture the same baseline and change runs withoutAPS5_TRACE_DIR(write the traces to the working directory), then replace the four summary lines and attach the corresponding traces and digest outputs. Keep the same title, scene, machine, and fixed input so the comparison remains controlled.
Non-blocking
- The current PR merge preview against
mainis clean. Its documented#7893prerequisite is common to both baseline and change captures, and the report explains why it is needed to reach the scene; the comparison isolates this PR's effect.
Not verified locally
I could not run the NVIDIA-specific workload or reproduce the Windows validation/test results on this Linux host. Native CTest stopped at the CPU guard, and QEMU with llvmpipe skipped the test because its Vulkan implementation lacks the required mesh-shader compiler. I therefore do not count those as passing local runtime tests.
To resolve
Please provide the four recaptured summary lines and their matching traces/digests without APS5_TRACE_DIR. I can then re-check the updated report and clear this request if the capture record complies.
|
@MarcZX10 thanks for recomputing the digests. Recaptured without digest-tr-pr-2.txt |
MarcZX10
left a comment
MarcZX10
left a comment
There was a problem hiding this comment.
Re-checked — approved
The requested performance capture correction is complete. The four current summary lines no longer include APS5_TRACE_DIR, and I reproduced each attached digest from its trace and summary.
What I ran
- Checked current head
7f0e19dd6d15017f8f281c1b3a98d1325f8da3d6and its updated description/comment. - Downloaded all four newly attached trace files and digest reports.
- Ran
tools/trace_digest.pyon each trace with its matching summary line. - Refreshed the hosted check state and re-read the exact-head author evidence and previous review findings.
Results
| Check | Result |
|---|---|
| Performance summary environment | Passed; only APS5_TRACE_SECONDS=260:60 is set among trace/profile switches |
| Recomputed trace digest: main run 1 | Exact match to attached digest |
| Recomputed trace digest: main run 2 | Exact match to attached digest |
| Recomputed trace digest: change run 1 | Exact match to attached digest |
| Recomputed trace digest: change run 2 | Exact match to attached digest |
| Author-reported NVIDIA validation pair | Pass on same RTX 3060 Ti machine; 26/26 baseline and 27/27 change tests, no VUID- errors |
| Hosted checks | Conventions, eligibility, labels, and report pass; Windows pending; Linux and specialized jobs skipped |
All four full trace_digest.py outputs match the attached reports exactly. The reported baseline and change use the same title, menu scene, fixed input, machine, driver, and warm-cache alternating capture procedure.
Negative control
| Control | Result |
|---|---|
APS5_NO_MESH_WINDOW_BUFFER=1 |
Author reports the focused test fails in waves 1–3 in two runs; normal PR configuration passes in two runs |
| Independent reproduction | Not run; this Linux host lacks the required CPU features and does not provide the author’s NVIDIA hardware |
Findings
Blocking
None found on this head. The prior report-format issue is resolved, and the attached trace digests verify against the current captures.
Non-blocking
- The performance report shows a substantial GPU-time cost for this path. The report also demonstrates that the baseline renders corrupted output and the change fixes it, so the measured cost is documented against the behavior being corrected.
- The Windows hosted job is still pending. The exact-head NVIDIA validation and performance evidence are reported by the author and were not rerun on my machine.
Sources
- Current reviewed head: mesh-window implementation and current report
- Current performance capture instructions
- Author’s current recapture and attached traces/digests
- Author’s exact-head validation pair and negative control
Not verified
- I did not execute the Windows/NVIDIA workload, the author-reported CTest validation pair, or the negative control locally.
- Windows hosted build remains pending at the time of this review; Linux product and specialized workflows were skipped by policy.
To get to a good state
No further changes are required for the findings in this review.
|
@beardsciences thanks, the fill now happens once in its own submit before the first draw that uses the window, which removes the dropped-command-buffer case. One request left: the Mesh vector windows entry in @adriadam10 courtesy: assigned to you; the rest of this update is code-pass. |
MarcZX10
left a comment
MarcZX10
left a comment
There was a problem hiding this comment.
Re-reviewed exact head 7f0e19dd6d15017f8f281c1b3a98d1325f8da3d6 after the new review comment. The implementation and matched validation/performance evidence remain as reviewed, but docs/dev/debt/shader.md:196 still names “PSPHD” twice. The repository debt convention uses PPSA IDs or “the title”; please replace both names with “the title” (PPSA07550 is already identified in this entry). The current-main merge preview is clean and git diff --check passes. No GPU tests rerun locally.
On NVIDIA's proprietary driver, function arrays that live in local memory and are read under a data-dependent branch return wrong values in subgroups 1-3 of a mesh workgroup; subgroup 0 is correct. A wave64 geometry program whose relative addressing keeps its VGPRs in the vector window therefore loses register writes in every wave but the first. In the clipping geometry program of Sony's PS5 PSP emulator (PPSA07550) the empty waves never cleared their candidate primitive slots and wrote degenerate triangles over wave 0's, so the 3D menu drew spikes. On that driver a mesh program's window now lives in a device-local storage buffer of 256 workgroup slots of 65536 dwords, claimed through a per-slot lock and released after the final barrier. The pool is created before the first draw that uses it records anything, and its header is zeroed in a batch of its own that is submitted and waited for; the pool is published only after that wait, and a failure releases it. Direct register accesses keep one function variable per register; a direct store also writes the buffer when a buffer read can see it, relative reads load the buffer, relative writes store only to it, and a direct read that may follow a relative write loads the buffer and refreshes the variable. A buffer entry is zeroed only when a buffer read can see it before a write. APS5_NO_MESH_WINDOW_BUFFER=1 keeps the arrays.
7f0e19d to
6f79055
Compare
|
@MarcZX10 @oneandonlydean thanks, done: both names in the |
MarcZX10
left a comment
MarcZX10
left a comment
There was a problem hiding this comment.
Re-checked — new head, approved
The only change since the previously approved head replaces the two title nicknames in the debt entry, resolving the outstanding repository wording request.
What I ran
Reviewed head 6f79055a0001703bc0419fb03a8dabd9ef1a2677 against current main 899ae3d1ad32ccea94dc24e66fdc948ce8a88171. The merge preview is clean (9f92fdb8033189e5630fb23bc56a08ad5b1aaa47). Environment: Debian GNU/Linux 12; GCC 12.2.0; CMake 3.25.1.
git diff 7f0e19dd6d15017f8f281c1b3a98d1325f8da3d6 refs/remotes/upstream/pr/12233-review -- docs/dev/debt/shader.md
git diff --check upstream/main...refs/remotes/upstream/pr/12233-review
git merge-tree --write-tree upstream/main refs/remotes/upstream/pr/12233-review
python3 tools/check_conventions.py --base upstream/main --head refs/remotes/upstream/pr/12233-review --title 'fix(shader): keep mesh register windows out of local memory on NVIDIA' --body /tmp/AnyPS5-12233-body.mdResults
| Check | Result |
|---|---|
| Current-head delta | One debt-document line changed; shader implementation and tests are unchanged |
| Conventions, including PR body | Passed |
| Diff check | Passed |
| Merge preview | Clean |
| Hosted conventions and labels | Pending when checked |
| Behavioral tests | Not rerun; the new head changes debt wording only |
Raw output
All conventions checks passed.
git diff --check produced no output and exited 0. git merge-tree --write-tree returned:
9f92fdb8033189e5630fb23bc56a08ad5b1aaa47
Negative control
| Assertion | Change made to code under test | Did the test fail? |
|---|---|---|
| Mesh-window behavior | No behavioral code changed on this head; no negative control rerun | Not applicable to the documentation-only update |
Findings
Blocking
None on this head.
Non-blocking
None on this head.
Sources
Not verified
I did not rerun the GPU workload, Vulkan validation pair, performance captures, or CTest because the changed line is documentation only. Hosted conventions and labels were still pending.
To get this to a good state
No further changes are required for the findings reviewed here. I will rerun these checks if the head changes.
✅ NVIDIA GPU test: passedPR head
Failing on main too (5, not counted against this PR)Validation IDs already present on main (4 IDs in 407 tests)Skipped (39)ReproduceAutomated build and test on a contributor's NVIDIA machine; it does not replace upstream CI. A new comment is posted for each new push. Full logs. |
|
Re-checked the new head: the only change is the debt entry, where both title nicknames now read "the title" (PPSA07550 stays). Code, test and traces are as reviewed before; the NVIDIA CI run on this head passed with no new failures; merged. |
What
On NVIDIA's proprietary driver, function arrays that the driver keeps in local memory return wrong values in subgroups 1–3 of a mesh workgroup when they are read under a data-dependent branch. Subgroup 0 is correct, and the same reads without a branch are correct. A wave64 geometry program that uses relative VGPR addressing (
v_movrels/v_movreld) keeps its registers in the vector window, which is two such arrays per invocation, so waves 1–3 of a 256-thread NGG group lose register writes.v3 = v22 << 2in the setup block. They never cleared their candidate primitive slots, read 0 there, and wrote 76 degenerate triangles over wave 0's 6, so the 3D menu drew spikes.On that driver a mesh program's window now lives in a storage buffer:
APS5_NO_MESH_WINDOW_BUFFER=1keeps the arrays. Other drivers are unchanged.Cost, measured in PPSA07550's 3D main menu with #7893 (needed to reach it), RTX 3060 Ti, warm caches,
APS5_TRACE_SECONDS=260:60and no other trace or profile switch, two alternating runs per build.mainis a59fd393ec6d (3a2b919 + #7893 76bea98), which draws the corrupt scene; the change is a6abcf9de1ec (this PR + the same #7893 head):main+ #7893, run 1main+ #7893, run 2The window buffer costs about 18–19 ms of GPU time per present in this menu. All four summaries report
trace_dropped(the event buffers of the queue worker and the draw committer filled after 9–38 s);totals_fullis false for every thread, so the summary numbers are complete. Traces andtools/trace_digest.pyoutput attached.Only mesh programs that keep a vector window for relative register addressing (
v_movrels/v_movreld) take this path, and only on NVIDIA's proprietary driver; every other shader, and every other driver, is compiled as onmain. The fastermainframes above render the corrupt scene, so the cost buys correct output rather than slowing down working output.Summary lines
{"commit":"a59fd393ec6d","dirty":false,"env":{"APS5_PAD_INPUT_FILE":"<path>","APS5_TRACE_SECONDS":"260:60"},"gpu":"NVIDIA GeForce RTX 3060 Ti","driver":"NVIDIA 621.01","os":"Windows 10.0.26200","cpu":"AMD Ryzen 5 5600X 6-Core Processor","cpu_threads":12,"memory_mib":32674,"title_id":"PPSA07550","title":"MediEvil: Resurrection","content_version":"01.000.003","resolution":"3840x2160","window":{"spec":"seconds 260:60","first_present":6193,"ms":60012.311},"presents":1654,"frame_ms":{"p50":38.023,"p90":43.264,"p99":52.195,"max":69.873},"gpu_busy_ms_per_present":24.174,"worker_busy_pct":{"queue worker 0x0":47.762},"top_phases_ms_per_frame":{"VideoOut.Flip":18.221,"Driver.Present":18.085,"VideoOut.Flip.present":18.073,"Driver.Present.render_fence_wait":17.454,"Vulkan.Retire":17.453,"Vulkan.Retire.wait":17.447,"Driver.Packet":17.321,"Graphics.Draw":15.833},"trace_dropped":true,"trace_dropped_threads":{"queue worker 0x0":{"events_full_after_s":9.625,"events_lost":940448,"totals_full":false},"draw committer":{"events_full_after_s":24.553,"events_lost":339616,"totals_full":false}},"trace":"aps5-trace-a59fd393ec6d-23000.json.gz"} {"commit":"a6abcf9de1ec","dirty":false,"env":{"APS5_PAD_INPUT_FILE":"<path>","APS5_TRACE_SECONDS":"260:60"},"gpu":"NVIDIA GeForce RTX 3060 Ti","driver":"NVIDIA 621.01","os":"Windows 10.0.26200","cpu":"AMD Ryzen 5 5600X 6-Core Processor","cpu_threads":12,"memory_mib":32674,"title_id":"PPSA07550","title":"MediEvil: Resurrection","content_version":"01.000.003","resolution":"3840x2160","window":{"spec":"seconds 260:60","first_present":5638,"ms":60026.885},"presents":1280,"frame_ms":{"p50":36.386,"p90":77.920,"p99":110.653,"max":183.849},"gpu_busy_ms_per_present":42.881,"worker_busy_pct":{"queue worker 0x0":92.287},"top_phases_ms_per_frame":{"Driver.Packet":43.262,"Graphics.Draw":41.560,"Driver.Packet.draw":41.526,"Driver.Draw":41.448,"Graphics.Draw.draw_and_resource_release":40.413,"VideoOut.Flip":36.666,"Driver.Present":36.499,"VideoOut.Flip.present":36.484},"trace_dropped":true,"trace_dropped_threads":{"queue worker 0x0":{"events_full_after_s":16.317,"events_lost":669945,"totals_full":false},"draw committer":{"events_full_after_s":33.279,"events_lost":205045,"totals_full":false}},"trace":"aps5-trace-a6abcf9de1ec-5384.json.gz"} {"commit":"a59fd393ec6d","dirty":false,"env":{"APS5_PAD_INPUT_FILE":"<path>","APS5_TRACE_SECONDS":"260:60"},"gpu":"NVIDIA GeForce RTX 3060 Ti","driver":"NVIDIA 621.01","os":"Windows 10.0.26200","cpu":"AMD Ryzen 5 5600X 6-Core Processor","cpu_threads":12,"memory_mib":32674,"title_id":"PPSA07550","title":"MediEvil: Resurrection","content_version":"01.000.003","resolution":"3840x2160","window":{"spec":"seconds 260:60","first_present":6228,"ms":60025.286},"presents":1646,"frame_ms":{"p50":37.986,"p90":43.180,"p99":53.654,"max":71.361},"gpu_busy_ms_per_present":24.428,"worker_busy_pct":{"queue worker 0x0":47.833},"top_phases_ms_per_frame":{"VideoOut.Flip":18.659,"Driver.Present":18.379,"VideoOut.Flip.present":18.375,"Driver.Present.render_fence_wait":17.833,"Vulkan.Retire":17.832,"Vulkan.Retire.wait":17.825,"Driver.Packet":17.435,"Graphics.Draw":16.070},"trace_dropped":true,"trace_dropped_threads":{"queue worker 0x0":{"events_full_after_s":10.260,"events_lost":920764,"totals_full":false},"draw committer":{"events_full_after_s":25.193,"events_lost":334873,"totals_full":false}},"trace":"aps5-trace-a59fd393ec6d-2260.json.gz"} {"commit":"a6abcf9de1ec","dirty":false,"env":{"APS5_PAD_INPUT_FILE":"<path>","APS5_TRACE_SECONDS":"260:60"},"gpu":"NVIDIA GeForce RTX 3060 Ti","driver":"NVIDIA 621.01","os":"Windows 10.0.26200","cpu":"AMD Ryzen 5 5600X 6-Core Processor","cpu_threads":12,"memory_mib":32674,"title_id":"PPSA07550","title":"MediEvil: Resurrection","content_version":"01.000.003","resolution":"3840x2160","window":{"spec":"seconds 260:60","first_present":5891,"ms":60022.217},"presents":1284,"frame_ms":{"p50":36.193,"p90":76.179,"p99":115.956,"max":183.309},"gpu_busy_ms_per_present":42.876,"worker_busy_pct":{"queue worker 0x0":90.375},"top_phases_ms_per_frame":{"Driver.Packet":42.230,"Driver.Packet.draw":40.516,"Graphics.Draw":40.486,"Driver.Draw":40.439,"Graphics.Draw.draw_and_resource_release":39.372,"VideoOut.Flip":36.761,"Driver.Present":36.625,"VideoOut.Flip.present":36.608},"trace_dropped":true,"trace_dropped_threads":{"queue worker 0x0":{"events_full_after_s":19.564,"events_lost":665710,"totals_full":false},"draw committer":{"events_full_after_s":37.669,"events_lost":200843,"totals_full":false}},"trace":"aps5-trace-a6abcf9de1ec-37332.json.gz"}Overlap: the overlap comment lists #6416, #6751, #6959, #7392, #8246, #10547, #10846, #11557, #11951, #12548 and #12662. None of them implements this or needs it, and this needs none of them: each adds its own flag, field, environment row or debt line next to this PR's (
SpirvTarget,CompiledShaderArtifact, the cache key,SpirvEmitterState, the draw paths inDraw.cpp,ENVIRONMENT.md,debt/shader.md), so whichever merges second keeps both lines. #6959 also changesEmitMeshEntryPoint, but in the layer output, not around the window claim and release.Tested
Windows 11, NVIDIA GeForce RTX 3060 Ti, driver 621.01 (32.0.16.2101).
mainis 3a2b919, the change is this PR on it (7f0e19d).Validation,
mainand the change:ctest --test-dir build -R "mesh|vector_window|relative|window_lane|ngg|provoking" -VwithANYPS5_REQUIRE_VULKAN=1,ANYPS5_NO_SHADER_CACHE=1andVK_INSTANCE_LAYERS=VK_LAYER_KHRONOS_validation; both logs attached. Neither containswas not found but was requested by env var VK_INSTANCE_LAYERS; the layer reports the same 10 messages in both, allWARNING-VkImageSubresourceRange-layerCount-compatibility, and noVUID-error.mainpasses 26 of 26, the change 27 of 27 (the extra test isagc_driver_mesh_vector_window_waves, whichmaindoes not have).New GPU test
agc_driver_mesh_vector_window_waves: a hand-written wave64 geometry program with 256 threads (no title code) fills 128 window registers with ordinary register writes. It reads them back throughv_movrelsunders_and_saveexec_b64+s_cbranch_execz, plus three direct reads, and stores a mismatch count per thread. WithAPS5_NO_MESH_WINDOW_BUFFER=1it fails in waves 1–3 (wrong threads per wave: 0 64 64 64), 2 of 2 runs. With this PR it passes, 2 of 2.MediEvil: Resurrection (PPSA07550), Sony's PSP emulator wrapping UCUS98620, together with fix(shader): hand a geometry program strip and fan triangles in the order of its provoking vertex mode #7893 (without it the menu aborts on a triangle-fan refusal first). The 3D main menu and its idle camera fly-through render correctly; on
main+ fix(shader): hand a geometry program strip and fan triangles in the order of its provoking vertex mode #7893 they show polygons stretched into spikes. Screenshots attached; the last visual check of the menu was on cccdaa2, and on 3a2b919 the traced runs above run the same menu.Before:
main+ fix(shader): hand a geometry program strip and fan triangles in the order of its provoking vertex mode #7893, main menu. Polygons stretched into spikes.After: this PR + fix(shader): hand a geometry program strip and fan triangles in the order of its provoking vertex mode #7893, the same menu.
After: this PR + fix(shader): hand a geometry program strip and fan triangles in the order of its provoking vertex mode #7893, during the menu's idle camera fly-through.
Not tested: AMD and Intel GPUs (the path is not taken there), other NVIDIA GPUs and drivers. TEKKEN: Dark Resurrection (PPSA31371) runs the same emulator, and TEKKEN: DARK RESURRECTION (PPSA31371) #10051 reports black and missing triangles with
APS5_NO_WINDOW_NARROWING=1; that may be this bug but was not checked.Checklist
main; no other open PR implements the same functionsNotImplemented_nid_no_patch); silent stubs are listed under Silent stubs in their library's technical debt file.mdfiles or images added to the repository (attach them to this PR)