WIP: fix(macos): an indirect fan of more than one record drawn a call a record on MoltenVK #702
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 milestone
No project
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set
Reference
MagicBeanDev/Longshot!702
Loading…
Reference in a new issue
No description provided.
Delete branch "mooki/Longshot:mac/fan-indirect"
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?
Type: fix · Risk: low · Tested on: macOS 26.7.1, Apple M4 Max (MoltenVK 1.4.2), arm64 and x86_64 under Rosetta · Needs: macOS, Apple silicon (a start to play on the rebased stack); Windows and Linux, any GPU (build and start)
Why
On a Mac an indirect triangle fan of more than one record can lose or garble triangles. MoltenVK 1.4.2 converts all of an
indirect fan's records in one compute pass into one shared temporary, each record's
3(n-2)triangle indices written from its ownfirst index (or first vertex), so records closer than their triangles write over each other's (
MVKCmdDraw.mm:1128-1211,:676-720,:227-263). This PR counts these draws and, on MoltenVK only, draws such a fan one call a record, each with a temporaryof its own. It is part of the macOS port. See #420.
How to reproduce
Seen on: macOS 26.7.1, Apple M4 Max, MoltenVK 1.4.2
Settings and switches: defaults
Steps: probe FI-01's
offarms (standalone MoltenVK process; command inreports/2026-11-14-fan-indirect.md§5): indirect fansof 2, 3 and 7 records, 16-bit, 32-bit and not indexed, packed (first indices closer than their triangles), spaced, far and same
Expected: each record draws its own fan, as it does alone
Actual: every packed arm of 2, 3 and 7 records wrong in every trial (later records unlit or wrong); every other arm right
What changed
On MoltenVK's driver only, an indirect
TRIANGLE_FANdraw of more than one record whose program does not read the draw index isrecorded as one
vkCmdDraw(Indexed)Indirecta record, record i fromoffset + i * stride(from the count-draw rewrite's recordswhere there are some, so a count applies); a program that reads the draw index keeps its one call (a call a record would give each
draw index 0), counted and logged at error level. Every other driver records as before.
gpu/vk/fan_indirect.h(new):choose()(Split, DrawIndex, One, NotFan, Driver),record_at(),rewrite_count().gpu/vk/vk_draw.cppfan_indirect_way,fan_indirect_split(both indirect branches call it first); a count draw thatsplits is rewritten also where the device has
drawIndirectCount(MoltenVK main), so no call goes through MoltenVK's count-plus-fanpath.
fan_indirect_noteatdraw()'s common end right after #701's census;draw_fan_indirect_reportwith the frame statistics (vk_replay.cpp,vk_draw.h).gpu/vk/VULKAN.md.tests/test_fan_indirect.py+tests/fan_indirect_test.cpp(new) runchoose,fan_indirect_splitand the indirect blockcut from the source on a model of the records (21,660 draws recorded as before off MoltenVK); #663's
tests/seg_gathers_test.cppgets
fan_indirect.hand stand-ins.Measurements
Verdict: fixed where measured: FI-01 drew 429 of 429 records right with the split, against every packed multi-record arm wrong
without it; each record split off is drawn exactly as MoltenVK draws it alone (858 of 858). No game run with it yet: the census line
will say whether LS draws such fans and how many calls the split costs.
Baseline: main does not start on macOS. FI-01 compares one call (
off, what main records) with the split (on) in one standaloneMoltenVK 1.4.2 process.
Hardware: macOS 26.7.1, Apple M4 Max (40-core GPU), MoltenVK 1.4.2, 16-core CPU, 64 GB
Scene: FI-01: indirect fans of 1, 2, 3 and 7 records; i16, i32, plain; packed, spaced, far, same; 264 trials
Method: FI-01
reports/FanIndirect-files/fi01/runs-fi01-arm64/result.json(a40f33f8…),probe.log(f9d8f6a7…), released onceon arm64
How to reproduce: FI-01's
runcommand (reports/2026-11-14-fan-indirect.md§5)6c9edffe(one call, off arms)Flags and settings
None: no switch. On MoltenVK's driver only.
Testing
Tested: on macOS 26.7.1, Apple M4 Max, at this head (main
6c9edffeplus #701, #680 and what it carries):tests/test_fan_indirect.pywith/usr/bin/clang++andclang++ -arch x86_64(Rosetta): pass, 77,586 checks, 0 failures; with thetest copied onto #701's head it fails (
the FanIndirect block (vk_draw.cpp) is missing);test_seg_gathers(--arch arm64,--arch x86_64) andtest_count_draws(both compilers): pass (reports/PubWave-files/PubLate3/proof-02/).gpu/vk/vk_draw.cppandvk_replay.cppcompiled whole for arm64 and x86_64 with main's renderer flags (#476's, #492's and #490'smacOS compile pieces merged in a scratch tree only): 4 objects, exit 0 (
reports/PubWave-files/PubLate4/compile/fan-indirect/).unifdef -U__APPLE__):vk_draw.cpp,vk_draw.h,vk_replay.cppdiffer andfan_indirect.his included (the gate isthe driver id); off MoltenVK the way is
Driver, the indirect calls and the report are as before(
reports/PubWave-files/PubLate4/offapple-fan-indirect.txt).Not tested: a game start with this stack (main does not start on macOS by itself); a fan whose program reads the draw index (kept,
counted); not built on Windows or Linux; its test request covers it.
Must be tested:
vk: indirect fans on MoltenVK (…)lines, and no
vk: an indirect fan of N records of program … drawn as one callline (by hand: the parent's game run).fa6dd43a, it carries #701, #680 and what it carries) and main6c9edffethesame way and start each to the title, expecting no
vk: indirect fans on MoltenVKline and every other line as main's (by hand:test request T-XXXX).
Risks
What could break: on MoltenVK only: what MoltenVK spends on one fan call (a compute dispatch that restarts the Metal render pass
and a 256 KiB-1 MiB temporary) is now spent a record; a count draw's split covers maxDrawCount records. A draw-index reader keeps
MoltenVK's one call and its defect (counted).
Turning it off: revert the PR (no switch).
Compatibility: Nothing changes for players outside macOS. No file in the shader stamp changed; no
MKWVK_API_VERSIONchange.Reviewer notes
592cd8d3..fa6dd43a(4 commits):cherry-pick -xofmacos/fan-indirect9667c26d(the count),8ab5cbe5(the split),b2652b4d(the test's position check: the census right after #701's),1e546072(the gathers fixture's stand-ins), cleanon #701 (
reports/PubWave-files/PubLate3/pick-fan-indirect-*-lines.txt).test_seg_gatherscompiles only from1e546072.tests/test_perf_lines.pylets thiscensus line sit after #701's at
draw()'s end (Integration 37's 05e35178); with #703,tests/test_fan_indirect.pycuts the indirect block to
draw()'s next branch, now ProvokingFirst's list (Integration 37's eb4d0204, one line). Both are in thewave merge
39da60e1.calls here and is counted there as before.
Issues
See #420
Checklist
Everyone
type(scope): description) of the status line's type.main(maine090cc48).python -I -B tests/run_all.pypasses (any failure that also fails onmainis named). (On the wave's merge39da60e1= the nine late renderer PRs' heads (fan-key592cd8d3, fan-indirectfa6dd43a, provoking-firstb8194380, perf-instrea90d3ca, readback-whole-waits203b8b7c, fix/indirect-quadsc6e0c37f, indirect-quads978079d4, fix/mesh-count0a61a9e7, mesh-provoking56c09f1b) with their sibling fixes: 570 passed, 31 failed, 18 skipped against main 6c9edffe's 531/32/17 on this Mac; the 31 failures are main's own and nothing that passes on main fails;test_collect_widenow passes; 38 new tests pass;reports/PubWave-files/PubLate4/wave4/full-39da60e1/.)third_party/untouched (a new folder with aPROVENANCE.mdinstead).When it applies
Assisted-by:/Co-Authored-By:trailers.mainand passing here.tests/that reads changed source updated in this PR, not weakened.Every scan down vk_memory.cpp's import table by start (pool_lookup's covering import and merge plan, bg_drain, prefetch_chunks, import_in_background, memory_note_gpu_write and memory_guest_imported) stopped once an import ended more than 1 GiB below its address, on the belief that no import is larger than that. pool_lookup's own merge escape leaves an import above 1 GiB over the smaller ones inside it ("left overlapping"), and the device allows imports up to max_import_size: a scan stopped at a small import inside a large one, missed the large one, and the same bytes were imported again, two memories over the same bytes. import_reach.h counts the live imports by span class; its bound is above every live span, within twice the largest. imported_add and imported_take, now the table's only ways in and out, keep the count. A scan stops only where no import further down can reach its address, so it also stops sooner than before when the imports are small. The covering-import checks in pool_lookup and import_in_background look past the last import at or below the range for the same reason. Tests: test_import_reach.py runs the actual scans with a 4 GiB import over a 16 MiB chunk, left by the merge escape (imports are recorded ranges: no payload, no Vulkan, no game). Its lookup, query, note, drain, background and prefetch cases fail on main5e072fc8and pass here, and a source check keeps every change to the table in the two helpers. test_import_windows runs pool_lookup with the helpers; test_mirror_gpu_only names the new publication point. Assisted-by: Claude Opus 5.5 Co-Authored-By: Claude Opus 5.5 <[email protected]>044635ea added vma_startup_rollback (vk_vma.cpp, run after a failed replay_start), which calls vmaDestroyAllocator. vk_vma.cpp includes vk_work.h last so that RX_VK_RECORDER=defer/thread drains before its destructions, and every such boundary call in a vk_work.h file must be in RX_REC_BOUNDARIES with a drain-first macro. This one was not, and test_recorder_sync caught it ("vk_vma.cpp calls ['vmaDestroyAllocator'], which vk_work.h does not drain before"): it passes at 58c5c5aa (044635ea's parent) and fails at 044635ea. Add it to the list and to vk_work.h, next to vmaDestroyBuffer and vmaDestroyImage. With the recorder off (the default) boundary() is a relaxed load and a branch. Rollback runs before any image or buffer exists, so nothing should be held there today, but the allocator is no longer destroyed while a recorder could still hold packets. The test is unchanged (sources: 42 boundaries drained, was 41). Assisted-by: Claude Opus 5.5 Co-Authored-By: Claude Opus 5.5 <[email protected]>A guest span larger than one device allocation (the cap on MoltenVK/M4 Max is 41,747,087,360 bytes) is bound as pieces: one descriptor per allocation segment, at the descriptor's own binding (reports/2026-11-14-full-range-design.md sections 4.1-4.4). This is the shader half; the draw side (descriptors, layout, gathers) comes later. - runtime_info.h: SegEntry {Storage|Table|TexBuf|ImgBuf, desc, pieces} and RuntimeInfo::segmented. - Back end: a segmented storage buffer is an array of pieces at its binding (a table count * pieces, entry k's piece j at k * pieces + j); its loads, stores and atomics call DontInline helpers made after main, which find the piece by the pieces' OpArrayLength in order, with constant indices. The global memory fallbacks sum the pieces' lengths for a segmented descriptor's bound end and branch to its piece (single, native candidate, table written out and in the loop). Texel and image buffers are arrays too, their fetches, reads, writes and atomics through helpers that pick the piece by OpImageQuerySize, and a texel buffer's size query sums the pieces. ImageQuery was declared for every module already. With RuntimeInfo::segmented empty nothing new is made: the SPIR-V is byte-identical. - vk_shader: SegSig (entries, hash) on Program, seg_sig and seg_variant; program_segmented finds or translates the variant of a plain program with plain's texture questions and answers and checks its descriptors are plain's; program_pick, mesh_program_locked, program_find_plain, loaded_variants and portable_variant keep the plain and segmented variants apart; every stage's RuntimeInfo::segmented comes from Program::seg. - vk_progcache: a segmented variant is never written to the program cache nor listed among the loaded programs. - tests/test_seg_shader.py + tests/seg_shader_ir.cpp: synthetic IR programs through the real back end, plain and segmented; structure checks, spirv-val (Vulkan 1.3, uniform buffer standard layout), and with --base-tree a byte comparison of the plain modules against an older tree. Five source-reading checks follow the new Program field and the atomics' new branch. The translator stamp changes with this (eden/, vk_shader.cpp, sizeof(Program)): every program cache is rebuilt once. Land it in the same release as the vk_api.h change that already does so. Assisted-by: Claude Opus 5.5 Co-Authored-By: Claude Opus 5.5 <[email protected]> (cherry picked from commit b7928876dab846485f667c1c934d34d25ecef035)The interfaces the four segmented-consumer lanes (SegBind, SegXfer, TexelDesc, NullShader; reports/2026-11-14-seg-consumer-plan.md section 3) code against, frozen before they fork. Inert: no caller of anything new, no SPIR-V, layout, push range or stream change, sizeof(Program) unchanged. - vk_internal.h / vk_memory.cpp, SegMem's consumer interface: gpu_span_any() (a relaxed count of the live domains, stored in seg_commit; domains are kept for the run), gpu_span (gpu_buffer's flavour of gpu_span_bind: import buffers, no written marks, no mirror), SpanLayout::bind_stamp (gpu_bind_stamp(): a segment can be imported again with its cuts kept, so cut_gen alone never keys piece buffers), span_unused_piece's semantics ({null_storage, 0, 1}), and the SegArena API (SegArenaRef, seg_arena_take, seg_gather, seg_scatter; stub bodies after stream_alloc that give no room and log, SegBind's to fill). - Review fixes in the same interface (renderer-core review P2-3, P3.2): every piece of gpu_span_bind carries its buffer's device address (the import's or the mirror's; 0 only with bufferDeviceAddress off), and its one-piece path stores the translated guest address, not a virtual one. - vk_texture.h/.cpp: texel_view_pieces / texel_piece_views, stubs that return 1 and false (TexelDesc's to fill). - vk_shader.h: Program::presence_words, in what was padding. - Back end: PRESENCE_PUSH_OFFSET (88), Profile::emulate_null_descriptors, Bindings::presence / presence_read with the bit numbering rule. - tests: test_resolver_coverage lists gpu_span_bind / gpu_span sites too (scoped to gpu/vk: gpu/render has an rx::gpu_span of its own), checks that gpu_span follows its own gpu_buffer, and splits SITES into SITES_BASE plus one block per lane. test_segmented_domains executes the contract (the count 0, 1, 2 -> 1 on a merge, 0; gpu_span's pieces; addresses with the fixture's gpu_buffer_bind returning none as the real one does; the bind stamp; a virtual address's guest). test_seg_shader accepts a base tree of this back end's generation and compares every module. - docs/segmented-binding.md: the gate (every new behaviour Apple-only, other platforms byte-identical) and one heading per lane. Conflict on this branch (not with main): tests/test_seg_shader.py's docstring, against the array_pointer_calls paragraph #P20's msl-arrayptr port added; both kept, this paragraph first. Assisted-by: Claude Opus 5.5 Co-Authored-By: Claude Opus 5.5 <[email protected]> (cherry picked from commit 95fa06d4e6526601bd2e1bc88c2f8f5720855350)The segmented single storage array's case (plan 4.3 item 5) had no negative: the check always cut its own tree. With --source <B2 archive> it fails as required ("moltenvk.segmented_single want=set got=push"). Conflict with #P28 (read_write textures; not with main): the docstring keeps #P28's paragraph with this usage line, and main() cuts from --source's vk_draw.cpp with #P28's read_write count (as in the macOS integration, Integration 17). Assisted-by: Claude Opus 5.5 Co-Authored-By: Claude Opus 5.5 <[email protected]> (cherry picked from commit 5824f6828e21cf810dcceaa6be425a9224b15994)plaine37c097da2plaina997b3629aKosmicKrisp has no fillModeNonSolid and MoltenVK no POINT polygons, and Metal restarts an indexed strip or fan at the all-ones index whatever Vulkan's restart enable says. Before anything is fixed, count whether the game needs it (mode_census.h, Apple only; nothing built or drawn changes): - the graphics pipelines built, where st_pipelines counts them, and the draws recorded, where st_draws counts them, by NVN's polygon mode, with the first 8 LINE and POINT pipelines (program, pipeline key, topology) and programs named; - the indexed strip and fan draws with restart off; a direct one's indices are read whole, a page at a time, through memory_read_as_submitted for the index type's all-ones value (0xFF, 0xFFFF, 0xFFFFFFFF). An indirect one's range is the GPU's and a range the CPU cannot read is not guessed: both are counted as not scanned. Any other draw costs restart_off_strip's compares and its polygon mode's count on the thread's own counters. Printed with the frame statistics every 120 presents ("at present N") and at close ("at close"). The two tests that compile build_graphics and draw()'s binds from the source get a stand-in for the counts. Assisted-by: Claude Opus 5.5 Co-Authored-By: Claude Opus 5.5 <[email protected]> (cherry picked from commit 1bd30dceb348909ca03c4cf12709dccbb0900f4a)DI-01 (reports/2026-11-14-drawid-direct.md) ran the direct and one-record indirect forms on MoltenVK 1.4.2: the indirect form was exact in every shape (0 stale of 3,744 draw indices), and the direct plain, indexed and quad draws read stale words, but tessellated direct draws read draw index 0 (64 of 64, with update tags in the pool) while their indirect form took about 135 MB of GPU memory and 3 ms of submit a draw. So 3f1ee3da's choice to keep them direct stands, and the error line it logged for each ("writes a direct draw's draw index to the wrong word") was wrong for them: they are now counted in the vk: direct draws line only, as fans are (0 of 64 stale direct in DI-01). drawid_direct.h and VULKAN.md give DI-01's numbers; the fixture expects an error line only for a first instance without drawIndirectFirstInstance. Assisted-by: Claude Opus 5.5 Co-Authored-By: Claude Opus 5.5 <[email protected]> (cherry picked from commit 0a6956568f871c66b08c29dffc0a1b9614dfb5de)fa6dd43a6b7260a150387260a15038d7878c9d1aFolded into #850 (fix(macos): draw-path emulation on MoltenVK)
Pull request closed