WIP: fix(macos): quads provoke the same corners where the first vertex provokes #706
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!706
Loading…
Reference in a new issue
No description provided.
Delete branch "mooki/Longshot:mac/indirect-quads"
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 both triangles of a quad take their flat values from the quad's first corner. A quad is drawn as
{c0, c1, c2}and{c0, c2, c3}; LS's pipelines provoke the last vertex where the device hasVK_EXT_provoking_vertex, so elsewhere the halves take c2and c3. MoltenVK 1.4.2 offers the extension only with Metal's private API (
MVKDevice.mm:3579), so on the Mac the first vertexprovokes and both halves took c0. This PR rotates each triangle on Apple, where the first vertex provokes, so the same corners provoke
as on every other device, for direct quad draws and #705' indirect ones alike. 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 QD-01's
prov_unrotatedstep (standalone MoltenVK process; command inreports/2026-11-14-indirect-quads.md§6):quads coloured flat by the provoking vertex's id
Expected: the halves show c2 and c3 (NVN's and every other device's corners)
Actual: both halves show c0
What changed
On Apple only, where the device provokes the first vertex (
quad_draws::rotated(apple, provokes_last)), each triangle is rotatedwith its winding and edges kept:
{c2, c0, c1}and{c3, c0, c2}. The same triangles, the same split along c0-c2, the sameprovoking corners as every other device. Elsewhere nothing changes.
gpu/vk/quad_draws.h: the rotated form,rotated(),corners(),provoking().gpu/vk/vk_draw.cpp:quad_rotated()(#if defined(__APPLE__)), the direct path's triangles and the indirect path's pattern andpush word from
quad_draws::corners(quad_rotated()); the quad report names the form in use.gpu/vk/VULKAN.md. Tests:tests/quad_draws_test.cppchecks that the rotated triangles are rotations of the drawn ones, keepthe winding and provoke the same corners, and runs every indirect case in both forms;
tests/test_quad_draws.pychecks the gate.Measurements
Verdict: fixed where measured: in QD-01
prov_rotatedread c2 and c3 in both arms (LS's indirect path and the direct path'squads), and
prov_unrotatedread c0 for both halves, as predicted.Baseline: main does not start on macOS. QD-01's provoking steps compare the rotated and unrotated corners in one standalone 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: QD-01's
prov_rotatedandprov_unrotatedsteps (quads coloured flat by the provoking vertex's id)Method: QD-01
reports/IndirectQuads-files/qd01/runs-qd01-arm64/result.json(271fb491…), released once on arm64How to reproduce: QD-01's
runcommand (reports/2026-11-14-indirect-quads.md§6)6c9edffe(unrotated corners)Flags and settings
None: no switch. On Apple, on a device without
VK_EXT_provoking_vertex's last vertex, only.Testing
Tested: on macOS 26.7.1, Apple M4 Max, at this head (main
6c9edffeplus #705, #679 and what it carries):tests/test_quad_draws.pywith/usr/bin/clang++andclang++ -arch x86_64(Rosetta): pass, 6,099 indirect draws, 91,542 records,204,237 checks, 0 failures; with the head's tests on #705's head it fails (
bool quad_rotated() is missing);test_seg_gathers(--arch arm64) andtest_draw_binds: pass (reports/PubWave-files/PubLate4/proof-01/iq-*,proof-02/iq-*).gpu/vk/vk_draw.cppcompiled whole for arm64 and x86_64 with main's renderer flags (#476's, #492's and #490's macOS compile piecesmerged in a scratch tree only): 2 objects, exit 0 (
reports/PubWave-files/PubLate4/compile/indirect-quads/).vk_draw.cppandquad_draws.hdiffer (the forms and the gate are compiled everywhere), butquad_rotated()isquad_draws::rotated(false, …), false, so the corners are main's (reports/PubWave-files/PubLate4/offapple-indirect-quads.txt).ce38f193, scratch), so it is the code QD-01 ran.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: quad draws (…)line to name{2,0,1, 3,0,2} (the device provokes the first vertex)once a quad is drawn (by hand: the parent's game run).978079d4, it carries #705, #679 and what it carries) and#705's head the same way, expecting the quad line to end
triangles {0,1,2, 0,2,3}(a field this PR adds) andevery other line the same (by hand: test request T-XXXX).
Risks
What could break: on Apple only: the order of each quad triangle's corners (its edges, winding and split unchanged). NVN's own
quad rule (the fourth corner for both halves) is not what any LS path draws, here or elsewhere (
reports/2026-11-14-indirect-quads.md§3).
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
c6e0c37f..978079d4(1 commit):cherry-pick -xofmacos/indirect-quads08cd778a, which in the topic came before theindirect path; here it follows #705 and also rotates that path's corners, as the topic's head did (its message says
so).
quad_draws.h, the indirect path) and through it #679 and #663.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)Quads have no Vulkan topology. draw() made a direct quad draw's triangles on the CPU and left every indirect quad draw out (SKIP_HERE), with or without a count buffer, on every driver. Now quad_rewrite, at draw()'s pre-pass point (after SegBind's gathers, outside the pass, between full barriers), rewrites the draw's records on the GPU (shaders/quad_draws.comp) into indexed triangle-list records, and one vkCmdDrawIndexedIndirect of maxDrawCount records draws them, each record with its own draw index, instances and first instance: - not indexed: {6 floor(V/4), I, 0, firstVertex, firstInstance} over a device-wide pattern (quad q's corner k is 4q + corner), so each vertex index is the direct path's. The pattern is sized from the quads the program's per-vertex streams can address and grown on demand; a record asking more draws nothing, is counted on the GPU, and grows it for the next draw. - indexed: a dispatch notes the quads each phase of firstIndex uses, records {6 quads, I, slot + 6 (f >> 2), vertexOffset, firstInstance} are written, and indirect dispatches make the triangles of those quads only, from the draw's indices, into a device-wide buffer of min(maxDrawCount, 4) slots of 6 ceil(K/4) indices (K: the index extent LS already admits). 8-bit indices become 16-bit triangles, 16/32-bit ones 32-bit, as the direct path's. Quads past the extent are counted on the GPU. - with a count: records past the count the GPU reads are written zero and never read, as in the count-draw rewrite; no driver count draw is needed. The triangles are quad_draws.h's ({v0, v1, v2, v0, v2, v3} a quad), the direct path's own: the direct path now makes its triangles with the same quad_draws::triangles. Primitive restart is not applied to quads, as on the direct path. The device-wide buffers are retired through vk_retirement_value() when grown. A quad draw's 8-bit indices no longer need VK_EXT_index_type_uint8 (neither path binds them). The quad report gains the indirect draws, their maxDrawCount, pass breaks, dispatches, those left out and the GPU census. draw_demands and SegBind's gathers cover indirect quad records, count and index extent. Tests: tests/quad_draws_test.cpp runs quad_draws.h and the shader's own code (quad_draws_record.glsl) on the CPU, each record drawing what the direct path draws; five shader mutants fail it. test_quad_draws.py checks the push block, the SPIR-V header and the renderer's order; seg_gathers_test case 8c the gathered records and extent; test_draw_binds counts the new bind site. (Every-platform PR fix/indirect-quads, picked onto the count-draw rewrite: the Mac's quad corners, 08cd778a, are not in this commit. quad_draws.h is made here with the one form LS has always drawn; the rotation follows on top in mac/indirect-quads. Without it, quad_draws_test.cpp runs the last-vertex corners only: 3,051 draws, 45,774 records, 102,131 checks.) Assisted-by: Claude Opus 5.5 Co-Authored-By: Claude Opus 5.5 <[email protected]> (cherry picked from commit de9f171626dda257ec72ca9390e8d5f0d83cc838)quad_draws::expansion bounded a piece by maxComputeWorkGroupCount[0] x [1] x 64 threads as one 64-bit product. MoltenVK reports 2^30 for both, so the product (2^66) wrapped to 0, no piece fitted, and every indexed indirect quad draw would have been left out there ("its index extent holds no quad, or no piece of it fits a binding"). The bound now saturates. Found by QD-01's plan (--plan-only with MoltenVK 1.4.2's limits). The CPU fixture now uses MoltenVK's limits by default and checks the saturation; from the previous commit's header it fails (1,801 failures), with this one it passes. Assisted-by: Claude Opus 5.5 Co-Authored-By: Claude Opus 5.5 <[email protected]> (cherry picked from commit fc8dd951f05790e6c429e84adaef21091dc8dc68)A quad is drawn as two triangles, {v0, v1, v2} and {v0, v2, v3}. LS's pipelines provoke the last vertex where the device has VK_EXT_provoking_vertex (build_graphics), so flat inputs come from v2 and v3. MoltenVK 1.4.2 offers the extension only with Metal's private API (MVKDevice.mm:3579), so on the Mac the first vertex provokes and both triangles took v0's flat values. On Apple, where the device provokes the first vertex, each triangle is now rotated with its winding and edges kept: {v2, v0, v1} and {v3, v0, v2}. The same triangles, the same split, the same provoking corners as on every other device. Other platforms draw exactly what they drew. quad_draws.h holds the two forms; the direct path's triangles and the indirect path's pattern and push word (fix/indirect-quads) both take them from quad_draws::corners(quad_rotated()). tests/quad_draws_test.cpp checks that the rotated triangles are rotations of the drawn ones, keep the winding and provoke the same corners, and runs every indirect case in both forms. The quad report names the form in use. (Picked onto fix/indirect-quads: in the topic this commit came before the indirect path; here it follows it and also rotates that path's corners, as the topic's head did.) Assisted-by: Claude Opus 5.5 Co-Authored-By: Claude Opus 5.5 <[email protected]> (cherry picked from commit 08cd778a7750948e10c496b62b972da322206b04)978079d47801700d975801700d97586e6bc8d921Folded into #851 (fix(macos): the Mac halves of indirect quads and mesh count draws)
Pull request closed