WIP: fix(macos): fans keyed apart from the other triangle topologies on MoltenVK #701
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!701
Loading…
Reference in a new issue
No description provided.
Delete branch "mooki/Longshot:mac/fan-key"
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 a triangle fan can be drawn as a triangle list, and a list or strip converted as a fan. LS keys its graphics
pipelines by topology class (lists, strips and fans share one) and sets the topology per draw; MoltenVK 1.4.2 decides whether
to convert a fan by the pipeline's static topology, the one the class's first draw built it with (
MVKCmdDraw.mm:183,:456,:731,:1052). So a fan that binds a pipeline a list built is drawn as a list, and a list or strip on a pipeline a fan built isconverted as a fan. This PR gives fans a key of their own on MoltenVK, and counts what the old key did in the game (log-only). 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 FK-01's
Barms (standalone MoltenVK process; command inreports/2026-11-14-fan-key.md§6): one pipeline per keyclass, a list or strip and a fan drawn through it in both orders, with all four draw calls
Expected: each draw covers its own topology's pixels
Actual: 96 of 288 draws wrong: the fans after a list drawn as lists, the lists and strips after a fan converted as fans
What changed
On MoltenVK's driver only,
TRIANGLE_FANis its own pipeline key class (kFanClass), so every pipeline a fan binds was built asa fan and every pipeline another triangle draw binds was not. Every other driver keys and stamps as before.
gpu/vk/fan_key.h(new)key_class();gpu/vk/vk_draw.cpppipeline_class(t)used at the three places that keyed byclass (
pipeline_key,graphics_pipeline's last-two cache,gpl_link's vertex input library). NVNPOLYGONmaps to fans.fan_key::Census;census_fan_madeatbuild_graphics's end and aftergpl_link;census_fan_drawright after the count atdraw()'s non-mesh sites;draw_fan_census_reportevery 120 presents andat close (
vk_replay.cpp,vk_main.cpp, declared invk_draw.h).vk_progcache.cpp): the wordfankon MoltenVK only, so the fan-keyed notes, prepare record andprogram cache are made once more on the Mac (old notes keyed by class would hand a fan's pipeline to lists). Other drivers: the
same stamp, byte for byte.
gpu/vk/VULKAN.md.tests/test_fan_key.py+tests/fan_key_test.cpp(new): the key over every topology and a grid of states on MoltenVK andelsewhere (elsewhere equal to the base's), the census, the stamp, the source;
tests/test_fallback_pipeline_state.pygets acensus_fan_madestand-in.Measurements
Verdict: fixed where measured: FK-01 drew every one of 288 draws right with the new key, against 96 wrong with the old one, each
as MoltenVK's source predicts, in all four draw calls. No game run with it yet: the census line will say how often the game hit it.
Baseline: main does not start on macOS. FK-01 compares the old key (
B, one pipeline per class) with the new one (A) in onestandalone MoltenVK 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: FK-01: list/strip/fan pairs in both orders, controls,
vkCmdDraw,vkCmdDrawIndexed,vkCmdDrawIndirect,vkCmdDrawIndexedIndirect, 3 trialsMethod: FK-01
reports/FanKey-files/fk01/runs-fk01-arm64/result.json(3f53d054…),probe.log(b7cdaabe…), released once on arm64How to reproduce: FK-01's
runcommand (reports/2026-11-14-fan-key.md§6)6c9edffe(the class key, B 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 #680 and what it carries):tests/test_fan_key.pywith/usr/bin/clang++andclang++ -arch x86_64(Rosetta): pass, 58,329 checks, 0 failures; with thetest copied onto #680's head it fails (
pipeline_class (vk_draw.cpp) is missing);tests/test_fallback_pipeline_state.pybothcompilers: pass (
reports/PubWave-files/PubLate3/proof-01/).gpu/vk/vk_draw.cpp,vk_main.cpp,vk_progcache.cpp,vk_replay.cppcompiled whole for arm64 and x86_64 with main's rendererflags (#476's, #492's and #490's macOS compile pieces merged in a scratch tree only): 8 objects, exit 0
(
reports/PubWave-files/PubLate4/compile/fan-key/).unifdef -U__APPLE__):vk_draw.cpp,vk_draw.h,vk_main.cpp,vk_progcache.cpp,vk_replay.cppdiffer andfan_key.his included (the gate is the driver id); off MoltenVK the key, the stamp and every census call are as before(
reports/PubWave-files/PubLate4/offapple-fan-key.txt).Not tested: a game start with this stack (main does not start on macOS by itself); not built on Windows or Linux; its test
request covers it.
Must be tested:
vk: fans on MoltenVK (…)lines with0 drawn wrong (by hand: the parent's game run).
592cd8d3, it carries #680, #679, #677, #663 and what they carry) and main6c9edffethe same way and start each to the title, expecting novk: fans on MoltenVKline, no new program-cache rebuild, andevery other line as main's (by hand: test request T-XXXX).
Risks
What could break: on MoltenVK only: one more graphics pipeline for each program and state drawn both as a fan and as another
triangle topology (its Metal pipeline the same as its sibling's); the first start after it remakes the Mac's notes, prepare record and
program cache once (the stamp word).
Turning it off: revert the PR (no switch).
Compatibility: Nothing changes for players outside macOS. On macOS the program cache stamp gains
fank(one retranslation).No
MKWVK_API_VERSIONchange.Reviewer notes
9c2bd272..592cd8d3(2 commits):cherry-pick -xofmacos/fan-keyb52fe890(the census) and1e8d1ba3(the key) onto#680; conflicts only where #680's lines sit beside this topic's (
vk_draw.cpp#252's Intel gate kept;vk_progcache.cpptheinclude), resolved by keeping both (
reports/PubWave-files/PubLate3/pick-fan-key-*-lines.txt).drawid_direct.h's fan exclusion is exact only with this key) and what it carries (#679, #677, #663,#662, #658, #649, #648, #647, #645, #644, #643, #491).
tests/test_perf_lines.pylets thiscensus line sit between a draw's count and the perf hook (Integration 36's 34438870); with #706, the indirect quads'
count gets
census_fan_drawandtests/test_fan_key.pycounts three non-mesh sites of six (Integration 36's 74da414a). Both arein the wave merge
39da60e1.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)592cd8d36029afee2a9629afee2a96cd1ce0b214Folded into #850 (fix(macos): draw-path emulation on MoltenVK)
Pull request closed