mirror of
https://github.com/MobileGL-Dev/MobileGL
synced 2026-11-07 01:53:24 +09:00
8b13d8a84cf9b7478acc3d7dfd7fbbf92f98ea29
3444
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 swung0x48
|
8b13d8a84c |
[MG_Remote, MG_Impl] (P6/dl): the device-lost latch, taken from a descriptor and never from a deadline
Before this, CONTRACT-P6 5.3's own sentence held: "a dead server leaves the
client silently DECLINE-ing every verb forever with glGetError clean".
FatalCode::DeviceLost had zero producers and zero consumers and
glGetGraphicsResetStatus returned GL_NO_ERROR unconditionally, so an
application had no way to learn that the process holding its GPU objects was
gone. Measured cost of that: a server killed on its first Clear left the
client waiting out the full 120 s barrier and reporting Fatal{BarrierTimeout}
- the wrong diagnosis, two minutes late. It now notices in 0.01 s.
THE MACHINERY WAS ALREADY THERE. Doorbell::Wait breaks out on Dead(),
SessionProducer::Park turns that into SessionWait::ShutDown, and ClientSession
already had the named DECLINED path. What did not exist was the death FACT:
D5c's finding is that the client parks on clientBell[0] and rings ITSELF
through clientBell[1], holding both ends, so EOF can never arrive on the one
descriptor it watches. A bell that can wake itself cannot hear a death.
SetDeathWitness(fd) fixes that with no new descriptor: the control socket's
far end is held only by the server. The subtlety is that the bell must watch a
socket it must NEVER read - the transport's reassembler owns those bytes and a
control reply is not a death. So the witness enters the poll set with
`events = POLLRDHUP` and nothing else: POLLHUP/POLLERR/POLLNVAL arrive in
revents unrequested, so the hangup is seen while POLLIN never is. Nothing is
recv'd from it and its ownership stays with SocketTransport.
Dead() AND PeerHungUp() ARE TWO FACTS. Park sets m_dead for POLLERR, POLLNVAL
and unrecognised revents too - each a fault in THIS process's descriptor - and
CondVarDoorbell::Kill() sets it on every orderly Stop(). A latch taken from
Dead() would report a lost GPU for a local fd bug and a context reset for
every clean exit, so only PeerHungUp() arms it.
GL_UNKNOWN_CONTEXT_RESET, not GUILTY or INNOCENT: those assign blame, and the
server died for a reason that never crossed the wire.
D5b, in the control-reply wait, which had the right comment and no code: EOF
on the control stream is device loss; a TIMEOUT with the socket still open is
"ALIVE BUT SILENT" and says so by name. 5.4 forbids calling a frozen or merely
slow peer dead, and under P5e run-ahead a server one frame behind is the
INTENDED steady state - a latch armed by a deadline would fire on a healthy
session under load.
MOBILEGL_IPC_RESPAWN is now a NAMED REFUSAL rather than a variable nothing
reads. The latch is one-way on purpose: every handle this client minted names
an object inside the server process, so recovery means re-pushing the world.
THREE TEST NAMES ADDED, none renamed (G14), and R-16 run both ways:
* remove the witness -> AKilledServerLatchesDeviceLostFromTheHangupAndNot-
ADeadline goes red, takes 2 s instead of 0.01 s, and names D5c.
* collapse PeerHungUp() into Dead() -> ADeadBellIsNotAlwaysAHungUpPeer goes
red naming the local-fd-bug consequence.
* AnOrderlyStopIsNotADeviceLoss stays GREEN under that collapse, and its
comment now says so. The first draft claimed it was the control for it;
running the falsification proved otherwise - under spawn the self bell is a
SocketDoorbell and Stop() never kills one. The comment was corrected rather
than the claim quietly kept.
G1 RE-MEASURED, NOT ASSUMED: 0 symbol diff against the c6 baseline and .text
unchanged at 0xa52203. That measurement earned its keep - it caught a 16-byte
drift that had nothing to do with dl: P6's O_APPEND log-sink fix was compiling
into the PULL build, where a second process can never exist. Now guarded, and
.text is back to the baseline exactly.
Full unit suite: 2350 tests, the same two pre-existing environmental failures
and no others. All six OpenRA arms (2 backends x monolith/inproc/spawn) green.
|
||
|
swung0x48
|
d83fc6643a |
[CI, android-plugin] (P6): CI starts a second MobileGL process on a device
The Android retrace matrix gains a third lane, spawn-acceptance, beside
monolith-control and inproc-acceptance. Until now nothing in CI had ever
started a second MobileGL process on a device at all.
The apk.yml retrace job already carried a `transport` matrix dimension and
already branched on it, so the lane itself is three edits: a row in
github_transport_matrix, an elif in the retrace step, and a summary to render.
What needed thought was the gate.
THE SPAWN LANE IS APK-ONLY, and that is a statement about where evidence is
missing rather than about what spawn can do. Linux already has
test.yml's `retrace-split`, which is {backend, case, transport} over inproc AND
spawn with the two negative controls that make it mean anything. A spawn row in
the Linux transport matrix would pay for a second full sweep proving nothing -
and test.yml's `retrace` job stages no server image, so the lane would not
merely be redundant, it would be red. self_test_transport_matrices now asserts
the absence in both directions, because an accidental removal of the `if apk`
has to be caught in the self-test rather than in CI.
--require-spawn REQUIRES TWO SENTENCES, and the second is the one that matters.
ConfigLoader's marker proves the VALUE RESOLVED - it exists only in the Spawn
arm, which exists only under MOBILEGL_BUILD_DISAGGREGATED - but that is a
statement about a parser. `spawn ARMED - the server role runs in pid N` is
written only after the launch, the connect and the handshake have all
succeeded, and the pid in it is the SERVER's. It is the only line in the log a
same-process run cannot produce.
Both halves red once, measured:
* a monolith run demanded to prove spawn: REFUSED, transport_resolution_marker
false, server_pid null - while the retrace itself scored ssim=1.000000,
which is exactly the trap. OpenRA matches its golden under monolith too, so
the picture has never been the evidence.
* the same run's real spawn log with the one ARMED line deleted: REFUSED,
transport_resolution_marker TRUE and server_pid null. That is the
"resolved the transport and then never left the process" case, isolated.
THE PROOF USES `any` WHERE THE inproc GATE USES `all`, and the difference is
load-bearing rather than a relaxation. Under spawn two processes append to one
MOBILEGL_LOG_FILE_PATH, and the launcher deliberately scrubs every
MOBILEGL_IPC_* from the child's environment (anti-recursion catch (b)) - so the
server's own `Config: IPC` line reads strict=0 role-split-state=0 BY
CONSTRUCTION. Demanding every line match would red every spawn run for the one
thing the design requires.
--use-pbuffer is not optional on this lane: until P12 a spawned server cannot be
handed a window, because an ANativeWindow* is a pointer into the CLIENT's
process (Rule H). MOBILEGL_IPC_SERVER_PATH is deliberately NOT passed -
nativeLibraryDir carries an install-time hash that nothing off the device can
spell, and the Activity resolves it.
The local runner now passes the same knobs and the same --require-* flag CI
does, so "it works locally" can no longer mean "it ran monolith locally". That
change immediately caught its own omission: the first local run with the gate
armed failed on client_ipc_line_present=false because the runner had been
sending MOBILEGL_TRANSPORT alone.
Verified on device 2f7cbe2e: OpenRA.DirectGLES spawn, ssim=1.000000,
mismatchPixels=0, "real spawn transport, server role in pid 9885, and zero
Fatal records confirmed".
|
||
|
swung0x48
|
7c8120cc49 |
[MG_Remote, android-plugin] (P6): a real phone retraces a real trace across two processes
OpenRA.DirectGLES, transport=spawn, on a Redmi: ssim=1.000000,
mismatchPixels=0, zero Fatal{ lines, three runs for three. The server is a
process the client launched out of the APK's lib/arm64/, and the client's own
log names its pid.
Four things stood between the desktop green and the device green, and every
one of them was invisible on desktop for the same reason: a build tree is not
a deployment, and Linux is not Android.
1. THE SERVER COULD NOT LINK. It links libMobileGL.so and is EXEC'd, not
dlopen'd, so it gets a fresh linker namespace with the DEFAULT search path
- and an APK's lib/<abi>/ is not on it. `CANNOT LINK EXECUTABLE ...:
library "libMobileGL.so" not found: needed by main executable`, before a
single line of MobileGL ran. On desktop CMake had been giving the target an
absolute build-tree RPATH, so the WSL server resolved its dependency
through a path no phone has. $ORIGIN is asked for via BUILD_RPATH (a raw
-Wl,-rpath,$ORIGIN reaches the generator unquoted and the SHELL eats it -
the device linker said `normalize_path - invalid input: "$"`), and because
the NDK drops RPATH from the link anyway, the LAUNCHER states
LD_LIBRARY_PATH: it is the one component that has already resolved where
the image lives, and a pair that is not co-located is not a pair.
2. THE RENDEZVOUS NEEDED A WRITABLE DIRECTORY. An Android app has no /tmp, so
bind() in the child had nowhere to put the node and the client refused by
name 20 s later. AF_UNIX endpoints now accept the ABSTRACT namespace with a
leading '@' - the systemd/D-Bus convention - which has no filesystem
presence at all: no directory, no mode bits, and no stale node to unlink
after a crash, which is the bind() failure Listen() has been working
around. Its reach is the network namespace, which for an app is the app.
StartSpawned mints '@' names; an explicit path still works everywhere, and
that is what P12's "connect to something somebody else is listening on"
will use.
3. PBUFFER WAS A DirectGLES-ONLY KNOB. Twice over: trace-replay-ci.sh gated
`--ez use_pbuffer true` on the backend name, and the JNI demanded a native
window for DirectVulkan unconditionally. A surface SHAPE is not a backend
property, and the coupling mattered because the Android GLWS makes a window
surface whenever gNativeWindow is non-null - so handing Magma a window was
the same as asking for one. Until P12 a spawned server cannot be given a
window at all: an ANativeWindow* is a pointer into the CLIENT's process and
SetWindowHandle is refused on the way across with Fatal{UnmigratedSurface,
"AndroidNativeWindow@P12"} (Rule H). Which is exactly what happened, one op
after the server had come up green.
4. THE RUNNER LOOKED IN ONE OF THE TWO OUTPUT DIRECTORIES. assembleTraceRelease
writes apk/trace/release; CI has staged the same artifact under
apk/trace/debug. A tree that has built locally has both, and picking the
wrong one retraces last month's APK and says nothing about it. Newest of
either, now.
MEASURED AND NOT FIXED: DirectVulkan on this device diverges under BOTH split
transports and does so nondeterministically - inproc scored ssim=0.988998 /
9836 px, spawn scored 0.976494 / 14658 px and then 0.988998 / 9836 px on a
repeat, while monolith with the same pbuffer surface scores 1.000000 / 0 px.
The two transports draw from the same set of outcomes, so this is the split
path on Magma against a real GPU, not the process boundary: P6 neither caused
it nor can fix it. DirectVulkan spawn IS green on desktop (lavapipe, 1.000000).
G14: one test name added, AnAbstractEndpointNeedsNoWritableDirectoryAndLeaves-
NoFile, which pins both halves - the handshake crosses an abstract name, and
nothing is left on the filesystem. A '@' mistaken for a path would have bound
a file of that literal name, passed here, and failed on the one platform this
exists for.
|
||
|
swung0x48
|
557dc2c109 |
[MG_Remote] (P6/dl): D5c - a bell that can wake itself cannot hear a death
Measured while running the 78-entry spawn matrix, not predicted.
minecraft-1.21.4-fabric-iris-iterationrp-in-world kills the SERVER process
outright on this machine - llvmpipe cannot lower the shader pack's
%llvm.x86.vcvtps2ph.256 and aborts at call 159429 - and the client did NOT
take the SessionWait::ShutDown path, which already exists and is already
correct. It sat out the full 120 s and reported Fatal{BarrierTimeout,
"ResourceRespecify"}: the wrong diagnosis, at two minutes a case, for a
server that had been gone since the first second. The same trace fails in 3 s
under monolith.
The cause is structural and is the price of `sm`'s two-fd bell. The client
parks on clientBell[0] and rings ITSELF through clientBell[1], so it holds
both ends of that socketpair. EOF arrives only when every writer closes and
the client is one of them, so that descriptor can never hang up no matter
what happens to the peer.
`dl` therefore needs a witness descriptor whose far end only the server
holds, polled beside the park fd and never read from, whose POLLHUP/EOF
latches Dead(). The control socket already is exactly that, so no new
descriptor is needed.
This is NOT a defect in `sm`: holding both ends is what self-notification
requires, and without it the server's control pump could not wake its own
apply thread - the bug that cost this branch an afternoon. The mistake would
be assuming one descriptor can answer both questions.
Recorded rather than fixed: it turns no red case green, and the only case
that reaches it fails identically in all three arms for a reason that is not
MobileGL's. `dl` owns the fix and S2 already names the red-once for it.
|
||
|
swung0x48
|
534067493c |
[MG_Remote, android-plugin] (P6): the device can run the split retrace, and only the app can spell the server's path
Two small pieces, so an Android spawn arm is a flag rather than a project. THE RUNNER GAINS --transport. It already had a generic env passthrough - one intent extra carries `K=V;K=V` all the way to setenv before the library is dlopened - so the transport itself costs one entry and no new plumbing in the five files between here and the device. monolith passes NOTHING rather than MOBILEGL_TRANSPORT=monolith, so the default arm's environment stays byte-identical to what it was before this option existed: a control that changed when the option landed would not be one. THE SERVER PATH CANNOT COME FROM THE HOST. Every other knob can, but this one names a file that must be EXEC-ABLE, and on Android there is exactly one such place: an APK's lib/<abi>/. W^X has forbidden the app's own data dirs since API 29, and the packager only puts a file in lib/<abi>/ if it is named lib*.so - which is why the server executable wears that name, and what P0 spike A proved on two devices. nativeLibraryDir contains an install-time hash, so no host-side script can spell the path; the process about to launch the server is the only one that can. TraceReplayActivity resolves it, and only when the caller actually asked for spawn. AN EXPLICIT MOBILEGL_IPC_SERVER_PATH STILL WINS. A caller that set one is pointing somewhere deliberately - at a pushed build, say - and silently replacing it would make that knob untestable on the one platform where it matters most. The APK must be built with -Pmobilegl.buildDisaggregated=ON or none of this is reachable: without it ConfigLoader has no transport parser, accepts the value, ignores it (CONTRACT-P5 rule 5), and the run is monolith under a name that says spawn. The --transport help says so where somebody will read it. |
||
|
swung0x48
|
dc5bd935de |
[MG_Remote, CI] (P6): the split retrace lane becomes the whole CI matrix, both backends, both transports
It was one case on one backend under one transport: OpenRA, DirectGLES,
inproc. It is now every CI case, both backends, inproc AND spawn - 154 CI
entries, 156 local ctest entries - because every defect the spawn work turned
up was a question about the SERVER's process and not about any one trace or
either backend.
THE DirectVulkan ARM. tools/trace_replay/CMakeLists.txt carried "a
DirectVulkan arm would be measuring a server nobody has written yet". True
when P5 wrote it; false since P5f/P7 landed Magma's server consumers and its
run-ahead readiness gate. Leaving it would have shipped a split shape whose
second backend nothing ever ran - the same class of hole as a lane that runs
monolith under a name that says split. Magma now brings up a server process
of its own and OpenRA retraces across it at ssim=1.000000, which is only
possible because this branch also taught the child to load its config: before
that, a DirectVulkan session brought up a DirectGLES server and the two
halves disagreed about which backend the capability mask describes.
`split` BECOMES AN OPT-OUT. It began as an opt-in naming one case, because
one case was the work. Marking 39 cases `"split": true` would carry no
information and would reinstate the hole the key was written to close, only
mirrored: a case added later would get the monolith arms and be silently
absent from the split ones, and absent is the one state nothing reds on. A CI
case is now in the subset unless it says `"split": false`, which is a
reviewable claim that a trace cannot cross a role boundary.
THE CI MATRIX GAINS A THIRD DIMENSION. The job took {backend, case} and
hard-coded MOBILEGL_TRANSPORT=inproc in its own run block, so `spawn` had no
way into CI at all; the .SPAWN ctest entries existed only for a local build.
It is {backend, case, transport} now, the backends are the case's own
ci_backends rather than a hard-coded DirectGLES, and the artifact names carry
the transport or the two arms of one {backend, case} would overwrite each
other's uploads.
inproc STAYS AS THE CONTROL. Same library, same codec, same applier, same
picture; the only difference is which process the server role runs in.
Running only spawn would leave "did the transport break it?" and "did the
phase break it?" answerable only together - which is the question every
failure in this phase has turned on, including the SIGSEGV that D1 explained.
BOTH NEGATIVE CONTROLS FOLLOW THE ARM. retrace_pull_library_control.sh and
retrace_drop_draw_control.sh each exported MOBILEGL_TRANSPORT=inproc
themselves, so on a spawn job they would have gone on proving something about
the other arm while spawn ran unguarded. They read SPLIT_TRANSPORT now, and
default to inproc so a caller that sets nothing behaves exactly as before.
scripts/ci/control_smoke_test.sh: 22 passed, 0 failed against the stubbed
ctest, including the two modes that matter most - a red whose SSIM never fell
and a knob that armed and dropped nothing.
The spawn arm needs a second binary and the inproc arm needs none, so the job
places libMobileGLServer.so only on the spawn arm and a missing one is a
named failure there rather than a LaunchServer refusal 78 jobs deep.
Measured locally as this lands: 39 of the 78 SPAWN entries complete, all
green. The rest are still running.
|
||
|
swung0x48
|
7e244b3775 |
[MG_Remote] (P6/cp): a real trace retraces across two processes, pixel-for-pixel
OpenRA.DirectGLES.SPAWN is green: ssim=1.000000, mismatchPixels=0, zero
Fatal{ lines, against a server in a process the client started by name. The
inproc SPLIT arm stays beside it as the control, so "did the transport break
it" and "did the phase break it" remain separately answerable.
Five defects stood between the handshake and that picture. Each one was
silent in its own way, and none of them looked like what it was.
1. THE DOORBELL COULD NOT WAKE ITSELF. SocketDoorbell's one-fd form sends on
the end it polls, so a write is delivered to the PEER - correct for waking
the other process, useless for waking a thread in this one. The server's
control pump posts into the apply thread's mailbox and must ring the bell
that thread parks on. Symptom: every eglInitialize crossed, landed in the
mailbox, and timed out at 5 s. The two-fd form (park on [0], notify on [1])
is restored here, and the bell handover goes from two descriptors to THREE
- the client needs the same split for the same reason. Seven offers now.
2. InitCapabilities HAD NO WIRE KIND. CONTRACT-P6 §2 predicted this exactly
("the request has a live client caller and the codec refuses to encode
it"). SurfaceOpKind gains 11, append-only; the decode range's upper bound
moves with it, or the new tag reads as out-of-range and surfaces as
Fatal{ProtocolCorruption} rather than the missing row it is. The op's name
loses its InprocOnly suffix, which stopped being true here.
3. THE CHILD LOADED NO CONFIG AT ALL. mobilegl_server_main calls
InitServerRoleForSpawn directly, and MG_ConfigLoader::Init is a step of
MobileGL::Initialize - so every MG_Config value in the server was its
static default. MOBILEGL_BACKEND_TYPE survives the launcher's scrub on
purpose and was read by nobody: a DirectVulkan session would have brought
up a DirectGLES server and then disagreed with its client about which
backend the capability mask describes.
4. AND THEREFORE Transport READ AS Monolith, which CONTRACT-P6 §3.1 (D1)
forbids in as many words. a6 measured the cost: 226 live lines test
`Transport != Monolith` to select the SERVER arm, 148 in MG_Backend.
Monolith flips all of them to frontend glue in the one process with no
frontend. Observed precisely as predicted - UnitTexturesByHandle() answered
false, Clear took the legacy arm that dereferences borrowed FRONTEND slot
pointers, and the apply thread took a SIGSEGV inside SyncNeccessaryTextures
on the first Clear record of the trace. It is pinned in-process rather than
through the environment because the environment is where it must NOT be:
LooksLikeAClientEnvironment exits 65 on a surviving MOBILEGL_TRANSPORT, and
that check is anti-recursion catch (b). Dial, not Transport, is that axis.
5. THE LOG WAS CORRUPT, NOT MERELY INTERLEAVED. Two processes opened the same
MOBILEGL_LOG_FILE_PATH with "w" and each carried its OWN file offset, so
they overwrote each other's bytes - half-lines like `ote client: CapsMirror
generation 1 adopted`, and the server's whole startup block vanishing under
the client's later writes. This cost real diagnosis time twice, because a
missing line read as a code path not taken. O_APPEND now; the client still
truncates once so a fresh run is not a pile of old ones, and the server
never does - it would delete the very startup lines a failed session needs.
run_trace_case.cmake's Fatal{ census reads exactly this file, so it was
also reading a lie.
The retrace runner's split assertions become transport-aware: one row per
transport it admits, and a transport with no row is a FATAL rather than a
fall-through, because the fall-through is the accident the block exists to
prevent. The spawn row carries a SECOND sentence and it is the stronger half
- ConfigLoader's marker proves the value RESOLVED, while `spawn ARMED - the
server role runs in pid N` is written only after the launch, the connect and
the handshake all succeeded, and carries a pid that is not this process's.
inproc resolves its own marker while running the server role on a thread
here; nothing on that path can write this one.
G14: one test name added (TheEleventhFramedOpIsInitCapabilitiesAndItCrosses-
TheWire), none removed. The ten-op table keeps its name and its scope rather
than being renamed to eleven.
Two unit failures predate this change and are environmental in this tree:
PipeCatalogue.FrontendNeverTakesAnApplierAddress (the frontend source root
resolves empty) and SessionHandshakeTest.TheAbiFingerprintChangesWhenAnyOf-
ItsInputsDoes (GIT_COMMIT_HASH_SHORT is empty here, which a6 already recorded
as red in this configuration).
|
||
|
swung0x48
|
5239ae0c19 |
[MG_Remote] (P6/cp): EGL control frames cross to the other process, and answer
An eglInitialize issued in one process is dispatched in another and its answer comes back. That is the last thing standing between the session and a native context, because the server creates its context on the apply thread when the client's first eglMakeCurrent crosses - and until now nothing crossed. ONE SEAM, NOT TWELVE. a6 row A4-13 counted twelve Server* forwarders, each ending in ServerLoopInstance().RunSurfaceControlFrame(frame) with no role branch, and concluded twelve edits. They all funnel through that one function, so the branch goes there and the forwarders are untouched. A HOOK, NOT A CALL. ServerLoop::SetRemoteControlSink takes a function pointer the client installs at handshake. The obvious alternative - the server half asking ClientSession::Active() - would have added a fifth symbol to the six MG_Remote already owes itself (a6-link-experiment §5), in the one file with a clean reason not to. The branch is BEFORE the local seq mint, deliberately. RunSurfaceControlFrame numbers unnumbered local posts; a wire request must keep the CLIENT's number all the way through dispatch and reply or its answer cannot be correlated (CONTRACT-P6 §6.3, which says the client mints, always). The client sink encodes with EncodeSurfaceOpFrame, sends, and waits for the SurfaceReply carrying its own seq - under one mutex, because the twelve forwarders are already caller-serialised on the server's m_callerMutex under inproc and two replies racing for one correlation is the same bug on either side. A CapsSnapshot arriving while it waits is ADOPTED rather than dropped: the server republishes caps from inside its MakeCurrent dispatch (a6 row A4-30) so a snapshot legitimately precedes the reply to the op that produced it. The wait is bounded and its expiry is NOT fatal - a peer that is gone is a different fact from a peer that is slow, and only the doorbell's death latch may say which. The server's pump decodes and applies through ServerApplyWireSurfaceOp, which a6 found had no production caller anywhere. It runs on ServerMain's own thread - NOT the apply thread - which is what §6.2 asks for: a control arrival must not wake the applier, and frame decode must not happen on the thread holding the native context. A frame that is not a verifiable SurfaceOp ends the session rather than being ignored, because a silent drop is the failure mode this project refuses everywhere else. THE TEST'S ASSERTION IS THE OUT-PARAMETERS, not the EGL result. A headless machine has no display to initialise, so the op is expected to be refused - and ServerInitializeEGLDisplay writes major/minor "whenever the dispatch ran, success or not" (the forwarder's own comment). They move off -1 if and only if a SurfaceReply came back from the other process; a transport failure returns before the write-back. I got this backwards first and the test said so. Gates: ServerSpawnTest 5/5; unit suite 2345 with the same two pre-existing environmental failures; G1 pull 0 added / 0 removed, .text 10822147 unchanged. Not TCP. It was considered and dropped for now: TCP has no SCM_RIGHTS, so the four segments cannot cross it, and a TCP control plane would have to refuse a data plane by name until StreamLink exists (P6.5). Recorded here so the reason is on file rather than re-derived. |
||
|
swung0x48
|
11807ac7d0 |
[MG_Remote] (P6/sm): two INDEPENDENT processes, joined by a name, sharing memory
The cross-process session is up. A server process is started on its own with an
endpoint and nothing else, it listens; a client started separately connects to
that name; Hello crosses, Welcome comes back with four SegmentRefs, six
descriptors cross by SCM_RIGHTS, and the client maps memory another process
created. ClientSession::Active() is non-null at the end of it, which is the flag
that arms the thirty-seven wire emitters.
INDEPENDENT, NOT FORKED - and that is a correction to this package's first
shape. The version committed an hour ago forked the server and handed it fds
3..6. It worked, and it was less code, but inheritance can only ever produce a
server that is a CHILD OF ITS CLIENT, which is the one arrangement the end state
cannot use: there the server is a standing application and the client is
somewhere else entirely, possibly on another kernel. CONTRACT-P6 §3.1 called
this Dial == Connect and filed it as a later shape. It is the primary one, and
fork/execve survives only as a way to START a process - every descriptor it used
to carry is gone.
What that costs and buys, concretely: the rendezvous becomes a NAME, so
SocketTransport grows Listen/AcceptPair/ConnectTo, with a bounded connect retry
that is the CLIENT's (a launcher that waited for the server to be ready would be
coupling the two again by the back door). The bells stop being inherited fds and
are created by the server and handed over with the segments - slots 4 and 5 of
six SCM_RIGHTS offers. And the client connects TWICE to the same path: an fd
offer is a sendmsg whose ancillary data rides with specific bytes, so it may not
share a socket with the framed control stream, or the frame reassembler and the
descriptor receiver race for the same bytes.
The pieces behind it:
- SessionSegments::AdoptFromDescriptors, which is the same Adopt+Map loop
AttachInProcess has run on every inproc session since P5. That was the point
of writing the inproc attach as dup+Adopt+Map rather than as an alias: only
the origin of the descriptor is new.
- ServerSession::SetExternalDoorbells, the narrow fix for a6 row A7-1 - the
§3.9 accessors abort for every role but InProcess, so a spawn apply thread
died before popping its first record. lk deletes them; until then an injected
pair is honest about where the bells come from.
- MG_Backend::InitServerRoleForSpawn: steps 1 and 2 of the split bring-up
extracted so the server process runs the SAME code the inproc server role
runs. The capability bits are a statement about the server's own backend and
a second copy would diverge the first time a backend registers a slot.
- ClientSession::FinishStartup: everything below the attach - producer, reply
pool, event ring, segment table, encoder, first CapsSnapshot, emitter tables -
shared verbatim between inproc and spawn. Its one parameter is not a
transport question: under inproc the client starts the apply thread because
there is no server-side main; under spawn there is one, and it started the
thread before it sent the first descriptor.
G1 CAUGHT A REAL LEAK on the way: InitServerRoleForSpawn was guarded on the
inside, so the pull build still DEFINED the symbol - 2 added, 1 removed, .text
+16 bytes. The whole function and its declaration are now inside the guard.
"It returns false there" is not the same as "it is not there", and the gate is
what said so.
Gates: ServerSpawnTest 4/4; unit suite 2344 with the same two pre-existing
environmental failures; G1 pull 0 added / 0 removed, .text 10822147 unchanged.
|
||
|
swung0x48
|
70c45ab9ce |
[MG_Remote] (P6/sm): two MobileGL processes, talking
The first time this project has had a second process. fork + execve of a real server image, the control socket surviving the exec, bytes crossing a process boundary, and the child exiting on EOF with nothing left behind. mobilegl_server_main EXISTS NOW. a6 found ARCHITECTURE.md:488-496 had described it for three phases while the symbol was nowhere in the tree, and the only MobileGLServer target was the P0 spike stub, which links no MobileGL library at all. The spike keeps its evidence and its name becomes MobileGLServerSpike; the real image links the library, so an undefined symbol is a build error rather than a dlsym failure in a child whose stderr is /dev/null. WHAT sm OWNS AND WHAT IT LEAVES: the process, the inherited descriptors, the transport and the exit discipline. Not the session, not the applier, not the four segments - wiring those from ServerMain before lk has a seam to wire them THROUGH is how call sites get written twice. The loop is a read/answer/exit-on- EOF stand-in whose SHAPE is the part that matters: one reader, EOF ends the process, and no timeout is ever allowed to stand in for the death fact (§5.4). ANTI-RECURSION IS TWO CATCHES, and neither is ARCHITECTURE.md:488's. That rule says the child hard-sets Transport to Monolith; a6 measured that 226 live lines test `Transport != Monolith` to select the SERVER arm, 148 in MG_Backend, so obeying it flips the whole backend to frontend glue in the one process with no frontend. Instead: (a) MOBILEGL_IPC_DIAL=no, structural and separate from role, and (b) MOBILEGL_TRANSPORT and every MOBILEGL_IPC_* scrubbed from the child's envp. The child refuses with 64 for (a) and 65 for (b) so the two safeties are distinguishable - two that cannot be told apart are one. TWO BUGS WORTH RECORDING, both found by the test rather than by review: dup2(n, n) IS A NO-OP THAT DOES NOT CLEAR CLOSE-ON-EXEC. FdPassing sets SOCK_CLOEXEC on the aux pair, so the child depends on dup2 to clear it - but in a gtest process a fresh socketpair lands on exactly fds 3..8, so a source fd often IS its own target. The no-op left CLOEXEC set and the following close() then shut the descriptor we had just "installed". The child came up with no socket and the parent saw TRANSPORT_CLOSED on its first send. Now spelled out rather than written cleverly. THE CHILD'S DIAGNOSTICS WERE INVISIBLE. WireLogError reaches the file sink only - Defines.h builds the logger with the console sink off - so a child that refused to start said so into a file the parent does not know the path of. ARCHITECTURE.md §15.2 warns about exactly this on Android. Both refusal paths now also write stderr directly, and both carry a distinct exit code the parent can name without reading anything. TESTS: six cases, 6/6. Each exists because something would otherwise pass silently - the reply carries the child's own getpid() so a test that talked to itself cannot pass (ID-124's lesson at the smallest scale); the process tree is COUNTED before, during and after, not promised; the child is proven not to exit while merely idle before it is proven to exit on EOF; an unresolvable image is a named refusal that spawns nothing (S1); a surviving client knob is caught by name (S3); and a descriptor crosses execve by SCM_RIGHTS, which is the mechanism the four segments will ride on and is now exercised before it is needed. Also records the asio decision in the contract: declined for P6 because spawn needs five platform mechanisms and asio covers one - its own socketpair returns operation_not_supported on Windows, it has no descriptor passing, segment delivery is POSIX-only by design, and there is no fork. Revisit at P6.5, where asio::generic::stream_protocol carries a raw family/type/protocol triple and therefore gives AF_VSOCK from one implementation. Gates: unit suite 2346, up 6, same two pre-existing environmental failures. G1 pull 0 added / 0 removed, .text size 10822147 unchanged, and zero spawn, server or SocketTransport symbols in the pull build. |
||
|
swung0x48
|
5f846d96e3 |
[MG_Remote] (P6/so): SocketTransport, green over a real socketpair before any second process
The first P6 code that runs. ITransport over a connected AF_UNIX SOCK_STREAM socket, with SCM_RIGHTS descriptor passing on a dedicated SOCK_DGRAM aux pair. No shared memory: this is the CONTROL plane, and the hot path bypasses ITransport entirely by its own header's account. What the socket carries is the handshake, surface ops, resync, aux requests and fatals - and, once sm exists, the four segment descriptors that let the peer map the data plane. It moves the keys, not the bytes. CONTRACT-P6 §8 and the spawn plan both require this green over a plain socketpair with two threads BEFORE a child is ever forked, for the reason SessionRings.h gives about the attach half: a transport first exercised on the day the process appears is a transport whose bugs all arrive at once, on the hardest day to debug them. That requirement is now met. It reinvents nothing. Framing.h already had AppendFrame and a real byte-stream reassembler with exactly the BUFFER_TOO_SMALL keep-the-message semantics ITransport specifies - written for this and never before driven by a socket. FdPassing.h already had SCM_RIGHTS over a dedicated datagram pair. What is new is ~250 lines joining them, plus the platform refusal: Windows has no SCM_RIGHTS and ShmSegment::Adopt is POSIX-only, so CreatePair refuses BY NAME at construction rather than handing back a transport that fails every later call. SOCK_STREAM rather than SOCK_SEQPACKET, deliberately: Doorbell.h:390-398 already chose a stream socket so a closed peer surfaces as POLLIN|POLLHUP with recv()==0 rather than as a hang, and the control plane may not disagree with the bell about what death looks like. Framing.h is what makes a stream safe - the message boundary is in the payload, not in the socket. TESTS: the same nine ITransport-contract cases InProcessTransportTest runs, copied rather than shared. Folding both suites into one parameterised fixture would rename the sixteen existing cases and G14 says names only grow; and the two suites do not share a handle anyway, since InProcessTransport's tests reach into condvar doorbells this class does not have. Nine names added, zero removed. The seven Doorbell* cases are deliberately absent: they exercise CondVarDoorbell, not ITransport. Plus three a stream has and a message queue cannot: a 256 KiB payload that provably crosses in several reads; a garbage stream that must latch failed and must NOT resync onto a boundary it invented; and adopting an already-connected fd with no aux channel, which answers UNSUPPORTED to ShareFd rather than pretending - rule G, a transport declares what it supports. Red-once, both run: drop the message on BUFFER_TOO_SMALL and six cases go red; make the framing latch recoverable and AGarbageStreamIsLatchedFailed fails by name. Gates: 12/12 in 677 ms. Unit suite 2340 tests, up 12, with the same two pre-existing environmental failures. G1 pull 0 added / 0 removed, .text size 10822147 unchanged, and zero SocketTransport symbols in the pull build. One note for the VM track this eventually serves: swapping AF_UNIX for AF_VSOCK is one line here and nothing at all in Framing. It is the shared-memory half that does not cross a kernel. |
||
|
swung0x48
|
db62d674d0 |
[MG_Remote] (P6/lk): regroup RingControl by writer, and make the asserts that pin it falsifiable
The shared control page both roles map had a mixed-writer cache line:
{appliedSeq, submittedSeq, retiredSeq, completedFrameSerial, presentAckSerial}
sat together, but submittedSeq is producer-written (Ring.h's "THE FIVE
WATERMARKS") while the other four are consumer-written. That is why no
single-writer progress block could be aliased over it, and it is what this
package fixes: submittedSeq moves up beside cmdHead, the other thing the
producer writes, and the four that remain become a named LinkProgress member -
the struct c6 declared in ILink.h - so a data plane with no shared page can own
the same four values and every wait predicate in SessionRings.h keeps ONE
implementation at source level.
The two event flags deliberately stay in the doorbell group. Promoting them
would put the client's per-drain exchange(0) on eventRingFull (EventRing.h:200)
onto the line the client's own spin predicate reads 1e5-1e6 times a frame.
73 call sites rewritten, 15 in the library and 58 in tests. The rewrite is
receiver-scoped rather than field-scoped, because SplitRuntimePeek.h:77 has its
own struct with a field of the same name and SplitRuntimePeek.cpp:146 has both
on one line - a field-scoped rename would have silently broken the peek struct
that MagmaRunAheadScenario reads nine times.
THE ASSERTS ARE THE PART THAT NEEDED A SECOND PASS. The first revision shipped
four aggregate asserts and an adversarial review broke them by counterexample:
swapping retiredSeq and completedFrameSerial inside LinkProgress compiled
clean, passed all four, and passed the runtime layout test too - because
appliedSeq stays at offset 0. Three wire-visible offsets on a shared page were
pinned by nothing. Nothing else in the tree covers it either: the ABI
fingerprint takes no input from this page, and sizeof(RingControl) is 4096
whatever the members do, because alignas(4096) makes it so.
So the block is now per-field as CONTRACT-P6 §8.3 actually asked, plus a
standard-layout precondition, a tail bound that alignas cannot satisfy for us,
and the LinkEventFlags mirror. Each was run red: the field swap reds the
per-field asserts by name, a fifth member inside LinkProgress reds sizeof.
One of them cannot go red, and Ring.h now says so rather than implying
otherwise. A SMALL member inserted between Progress and serverEpoch lands at
offset 288, inside Progress's own line, and serverEpoch's alignas(64) absorbs
up to 32 bytes of it - so both line asserts still pass. The review's proposed
fix does not catch it either; I tried it. A gate nobody can falsify is worse
than no gate, so the limit is written down beside the asserts.
Also: RingTest.ControlPageLayoutIsTheSharedContract said nothing about the one
field this package moved, and now asserts the producer/consumer separation and
the event flags' exclusion itself rather than borrowing the claim from Ring.h.
Ring.h grew 30 lines, so the Ring.h:NN citations this package and c6 authored
are re-anchored to named blocks instead of line numbers. ARCHITECTURE.md §11.2
is updated for the new layout, and a factual error introduced in the previous
revision of that paragraph - submittedSeq annotated as releasing staging, which
retiredSeq does - is corrected.
Gates: pull 0 added / 0 removed, .text size 10822147 unchanged; split 0/0,
.text size 11681395 unchanged; unit suite 2326/2328 with the same two
pre-existing environmental failures as the baseline. The five edits behind
MGITEST_SPLIT_RUNTIME_PEEK were proven compiled by a red-once, not assumed.
Note on the .text claim: that is a SIZE, not the bytes - field offsets moved,
so immediates moved with them. And this page's layout IS observable across
binaries; what is unobservable is behaviour within one binary. The contract
already books the layout change deliberately, before spawn exists to care.
|
||
|
swung0x48
|
99649a9087 |
[MG_Remote] (P6/lk): the pre-flight itemisation, and a correction to c6's regroup design
CONTRACT-P6 §8.5 requires lk to publish its blast radius before it starts, because an earlier draft's ratchet baseline was wrong by a factor of three and a ratchet installed on the wrong number is red on its first CI run for reasons that have nothing to do with a leak. The itemisation is mechanical, per file, with the grep spelling stated: 250 seam-identifier references outside Transport/ across 23 files (127 production, 123 test), and 44 member-access sites that the regroup itself must rewrite, across 9 files - 17 of them in MG_IntegrationTest, which is why lk builds with the integration tests ON. The earlier build did not, and a missed edit there would have been green. THE CORRECTION. Measuring that radius turned up a mistake in c6, landed two commits ago. §8.3 and ILink.h both said lk should promote eventRingFull and eventDropped into the watermark group and called the result single-writer. eventRingFull has TWO writers: the server latches it with store(1) when SEG_EVENT fills (EventRing.h:140) and the CLIENT clears it with an exchange(0) on EVERY DRAIN (:200). Promoting it would put a per-drain read-modify-write by the client on the same cache line as appliedSeq - which the server release-stores every 64 records and the client's spin predicate reads 1e5-1e6 times a frame. That is new false sharing on the hottest line in the system, and it would land in exactly the numbers this package's gate has to hold constant. It is also the same class of error the earlier draft made with submittedSeq, one field over. So LinkProgress is now the four consumer-written watermarks and nothing else - genuinely single-writer - and the two flags stay in the doorbell/generation group, where mixed writers already live beside consumerParked/producerParked, crossing the seam through a separate EventFlags() accessor on its own line. The contract records the correction and its reasoning rather than quietly restating the rule. ALSO CORRECTED, from running the baseline suite: a6's link-experiment report said the silently-empty build stamp had "no log and no gate to say so". That is false. SessionHandshakeTest.cpp:140 asserts BuildStamp[0] != '\0' and :180 asserts that perturbing it changes the fingerprint, and BOTH GO RED in this configuration right now - the worktree's .git holds a Windows path WSL's git cannot resolve, so GIT_COMMIT_HASH_SHORT is "". The finding is therefore stronger, not weaker: the gate that catches it already exists and is already red. What is silent is the BUILD, and hs owes a named configure-time failure rather than a new gate. Baseline for lk, recorded: 3878 tests, two failures, both environmental and understood - the fingerprint one above, and PipeCatalogue.FrontendNeverTakes- AnApplierAddress, which walks up from cwd looking for MobileGL/MG_Impl and so fails for any out-of-source build directory. No behaviour change. The headers are still included by nothing. |
||
|
swung0x48
|
b6293957e9 |
[MG_Remote] (P6/c6): the normative contract, and the data-plane seam declared but not wired
c6 is P6's contract package. It lands CONTRACT-P6.md plus two headers and changes nothing: no caller moves, no byte moves on any wire. THE CONTRACT supersedes P6-CONTRACT-DRAFT.md, which was written before its own audit and is wrong in four places - the ABI row, "the latch is implemented unchanged", "cp is transport only", and "a transport swap and nothing else". a6 measured all four. It carries 21 rulings, of which the load-bearing one is D1: the spawned child keeps Transport == Spawn and anti-recursion becomes a separate Dial axis. ARCHITECTURE.md:488's literal instruction is amended, because 226 live lines test `Transport != Monolith` to select the SERVER arm and forcing Monolith flips 148 MG_Backend lines to frontend glue in the one process that has no frontend. That ruling can be stated normatively without changing behaviour because of something a6 found and the contract exploits: TransportMode::Spawn is UNREACHABLE at this head. ConfigLoader.cpp:344 is the only writer for the string "spawn" and it writes Monolith. So every rule conditioned on Transport == Spawn constrains a state no process can enter - and the same fact is why ConfigLoader's named refusal cannot currently falsify itself, which is sm's to fix and §9.5's arm-proof gate to catch. THE SEAM. ITransport says in its own header that the hot path bypasses it; below it there was no interface of any kind, because Ring.h, ReplySlot.h and EventRing.h each take a raw void* plus a capacity. ILink.h is that missing thing. It is setup-time, not hot-path: no method is called per record or inside a spin predicate, because Doorbell::Wait's predicate runs 1e5-1e6 times a frame reading watermarks straight off the page, and because the only lane that could measure a virtual there has four times the dispersion of the effect - a gate that can only print "within noise" is ID-124 one level down. StreamLink.h is declared and implemented by nobody, and that is its job: ILink's adequacy is a claim only a compile can settle. Writing it forced three methods the brief did not name - ResolveSpan, FlushProgress, and the reply pair - so it has already earned its keep. Delete one ILink method it declares and the build goes red. Deliberately NOT here, each for a reason the contract records: Watermark::Reached is not redefined (SessionRings.h owns it); Ring.cpp's reserve/pop bodies are not relocated into a header that would then need the logger; RingControl is not regrouped, because that rename is 109 production sites with five behind a non-Android #if, so it is lk's with a published itemisation; and the schema rows are specified but left to hs, because deprecating SurfaceReply.defaultFb while appending `result` changes CreateSurfaceReply's arity and the existing six-argument call would silently bind its defaultFb 0 to result. INERTNESS IS STRUCTURAL, not measured. Zero object dependency files reference either header, zero build rules do, and nothing in the tree includes them. Verified anyway on both flavours: pull and split each have 0 symbols added, 0 removed, and .text byte-identical (10822147 / 11681395). check_doc_citations resolves all 61 citations. |
||
|
swung0x48
|
c25a776090 |
[Docs] (Disaggregated): P6 a6 close-out, and the end-state review it was re-scoped from
a6 is the read-only audit P6-SPAWN-PLAN puts first. Seven static items, each
produced by one auditor and re-resolved line by line by one adversarial
verifier; 198 rows, 65 of them contested, and only the surviving ones carry
into the report. Item 5 - can the server link without MG_Impl - is the one
experiment, and it is answered.
The headline is that P6's transport half is smaller than the plan assumed
(fc and fs really did land) and its process/role half is much larger. Four
statements in P6-SPAWN-PLAN / P6-CONTRACT-DRAFT are false at this head:
- `spawn` is not an observable state at all. ConfigLoader.cpp:339-346
recognises it and then writes Transport = Monolith, so "the child was
forced to monolith" and "the parent fell back" are the same value and
that named refusal cannot falsify itself. Meanwhile 226 live lines test
`Transport != Monolith` to select the SERVER arm, 148 of them in
MG_Backend - so ARCHITECTURE.md:488's rule, implemented literally, flips
the whole backend to frontend glue in the one process with no frontend.
- `cp` is not transport-only: the InitCapabilities REQUEST has no wire form,
and the codec refuses to encode the kind a live client still calls.
- the device-lost latch is not implemented; it was booked as carriage.
- the server cannot link without MG_Impl: 184 symbols, 177 of them real
function calls, owed 101 to P7 / 14 to P3b-P4b / 60 shared / 6 to P6.
There is also no module target to link, so the question could not have
been answered by editing a link line.
Four more blockers the earlier review missed: the apply thread aborts in
ConsumerDoorbell before popping its first record (contract 3.9's ruling, which
lk should retire rather than amend); neither side of the control plane is
wired to a socket; the control-reply wait has no deadline, so a frozen peer
wedges the GL thread inside eglMakeCurrent; and SurfaceReply cannot carry a
MobileGLResult, so a client cannot tell a dead server from a legitimate
refusal.
The abort census makes dl's funnel a redesign rather than a rename: 92 sites,
30 distinct family words against a 7-value wire enum, and three unbounded
peer-driven allocations with no try/catch anywhere under MG_Remote.
P6-ENDSTATE-REVIEW.md is the re-examination of P6 against the gfxstream-shaped
end state that produced a6's three added items. a6 overturned four of its own
claims - the MG_State eight's shape, the abort count, m_windowHandle's
injection points and a predicate name collision - and those are corrected in
place rather than left to be rediscovered.
Docs only. No code, no gate, no behaviour.
|
||
|
swung0x48
|
b3b9c2ee12 |
[MG_Backend] (DirectGLES): the guard comment cites the visibility block it means
Two comments from the previous commit, corrected. The CMakeLists.txt reference in DirectGLES.cpp pointed at the link libraries instead of the Debug/Release visibility preset it names (707-719, not 629-641). The Managers.h note said the overload is "reached by the pull, verify and push build" through "the three wrappers", which reads as if all three called it; only VertexArrayImpl::SyncClientSideVertexArraysForFetch does, and what those flavors compile is the declaration, which is the point being made. No code change: the four flavors' libraries are byte-identical to the previous commit's (comment-only). |
||
|
swung0x48
|
3e3405e3a8 |
[MG_Backend] (DirectGLES): the monolith client-array snapshot compiles in every flavor
The indexed and indirect client-memory upload added by
|
||
|
swung0x48
|
e56bf610b5 | Merge remote-tracking branch 'origin/codex/fix-github-ci' into feat/disaggregated | ||
|
swung0x48
|
eae947050f |
[CI] drop the APK disaggregated-proof step
The step located the Android CMake identity by reading CMakeCache.txt next to each compile_commands.json, but the trace flavor's build also produces .cxx/tools/<flavor>/<abi>/compile_commands.json copies whose cache lives in the real per-ABI build dir, so the proof failed on a file-layout assumption rather than on missing evidence. Remove the step and its artifact upload. |
||
|
swung0x48
|
f99fbf717e | Merge remote-tracking branch 'origin/codex/fix-github-ci' into feat/disaggregated | ||
|
swung0x48
|
eb2747c966 | [Docs] (Disaggregated): record the CI/inproc close-out in the roadmap and stage status | ||
|
swung0x48
|
e1bf36775f | Merge remote-tracking branch 'origin/codex/fix-github-ci' into feat/disaggregated | ||
|
swung0x48
|
0689fc9637 |
[Docs] (Disaggregated): the P5b review row keeps its historical 30 s and points at the current 120 s
MEASUREMENTS 7.3 records what
|
||
|
swung0x48
|
68b55ea371 |
[MG_Remote] (Disaggregated): the inproc barrier watchdog is a deadlock detector, so its budget leaves room for a leg
lavapipe/llvmpipe stall the apply thread for tens of seconds inside texture-handle
registration (vkCreateImageView -> lvp_CreateImageView -> llvmpipe_register_texture,
a JIT/futex path): one measured stall is 29.3-39.7 s and a whole inproc retrace run
accumulates ~44.6 s of it. At a 30 s bound those runs red intermittently on
Fatal{BarrierTimeout, "ResourceRespecify"} while the same stall under monolith is
merely slow - monolith has no cap on that path. Since the monolith shares the stall,
the bound, not the workload, was the defect.
kBarrierTimeoutMs becomes 120000, >3x the largest observed stall, which still leaves
the watchdog job: a record that is never retired still terminates the run and the
Fatal still names the seq. The comment now says deadlock detector rather than
performance gate and carries the measurement.
The fake-clock case that pins the budget moves with it: FenceWaitBudgetHonorsGl-
TimeoutAndFiniteTransportChunks expected 30000/30001/90000 and now expects
120000/120001/180000, so the assertion still pins the value instead of drifting.
Texts that wrote the old number down are updated: ARCHITECTURE 17.1, CONTRACT-P5E
2.4 (which now names kBarrierTimeoutMs rather than the literal), and the two
bring-up comments in PipeRoute.h / WireTables.cpp that dated a self-wait to
"thirty seconds".
|
||
|
swung0x48
|
17bbb4482c | [Build] (Android): keep the embedded-FCL arm64 pin beside the disaggregated wiring | ||
|
swung0x48
|
d56c0e8346 | [MG_Backend] (DirectVulkan): the shared depth/stencil packer's call site keeps the file's argument alignment | ||
|
swung0x48
|
a7d7e2bc44 |
[MG_Backend] (DirectGLES/DirectVulkan): the stencil-only scratch attachment and the aspect staging buffer's lifetime
Three follow-ups the first pass needed: DirectGLES glGetTexImage has to attach a STENCIL_INDEX texture at GL_STENCIL_ATTACHMENT. It went through the colour branch, which left the scratch READ framebuffer incomplete for a stencil-only texture, so the readback bailed out before the new stencil branch could run. ScratchFBOImpl::EnsureDepthAttachment2D gains the stencil-only point; the shadow remembers which of the three points it holds so a later depth attach is not deduped against it. DirectVulkan's aspect staging buffer is recorded into the current frame's command buffer and outlives the glBlitFramebuffer call that records it, so it retires with the frame. VkBufferManager::DeferRelease is the manager's own deferred-release entry point and moves to its public section for that. |
||
|
swung0x48
|
5264f9a966 |
[MG_Backend] (DirectGLES/DirectVulkan): the monolith readback and resolve arms get the depth/stencil fixes the wire arm already had
|
||
|
swung0x48
|
e258a822e4 | [MG_Backend] (DirectGLES): one blank line between the two upload arms | ||
|
swung0x48
|
cb64b62b57 |
[MG_Backend] (DirectGLES): both client-snapshot wrappers are monolith-only
The indexed wrapper reads the bound VAO to tell a client index array from an
element-buffer offset, and the indirect one reconciles the command buffer's
shadow - both frontend reads, and under a transport the entry points run on the
apply thread where the client owns those bytes (Fatal{RoleViolation}, which the
split lane aborted on).
|
||
|
swung0x48
|
82f509c535 |
[MG_Backend] (Magma): reconcile a shader-written element buffer before the draw records
The client-memory upload's index scan waits for the GPU (SyncGpuWrites), and that wait flushes the batch the frame's command buffer belongs to - so a handle taken before it is stale by the time the draw records into it. Reconciling in the entry point, before SetupDraw starts recording, keeps the wait out of the recording. |
||
|
swung0x48
|
64182b76dc | [MG_Backend] (Magma): the indirect client-array staging block is monolith-only | ||
|
swung0x48
|
92fe12a2f4 |
[MG_Backend] (DirectGLES): the indexed client snapshot is monolith-only
The backend draw entry points run on the apply thread when a transport is live,
where MGB_CTX->GetBoundVertexArray() is a frontend read the server role must not
make (Fatal{RoleViolation}) - which is what the split lane aborted on. With a
transport the CLIENT owns those bytes and snapshots them from the GL thread, so
the server has nothing to stage.
|
||
|
swung0x48
|
e550d10339 |
[MG_Backend] (Magma): an indirect draw's client-memory arrays are staged from the commands
The commands live in GPU memory, so the draw is issued natively and the vertex range was left unknown (vertexCount = 0, instanceCount = 1). That is exactly right for a buffer-backed array and wrong for a client-memory one: its bytes must be in the store before the GPU reads the command, and the range the upload is bounded by came out as one element - so the quad the command named was never staged and the draw painted nothing. The words are now resolved on the CPU for exactly that case, with the shadow reconciled first (they may be shader-written), and the draw stays native. |
||
|
swung0x48
|
cf7366276e |
[MG_Backend, MG_Impl] (DirectGLES, Magma): the monolith arm's client fetch bounds
DirectGLES: the client snapshot's baseInstance shift belongs to the backend's emulation, not to the element set. A driver that applies baseInstance itself (UseNativeBaseInstance) must not also receive a shifted pointer - doing both moved instance 0 from element 1 to element 2 and left the instance that reads element 1 outside the snapshot. EmulatedFetchBaseInstance is not declared this early in the file, so the wrapper states the same answer inline. Magma: ResolveIndirectCommandBytes reconciles a GPU-written command block before the callers read it. Every one of them reads the words on the CPU - the per-command expansion, the baseInstance the shader emulation is fed, and the range a client-memory vertex array's upload is bounded by - so a command a compute shader wrote this frame was expanded as the poison the shadow still held, which is no draw at all for glDrawArraysIndirect. Magma: a client-memory INSTANCE-RATE binding is bounded by its instances (baseInstance + (instanceCount - 1) / divisor), not by the vertex range; the upload now covers the union of the two, which is what instances shifted off the front of the array needed. |
||
|
swung0x48
|
ae418e80a0 |
[MG_Backend] (DirectGLES): the client snapshot's baseInstance shift is the emulated one
The element set a divisor'd client array fetches starts at GL's raw baseInstance, but the pointer shift that carries it belongs to this backend's emulation: when the driver applies baseInstance itself (UseNativeBaseInstance), EmulatedFetchBaseInstance answers 0 and shifting the pointer as well would move the fetch twice - instance 0 read element 2 instead of element 1, and the instance that used element 1 read past the snapshot and vanished. Also drops the two 'continue' statements the indirect executors' lambdas cannot carry (a lambda is not a loop; 'return' leaves the pass). |
||
|
swung0x48
|
62bfe4615d |
[MG_Impl, MG_Backend] (Disaggregated): the monolith arm fetches the client arrays a draw names
The wire arm snapshots the elements a draw fetches out of the application's own vertex/index memory (beba0256's MGPipeOwnedDrawInputs). The monolith arm's upload answered only the non-indexed family's contiguous (first, count) run, so an INDEXED or INDIRECT draw over a client-memory array staged nothing: the enabled array kept whatever the ES context last held for it - normally disabled, i.e. every vertex reading the generic current value - and the draw painted nothing with no GL error anywhere. ClientFetchPlan.h is that question factored out once (the fetches, the referenced vertices, the per-divisor element sets) so both arms answer it from one place; OwnedDrawInputs now calls it and keeps its behaviour byte for byte. DirectGLES: SyncClientSideAttributesForDraw stages the sparse snapshot for a draw whose elements the (first, count) run does not describe, driven by the plan; the indexed entry points and both indirect executors ask for it, and the arrays path learns the baseInstance shift a divisor'd client array needs. W3: DirectGLES' six monolith failures are the four indexed/indirect shapes. |
||
|
swung0x48
|
e4ae19643d |
[CI] (Disaggregated): P7's packed-float mip gate flips to green, by name
|
||
|
swung0x48
|
3542f64f8c |
[MGPipe] cover MGPSubData's four appended extent words in PipeFields.def
|
||
|
swung0x48
|
0fa4a98139 |
[MG_Backend] drop the invented point count from the handle arm's memo comment
The record carries eleven surfaces, but the walk writes the attachment points only - eight colour plus depth and stencil - and the read surface is applied by ApplyReadBufferFromRecord rather than by the walk. Say "the SAME attachment points on it" instead of a count that is wrong for the walk. |
||
|
swung0x48
|
fd30b5c30a |
[MG_Backend] (Magma): keep a view's mip window and readback inside the layers it names
Magma's generate path (VulkanRenderer::GenerateMipmap) took its window from the
BOUND object and its blits from the STORAGE image, and the two do not coincide
for a texture view: glTextureView gives a view its own BASE_LEVEL/MAX_LEVEL and
layer slice over somebody else's image. Three things followed from the mix-up,
all visible through a two-layer view over a four-layer 2D array:
* EnsureGenerateMipmapStorageAllocated allocated the chain THROUGH the bound
object from that object's OWN base extent, so the storage owner's level was
resized to the view's layer count - the storage behind its other layers was
gone - and the levels outside the view's window were redefined with it.
Immutable storage and views already own every level generation can write,
so neither allocates anything now, and a mutable owner gets only the levels
its window is missing (AllocateStorage RESIZES the level it is handed, so
an existing level keeps the shadow bytes its own upload path wrote).
* The level the chain starts at was the view's, applied to the storage image:
a view whose MIN_LEVEL is 1 generated from the storage's level 1 instead of
its level 2. The window now comes from the frontend's own plan
(MipmapGenerationPlan.h - the same numbers MGPMipPlan::LevelCount publishes)
and is shifted into the storage's numbering before any blit or barrier.
* Every layer of the storage was written instead of the view's window. The
blits carry the view's baseArrayLayer and layerCount now; every layer
outside that window belongs to the storage owner and is left as it was.
The end of the window follows MAX_LEVEL for the same reason the frontend's plan
does: a chain MAX_LEVEL cuts short is no longer generated past it.
The readback had the same shape of bug pointing the other way.
VulkanRenderer::GetTextureImage sized its copy by the storage image's array
layer count while starting it at the caller's layer origin, so a two-layer view
of a four-layer array copied four layers - two outside the window, and the last
one past the end of the image. glGetTexImage carries no destination size, while
the size glGetTextureImage validates is built from the VIEW's own extents, so a
caller that sized its buffer for the view had it overrun by exactly the layers
outside its window. The copy covers the caller's own layers now.
TextureViewScenario.GenerateMipmapThroughAViewLeavesTheOwnerShapeAlone runs on
both backends now. Its DirectVulkan skip named the reallocation above, and the
sub-image readback the skip blamed as well is not broken: a control run of the
same scenario with the generate removed reads every seeded level and layer of a
2D array back, mip 2 and 3 included, through both glGetTexImage and a per-layer
FBO attachment - the two mismatches that control reported are the colours the
generate was supposed to write. GetTexImageThroughAViewReturnsOnlyItsOwnLayers
is new and pins the readback window with canary slices past the view's last
layer; it fails on the reverted readback and passes on DirectGLES too.
|
||
|
swung0x48
|
b16531c3a8 |
[Docs] record the landed stage chunking in the R-10 texts
Every place that stated "P5 does not chunk" as current behaviour except the Fatal texts (previous commit) now records the stage chunk budget and what it does not cover. - CONTRACT-P5.md section 5: the STAGE_MB row names MGPipeStageChunkBytes() = clamp(segment/4, 4096, segment), the two buffer walks ( |
||
|
swung0x48
|
a63577354d |
[MGPipe] the Fatal texts no longer claim P5 does not chunk
R-10's "P5 does not chunk (R-10)" wording predates the stage chunk budget: buffer content ranges are cut at MGPipeStageChunkBytes() ( |
||
|
swung0x48
|
e004269d96 |
[MG_Backend] key the handle arm's attachment walk on the object, not the binding
SyncToBackendByHandle gated its record-driven attachment walk (the eleven applyPoint calls plus NoteFramebufferTextureAttachments) on m_syncedRecordHashes[asTarget], a PER-TARGET slot, while the work inside is object-level: glFramebufferTexture2D/glFramebufferRenderbuffer write the driver FBO behind whichever binding is current, so a Draw walk and a Read walk write the same points. Two slots can therefore hold two individually true, jointly false accounts of one physical state. improved-transparency flips a composite framebuffer between two attachment states and syncs one of them as DRAW in a depth pre-pass. That walk detaches the colour point, stamps the Draw slot; the end-of-frame READ-side sync of the other record then found its own slot already stamped by an earlier identical sync, walked nothing, left the point detached, and the driver refused the following glBlitFramebuffer with INVALID_OPERATION - the default framebuffer froze from frame 835. The object form is immune because a per-point version array gates it underneath; the handle arm has no frontend array, so this memo is its only gate. Replace it there with m_syncedAttachmentRecordHash, one slot for the object. The per-target array stays for the object arm, where it is a re-arm trigger over that version array rather than a claim. InvalidateSyncedState now zeroes both memos: it invalidates the version array, which is not the handle arm's gate, so a memo left standing would let a re-sync of the same record skip the re-attach that call exists to force. |
||
|
swung0x48
|
7059869848 |
[Test] fix the two new stage-chunk cases' own expectations
Both cases were written against the wrong property and went red on the first WGB build, which is the one thing a case may not do. TextureEmit.ALevelTooLargeForTheStageChunkIsCutIntoSlabs asserted D == 8 slices for every slab of the 512x128x33 level, whose last slab is the 33rd slice alone. The per-slab check now names the scheme (Z = 8 * index, D = 8 for every slab but the remainder) instead of restating one number for all of them. StagedTextureStoreTest.ALevelWithAPieceMissingIsRefusedAsAWholeLevelReadByName wanted "not covered" out of a single LEADING run, and the store cannot answer that: it knows no format, so it cannot know how many bytes the level should have - the check is "the range the reader asks is covered". The case now uses the shape the check really is about, two runs with a hole between them (ServerLoopTest's R-11 coverage case, one store over), and the looseness is written down: in the header's coverage ruling, and as an assertion on the leading-run case beside it. |
||
|
swung0x48
|
9469d48edb |
[MGPipe, MG_Remote] cut a texture level's staged run at the stage chunk
ONE RECORD'S BLOB IS STAGED WHOLE IN SEG_STAGE, a linear arena, so a texture
level larger than the segment's chunk budget was Fatal{RingOverrun, "SEG_STAGE"}
at the encoder rather than a split - and the CI traces hit it: the texture half of
resource_subdata stages the whole level shadow regardless of how little of it
changed, and a 512x128x33 GL_RGBA32F level is 34,603,008 bytes while a 192-cube
GL_RGBA16 level is 56,623,104, both against the default 32 MiB segment. Three
retrace cases aborted there: iterationt-in-world and iterationt-nodsa-in-world in
both backends, and iterationrp-in-world in DirectVulkan.
THE CLIENT now cuts such a level into WHOLE-WIDTH SLABS, one resource_subdata
each (TextureEmit.h's MGPipeForEachTextureSlab, driven at MGPipeStageChunkBytes -
the same cap PipeFill.cpp's buffer walks are cut at). A piece is a set of
consecutive rows of the level shadow, so its box names both its texels and its
byte run; whole slices are the coarsest cut, a slice bigger than the cap falls
back to whole rows and a row bigger than the cap to whole texels inside one row.
Every piece is emitted, dirty or not: the server assembles one level image out of
the runs, so a piece left out would leave a hole no texel read can answer. Each
piece carries at least one region (its clipped dirty rects, or its own box when
none reaches it) because RegionCount == 0 remains the whole-level spelling, and
each region's SrcOffset is rebased onto its own piece's run - MGPipeTypes.h has
always said SrcOffset is "into the blob" - while the strides stay the level's. A
level that FITS keeps the single whole-level record, field for field what it has
always been, which is what every existing gate is written against.
THE SERVER now assembles levels from those runs. StagedTextureStore::AdoptRun
places one run at an image offset, its coverage set is exactly the runs it has
adopted (StagedShadowStore's CoverageAdd, the buffer half's helpers), and only a
FULLY covered level satisfies IsCovered / RequireLevelBytes - a level with one
piece missing is as unreadable as one that never arrived, and it is the same
Fatal{StageSnapshotTooNarrow}. The whole-level spelling survives as Adopt, its
offset-0 replacement, and the two are told apart by the run's own LENGTH: the
whole-level run is the level's complete shadow whatever its box says, a piece's
run is exactly its box's byte extent. Both adoption call sites -
Ops_H_TextureSubData and PipeApply.cpp's hookless fallback, which is what Magma
takes - read the offset that way.
THE PROTOCOL IS UNCHANGED: no field moved, no caps bit was added. The one
compatibility statement is that a split level now crosses as several records of
one apply batch, so an OLD server (which replaces the level on every record) would
keep only the last piece - inproc runs both roles out of one library, so the
constraint is noted here rather than armed with a gate.
Tests. TextureEmit.ALevelTooLargeForTheStageChunkIsCutIntoSlabs drives the split
at MGPipeStageChunkBytesFor's clamp with both trace shapes (5 and 7 slabs), the
row-granularity and texel-granularity fallbacks, the three shapes that refuse to
cut, the region clipping/rebasing, and the piece no rect reaches.
StagedTextureStoreTest gains the multi-run assembly, the gap that must not read as
coverage, the partial-level Fatal by name, the run-offset discriminator, and two
production cases - the applier's hookless arm and DirectGLES's real TextureSubData
hook - that would put both halves at the front of the image if the offset were
dropped.
|
||
|
swung0x48
|
ed919f0d64 |
[MG_Impl] guard the monolith mip generate arm against immutable storage and views
EnsureGeneratedMipmapStorageAllocated branched on the transport. The split arm
planned the generation window from BASE_LEVEL/MAX_LEVEL and refused to touch
immutable storage (
|
||
|
swung0x48
|
f1d3fa8f1a |
[MG_Backend] keep the default framebuffer on the image GL last rendered into across a swap
DirectVulkan resolved every default-framebuffer role against m_imageIndexAcquired. Present() advances that to the image it acquires for the frame that is about to start, and a present record is kWaitPresent - the client does not wait for the apply - so a readback belonging to the frame being presented (the retrace snapshot, taken at the swap call) can be applied after the acquire and then resolve an image nothing had written. That is the pure-black sundial-lite DirectVulkan retrace frame; DirectGLES is immune because the driver keeps handing back the buffer the app drew into. Split the GL-visible index out (VulkanRenderer::m_defaultFramebufferImageIndex) and keep it out of the present/acquire path: it only re-points at the acquired image when something WRITES the default framebuffer (DefaultFramebufferWriteIndex, called at the FBO-0 render-pass creation, the deferred-clear-free direct writes and the wire draw/clear/blit resolutions) and is reset on RecreateSwapchain. Reads - glReadPixels both arms, the parked-clear materialization, a blit or copy SOURCED from FBO 0, ResolveWireImage's read role (carrying the resolved index in WireImage) - resolve DefaultFramebufferReadIndex instead, i.e. whatever that is. A readback that resolves a swapchain image with no recorded write now says so once (MGLOG_W_ONCE, both arms) instead of silently handing back raw pixels. New integration scenario DirectVulkan|DirectGLES.DefaultFramebufferAcrossSwap pins both halves: a readback after a swap still sees the drawn frame, and the first draw after a swap lands in the image the readback then resolves. |
||
|
swung0x48
|
73b37bbe83 |
[MG_Backend] (DirectGLES): clip view mip generation to the view's layer window
glGenerateMipmap's native arm carries BASE_LEVEL/MAX_LEVEL of the view's own
ES name, so the LEVEL axis was already clipped the way the view describes, but
the layer set it writes is every layer of the storage the view aliases. A
generation through a windowed texture view therefore rewrote the owner's layers
outside the window (F1WireScenario
.GenerateMipmapThroughViewKeepsOwnerLevelsAndLayersOutsideItsWindow).
Both arms now read the window and generate the chain with per-layer blits
between the owner's levels MinLevel + L, over layers
[MinLayer, MinLayer + NumLayers), and leave the native call for the shapes it
already gets right: no view, a view whose layer window covers the whole
storage, a multisample view, and aspects the layer blits do not carry. The
record arm reads the window from the sampler view CSO with the same validation
SyncTextureViewToBackendByRecord uses (Fatal{ProtocolCorruption} on a stale or
foreign record); the monolith arm reads the same numbers off the frontend view
object. ScratchFBOImpl gains EnsureDepthAttachmentLayer for the depth-only
aspect.
|
||
|
swung0x48
|
8ffb87109a |
[MGPipe] include the stage-chunk helper where it is called
PipeFill.cpp reaches MG_Remote::Client::MGPipeStageChunkBytes through GpuWritePending.h, which the TU did not include: the declaration was visible to the helper's own TU and to no other, so both MobileGL translation units for PipeFill.cpp failed to compile while the shared objects it defines built fine. PipeFill.cpp. |