mirror of
https://github.com/MobileGL-Dev/MobileGL
synced 2026-09-08 04:08:32 +09:00
5f8e8db1b96d1663bfe67155efb5a0c0c370b6dc
2663
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 swung0x48
|
5f8e8db1b9 |
[Test] (Retrace): make a retrace under MOBILEGL_PIPE_VERIFY prove it armed, and give the lane a label
- A retrace that exports MOBILEGL_PIPE_VERIFY=1 at a library configured without
-DMOBILEGL_PIPE_VERIFY=ON is a no-op: the frames still match their goldens and the
case reports green having compared nothing. run_trace_case.cmake now demands the
evidence whenever the variable is set to anything but 0/false - mobilegl.log must
exist, must carry "MGPipe: verify armed", and must carry neither
Fatal{PipeVerifyDiffer nor Fatal{UnmigratedPipeInput. With the variable unset the
script is byte-for-byte the old one.
- The Fatal scan is not redundant with the replay's exit status:
MOBILEGL_PIPE_VERIFY_FATAL=0 is the supported triage configuration, and there a
divergence is logged and counted rather than aborted, so the run would finish 0
with its own report sitting unread in the log.
- LABELS retrace on both registrations: the lane was selectable only by regex, so
`ctest -L retrace --no-tests=error` - the spelling that reds a lane which
registered nothing - could not be written at all.
- "verify": true on eight cases (the five 180s cases plus minecraft-1.21.4-in-world,
minecraft-1.21.4-fabric-sodium-in-world and improved-transparency-minecraft-26.3),
and --format github-verify-matrix over that subset: 16 entries, against the full
lane's 77. The verify build compares at every verb boundary and again at every
accessor read, which the design budgets at 5-10x, so the per-push job runs the
subset and the full sweep is a phase-exit / workflow_dispatch run.
- The flag is validated in the manifest loader, not at the matrix, so a non-boolean
or a verify case excluded from CI is a loud error in every consumer instead of a
subset that is quietly one case short.
|
||
|
swung0x48
|
bdf05514c3 |
[Test] (Pipe): the integration-verify lanes and their two always-on negative controls
- ARCHITECTURE.md 13.2-(2) asks for a third CI mode, and a third mode whose only evidence is "ctest was green" proves nothing: MOBILEGL_PIPE_VERIFY=1 against a library that never compiled the comparator in is a silent no-op that looks exactly like a clean pass. Six registrations, all under if (MOBILEGL_PIPE_VERIFY) and all labelled integration-verify, make both halves falsifiable - a mis-configured build registers nothing and --no-tests=error reds the lane, and PipeVerifyArmingScenario.Armed fails a lane whose library never printed its arming line. - PipeVerifyArmingScenario.CorruptedFieldIsReported is negative control A (G4): its lane pins MOBILEGL_PIPE_VERIFY_CORRUPT=GetRenderStateParameters with MOBILEGL_PIPE_VERIFY_FATAL=0 so the process survives its own divergence and can read the report back; the CI step that exports the same knob against the ambient lane, where FATAL keeps its default, asserts the other half - the abort. - PoisonOmissionScenario is negative control B (G5), in two cases that cannot share a process because the knob is process-wide: the omitted (verb, field) pair must abort the glGenerateMipmap and NOT the draw before it, and the same sequence with the knob unset must complete with no Fatal at all. - The sequence runs in a fork()+execve() of this same binary rather than a bare fork(): the fixture has already brought a context up, and a bare fork of a process holding a live Vulkan device inherits the driver's mutexes with no threads to release them - measured here as a 120s wedge on DirectVulkan against a clean pass on DirectGLES. The child gets its own MOBILEGL_LOG_FILE_PATH because the library opens its log with fopen(path, "w") and would otherwise truncate the file the parent is about to read. - No ambient Verify. entry names MOBILEGL_PIPE_VERIFY_CORRUPT or MOBILEGL_PIPE_POISON_OMIT in its ENVIRONMENT property, because a property entry overrides the job environment for the names it lists: the two CI negative-control steps export those knobs into the job environment and must reach the processes. Every list appends MGL_ITEST_COMMON_ENV / MGL_ITEST_VULKAN_ENV for the same reason, so the vendor and ICD pinning survives. |
||
|
swung0x48
|
bf8b39a867 |
[Refactor] (Magma): route every frontend read through MGB_CTX - 164 arrow sites sed'd, 49 non-arrow lines converted (43 asserts keep their meaning as MGB_CTX_LIVE); pull build byte-identical
- Every MG_State::pGLContext-> in the seven DirectVulkan TUs becomes MGB_CTX-> (DirectVulkan.cpp 12, BackendObject_DirectVulkan.cpp 2, UniformManager.cpp 14, VkClearManager.cpp 1, VkRenderPassManager.cpp 3, VkTextureManager.cpp 2, VulkanRenderer.cpp 130 occurrences on 127 lines); each TU includes <MG_Pipe/PipeInputsSwitch.h> right after its MG_State/GLState/Core.h include (VkRenderPassManager.cpp after its include block, it never included Core.h). - The 43 MOBILEGL_ASSERT(pGLContext) / (pGLContext != nullptr) lines become MOBILEGL_ASSERT(MGB_CTX_LIVE, ...): identical in every INFO build (the macro is empty there) and still a null-context assert in a DEBUG build. - The six code-bearing guards are like-for-like pointer tests in the pull arm: if (MGB_CTX_LIVE) at the two InvalidateCompileEnv sites, MGB_CTX_LIVE in the XFB query counter condition and the ternary that snapshots the paused-primitive counter, !MGB_CTX_LIVE in BeginXfbCaptureForDraw, MGB_CTX_LIVE && in the provoking-vertex resolve. No semantic rewrite: under push MGB_CTX_LIVE is simply "a context exists", the real meaning of these guards is P2's business. - VertexInputStateFactory.h's comment stops naming pGLContext so purity gate C (grep -rc pGLContext MobileGL/MG_Backend/DirectVulkan) reads 0 in every file. - The 12 SyncPersistentMappedRange and 3 SyncGpuWrites sites of the D10 table are untouched; PipeStats AddCalls literals unchanged. - Proof on the pull build (Release/INFO, LTO off, vs feat/disaggregated@087685d1): symbol_report --threshold 0 -> 27799 symbols, 0 added / 0 removed / 0 resized / 0 renamed, .text 10792579 -> 10792579 (+0); nm --defined-only name set identical; ctest -N name set identical (2334); unit 1466/1466; DirectVulkan integration lane 427/427 on lavapipe (two ArmedWhenTheEnvironmentPinsItOn entries flake under -j 4 exactly as on the baseline and pass serially). The push build compiles and links. |
||
|
swung0x48
|
d4504e30f8 |
[Refactor] (Espryt): route every frontend read through MGB_CTX - 113 arrow sites sed'd, 9 non-arrow lines converted (the fb-slot cache keys on MGB_CTX_IDENTITY); pull build byte-identical
- P1 package B (BRIEF-P1 C.2): the four DirectGLES TUs include <MG_Pipe/PipeInputsSwitch.h> right after
their MG_State/GLState/Core.h include (Managers.cpp after its Managers.h include) and spell every
frontend read MGB_CTX->Accessor(...). In the pull build MGB_CTX is MG_State::pGLContext, so the
code is the tree before this commit token for token; in the push build it is &gPipeInputs, the block
the frontend fills at every verb boundary.
- 113 arrow occurrences on 113 lines went through the mechanical sed (DirectGLES.cpp 91, Managers.cpp 15,
MultiDraw.cpp 5, Utils.cpp 2). The 9 non-arrow lines follow D9: Managers.cpp's five bare/compound
guards and three ternary conditions become MGB_CTX_LIVE (UniquePtr::operator bool spelled out, so the
pull build does not move); DirectGLES.cpp's GetFramebufferBindingSlotFast keys its static cache on
MGB_CTX_IDENTITY (a const void* compare) instead of pGLContext.get().
- The cache refill loop dereferences MGB_CTX once into a local reference and reads the slots through it,
instead of &MGB_CTX->GetFramebufferBindingSlot(i) per iteration as D9 spells it: a store into
g_fbSlotCache (a pointer) may alias the unique_ptr's own pointer under clang's TBAA, so the per-iteration
spelling re-reads pGLContext inside the loop and grows the three functions the loop is inlined into
(SyncCurrentProgram +16, ForceBindCurrentFBO +9, BlitNamedFramebuffer +1; .text +32). Hoisting the
dereference restores the single load and a zero .text delta.
- grep -rc pGLContext MobileGL/MG_Backend/DirectGLES reports 0 in every file; symbol_report --threshold 0
against the
|
||
|
swung0x48
|
9087f13308 |
[Fix] (Pipe): let a readback fill the transform-feedback state its emulation reads
- the depth/stencil read emulation opens a ScopedEmulationDrawState that pauses an active
capture around its own draw, so ReadPixels/GetTexImage read IsTransformFeedbackActive and
IsTransformFeedbackPaused; without the two kReadback rows every emulated readback aborts
with Fatal{UnmigratedPipeInput, "IsTransformFeedbackActive@ReadPixels"} once the Espryt
sites are converted
|
||
|
swung0x48
|
44ffafb2dc |
[Test] (Pipe): pin the pre-fill window - every stamp of 0 is stale at serial 0, a read before any fill aborts naming "<none>", and the FATAL=0 child leaves through std::exit so the teardown summary is observed
- PipeCatalogue.PipeInputFieldsStartUnfilled asserted "unfilled" only after setting the serial to 1 by hand, stepping around the serial-0 window; it now asserts every field stale on a value-initialised state first (sticky fields included).
- PipeInputsTest.ReadingBeforeAnyFillAbortsNamingNoVerb: a live context, no fill, gPipeInputs.GetLineWidth() in a forked child; the parent expects SIGABRT, exactly Fatal{UnmigratedPipeInput, "GetLineWidth@<none>"}, no PipeVerifyDiffer and no arming line. In a verify build the child sets Features.PipeVerify first, the lane's shape.
- VerifyFatalOffLogsTheDivergenceAndContinues: the child _exit(0)ed, so VerifyState's destructor never ran and the "verify summary" line was covered only by a lane run; std::exit(0) runs it, and the parent now asserts "2 divergence(s) survived MOBILEGL_PIPE_VERIFY_FATAL=0".
|
||
|
swung0x48
|
12b57055b7 |
[Docs] (Pipe): record why the forwarders carry no poison check, the InHook re-entry guard, and the Index slot no FillPoints.def row can fill
- The seven F-class forwarders are the declared exception to D4's "every accessor body": a forward is a live call, not a stored value, and InvalidateCompileEnv is reached from backend initialisation before any verb has filled, where a check would be Fatal{...@<none>} on every start. Their sticky stamp is consulted by no accessor; the tests pin it through MGPipeInputFieldIsFresh directly. Written down so P2's tracker does not "fix" the missing check.
- MGPipeVerifyReadHook: InHook is what keeps a GetProgramForDraw-triggered backend re-entry from recursing into a second hook, and the whole-field re-read is a per-read cost to keep in mind when reading the verify lane's wall time.
- GetBufferBindingSlot(Index) is polymorphic on GLContext (the bound VAO's element-buffer slot) and null in the block by design: a push Fatal there is not a missing row. No backend reads it today.
|
||
|
swung0x48
|
d0ff647581 |
[Fix] (Pipe): re-arm the verify comparator when any of its three knobs changes, not only when PipeVerify does
- ArmVerify latched PipeVerifyFatal and PipeVerifyCorrupt at the moment PipeVerify changed; a later change to either without a PipeVerify toggle was not seen, so 878db2c4's "re-parse when their value changes" held for one knob of three. - The latch now keys on all three Features values. A lane loads Features before its first fill, so it still arms once per process; cost is two Bool compares and one String compare per fill, verify builds only. |
||
|
swung0x48
|
30d72c5b4e |
[Fix] (Pipe): refuse a stamp of 0 as fresh on both branches - a read before the first fill is Fatal{UnmigratedPipeInput, "<Field>@<none>"}, not default storage
- MGPipeInputFieldIsFresh (gen_pipe.py gen_filled, emitted into PipeFilled.inc) compared FilledGen == CurrentVerbSerial for a non-sticky field; before the first MGPipeFillForVerb both are 0, so 55 of the 63 fields read as fresh and served their default-constructed storage silently, with no log line and no abort. Only the four raw-pointer O-class accessors tripped, through their null-base checks.
- D6 says the serial starts at 1 so that FilledGen == 0 means never filled, and names the window "<Field>@<none>"; the predicate now refuses a stamp of 0 before consulting the sticky branch or the serial, so the window is the poison's case as documented. This is the window E's risk table expects the verify lane to find (an init-time read, the first link, anything reached from eglMakeCurrent).
- Reproduced with the reviewer's pre-fill program (a live context with line width 7, no fill, gPipeInputs.GetLineWidth()): stored=0, rc=0 before; rc=134 with exactly Fatal{UnmigratedPipeInput, "GetLineWidth@<none>"} after, with and without Features.PipeVerify set first.
- The compare-at-read hook arms at the first fill and cannot see this window either; a static_assert next to it pins that a verify build always carries the poison, which is what covers the reads before arming.
|
||
|
swung0x48
|
d9f4698d98 |
[Test] (Pipe): give every PipeInputsTest process its own log file, and cover the compare-at-read arm, the three knob parsers and VERIFY_FATAL=0 with forked children
- gtest_discover_tests runs each case as its own process; under ctest -j the five shared one fixed log path, each main() unlinked it and each fork parent re-read it by path, so a sibling's Fatal line or unlink landed in another case's assertion (red 40/40 at -j 8, for a reason unrelated to the poison). The name now carries the pid and the file is removed on the way out; a forked child inherits the path on purpose.
- MutatedFieldIsNamedAtRead: the first falsifier of MGPipeVerifyReadHook - the child arms verify, fills DrawArrays, reads GetLineWidth (completes), mutates the live context's line width and reads again, and the parent expects SIGABRT with Fatal{PipeVerifyDiffer, "GetLineWidth@DrawArrays", verb=<serial>, where=read} and no where=entry.
- PoisonOmitKnobArmsTheOmission / BadPoisonOmitKnobIsFatalNamingTheKnob / VerifyCorruptKnobNamesTheFieldAtEntry / BadVerifyCorruptKnobIsFatalNamingTheKnob / VerifyFatalOffLogsTheDivergenceAndContinues: the knob parsers through MG_Config::Features, their arming lines, the exact Fatal{PipeVerifyBadKnob} text, and a FATAL=0 run that logs two divergences at consecutive serials and exits 0.
- OmittingOneFieldForOneVerbLeavesExactlyThatFieldStale now loops every field: fresh iff in kTextureOp's mask and not the omitted one, so the name is true.
- The fork/waitpid/log-delta shape is one RunInChild helper; every case is still a visible skip where its switch is off.
|
||
|
swung0x48
|
878db2c405 |
[Fix] (Pipe): re-parse the POISON_OMIT and VERIFY knobs when their Features value changes, not once per process
- The two parsers (ParsePoisonOmissionKnob, ArmVerify) latched on the first fill, so the only way to reach them was a lane that loads MG_Config::Features before any fill; no unit test could exercise the parse, the arming lines or Fatal{PipeVerifyBadKnob}.
- The latch now keys on the value: a lane still parses once (Features is loaded before the first fill), while a forked test child that sets Features after its parent filled gets its own parse. An empty omission value never clears an omission armed through MGPipeSetPoisonOmission.
- Re-arming resets the CORRUPT field so a stale corruption cannot outlive the knob that named it.
|
||
|
swung0x48
|
77ecde1524 |
[Fix] (Pipe): refuse an eighth sticky row at compile time - the forwarded count equals the generated sticky count
- kMGPipeForwardedFieldCount = 7 was a hand-written twin of the generated kMGPipeInputStickyFieldCount, tied only by PipeCatalogue.StickyFieldsAreExactlyTheSeven; a static_assert in the header makes a PipeFields.def sticky row without a forwarder a build error instead of a test failure. |
||
|
swung0x48
|
510ecd9293 |
[Fix] (Impl): move the DeleteSync fill of the orphan sweep inside its null-entry guard
- D7 says a verb whose table entry is null never bumps the serial; the sweep's fill sat before `if (backendDeleteSync && syncObject->backendHandle)`, so a backend without DeleteSync, or a sync without a backend handle, bumped once per orphan with nothing reading the fill. - The sibling sweep in GL_Query.cpp (DeleteBackendQuery) already fills inside its guard; this makes the two the same shape and leaves the declared list of guarded-expression sites at nine. - Pull build unchanged: MGP_FILL is ((void)0) there. |
||
|
swung0x48
|
440d3c5253 |
[Test] (Pipe): pin the P1 poison and verify shapes - 63 fields, 69 verbs, the seven sticky fields, an omitted GenerateMipmap field leaves exactly that field stale, a corrupted snapshot names its field, reading an unfilled field aborts with SIGABRT
- PipeCatalogueTest (header-only, every build): VerbTableIsTheFunctionTable (69 verbs, 9 non-empty classes, the seven sticky bits in every class mask, D7's edges), StickyFieldsAreExactlyTheSeven, FloatVectorsCompareBitwise (a NaN FloatVec4 equals itself, -0.0f differs from 0.0f, a differing BlendStates[3] names RenderState then BlendStates), SixValueStructsHaveFieldLists (69 payloads, PixelStoreParameters and MGHostSpan compared member-wise with Pad0 ignored). - PipeInputsTest, a new unit target linking the static library with its own main() that points MOBILEGL_LOG_FILE_PATH at a temp file: a fake GLContext, the real filler and accessors. OmittingOneFieldForOneVerbLeavesExactlyThatFieldStale (G5 layer 1), ReadingAnOmittedFieldAbortsNamingTheVerb (G5 layer 2: fork, the child reads the omitted field after a draw and a sibling read that must not abort, the parent expects SIGABRT and the exact Fatal line and no @DrawArrays), ReadingAFilledFieldCompletes (the sibling without the omission: _exit(0), no Fatal), CorruptedSnapshotFieldIsNamedWithItsSerial (G4 at block level: clean compare true, a corrupted GetRenderStateParameters is named, its stamp is the fill serial, a corruption outside the mask is not seen), EveryVerbFillsItsClassAndNothingElse (after each of the 69 fills a field is fresh iff its class bit is set). - Every PipeInputsTest case is a visible GTEST_SKIP in a pull build and the poison/verify cases skip in a push build without them; the ctest name set is the same in all three configurations (additions only: 9 names). |
||
|
swung0x48
|
83b16561c2 |
[Feat] (Pipe): give the six value structs G4 field lists and assert the lists cover their members - the memcmp fallback is now a compile error, floats in vector types compare bitwise
- PipeFields.def gains MGP_FIELDS_RenderStateParameters (65 members), PixelStoreParameters (8), PerBufferBlendState (7), StencilFaceState (7), DynamicBackendParameters (85) and MGHostSpan (Ptr, Seg, Size, Offset), all appended to MGP_VERIFY_PAYLOAD_LIST: kMGPipeVerifiedPayloadCount 63 -> 69, and ResidualValueBlock / MGPPixelPackState / MGPCaps are now compared field by field all the way down.
- gen_verify: MEMCMP_FALLBACK_TYPES is empty and the generic MGPipeFieldEqual's last branch is static_assert(sizeof(T) == 0) - a struct without a field list is a compile error, not a padding false positive; Array<T, N> gets an element-wise overload, and a VecBase-derived vector (FloatVec4, IntVec4, BoolVec4...) is detected by a probe and compared bitwise over its data, because VecBase::operator== is IEEE == and a derived-to-base overload would lose resolution to the exact-match generic template.
- check_field_lists_cover_struct_members(): for every payload in MGP_VERIFY_PAYLOAD_LIST, parse `struct <Name> {` out of MGPipeTypes.h / MGPipeValueTypes.h / MGPipeHostSpan.h / BackendObject.h (comments and strings masked, statics, functions, nested types and Pad<n> members excluded) and refuse a member without an F(...) or an F(...) that is not a member; runs in both modes, so it is a pipe-gates gate.
- scan_live_accessors(): every MGB_CTX-> / pGLContext-> read under MG_Backend must have a Coverage.def row (rows nobody reads are printed: today only the dead GetBoundTransformFeedbackName).
- --self-test: six negative controls (struct member without F, F without member, payload without struct, verb missing from FillPoints.def, verb outside GLFunctionsTable, field row naming a non-accessor) that must each trip, plus a positive control; zero trips is itself an error.
|
||
|
swung0x48
|
275dd3edb4 |
[Feat] (Pipe): the MOBILEGL_PIPE_VERIFY shadow comparator - a per-verb entry compare over the fill set and a compare-at-read in every accessor, first differing field and verb serial, fatal by default
- Entry compare: at the end of MGPipeFillForVerb a file-static second PipeInputs is filled by SnapshotFromGLContext (the branch that survives P13) over the same class mask, MOBILEGL_PIPE_VERIFY_CORRUPT perturbs one field of that snapshot arm, and MGPipeVerifyInputs compares every field in the mask through MGPipeInputsFieldEqual (V by G4's MGPipeFieldEqual, O by identity, F equal by definition). Both arms come from the same context at the same instant, so this arm is tautological until P2 - the CORRUPT knob keeps it falsifiable.
- Compare-at-read: MGP_INPUT_VERIFY_READ now calls MGPipeVerifyReadHook(*this, field, i0, i1), which re-reads the whole field from the live context into a scratch block and compares it against the stored value on the live block only - a superset of "the same indices"; the indices decorate the report. This is the arm that is real in P1.
- Reporting per D8: MGLOG_F("MGPipe: Fatal{PipeVerifyDiffer, \"Field@Verb\", verb=<serial>, where=entry|read}") then abort unless MOBILEGL_PIPE_VERIFY_FATAL=0, which counts and summarises at teardown with MGLOG_E; arming logs "MGPipe: verify armed - 63 fields, 69 verbs, fatal=N" once, the knobs acknowledge themselves, an unknown field name is Fatal{PipeVerifyBadKnob}; a push build without the comparator answers MOBILEGL_PIPE_VERIFY=1 with one MGLOG_W_ONCE.
- MGPipeVerifyInputs carries default visibility so the retrace-verify job's nm -D probe can prove the verify library was the one swapped in; pointer corruption flips low bits instead of nulling (a null pointer already null was invisible to the compare), a SharedPtr becomes an aliasing pointer with no control block; CurrentVertexAttributeValue gets its own bitwise equality.
|
||
|
swung0x48
|
a196ada4c1 |
[Feat] (Impl): call MGP_FILL before every GLFunctionsTable entry - 83 statements over 69 verbs, a no-op in the pull build
- Every call through gBackendFunctionsTable.GL in the seven MG_Impl TUs (Drawing 36, Framebuffer 11, Texture 8, Getter 3, Program 1, Query 18, Sync 6) is preceded by MGP_FILL(<Verb>); placed after every early return the call sits behind - the conditional-render check, the null-entry guards, the loop bodies - so a verb whose entry is null on this backend never bumps the serial. - Seven calls sit on a continuation line of a guarded expression (GetQueryResult64 x2, BeginXfbPrimitivesQuery, BeginTimeElapsedQuery, QueryCounterTimestamp, IsTimerQuerySupported, GetSyncStatus) and two more fold the null guard into the same expression (IsQueryResultAvailable, ClientWaitSync's guarded return); there the fill precedes the statement, so a null entry bumps the serial once with nothing to read it - harmless for the poison, recorded for the record. - Each TU includes <MG_Impl/Pipe/PipeFill.h> after its last MG_State/MG_Backend include; under MOBILEGL_PIPE_PUSH=OFF the macro is ((void)0) and the pull library is symbol-identical with a .text delta of zero. |
||
|
swung0x48
|
3aa4d8af1f |
[Feat] (Pipe): fill PipeInputs per verb class from GLContext and stamp per-verb generations - a read of a field the verb did not fill is Fatal{UnmigratedPipeInput}
- MGPipeFillForVerb now walks kMGPipeClassFieldMask[kMGPipeVerbClass[verb]] and copies every field in it by calling the GLContext accessor of the same name (MGPipeFillAccess::CopyField, one switch over the 56 stored fields; the seven forwarded fields copy nothing), stamping each with the new serial; the sticky seven get FilledGen = 1 on the first live fill through the same mask walk, since every class mask carries them.
- MOBILEGL_PIPE_POISON_OMIT (<Verb>:<FieldName>) is parsed once on the first fill and MGPipeSetPoisonOmission is the programmatic form for the unit tests; the omitted pair keeps its value copy and loses only its stamp, so the omission is indistinguishable from a forgotten FillPoints.def row. An unknown name is Fatal{PipeVerifyBadKnob}; a non-poison push build acknowledges the knob with one warning because no stamp exists to omit.
- PipeInputs names one friend, struct MGPipeFillAccess, instead of two friend functions, and VisitStorage gains a const overload for the comparator that follows.
|
||
|
swung0x48
|
bf86b1ede6 |
[Feat] (Pipe): land the P1 contract - PipeInputsSwitch.h with MGB_CTX, the 63-field PipeInputs block with type-identical accessors, FillPoints.def and its G5b generator, the MOBILEGL_PIPE_PUSH/VERIFY options and the three verify knobs; pull build unchanged
- MG_Pipe/PipeInputsSwitch.h is the strangler switch (ARCHITECTURE.md 9.2): MGB_CTX is the
live GLContext in the pull build and &gPipeInputs under MOBILEGL_PIPE_PUSH, so the pull
arm's pGLContext spelling stays outside MG_Backend/ and purity gate C's grep.
- MG_Backend/MGPipe/PipeInputs.h holds one struct with every accessor a backend reads (63:
the 61 Coverage.def rows plus GetBoundTransformFeedbackLifetimeId and
HasOpenTransformFeedbackSpan), each keeping its GLContext name, parameters and return
type so the site conversion is type-neutral; V fields are copied values, O fields are
SharedPtr copies or pointers into the context, the seven F fields forward to the live
context from MG_Impl/Pipe/PipeFill.cpp and are the only sticky ones (Coverage.def's
MGP_COVERAGE_STICKY_LIST argues each: argument-keyed lookups and reverse-channel writes,
never a version or generation counter).
- MG_Pipe/FillPoints.def is the verb table: one row per GLFunctionsTable function pointer in
declaration order (69), nine classes and the may-read field rows; gen_pipe.py parses the
struct and refuses a row set that is not exactly its member set, then emits
generated/PipeFillPoints.inc (verb enum, class tables, per-class field masks with the
sticky fields OR'ed in). MGP_FILL(Verb) in MG_Impl/Pipe/PipeFill.h is the fill point;
MGPipeFillForVerb only bumps the serial, records the verb and stamps the sticky fields
here - the per-class copies land in the next commit, the fill points in MG_Impl after.
- MOBILEGL_PIPE_POISON is derived once in PipeInputs.h from MOBILEGL_PIPE_PUSH and the DEBUG
level, MOBILEGL_BUILD_DISAGGREGATED or MOBILEGL_PIPE_VERIFY (the tree has no
MOBILEGL_DEBUG); under it every accessor is a read-side freshness check that aborts with
Fatal{UnmigratedPipeInput, "Field@Verb"}.
- CMake: MOBILEGL_PIPE_PUSH and MOBILEGL_PIPE_VERIFY options (VERIFY forces PUSH on), the two
new sources appended only under PUSH, the compile definitions; Config.h/ConfigLoader.cpp
gain PipeVerifyFatal / PipeVerifyCorrupt / PipePoisonOmit under #if MOBILEGL_PIPE_PUSH so
the pull build's FeaturesTable does not change size.
- Pull build proof: symbol_report.py against the
|
||
|
swung0x48
|
087685d19b |
[CI] (Purity): install libx11-dev for the include-graph-check job
- Includes.h defines VK_USE_PLATFORM_XLIB_KHR before vulkan.h, so the clang-mode probes and the negative controls need X11/Xlib.h on the runner; run 34008829271 failed on exactly that |
||
|
swung0x48
|
5635e33ffe |
[Docs] (Disaggregated): record the P0.5 landing
- README status, the ROADMAP P0.5 row (measured outcome, the 8-includer correction, the DynamicBackendParameters exception) and the ARCHITECTURE note on which header gate A asserts |
||
|
swung0x48
|
5d99ee435f |
[CI] (Purity): require both P0.5 closure probes now that the headers exist
- the ctest and the include-graph-check job pass --require-all, so a SKIP on MGPipeValueTypes.h or ProgramArtifacts.h is red from here on |
||
|
swung0x48
|
b566bf4db9 |
[Fix] (Purity, Program, Pipe): close the review minors of the three P0.5 packages
- check_include_closure.py keeps the directory of a two-token -isystem, makes --require-all fail on a missing required header even when --probe narrowed the run, and removes its temp dir at exit - ProgramArtifactsTest follows whichever STL branch the header pinned (#ifdef the size macro) instead of re-spelling the libstdc++ condition, and loses its stray executable bit - MGPipeTypes.h's debt comment says what its closure still reaches (TextureEnum.h via BackendObject.h), which is why gate A asserts MGPipeValueTypes.h and not this header |
||
|
swung0x48
|
09d2bb11b7 |
[Feat] (Program): add the reflection-archive field tables and sizeof trip wires to ProgramArtifacts.h
- One VisitFields table per type (ARCHITECTURE.md:259), a free constrained template so the moved struct bodies stay verbatim and one table serves both the const (serialize) and non-const (deserialize) direction. LinkArtifacts::program is deliberately absent: it is null for every archived instance and must never be serialized. - Trip wires: TypeFacts is pinned at 44 bytes on every ABI; the container-bearing four are pinned per standard library - libstdc++ 64-bit here (128/128/1056/88, measured on this build), the libc++ branch left inert for the integrator to pin from the NDK build, MSVC unasserted. A member added without a table entry changes the size and the assertion message sends the author to the table. - ProgramArtifactsTest counts the tables (20/14/11/57/8; const and non-const walks agree, names distinct), proves constness passes through, and records every sizeof as a ctest property so a new toolchain's numbers are readable from any `ctest -V` log. |
||
|
swung0x48
|
fee3902472 |
[Refactor] (Program): stop ProgramObject.h including SpvcSession.h - it names no spirv-cross or SPIRV-Reflect symbol
- The include line was the header's only match for spvc_*/SpvReflect*/SpvcMetadata/ SpvcSession; it only ever forwarded <spirv_reflect.h> and the session type to the eight includers, every one of which builds without it (no consumer needed a direct include added). - One less transpiler header behind ProgramObject.h, on the way to a ProgramArtifacts.h closure that stays clear of MG_Util/ShaderTranspiler/ (P0.5 gate A). |
||
|
swung0x48
|
da249f30e2 |
[Refactor] (Program): extract ProgramArtifacts.h - move TypeFacts, ResourceReflection, XfbVarying, LinkArtifacts and SpirvArtifacts to namespace scope with in-class aliases so every existing spelling compiles unchanged; pure move
- P0.5 of the MGPipe disaggregation (ROADMAP P0.5, ARCHITECTURE.md:260): the five reflection types a link produces now live in a header that includes only <Includes.h> and <set>, so a future server-side consumer can name them without dragging ShaderObject.h / SpvcSession.h / the transpiler behind it. - Struct bodies move verbatim, comments included, re-indented one level; no field is added, removed, reordered or re-typed. The two glslang-typed members (LinkArtifacts::program, uniformInitialValues) stay as they are (B.0 D5); the comment mentions of glslang types are respelled without the scope token so the include-closure gate's "glslang:: exactly twice" limit holds. - kInvalidUniformOffset moves to namespace scope (SpirvArtifacts defaults to it); ProgramObject::kInvalidUniformOffset is defined from it, so the two cannot drift. - ProgramObject keeps in-class aliases (fully qualified on the right-hand side) for all nine names, so none of the 8 includers nor any ProgramObject::X spelling changes. - ProgramArtifactsTest pins the aliases as the same types (is_same_v) and TypeFacts as a 44-byte POD; its first include is the new header, so it is also the proof that the header is self-contained. |
||
|
swung0x48
|
8566a288f8 |
[Refactor] (Pipe, State): extract MGPipeValueTypes.h - move the render-state, sampler and vertex value types and their enums out of MG_State/GLState so MG_Pipe no longer reaches RenderState.h; pure move, member order and namespaces unchanged
- ROADMAP P0.5 / ARCHITECTURE.md value-header manifest: MGPipeTypes.h embedded RenderStateParameters and PixelStoreParameters through RenderState.h, which drags FramebufferObject.h and the whole texture/renderbuffer/sampler chain into MG_Pipe; purity gate A (no MG_State/MG_Impl/MG_Backend/MG_Remote in the closure) could not be armed for anything in MG_Pipe while that include existed. - MGPipeValueTypes.h is a verbatim cut, comments included: the eleven RenderState.h enums (all of them - a split would be the maintenance trap), PixelStoreParameters, PerBufferBlendState, StencilFaceState, RenderStateParameters (member order untouched: DirectGLES' offsetof spans and PipeSpanTable.inc name the members), the six SamplerObject.h enums and SamplerParameters (BorderColorForm stays Uint8, it sets the tail padding), and VertexAttribute / VertexBufferBindingPoint / VertexAttributeVersion, which keep namespace MobileGL::MG_State::GLState with a forward-declared BufferObject so no mangled name changes. - MAX_DRAW_BUFFERS becomes inline constexpr kMGMaxDrawBuffers in namespace MobileGL and FramebufferObject::MAX_DRAW_BUFFERS is defined from it, so the eighty existing spellings keep working and the two cannot drift. No other constant is added. - The four MG_State headers become forwarders (include the value header, keep their class definitions); RenderState.cpp gains a direct FramebufferObject.h include because it spells FramebufferObject::MAX_DRAW_BUFFERS and only ever got that header transitively. No other TU lost a transitive include: the full build (Release, clang, tests + integration tests) passed without touching anything under MG_Backend, MG_Impl or MG_Util. - DynamicBackendParameters does NOT move (SizeT members and TextureTarget-taking member functions make that a type change, not a move); MGPipeTypes.h keeps its BackendObject.h include and the debt comment now says so, which is why gate A asserts MGPipeValueTypes.h rather than MGPipeTypes.h. - New trip wires in the header: trivially-copyable + exact sizeof for PixelStoreParameters/PerBufferBlendState/StencilFaceState (28), SamplerParameters (100), VertexAttributeVersion (6), RenderStateParameters (1168, standard layout, BlendStates before LogicOp, BlendStates sized by kMGMaxDrawBuffers). Their runtime twins ValueTypeLayoutsArePinned and the carrier check ResidualBlockIsExactlyItsTwoValueStructsPlusPatchTail (Pack at 1168, CapabilityBits at 1200) are added to PipeCatalogueTest without a new include. - gen_pipe.py's "field lists of their own in P0.5" comment now says P1 (the comparator needs std::array<struct> support first); PipeVerify.inc regenerated. - Verified: ctest -L unit 1460/1460 and -L integration-gpu green; ctest -N names a superset of feat/disaggregated@6672778b (two added, none lost); one definition per moved type; the -H closure of the new header contains no MG_State/MG_Backend/ MG_Impl/MG_Remote header and the header compiles alone; nm --defined-only -S against the base libMobileGL.so: 0 added / 0 removed / 0 resized, .text byte-identical, the only differing bytes are the build-id and two stamp strings. |
||
|
swung0x48
|
2318f6ae44 |
[Tooling] (Purity): add scripts/symbol_report.py - per-symbol nm/.text attribution between two libMobileGL.so builds
- P0.5's acceptance gate requires every `nm --defined-only -S` delta to be explainable per symbol, and nothing in the tree reads nm or size today. - The problem the tool exists to solve: de-nesting a type renames every mangled name that mentions it, including inside template arguments, so a raw nm diff of a pure move looks catastrophic. --strip-scope 'A::B::C::' rewrites 'A::B::C::X' to 'A::B::X' on the demangled name before comparing, which folds those into a renamed-only bucket - same normalised name, byte-identical size - and leaves the real churn visible. - Buckets sorted by |delta|: removed, added, resized, renamed-only, unchanged, plus .text/.data/.bss/Total from `size --format=sysv`; --only-names narrows the listing, --markdown/--json write the report the integrator pastes into the merge commit. - Always exits 0 (this is informational, ARCHITECTURE.md:507); --fail-on-added-bytes is accepted and documented as reserved for the day it becomes a hard gate. - The docstring carries the guard rails a reader would otherwise supply by assumption: same CMAKE_BUILD_TYPE (the visibility presets differ per configuration), LTO off on both sides, same compiler - and every report prints both paths and their byte sizes. - --self-test runs two canned nm/size transcripts through the same parser and bucketer and pins all five buckets, including that a de-nested member folds to renamed rather than to an added+removed pair. |
||
|
swung0x48
|
fe3dc1dde8 |
[CI] (Purity): add scripts/check_include_closure.py - the -H include-closure gate for the P0.5 headers with an always-on negative control, wired as a unit ctest and the include-graph-check job
- ROADMAP P0.5 asks for a CI assertion on the include closure of the two headers the phase extracts, and ROADMAP.md:7 asks every gate to be able to fail for the reason it exists. `nm --undefined-only` cannot express either: a header that is included but whose types are never named leaves no symbol behind, and "included at all" is exactly the coupling P1 and P7 have to sever. The preprocessor's own `-H` transcript can. - Three probes, coded against the fixed path contract of the P0.5 brief so this package lands before the headers do: value-header (MG_Pipe/MGPipeValueTypes.h: no MG_State/, MG_Impl/, MG_Backend/, MG_Remote/), artifacts-header (ProgramArtifacts.h: no ShaderObject.h, ShaderTranspiler/, Config.h, MG_Backend/, BufferState/, ProgramState/ Shader*, plus a budget of two `glslang::` tokens for the two members D5 keeps verbatim) and wire-header (ITransport.h must not reach Includes.h - green today, so the gate has a live probe from its first commit). - The forbidden sets say nothing about glslang, spirv-cross or vulkan on purpose: Includes.h pulls all three unconditionally and both new headers are allowed <Includes.h>, so a textually glslang-free closure is unsatisfiable by construction. P7 measures that with `nm -D | grep glslang` on the server binary instead. - Two modes because they check different things. Text mode walks literal #include lines, needs no compiler and no submodules, and is what the ctest runs (the CI `test` job's runner has neither); clang mode is the arbiter, and adds a -fsyntax-only pass proving the header is self-contained. `--mode both` additionally fails on a disagreement between the two violation sets, so text mode's blindness to `#if` cannot hide a hit. - -H parsing normalises before matching (today's transcripts contain TextureState/../SamplerState/SamplerObject.h) and accepts only `^\.+ ` lines, which discards the "Multiple include guards may be useful for:" paragraph g++ appends. Both are pinned by a canned-transcript check inside --self-test. - D9 skip semantics: a probe whose header does not exist prints SKIP and is counted, and --require-all turns every SKIP into a failure. That is what lets the gate land first and still stops an all-SKIP run from passing for free once the headers exist; the integrator flips --require-all on after all three P0.5 packages land. - --self-test is always on in both the ctest and the CI job: it synthesizes its TUs in a tempdir (it never touches a tracked file) and requires a negative control that does not depend on P0.5 at all - MGPipeHandles.h plus RenderState.h checked against the value-header list - to report RenderState.h as a depth-1 violation in every enabled mode. Zero trips anywhere is an ::error:: and exit 1, because a gate that cannot go red is not a gate. Controls 2 and 3 arm themselves as the two headers appear. - Registered as MobileGLPurity.IncludeClosure with LABELS unit so `ctest -L unit` runs it, and with no ENVIRONMENT property, which would replace the job env wholesale. |
||
|
swung0x48
|
6672778b80 |
[Merge] (dev): merge dev@50fb1343 into feat/disaggregated
- brings the write-map landing into GPU-resident stores, the open-ended fp64 storage block flattening, the idle-based CTS chunk timeout and the MSVC /WHOLEARCHIVE test link - verified on the merged tree: unit 1458, integration 868, Wire 54, retrace 79/79 |
||
|
swung0x48
|
6e0e3df372 |
[Docs] (Disaggregated): point the history note at commit hashes only
- the retired drafts stay in git history; the README no longer names or characterises them |
||
|
swung0x48
|
bee22f9d26 |
[Docs] (Disaggregated): separate the dirty-surface totals from the immediate-publish subset in MEASUREMENTS
- the sentence read as if 836 of the 92 immediate calls were RecordError; 836 is RecordError's share of all 926 calls, and most of the 92 immediate ones are RecordError |
||
|
swung0x48
|
8952b14024 |
[Docs] (Disaggregated): rewrite the MGPipe plan into a design and architecture set - README, ARCHITECTURE, ROADMAP, MEASUREMENTS - and retire PLAN.md and REVIEW.md to git history
- README.md: what MGPipe is, the one-paragraph architecture, the P0 status line, the file and code map, and the commit range where the design competition and the three adversarial review rounds live
- ARCHITECTURE.md: the design as decided, one reason per decision - handles and generations, the 71-call catalogue by class with flags and cap bits, record and payload conventions, the tracker, texture subdata and dirty ownership, shader state and the P0.5 header extraction, the reverse channel, the backend strangler, the server side and the index host mirror, the transport as landed in MG_Remote, the persistent-map tiers as measured, roundtrips, present and threads, process and platform delivery, build shapes and purity gates, the five-part verification gate, and the knob tables marked landed versus planned
- ROADMAP.md: P0..P13 as one table of what lands, the gate and the dependency, the two tracks and milestones, the day-43 GO/NO-GO checklist with both exits, the re-baseline checkpoints, and the questions still open after P0
- MEASUREMENTS.md: spike A on both devices, the spike B tier matrix, the four-trace boundary-counter baselines on both backends, the desktop and corpus facts, and the harness traps with the exact commands
- every file:line kept is verified at
|
||
|
swung0x48
|
458ccde176 |
[Feat] (Metrics, Config): make the boundary-counter summary cadence a knob, because the device harness never reaches the teardown dump
- MOBILEGL_PIPE_STATS_PERIOD (default 120, clamped to [1, 1000000]) sets the frames per summary line; Init() latches it and a zero falls back to the default - the trace APK's replay never tears MobileGL down, so MOBILEGL_PIPE_STATS_FILE never fires on device and a fixture shorter than the period (create-indirect) reported nothing - PipeStatsTest pins the latch and the zero fallback |
||
|
swung0x48
|
901d48a678 |
[Fix] (MGPipe, Metrics, Config, CI): exchange the per-frame stats instead of racing a store, carry the three uncarried table entries, drop the inline host span from a buffer range, spell the buffer subdata range, and close the small gate holes
- S-1 PipeStats::OnPresent read each frame accumulator and then store(0)'d it; a Bump from a staging thread landing in between was lost from the Tracy plot and from every frame. Each accumulator is now exchange(0, relaxed) and the exchanged value is what is plotted, so every add lands in exactly one frame. - T-3 FdPassing without MSG_CMSG_CLOEXEC (macOS, BSD) handed back descriptors that survived exec; every received fd now gets FD_CLOEXEC by hand under !MSG_CMSG_CLOEXEC. MSG_NOSIGNAL is defined to 0 where the platform lacks it (FdPassing.cpp, Doorbell.cpp) and SO_NOSIGPIPE is set on the socketpair and on a SocketDoorbell's descriptor where it exists, so a write to a hung-up peer is EPIPE rather than a fatal signal. - T-4 the missing-flatbuffers fallback wrote OFF into the cache with FORCE, so a plain re-configure after `git submodule update` stayed OFF silently. It is a normal-variable set now, shadowing the cache for that configure only; verified by hiding flatbuffers.h, configuring with ON (warning, transport off, cache still ON) and re-configuring plainly with the header back (transport ON). - P-2 three LIVE GLFunctionsTable entries had no carrier: GetGpuTimestampNs (glGetInteger64v(GL_TIMESTAMP), a synchronous server answer), QueryCounterTimestamp (glQueryCounter, a one-shot stamp, not a begin/end pair) and WaitSync (the GPU-side wait FenceWait's client wait does not express). QueryTimestamp (MGPTimestampRequest, kCtxQuery, kReplySlot), QueryCounter (MGPQueryDesc with Kind = GL_TIMESTAMP, kCtxQuery) and FenceWaitServer (MGPFenceWait, kScreen) are APPENDED at the end of PipeCalls.def because the opcode is the position: SetSwapInterval stays 68, the three take 69-71, and PipeCatalogue.LateArrivalsAreAppendedWithoutRenumbering pins that. Header counts 71 (screen 11, query 8); the seven generators regenerated. - P-3 MGPBufferRange inlined a 32-byte MGHostSpan into every range of every class - dead space on every SSBO, atomic-counter and XFB range, and D-B8 says not to freeze the named-UBO payload before the stage-ubo-named numbers exist. The range is 24 bytes now; the host spans are an optional second var-tail behind the ranges, announced by MGPShaderBuffers::HostSpanCount (0 or Count), with set_shader_buffers keeping its kVarTail|kHostSpan flags. PipeCatalogue.BufferRangeCarriesNoInlineHostSpan pins the sizes, the flags and the comparator's view of the count. - P-4 QueryEnvUint64 parsed with base 0 (a leading zero meant octal: MOBILEGL_PIPE_PUSH=010 read as 8) and accepted -1 as every bit set; it is decimal or explicit 0x now and a '-' anywhere is refused with the warning (smoke through the integration binary: -1 and 12abc warn, 010 and 0x10 parse). The CI stdio gate's alternation now also catches fprintf(stdout, puts( and std::cout/cerr; it is green over MG_Backend and MG_State. MGPSubData states how the buffer half expresses [offset, size): UnionBox.X / UnionBox.W with Target == Buffer, Y = Z = 0, H = D = 1, one record bounded at a 2^31-1 offset and 2^32-1 size beyond which the emitter splits (the same rule the ring's half-capacity bound already imposes); MGPipeSetSubDataBufferRange / MGPipeSubDataBufferOffset / Size are the only spelling and PipeCatalogue.SubDataBufferRangeRidesInTheUnionBox pins the encoding and its bounds. gen_pipe.py now refuses, in both modes, a call payload named in PipeCalls.def with no field list in PipeFields.def (the four memcmp-fallback member types are the documented exception); shown by dropping P(MGPSwapInterval), which exits 1 naming the payload. - The MGPPixelPackState size assertion compared sizeof against itself; it asserts the literal 28 PixelStoreParameters measures. - Verified: ctest -L unit green in both the default and the split configuration, gen_pipe.py --check clean with the generated files committed, nm --defined-only of the default libMobileGL.so has no MG_Remote symbol, and the full integration-gpu suite passes (the *IsActuallyArmedWhenTheEnvironmentPinsItOn family trips under -j 8 as documented and passes serially). |
||
|
swung0x48
|
e8ee7b1a88 |
[Feat] (Backend, MGPipe): carry the six per-axis compute limits in DynamicBackendParameters so MGPCaps has every backend-owned indexed answer, and pin them against glGetIntegeri_v on both backends
- P-1: MGPCaps is DynamicBackendParameters by inclusion (plan B section 4.4.1), but that struct carried MaxComputeWorkGroupInvocations and no per-axis GL_MAX_COMPUTE_WORK_GROUP_COUNT / GL_MAX_COMPUTE_WORK_GROUP_SIZE - the six numbers that ARE the backend-owned indexed answers surviving the getter retirement (GL_Getter.cpp and CompileEnv.cpp ask GLFunctionsTable::GetIntegeri_v for exactly these, DirectVulkan answers them from VkPhysicalDeviceLimits), so the interface had a hole where its only genuine indexed carrier should be. DynamicBackendParameters now has MaxComputeWorkGroupCount[3] / MaxComputeWorkGroupSize[3] with the GL 4.3 minimums as the no-backend defaults; DirectGLES fills them from glGetIntegeri_v inside the loader's bracketed probe run (GLESCapabilities carries them, logged with the other limits) and DirectVulkan from maxComputeWorkGroupCount / maxComputeWorkGroupSize through the loader's SaturateToInt like every other limit. Raw driver answers, as the invocations limit is: the frontend floors them at the shared MIN_COMPUTE_WORK_GROUP_* minimums itself. - The GetIntegeri_v table path is untouched, as is GL_Getter and CompileEnv behaviour: retiring the getter in favour of the caps is P0.5, and this only makes sure the caps have what P0.5 needs. - PipeCalls.def's footer no longer claims that "only GL_COMPUTE_WORK_GROUP_SIZE is a real backend answer and it lives in MGPCaps": the six limits live in MGPCaps, and GL_COMPUTE_WORK_GROUP_SIZE is a frontend link artifact (ProgramObject::GetComputeLocalSize, what GL_Program.cpp answers from), which AdvertisedLimitsScenario.ComputeLocalSizeComesFromTheLinkedProgram already pins. The MGPCaps size assertion is a composition of sizeof(DynamicBackendParameters) and follows the struct. - AdvertisedLimitsScenario.ComputeWorkGroupLimitsAreTheCapsBlocksAnswer pins, on both lanes: answerability, the GL 4.3 floors, vector/indexed agreement, INVALID_VALUE past axis 2, and - through the new Harness/BackendCapsPeek translation unit, which is the one place the module looks past the GL API - that max(caps, minimum) equals the live glGetIntegeri_v answer axis by axis. Shown live by halving each backend's caps copy: both lanes fail with "MGPCaps carries 512 but glGetIntegeri_v answers 1024". On Android the module links the shipping .so (hidden visibility), so the peek returns false there and only the GL-visible half runs. ComputeWorkGroupCapabilities.TakesEveryAxisFromTheIndexedQuery in BackendLoaderTest pins the DirectGLES loader half against the fake driver, per axis and above the initialisers. - Verified: AdvertisedLimitsScenario 20/20 on DirectGLES and DirectVulkan (llvmpipe / lavapipe), BackendLoaderTest green. |
||
|
swung0x48
|
1154f9a00d |
[Fix] (MG_Remote, Transport): give the inproc doorbell a death state so Shutdown can join a parked waiter, and bound a ring record at half the capacity so a refusal can never look like backpressure
- T-1: InProcessChannel::Close rang each CondVarDoorbell once and claimed that unparks a peer mid-frame. It does not. Doorbell::Wait consumes the one ring, re-tests a condition nothing published, finds the bell alive (CondVarDoorbell never overrode Dead(); Doorbell.cpp had no death state at all) and with kWaitForever parks again for good - so Shutdown could never join a server thread sitting in the design's own steady state (spun, set consumerParked, blocked; plan section 8.1 inheriting the earlier plan's 6.2a). CondVarDoorbell now carries an atomic death latch: Kill() sets it under the mutex and notify_all's, Dead() reports it, Park returns false at once on a dead bell (and the wait predicate includes it, so a Kill cannot slip between the test and the wait), and Close kills both bells instead of ringing them. Same shape as SocketDoorbell's EOF latch; Notify stays the ordinary wakeup. - T-2: RingProducer::Reserve refused only total > capacity, but a record with capacity/2 < total <= capacity is unplaceable at every head offset where neither the space to the wrap boundary nor the space before it holds it - even in an EMPTY ring, because a wrap pad costs spaceToEnd bytes on top of the record. Concretely: head offset 16 of an empty 256-byte ring, a 248-byte record; FreeBytes() says 256, Reserve says nullptr, forever, and a producer waiting for FreeBytes() >= 248 stalls with nothing logged. The bound is now capacity/2, which is exact rather than conservative (worst case 2*total-8 <= capacity-8), exposed as MaxRecordBytes() for the emitter to chunk against; the minimum ring is two headers so the smallest record still fits the bound. Ring.h states Capacity()/2 as the chunking bound and the G3 header comment in gen_pipe.py now states the chunking rule plan section 8.2 asks G3 to define (PipeWire.inc regenerated). - Tests, each shown red with only the fix site reverted and green with it: InProcessTransportTest.ShutdownUnparksAWaiterWithNoDeadline (bounded join through a shared_ptr-owned waiter: 5 s red instead of a hung job; reverted it hangs and fails at 5051 ms), RingTest.RecordLargerThanHalfTheRingIsRefused (reverted, the 248-byte record is accepted), RingTest.RecordPlaceabilityDoesNotDependOnTheHeadOffset (the offset-0 vs offset-16 negative control), RingTest.HalfCapacityRecordFitsAtEveryHeadOffset (the positive half: the maximal record at all 32 head offsets of a 256-byte ring) and RingTest.RejectsARingTooSmallForTheSmallestRecord. |
||
|
swung0x48
|
7ef7c7e543 |
[Fix] (Spikes): answer the tier question for DirectGLES too, make an OK mean bytes round-tripped through a real GPU access, and exercise T3 in the direction that makes it a tier
- plan-B §8.3 asks which tier `AcquirePersistentMap` lands in, but the probe only
asked Vulkan. DirectGLES ("Espryt") reaches a persistent map through
`glBufferStorageEXT` + `glMapBufferRange(PERSISTENT|COHERENT)`, not a
`VkDeviceMemory` map, so a Vulkan-only answer decides nothing for that backend.
Add a GLES leg to T1: the exported fd imported with `glCreateMemoryObjectsEXT`
+ `glImportMemoryFdEXT` + `glBufferStorageMemEXT`, then mapped
PERSISTENT|COHERENT -- in-process first (isolates "GL can import this fd" from
"the fd survives a process boundary"), then cross-process (new `t1gl` child).
Drivers disagree about how the import must be phrased, so each attempt walks a
ladder over {dedicated flag} x {import size = memory requirement or the fd's
own size} x {buffer size} and reports the rung the driver accepted plus every
rejected rung with its GL error -- a driver *preference* must never be reported
as a missing capability. A driver that backs the storage but refuses
PERSISTENT|COHERENT is reported separately from one that refuses the storage:
that distinction is exactly T1 vs T2 for DirectGLES. The T0 GLES leg
(`EGL_ANDROID_get_native_client_buffer` + `glBufferStorageExternalEXT` +
persistent map, verified by `AHardwareBuffer_lock` on the client side) now
reports every step's GL enum and requires the persistent flags for OK.
- the verdict was unfalsifiable: T1 reported PARTIAL when neither leg had moved a
byte. Replace it with an explicit decisive-leg model -- OK only when every
decisive leg round-tripped in both directions, PARTIAL when at least one did,
FAIL otherwise with the failing step and its driver error named in `why:`.
Every row now opens with a per-leg trace (`vkimport[D]=rt gpu[D]=rt`). The raw
`mmap` leg is informational for opaque-fd (Vulkan forbids interpreting that
payload outside the driver, so a refusal is conformant) and decisive for
dma-buf, where a CPU mapping is the point of the handle type.
- T3 never ran the direction that would make it a tier: both ends were the
importing process. Add `T3-client-memfd-server-import` (new `t3c` child) -- the
client creates and writes the memfd, the server mmaps the received fd, imports
the client's host pointer into a `VkDeviceMemory`, reads what the client wrote,
writes back, and takes a GPU access on the client's memory, which the client
then verifies through its own mapping.
- no route touched the GPU, so an OK proved only that a map call returned a
pointer. Every tier row now takes a real GPU access before it can be OK:
`vkCmdCopyBuffer` out of the shared allocation into private staging (mismatch =
the GPU could not read what the peer wrote) plus `vkCmdFillBuffer` into it,
queue-idle and an explicit host-read barrier, with the peer checking the filled
region through its own mapping. VkCtx grows a queue and command pool for it.
- the device run executes in the `shell` SELinux domain, not the `untrusted_app`
domain MobileGL runs in, and the two do not share dmabuf/gralloc rules. Print
uid/pid/`/proc/self/attr/current` in a run-context header, repeat the caveat in
the summary, and document in README.md how to answer it for the real domain
later (exec the same binary from the trace app's spike hook, spike-A package)
without implementing that here.
- `vkStr()` returned a pointer into one static buffer while several results
routinely appear in one format call, so all of them showed the last one; it
returns std::string now, `memFlagStr` likewise, and `fmt`/`pr` carry
`format(printf)` so a missed `.c_str()` is a compile error rather than UB.
- `advertisedExportable` decided the status at the allocate site but not at the
`vkGetMemoryFdKHR` site. One rule at every export failure now
(`exportFailStatus`): advertised EXPORTABLE and then declining is FAIL, never
advertised is UNSUPPORTED. Export + map + fd is factored into `exportHostVisible`.
- `T0-ahb-blob-transfer` was recorded OK on the socket handoff alone. The handoff
keeps its own informational row; the tier row is now composed at the end from
the full import+map+compare+writeback chain over the Vulkan, GL and GPU legs.
- `mmapErrno` kept the first attempt's errno after the second-chance mmap
succeeded, so a working mapping carried a failure code; it is cleared on
success and the first errno moves into the note.
- a failed `glImportMemoryFdEXT` no longer closes the fd: EXT_memory_object_fd
does not say whether ownership still transfers on failure and Mesa closes it
either way, so closing risks a double close landing on the socket. Leaking a
handful of dups in a short-lived probe is the safe side of that trade.
- validated end to end on the host harness (lavapipe + llvmpipe,
`VK_DRIVER_FILES=lvp_icd.json EGL_PLATFORM=surfaceless`): T1-opaque-fd OK,
T3-external-memory-host OK, T3-memfd-cross-process OK,
T3-client-memfd-server-import OK, T1-dma-buf UNSUPPORTED (not advertised
exportable). The two T1-gles rows FAIL there with GL_OUT_OF_MEMORY on every
ladder rung although GL_DEVICE_UUID_EXT matches the Vulkan deviceUUID --
llvmpipe's GL does not implement importing a lavapipe opaque-fd allocation,
a Mesa interop gap recorded in README.md so a device FAIL stays attributable.
Rebuilt for arm64-v8a with NDK r27d (PIE, android-30); the device run is
pending, both device locks are held by another campaign.
|
||
|
swung0x48
|
6c7ad0a1bf |
[Feat] (Spikes): add the standalone external-memory probe that decides the persistent-map tier
- Plan B §11 P0 requires spike B ("external memory 导出，两台设备") to run before the
persistent-map decision of §8.3 can be taken: T0 (server imports a client
allocation), T1 (server exports its own HOST_VISIBLE|HOST_COHERENT allocation)
or T2 (AcquirePersistentMap returns nullptr, making the §5.10 client-side block
push mandatory). §8.3 says the answer must be measured on the two campaign
devices, and that a platform unknown must not block the interface work.
- tools/spikes/extmem_probe/ is a self-contained NDK command-line program: it
links vulkan/EGL/GLESv3/android/log and nothing from MobileGL, is configured by
its own CMakeLists with the android toolchain file, and is deliberately absent
from the project's build graph (the root CMakeLists only pulls in
tools/trace_replay), so the default ALL target is untouched.
- Phase A enumerates VK_KHR_external_memory_fd, VK_EXT_external_memory_dma_buf,
VK_EXT_external_memory_host, VK_ANDROID_external_memory_android_hardware_buffer
and, through a headless EGL pbuffer context, GL_EXT_memory_object{,_fd},
GL_EXT_external_buffer, GL_EXT_buffer_storage, GL_OES_EGL_image_external{,_essl3}
and EGL_ANDROID_get_native_client_buffer, plus the memory-type table and the
vkGetPhysicalDeviceExternalBufferProperties verdict per handle type for the exact
buffer usage set MobileGL needs.
- Route T1 allocates a HOST_VISIBLE|HOST_COHERENT buffer memory with
VkExportMemoryAllocateInfo, writes a pattern through vkMapMemory, exports an fd
with vkGetMemoryFdKHR (opaque-fd and, where advertised, dma-buf), hands it to a
second process over SCM_RIGHTS, and has that process both mmap() the fd and
import it into its own VkDeviceMemory + vkMapMemory. Both sides write and both
sides compare, so a copy-on-import or one-directional mapping is reported as
PARTIAL rather than as success.
- Route T0 has the second process allocate an AHardwareBuffer BLOB
(CPU_READ_OFTEN|CPU_WRITE_OFTEN|GPU_DATA_BUFFER), send it with
AHardwareBuffer_sendHandleToUnixSocket, and the first process import it three
ways -- AHardwareBuffer_lock, VkDeviceMemory via
VK_ANDROID_external_memory_android_hardware_buffer, and a GL buffer via
eglGetNativeClientBufferANDROID + glBufferStorageExternalEXT mapped
persistent/coherent -- with a write-back leg the allocating process verifies.
- Route T3 covers VK_EXT_external_memory_host: a memfd-backed mmap region aligned
to minImportedHostPointerAlignment, imported through
VkImportMemoryHostPointerInfoEXT, plus the same memfd handed to another process.
- The second process is /proc/self/exe re-exec'd with --child=<route> and one end
of a socketpair on fd 3. A bare fork() is not usable: neither side's Vulkan
driver survives fork, and both sides need live Vulkan. It is also the topology
the transport actually has (§8.1, inheriting PLAN.md §11.1-§11.6: the client
spawns the server), so the probe measures the arrangement that would ship.
- The probe also builds for the host with T0 compiled out. That is not scope
creep: a negative device result is only worth something if the harness is known
to report a working route as working. Running it on lavapipe did that, and paid
for itself immediately by exposing two harness bugs that would have produced
false negatives on the devices -- (a) the child wrote through its plain mmap
before reading through the Vulkan import, so on a driver whose exported fd maps
at an offset the probe overwrote the very payload the second read compares
(lavapipe reports payloadAt=4096); reads through both mappings now precede
writes through either, and the offset is searched for and reported; (b) an
export failure on a handle type the driver never advertised as EXPORTABLE was
classified FAIL instead of UNSUPPORTED (lavapipe's dma-buf answer).
- Output is a RESULT/summary table carrying the raw driver verdicts (VkResult
names, errno, GL enums) because those codes -- not a pass/fail bit -- are what
§8.3 needs in order to pick the tier.
- Built with NDK 27.3.13750724 for arm64-v8a / android-30, RelWithDebInfo, PIE,
warning-clean; host build clang/RelWithDebInfo, warning-clean.
|
||
|
swung0x48
|
38d4c2372c |
[Feat] (TraceApp, CI): pass arbitrary env vars through the retrace lane
- PLAN-B.md §8.2 and appendix B add a batch of new runtime switches
(MOBILEGL_PIPE_PUSH / _VERIFY / _STATS / _LEGACY_MEMOS / _TEXEL_RETAIN_MB /
_INDEX_MIRROR_MB, plus MOBILEGL_IPC_* later), and §11 P0 wants them parsed beside
the existing ones. Today every knob that has to reach an Android replay costs an
edit in five files - run_android_retrace_local.py, trace-replay-ci.sh,
TraceReplayActivity's request record, the JNI marshalling, and the setenv block in
trace_replay_core.cpp. That per-knob tax is what this replaces: one extra,
`--es mobilegl_env "K=V;K=V"`, carries all of them.
- Applied last, immediately before dlopen(libMobileGL.so), so it can also override
the dedicated fields above it - MobileGL's config is read during the load, and an
escape hatch that cannot beat the defaults is not one. An entry with no '=' unsets
the variable, which is the only way to clear a default the marshalling sets.
- The existing per-knob flags stay: they carry semantics beyond a setenv (use_angle
also selects a variant, the dump lists are joined, DirectVulkan forces the
R11G11B10F fallback), and rewriting them as env strings would move that logic into
the callers.
- Surface: --env / MOBILEGL_TRACE_ENV in trace-replay-ci.sh, repeatable --env
KEY=VALUE in run_android_retrace_local.py, `mobilegl_env` intent extra,
Request::envOverrides.
- The two-level parse now lives in trace_env_overrides.hpp, beside the semicolon
splitter it shares with the texture and FBO dump lists, and
tools/trace_replay/trace_env_overrides_test.cpp pins it: the empty entries a
trailing ';' leaves behind must not become unsetenv(""), `K=` must stay a Set of
the empty string rather than an Unset (a knob read with getenv() != nullptr sees
those as opposite answers), and only the FIRST '=' may separate, or a value
carrying '=' is truncated without a word of warning. The whole MOBILEGL_PIPE_*
batch rides on this parse, and the only lane that exercised it end to end was an
on-device retrace, which would have reported a splitting bug as "the knob had no
effect".
- The check is built and RUN at build time and mobilegl_trace_replay depends on it,
so `cmake --build ... --target mobilegl_trace_replay` - the exact command of
test.yml's "Build trace replay" job, which never invokes ctest - runs it. It is
assert-free on purpose: that lane configures Release, and <cassert> under NDEBUG
would compile every check into a green run that checked nothing. Negative control:
swapping find('=') for rfind('=') fails 2 checks, and keeping the splitter's empty
entries fails 2 more.
|
||
|
swung0x48
|
8a239177ac |
[Feat] (Build, TraceApp): ship and exec a second native binary on android
- PLAN-B.md §11 P0 lists spike A (the Android delivery chain) as a P0 deliverable, inherited verbatim from PLAN.md §15 P0; §8.1 inherits PLAN.md §11.1-§11.6, whose Android path needs a second process. Android gives an application no writable exec-able directory, so the only supported route is to name the binary lib*.so, let the packager put it in lib/<abi>/, and exec it out of getApplicationInfo().nativeLibraryDir. This builds that route end to end so the spike can be answered with evidence instead of folklore. - New root option MOBILEGL_BUILD_SERVER_SPIKE (OFF, ANDROID-only) adds the MobileGLServer target from tools/spikes/server_stub/main.cpp with PREFIX "lib" / SUFFIX ".so" and -fPIE/-pie: an .so name does not exempt the file from Android's PIE requirement. Its RUNTIME_OUTPUT_DIRECTORY is pointed at CMAKE_LIBRARY_OUTPUT_DIRECTORY, because AGP packages what lands in the per-ABI library output directory and CMake would otherwise put an executable elsewhere. - The option is opt-in on both sides. The plugin flavour cannot turn it on at all, and the trace flavour builds it only when asked, with `-Pmobilegl.buildServerSpike=ON` or MOBILEGL_BUILD_SERVER_SPIKE=ON in the environment; a flavour that silently carries an executable nothing loads is the kind of thing nobody notices until it ships. Verified both ways: assembleTraceDebug -Pmobilegl.buildServerSpike=ON packages lib/arm64-v8a/libMobileGLServer.so and `file` reports "ELF 64-bit LSB pie executable, ARM aarch64 ... interpreter /system/bin/linker64, for Android 26"; the same task with no property packages only libMobileGL.so and libtrace_replay_runner.so. - The stub prints one line to stdout and writes the same line to the file named by argv[1], then exits 0. The line carries pid/ppid/uid/gid and, decisively, the child's own /proc/self/attr/current: only `u:r:untrusted_app:...` proves an ordinary app process did the exec. An `adb run-as` shell runs in a different SELinux domain, so a success there would prove nothing. - RunSpawnSpike() starts the stub with argv [serverPath, markerPath], redirects the child's stdout/stderr into a captured file (an app process has stdout on /dev/null, so a printed line would otherwise vanish), waits for it, and reports exit status, signal, the exec errno, the parent's own SELinux context, the marker content and the captured stdout - to logcat, to the returned string, and to a <marker>.report file, because the Activity finishes immediately afterwards. - The child reports the errno of a REFUSED execve through a close-on-exec pipe. Without it the one datum the spike exists to produce is lost: the parent only ever sees a wait status, in which every reason has already been flattened into one exit code, and EACCES (SELinux, or a noexec mount) versus ENOEXEC (a packager that mangled the file) are opposite verdicts for the design. A successful exec closes the write end for free, so the parent reads EOF and reports execErrno=0. - fork/execve only. The earlier draft also carried a posix_spawn arm behind `__ANDROID_API__ >= 28`, which was dead code in every configuration this repo can build - bionic declares posix_spawn from API 28 and the root CMakeLists.txt pins MOBILEGL_ANDROID_API_LEVEL to 26 and refuses to configure lower - and would have silently become the production path, untested, on a minSdk bump. Keeping the arm that actually ships means the spike measures the code the server would really use. Nothing happens between fork and execve except open/dup2/execve/write/_exit, all async-signal-safe, because the parent is a multi-threaded JVM process. - The spike lives in its own TU, spawn_spike.cpp/.hpp, listed only by the trace APK's CMakeLists. Its sibling trace_replay_core.cpp is compiled verbatim by the DESKTOP mobilegl_trace_replay runner (tools/trace_replay/CMakeLists.txt names the same file), where <android/log.h> does not exist, so nothing Android-only may live there; spawn_spike.cpp carries an #error for anyone who adds it to that list. - The Activity runs the spike, and nothing else, when launched with the `mobilegl_spike_spawn` intent extra; that mode needs no trace, no golden and no render surface. It is a separate JNI entry point rather than another parameter on the 30-argument replay call, which it shares nothing with. - Not yet run on a device: both device locks are held by another campaign. The on-device verdict is the coordinator's step. |
||
|
swung0x48
|
87ee17c68c |
[Docs] (Disaggregated): fold the P0 measurements and corrections into the plan
- GL_COMPUTE_WORK_GROUP_SIZE is answered by MG_Impl from ProgramObject::GetComputeLocalSize, not by a backend; only the compute limits are caps, and the two dead table entries (GetInteger64i_v, GetProgramiv) were retired in P0 - the glRenderbufferStorage OOM-probe idiom appears in 0 of 41 fixtures, so kNeedsAck is carried by glBufferStorage only - FramebufferSrgb/DepthClamp: six readers of a constant false and zero readers respectively, no fixture enables either; recorded as a decision to take before the render-state chunk table freezes - the call catalogue is 68 unique records (screen 10, ctx-query 6, CSO 13, kCtxState 17, kCtxObject 9, kCtxVerb 13); PipeCalls.def is the single source of truth and the wire opcode is a line's position - measured layouts (MGPDrawInfo head 56 B, RenderStateParameters 1168 B, ResidualValueBlock 1248 B, MGPipeContext 464 B ...), the 926/73 MG_Impl mutator surface, the first per-draw accessor numbers on lavapipe (Espryt 20.65, Magma 15.54), the EndTransformFeedback null-as-capability trap, and the host-side spike results |
||
|
swung0x48
|
aa005720d0 |
[Test] (MG_Remote, Wire): pin the hung-up doorbell, the wakeup that must not be eaten, the ring's capacity ceiling and the publish-then-ring handoff
- FdPassingTest.SocketDoorbellStopsParkingWhenThePeerHangsUp builds a SOCK_STREAM socketpair - deliberately not FdPassing::CreateSocketPair's datagram pair, because only a stream end reports the hangup at all - closes the notifier, and asserts Park returns false, latches Dead(), stays latched, and that a Wait with kWaitForever gives up in under a second instead of spinning. - FdPassingTest.SocketDoorbellStillDeliversTheLastRingBeforeAHangup rings and then closes: detecting death must not swallow the wakeup already sitting in the socket buffer, since the peer's last publish is the one a waiter is most likely to be blocked on. - InProcessTransportTest.AFrameWakeupIsNotEatenByAWaiterOnDescriptors blocks two readers on one endpoint with two different predicates and requires the frame to arrive within 2s rather than "eventually, when a receive timed out". - RingTest.RejectsACapacityTheRecordHeaderCannotDescribe refuses 4 GiB from both roles without mapping anything (the constructor rejects before it touches the base pointer) and keeps 2 GiB accepted as the positive control. - RingTest.DoorbellHandoffWakesBothSidesOnEveryPublish runs 2000 records through the ring with real parking in both directions, in the publish-then- NotifyIfParked order the fences assume. It cannot prove the fence pairing - no test can, since x86 has to actually hold the release store in the store buffer across the flag read - but it exercises the exact call order, and a lost wakeup surfaces as a Wait that times out with work available (a red test) rather than as a hung CI job. - Result: 1429 unit tests pass with MOBILEGL_BUILD_DISAGGREGATED=ON (1424 before this commit; the wire subset is 47), 1382 pass with it OFF, and the same three pre-existing skips appear in both. Every new case was verified to fail with its fix reverted and to pass again with it restored. |
||
|
swung0x48
|
c1a7ffac94 |
[Fix] (MG_Remote, Transport): give descriptor offers their own condition variable, cap the ring at what a 32-bit size field can describe, and keep the frontend umbrella out of Framing.h
- InProcessChannel::Direction served two different predicates from one
condition_variable signalled with notify_one, so a SendFrame wakeup could be
delivered to a thread blocked in ReceiveFd, which re-tests its own predicate
and goes back to sleep - leaving a queued message undelivered until some
unrelated later event. ITransport narrows the contract to one dedicated
reader thread, but that is a comment, not a mechanism, and the first caller
that splits its reader should not have to discover this. Offers now have
their own fdCv, and Close() notifies both.
- RingProducer/RingConsumer accepted any power-of-two capacity while
RingRecordHeader::size is 32-bit by wire contract (plan section 8.1 ->
PLAN.md section 6.3: 8-byte RecHeader). At 4 GiB or more a record's size -
or a wrap filler's, which is sized by the distance to the boundary - would be
truncated on the way in, and the consumer would then bounds-check the
truncated value against the real one. kMaxRingCapacity rejects that at
construction, the same class of guard as the power-of-two and
smaller-than-a-header checks beside it. Unreachable today (SEG_CMD 8 MiB,
SEG_STAGE 32 MiB), which is the point of catching it now.
- Framing.h included MG_Util/Debug/Log.h, which includes Includes.h, the GL
frontend's umbrella header: 661 headers by `clang++ -H`. It is the one header
under Transport/ that broke the rule ITransport.h states for this layer
("nothing about a byte pipe needs the GL frontend's umbrella header"), which
matters when the server-side binary links this and when the include-graph
purity gate of plan section 10.3 (gate A, asserted on -H output rather than
on symbols) lands. Its three error paths now call WireLogError, declared in a
new dependency-free WireLog.h whose .cpp owns the umbrella. Framing.h is down
to 134 headers, none of them MG_State, Includes.h or Log.h.
- Two documentation corrections. ITransport::Shutdown documented a one-sided
"releases the endpoint" while InProcessTransport::Shutdown closes both
directions - which is what closing a socket does, so the spawn transport will
behave the same way; the interface now says whole-connection teardown, and
keeps the promise that queued messages stay readable until drained.
InProcessTransport.h cited a CMake option
MOBILEGL_BUILD_DISAGGREGATED_INPROC that grep finds nowhere: plan appendix B
reserves it for the role-isolation shim, this skeleton does not add it, and
the delivery mode is a runtime choice (MOBILEGL_TRANSPORT), not a build one.
- Evidence: both configurations reconfigured and rebuilt; nm --defined-only on
the OFF build still reports 0 MG_Remote symbols and the ldd dependency set is
byte-identical to the OFF link (plan section 10.3, the two surviving
byte-level equalities). Negative controls: removing the capacity ceiling
makes RingTest.RejectsACapacityTheRecordHeaderCannotDescribe fail on both
roles; collapsing fdCv back into cv makes
InProcessTransportTest.AFrameWakeupIsNotEatenByAWaiterOnDescriptors fail at
3950ms against its 2000ms bound.
|
||
|
swung0x48
|
bdd4bed431 |
[Fix] (MG_Remote, Transport): close the doorbell's lost-wakeup window with two seq_cst fences and stop a hung-up peer turning every park into a spin
- The header claimed the park flag's own seq_cst store and load closed the lost-wakeup window. They cannot: that Dekker argument needs all FOUR accesses in the seq_cst total order, and the other two are not in it - the watermark publish is a release store (RingProducer::Publish) and the condition re-test is an acquire load. There was no atomic_thread_fence anywhere under MG_Remote. On x86 a release store and a seq_cst load are both plain MOVs, so the notifier can read parked==0 while its head store still sits in the store buffer, and the waiter then parks on a stale watermark forever; ARMv8 survived only because STLR->LDAR is RCsc, which is luck, not the design. Doorbell::Wait now fences after announcing and NotifyIfParked fences before reading the flag - the pairing the standard actually guarantees ([atomics.order]) - and the header says so, including the other half of the contract: publish the watermark BEFORE ringing, because a fence only orders what precedes it. This is the claim the whole P5/P6 wait discipline (present credit, kNeedsAck blocking requests, full-ring escalation) will be built on, and its failure mode is a silent cross-process hang. Inherited design, plan section 8.1 (PLAN.md section 6.2/6.2a: bidirectional doorbell, MOBILEGL_IPC_SPIN_US default 50us, condvar for inproc). - SocketDoorbell::Park treated any poll() return > 0 as a wakeup and never looked at revents. Measured on this machine: an AF_UNIX SOCK_STREAM socketpair whose peer has closed returns revents=POLLIN|POLLHUP with recv()==0 immediately and forever. Park therefore returned true, Wait stored parked=0, found its condition still false and re-parked - so a waiter with kWaitForever burned a big core at full clock with no bound. That is exactly the pathology the bidirectional doorbell exists to prevent (a whole 16.6ms frame of a big core on a phone, competing with the GPU and the game's JVM), reached from the other side. Park now branches on revents, a new Drain() latches death on EOF (and on ECONNRESET/EPIPE from Notify), and the new Doorbell::Dead() lets Wait give up instead of re-parking on a descriptor that can never deliver another wakeup. - Same commit, same defect class: `fd` is documented as one end of an AF_UNIX socket pair, not "a socket or pipe end". Notify uses send(MSG_DONTWAIT| MSG_NOSIGNAL) and Park uses poll()+recv(), which a pipe end refuses with ENOTSOCK, and a SOCK_DGRAM pair reports no readiness at all when the peer closes (measured), so the spawn transport wants SOCK_STREAM. - Evidence: build-linux-split rebuilt clean; the wire suite is green (47/47). Negative control - reinstating the revents-blind Park makes FdPassingTest.SocketDoorbellStopsParkingWhenThePeerHangsUp fail on both Park assertions, and restoring this code turns it green again. |
||
|
swung0x48
|
10315e71f3 |
[Test] (MG_Remote, CI): cover the wire layer with five suites and gate the generated header with flatc-check
- MobileGL/MG_Test/Wire, 42 gtest cases in five binaries, ctest label `unit`, registered only when MOBILEGL_BUILD_DISAGGREGATED is ON so the default configuration is untouched. - FdPassingTest is the one the earlier branch could not have had: it forks a child that creates a shared segment, fills 64 KiB with a pattern and hands the descriptor over SCM_RIGHTS; the parent adopts it, maps it read-only and compares every byte. Everything in Feat/CS-Delta-IPC ran in one process, which is why `out->fd = -1` survived unnoticed. It also pins that a too-small sideband buffer is refused before the datagram is consumed (so no descriptor is dropped), that an empty sideband still carries its fd, and that a receive with no offer times out. The `spawn` SocketDoorbell is covered in the same file because it rides the same kind of socket: a parked waiter woken through NotifyIfParked, a clean timeout, and a wakeup that arrives before anyone parks being remembered and then consumed exactly once. - RingTest: control-page size/alignment/cache-line layout, a non-power-of-two capacity refused, payload alignment, 200 records driven through a 256-byte ring so the wrap filler path runs repeatedly and no record ever straddles the boundary, backpressure (full ring refuses, applied alone frees nothing, retired frees), a record larger than the ring refused, the generation bump refused while records are in flight and accepted once quiesced, a corrupt header refused instead of dispatched, and a 20000-record two-thread producer/consumer run checking order, content and the final cursor equality. - FramingTest: byte-at-a-time reassembly of two frames, the magic reading "MGLF" on the wire, empty payloads, a bad magic and an oversized length each latching the reader dead (the old code hung silently instead), the send-side cap, and the buffer-too-small contract keeping the message so the retry still finds it. - InProcessTransportTest: both directions, ordering, buffer-too-small, poll and timeout, shutdown draining queued messages before it closes, a blocked receiver woken by a send and by a shutdown, the frame cap, descriptor hand-off with its sideband, and three condvar doorbell cases - a parked waiter woken through NotifyIfParked, an already-true condition that must never park, and a clean timeout. - ProtocolSmokeTest: a Hello built and read back through the committed generated header, a Welcome carrying the four segment announcements, the same buffer travelling across the transport unchanged, a truncated buffer failing verification rather than being read, and the CtrlMsg / SegmentKind / LogLevel / FatalCode tag values frozen - they are wire numbers, so if that test has to be edited the schema change was a wire break. - CI: a `flatc-check` job in test.yml that checks out only the flatbuffers submodule, builds the pinned flatc through scripts/gen_protocol.py into the runner temp directory, regenerates and runs `git diff --exit-code` on protocol_generated.h. It needs no MobileGL build, so it does not depend on build-linux (plan B section 8.1 / earlier section 7.1: the committed header plus a CI regeneration check, and no flatc in the default build graph). - Verified: `ctest -L unit` is green in both configurations - 1382 cases with the option OFF and 1424 with it ON (the same 1382 plus these 42); the nm gate is 0 matches OFF and 93 ON; regeneration of protocol_generated.h is byte-identical, and the flatc-check gate goes red on a hand-edited header and green again after a clean regeneration. |
||
|
swung0x48
|
bfa087d0f7 |
[Feat] (MG_Remote, Transport): land the transport skeleton behind MOBILEGL_BUILD_DISAGGREGATED - framing, the SPSC ring, doorbells, shm segments and SCM_RIGHTS
- New CMake option MOBILEGL_BUILD_DISAGGREGATED (default OFF, plan B appendix B). ON appends the MG_Remote sources to SOURCE_FILES, puts 3rdparty/flatbuffers/include on the include path and defines MOBILEGL_BUILD_DISAGGREGATED=1. OFF compiles nothing from MG_Remote and adds no include path and no library, which is one of the two byte-level equalities plan B section 10.3 keeps: measured `nm --defined-only build-linux/libMobileGL.so | grep -ic MG_Remote` = 0 with the option OFF and 93 with it ON, with an identical ldd set in both configurations.
- The option forces itself OFF with message(WARNING) when 3rdparty/flatbuffers/include/flatbuffers/flatbuffers.h is missing, so a checkout without the submodule still configures and builds rather than failing a hundred lines later on a missing header.
- ITransport.h: SendFrame / ReceiveFrame / PeekFrameSize / ShareFd / ReceiveFd / Shutdown. ReceiveFrame's contract is the fix for a defect of the earlier branch: a destination buffer smaller than the pending message returns MOBILEGL_ERR_BUFFER_TOO_SMALL with the required size and KEEPS the message queued. The earlier transport failed the call and popped the message anyway, which wedges the stream permanently the first time a reader guesses a size wrong.
- Framing.h: [u32 'MGLF'][u32 len][payload], 64 MiB cap, validated on read. A bad magic or an oversized length latches the reader dead, is logged at ERROR and makes every later call return MOBILEGL_ERR_PROTOCOL_MISMATCH. This is the second inherited defect: Feat/CS-Delta-IPC's Framing.h:41-45 Feed() always returned OK and its header peek merely returned false, so a desynchronized stream became a silent permanent hang; its LocalSocketTransport.cpp:232-236 then allocated on the peer-supplied length with no cap. The magic is also byte-ordered so it reads "MGLF" on the wire, where the earlier constant spelled "FLGM".
- Ring.h/.cpp: RingControl exactly as the inherited design (plan B section 8.1 -> earlier section 6.2): two independent cursor triples (cmd and stage, each {head, appliedTail, retiredTail}), appliedSeq / submittedSeq / retiredSeq / completedFrameSerial / presentAckSerial, serverEpoch, ringGeneration, consumerParked, producerParked, eventRingFull, eventDropped. One 4096-byte page with each contended group on its own cache line, pinned by static_assert on size and alignment and by an offset test.
- The SPSC pair uses monotonic byte cursors and a power-of-two mask, so a torn read can never look like a valid earlier position. A record never straddles the wrap: the producer emits a kPad filler to the boundary, which is always a multiple of 8 and therefore always has room for a header. The producer reclaims against retiredTail rather than appliedTail, so the day the server borrows a ring slot into the GPU timeline (kRecBorrowSlot) it does not silently degrade to early recycling. HardDrainRing bumps ringGeneration only when the ring is quiesced, and leaves the cursors monotonic so cached offsets are recognisably stale.
- The consumer bounds-checks every record header before dispatch (8-aligned, at least a header, no larger than what the producer published, contiguous inside the mapping) and reports corruption to the caller instead of dispatching into undefined behaviour. That is the runtime half of the earlier section 6.3 discipline: SEG_CMD is written by another process, so a compile-time static_assert on record sizes proves nothing about what is in the mapping.
- Doorbell.h/.cpp: spin then park, both directions (earlier section 6.2a). CondVarDoorbell for inproc, SocketDoorbell for spawn (one byte, codes 0x01 ring-advanced and 0x02 watermark-advanced) - no futex, eventfd or named event anywhere. The lost-wakeup window is closed by ordering: the waiter stores its park flag seq_cst and then re-tests the condition, NotifyIfParked loads the same flag seq_cst after the watermark is published, so one of the two always sees the other. kDefaultSpinUs is 50, the MOBILEGL_IPC_SPIN_US default.
- ShmSegment.{h,cpp} + ShmSegmentPosix.cpp: memfd_create by raw syscall on desktop Linux (the glibc wrapper is too recent to rely on), ASharedMemory_create on Android (API 26; libc's memfd_create wrapper is API 30, above MobileGL's floor), shm_open + immediate shm_unlink as the fallback. Adopt() refuses a descriptor whose fstat size is smaller than the size the peer announced, so a short segment cannot turn every later offset into an out-of-bounds map. ShmSegmentWin32.cpp (CreateFileMappingW in Local\) is compile-guarded and untested - this project's Windows machine is not a correctness gate.
- The Android path is compile-verified, not just written: an arm64-v8a NDK build with the option ON links, ShmSegmentPosix.cpp.o carries an undefined ASharedMemory_create, and ShmSegmentWin32.cpp.o is empty there. That build is also what caught the missing <cstddef> in ShmSegment.h and Framing.h, where std::size_t / std::ptrdiff_t only resolved through a transitive include on the host sysroot.
- FdPassing.{h,cpp}: SCM_RIGHTS in the FIRST transport commit, as plan B section 8.1 demands, over a dedicated AF_UNIX SOCK_DGRAM socketpair rather than the control byte stream (message boundaries survive on every POSIX - SOCK_SEQPACKET does not exist on macOS - and ancillary data can never be split from its payload). The third inherited defect this replaces: Feat/CS-Delta-IPC deferred fd passing and hardcoded `out->fd = -1` in LocalSocketTransport.cpp:296, so on the only platform that matters its data plane could not move one byte between processes. MSG_CTRUNC, an unexpected descriptor count and a malformed sideband header all close every descriptor received before failing, and a too-small sideband buffer is refused before the recvmsg so a datagram is never half-consumed.
- InProcessTransport.{h,cpp}: two in-memory queues plus the condvar doorbell pair. It keeps the frame size cap so nothing that passes in inproc becomes illegal after the switch to spawn, hands descriptors over with dup() under the same ownership rule as SCM_RIGHTS, and lets a peer's queued messages be drained after Shutdown - usually the last one says why it is going away.
- Not done here on purpose: the MOBILEGL_IPC_* environment variables are parsed in ConfigLoader.cpp, which belongs to another P0 work package running in parallel; this commit only exposes the constants (kDefaultSpinUs, kMaxFramePayloadSize, FdPassing::kMaxSidebandBytes) so that plumbing has something to set.
|
||
|
swung0x48
|
a1e22c26ab |
[Build] (MG_Remote, Protocol): pin the flatbuffers submodule, add the control-plane schema and commit its generated header
- 3rdparty/flatbuffers submodule pinned to the latest release tag v25.12.19 (7e163021). The runtime is header-only, so only 3rdparty/flatbuffers/include is ever used and no library is linked; CMake never calls add_subdirectory on it and flatc is not in the build graph (plan B section 8.1, inheriting the earlier plan's section 7.1).
- MobileGL/MG_Remote/Protocol/protocol.fbs carries the CONTROL PLANE only: SegmentRef, Hello, Welcome, CapsSnapshot, DefaultFramebufferInfo, SurfaceOp, SurfaceReply, ResyncRequest, ResyncDone, AuxRequest, Fatal, LogLine, union CtrlMsg and the CtrlEnvelope root with a file_identifier. Hot-path records are FlatBuffers structs generated from MG_Pipe/PipeCalls.def in a later package and are deliberately absent here, so record numbering never churns.
- Two deviations from the earlier plan's section 7.1 sketch, both deliberate: (a) ProgramReflection is not a union member - plan B ships program artifacts inside the create_shader_state CSO blob (section 8.2), and union tags are wire values that may only ever be appended, so reserving a tag for a message that may never exist is worse than appending one later; (b) maxComputeWorkGroupCount/Size are vectors, not [int:3] - fixed-size arrays are legal only in FlatBuffers structs, never in tables.
- scripts/gen_protocol.py resolves flatc as MOBILEGL_FLATC_EXECUTABLE, otherwise builds the pinned flatc ONCE into <repo>/../flatc-build (override with MOBILEGL_FLATC_BUILD_DIR), outside the project build graph. A flatc found on PATH is deliberately refused and a version mismatch against the pinned runtime is a hard error: the generated header static_asserts FLATBUFFERS_VERSION, so a stray flatc either fails to compile or churns the committed file on every machine. The earlier branch did the opposite - Protocol/CMakeLists.txt:22-38 add_subdirectory'd the FlatBuffers tree with FLATBUFFERS_BUILD_FLATC=ON whenever MOBILEGL_FLATC_EXECUTABLE was unset, which is exactly the NDK trap it claimed to avoid (cross-compile an arm64 flatc, then run it on the host).
- protocol_generated.h is committed with the project source header prepended by the generator, so regeneration is byte-identical: verified by running gen_protocol.py twice and by perturbing the file and regenerating it back.
- Reuse from Feat/CS-Delta-IPC: MobileGL/Protocol/mg_protocol_base.h, kept as the shared C vocabulary (result codes, byte spans, shm region, id typedefs) and keeping the structSize-first versioning discipline that section 14.2 calls the answer to risk B-R10. Dropped from it: MobileGLObjectKind / MobileGLObjectScope / MobileGLObjectHandle - plan B never puts GL object identity on the wire (the frontend allocates {slot, generation} handles in MG_Pipe, section 4.2.1), so a second identity vocabulary would be a drift surface with no reader. Added MOBILEGL_ERR_BUFFER_TOO_SMALL as an append-only code for the receive contract.
|
||
|
swung0x48
|
bd2b4158e0 |
[Fix] (DirectVulkan, State): key the transform-feedback counter slots on the object's identity, not on its recycled GL name
- VulkanRenderer::CurrentXfbCounterSlot keyed m_xfbCounterSlotByObject on
GetBoundTransformFeedbackName(). glGenTransformFeedbacks hands a deleted name
straight back (IndexGenerator is LIFO) and nothing ever removed a map entry, so
a transform feedback object created on a recycled name was served the DEAD
object's counter group - and with it that group's m_xfbCountersValid and
m_xfbLastSeenGeneration entries, which are the resume/fresh decision for
vkCmdBeginTransformFeedbackEXT. This is D21 in plan B v2 4.7.3, the one entry
in that table whose today-key "guards nothing", and 10.4-5 asks for it to land
on dev on its own - hence this separate commit, kept in files no other commit
on this branch touches so the cherry-pick applies unaided.
- Frontend: TransformFeedbackObjectState gains a never-reused `lifetimeId`
through a default member initialiser, so every route into existence
(operator[] materialisation, `= {}` in GenTransformFeedbackNames and
CreateTransformFeedbackObject) mints a fresh one and a recycled name cannot
carry the dead object's id back. The allocator is the same shape as
BufferObject::AllocateLifetimeId (atomic, starts at 1 so a zeroed backend slot
is never a live object).
- The bound object's id is mirrored in m_boundTransformFeedbackLifetimeId,
refreshed by RestoreBoundTransformFeedbackState - which every bind, and the
revert that deleting the bound object performs, goes through - and seeded for
the default object by the GLContext constructor. GetBoundTransformFeedback
LifetimeId is therefore a const load. Reading it through operator[] instead
would have been an INSERT on the per-draw path, and UnorderedMap is
ska::flat_hash_map, whose rehash invalidates every reference into the
container, not just its iterators.
- Backend: the UnorderedMap is replaced by a fixed 16-entry owner table, which
fixes the second half of the same defect - the map was keyed on a value that
recycles yet was never pruned, so it grew for the life of the context. With
lifetime ids as keys a map would have grown without bound instead, so the
bounded table is required, not cosmetic.
- Slot exhaustion: past sixteen owners a group has to be taken over, and the
victim is chosen among owners with NO OPEN SPAN, which
GLContext::HasOpenTransformFeedbackSpan answers; an identity no live object
carries any more answers false, and that is what lets a dead owner's group
come back. Least-recently-used ALONE would have been exactly the wrong rule:
GL only permits another object to capture while this one is PAUSED, so the
paused span these groups exist to protect is by construction the least
recently used entry, and an LRU takeover would reset the one resume offset
that still matters. LRU is now only the tie-break among reclaimable groups.
Sixteen genuinely open spans at once is reported (MGLOG_E_ONCE) rather than
resolved silently, because whatever is taken then restarts at offset 0.
- Not done, and why: the natural place to hand a group back is
glEndTransformFeedback, but registering DirectVulkan's EndTransformFeedback
table entry would flip the test FixupGsStripCaptureOrder makes of that same
pointer (GL_Drawing.cpp:1255) to decide whether the backend already captured
in GL's vertex order, silently disabling the geometry-stage strip fixup for
DirectVulkan. Giving that discriminator a name of its own is a separate
change; until then the no-open-span rule is what keeps the table honest.
- CurrentXfbCounterSlot asserts the identity is never 0. Zero is the free-slot
sentinel, so an identity of 0 would match every free slot as "mine" without
ever claiming one - this bug reintroduced, with no symptom at the call site.
- TransformFeedbackLifetimeIdTest, in its own translation unit, pins the
frontend halves: an object created on a recycled name must not report the dead
object's id, the default object has an identity before anything binds it, and
a PAUSED span still reads as open while another object is bound and capturing
- which is the whole correctness argument for the eviction rule. The name
reuse is not simulated: the test asks the real generator and skips (loudly) if
it never recycled. Still untested: the >16-owners path itself, which needs a
backend scenario with seventeen capturing objects and there is none.
- Negative controls, each applied then reverted: making a non-bound object's
span read as closed reddens APausedSpanStaysOpenWhileAnotherObjectCaptures;
making a vanished identity read as open reddens the same case on its delete
assertion; dropping the constructor's seeding reddens
AnObjectAtARecycledNameCarriesAFreshLifetimeId.
- Tested: cmake --build build-linux -j 24 (clean, 166 targets); ctest -L unit
-j 12 -> 1386/1386 passed; ctest -L integration-gpu -> 866/866 passed
serially, and 866/866 on one of two -j 8 runs. The other -j 8 run failed
DirectGLES.PointSizeDemotionScenario.TheDemotionIsActuallyArmedWhenTheEnviron
mentPinsItOn, a member of the pre-existing parallel-ctest flake family: it
passes in isolation here, and the unmodified parent tree
(~/w7/p0-noop-wins-base) reproduces the same family under -j 8.
|
||
|
swung0x48
|
9c7339b214 |
[Feat] (State): give RenderbufferObject the never-reused lifetime id every other cache-keyable state object already has
- RenderbufferObject was the last state object a backend twin registry keys on that could only be identified by its heap address or its GL name - both of which recycle. BufferObject, VertexArrayObject and ProgramObject all carry a process-wide, never-reused id for exactly this; the renderbuffer's absence is named in plan B §10.4-5 as one of the two latent problems P0 closes. - Mirrors BufferObject.h:202-208 / BufferObject.cpp:19-24 verbatim in shape: a private static AllocateLifetimeId() over a namespace-scope std::atomic<Uint64> starting at 1 (so a zero-initialised memo slot can never carry a live object's id), a const member initialised from it at construction, and an inline const getter. The doc comment is the buffer one restated for the renderbuffer's own recycling sources. - Deliberately NO GetVersion(): plan B §11 P0 says the id only. A mutation counter would be a second, independent invalidation surface to keep correct, and nothing needs one yet - the renderbuffer's mutable content already reaches the backends through AllocateStorage / SetInternalFormat / SetSamples. - No caller yet, by design: the id exists so §4.7.3's D1 rekey (and the DirectGLES renderbuffer twin registry at Managers.h:1858) has something to key on. It is a pure addition - no existing field, signature or answer changes. - ObjectLifetimeIdTest gains the two cases the other two object types already have, so the renderbuffer is covered by the same allocator-reuse probe: an object rebuilt at a freed address must not answer to the dead one's id, and two live ones must differ. - Tested: cmake --build build-linux -j 24 (clean); ctest -R ObjectLifetimeId -> 6/6 passed (4 pre-existing + 2 new). |