From 74060e8b5d754612a54d71c37e74973c7a18e33b Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Wed, 16 Sep 2026 10:43:33 -0400 Subject: [PATCH] [Fix, Test] (DirectGLES/BackendObject, MG_Remote/Server, MG_Test/Wire): ID-67 - the native skip in BackendObject_DirectGLES::MakeEGLCurrent is keyed on the VIRTUAL (dpy, draw, read, ctx) tuple, not the surface: the surface's own creation bind is adopted by the first tuple, an identical tuple is a native no-op and publishes no caps snapshot (the client's mirror generation does not move), and a DIFFERENT tuple onto the same surface binds natively again (DirectGLES's invalidations describe the frontend context that changed) and republishes (R-12 arm (a)); ServerLoop tallies MakeCurrentRepublishCount and the control asserts 1 bind + 1 republish for a different tuple and 0 + 0 for a repeat at the EGL table and the tally; the EGL fixture gives the session its backend so the republish is real --- .../DirectGLES/BackendObject_DirectGLES.cpp | 102 +++++++++++++++++- MobileGL/MG_Remote/Server/ServerLoop.cpp | 23 ++-- MobileGL/MG_Remote/Server/ServerLoop.h | 20 +++- MobileGL/MG_Test/Wire/ServerLoopTest.cpp | 64 ++++++++++- 4 files changed, 192 insertions(+), 17 deletions(-) diff --git a/MobileGL/MG_Backend/DirectGLES/BackendObject_DirectGLES.cpp b/MobileGL/MG_Backend/DirectGLES/BackendObject_DirectGLES.cpp index 06de2734..76a1d0fe 100644 --- a/MobileGL/MG_Backend/DirectGLES/BackendObject_DirectGLES.cpp +++ b/MobileGL/MG_Backend/DirectGLES/BackendObject_DirectGLES.cpp @@ -32,6 +32,54 @@ namespace MobileGL::MG_Backend::DirectGLES { return draw == EGL_NO_SURFACE && read == EGL_NO_SURFACE && ctx == EGL_NO_CONTEXT; } +#if MOBILEGL_BUILD_DISAGGREGATED + // ID-54 / ID-67 (v1, under ID-52/ID-59's grant for this file). Which VIRTUAL (dpy, draw, + // read, ctx) the process's one native ES context is currently bound FOR, on the apply + // thread. DirectGLES has one native context and one native surface (g_Context, g_Surface) + // whatever virtual handles the client uses, so "is a native eglMakeCurrent needed" is never + // "is the native triple different" - it is "did the VIRTUAL context change": + // DirectGLES::MakeCurrent is also where the seven caches that describe the frontend context + // are invalidated, and a different virtual context needs them invalidated even though the + // driver binds the same triple. ID-67: a make-current with a DIFFERENT tuple is a real + // native bind (and a caps republish, ServerLoop's half); an IDENTICAL one is neither. + // + // Three states. NotBound: the next bind is native. FreshFromSurfaceCreation: the surface's + // own creation (InitPbufferSurface / InitWindowSurface) bound natively and invalidated, and + // no virtual tuple has claimed that bind yet - the first tuple adopts it, which is the + // "2 -> 1 native binds per process" of round 3. BoundForTuple: bound and invalidated for + // the recorded tuple; only that exact tuple may skip. Process-wide like the native state it + // mirrors; under split exactly one DirectGLES object exists (the server's), and only the + // apply thread reaches these. + enum class NativeBindState : Uint8 { NotBound, FreshFromSurfaceCreation, BoundForTuple }; + NativeBindState g_nativeBindState = NativeBindState::NotBound; + EGLDisplay g_nativeBoundDpy = EGL_NO_DISPLAY; + EGLSurface g_nativeBoundDraw = EGL_NO_SURFACE; + EGLSurface g_nativeBoundRead = EGL_NO_SURFACE; + EGLContext g_nativeBoundCtx = EGL_NO_CONTEXT; + + void NoteNativeContextGone() { g_nativeBindState = NativeBindState::NotBound; } + void NoteNativeContextFreshFromSurfaceCreation() { + g_nativeBindState = NativeBindState::FreshFromSurfaceCreation; + } + void NoteNativeBoundFor(EGLDisplay dpy, EGLSurface draw, EGLSurface read, EGLContext ctx) { + g_nativeBindState = NativeBindState::BoundForTuple; + g_nativeBoundDpy = dpy; + g_nativeBoundDraw = draw; + g_nativeBoundRead = read; + g_nativeBoundCtx = ctx; + } + Bool NativeBindCanBeSkippedFor(EGLDisplay dpy, EGLSurface draw, EGLSurface read, EGLContext ctx) { + switch (g_nativeBindState) { + case NativeBindState::NotBound: return false; + case NativeBindState::FreshFromSurfaceCreation: return true; + case NativeBindState::BoundForTuple: + return g_nativeBoundDpy == dpy && g_nativeBoundDraw == draw && g_nativeBoundRead == read && + g_nativeBoundCtx == ctx; + } + return false; + } +#endif + void ClearGLErrors(const MG_External::GLESFunctionsTable& gl) { if (!gl.glGetError) return; while (gl.glGetError() != GL_NO_ERROR) {} @@ -843,6 +891,9 @@ namespace MobileGL::MG_Backend::DirectGLES { BackendObject_DirectGLES::~BackendObject_DirectGLES() { DestroyEGLContext(); +#if MOBILEGL_BUILD_DISAGGREGATED + NoteNativeContextGone(); +#endif } Bool BackendObject_DirectGLES::InitWindowSurface() { @@ -920,7 +971,17 @@ namespace MobileGL::MG_Backend::DirectGLES { ResetEGLRuntimeState(); } - return BackendObject::CreateEGLWindowSurface(surface, handle); + const Bool created = BackendObject::CreateEGLWindowSurface(surface, handle); +#if MOBILEGL_BUILD_DISAGGREGATED + // ID-54 / ID-67: the surface's creation bound natively (InitWindowSurface -> MakeCurrent); + // the first virtual tuple adopts that bind. On failure nothing is known to be bound. + if (created) { + NoteNativeContextFreshFromSurfaceCreation(); + } else { + NoteNativeContextGone(); + } +#endif + return created; } Bool BackendObject_DirectGLES::CreateEGLPbufferSurface(EGLSurface surface, EGLint width, EGLint height) { @@ -939,7 +1000,16 @@ namespace MobileGL::MG_Backend::DirectGLES { ResetEGLRuntimeState(); } - return BackendObject::CreateEGLPbufferSurface(surface, width, height); + const Bool created = BackendObject::CreateEGLPbufferSurface(surface, width, height); +#if MOBILEGL_BUILD_DISAGGREGATED + // ID-54 / ID-67: as for the window surface - InitPbufferSurface bound natively. + if (created) { + NoteNativeContextFreshFromSurfaceCreation(); + } else { + NoteNativeContextGone(); + } +#endif + return created; } Bool BackendObject_DirectGLES::InitPbufferSurface(EGLint width, EGLint height) { @@ -952,6 +1022,9 @@ namespace MobileGL::MG_Backend::DirectGLES { if (!DirectGLES::ReleaseCurrent()) { return false; } +#if MOBILEGL_BUILD_DISAGGREGATED + NoteNativeContextGone(); +#endif return BackendObject::MakeEGLCurrent(dpy, draw, read, ctx); } @@ -987,14 +1060,24 @@ namespace MobileGL::MG_Backend::DirectGLES { // native call is skipped and only the base class's bookkeeping below runs - which is // still required: it is what InitCapabilities and SwapEGLBuffers' current-thread record // hang off. Monolith transport in this build, and the pull build, bind exactly as before. - // Red once by making this arm unconditional: ServerLoopTest's C7 control reads 2 native - // binds at the EGL function table instead of 1. + // + // AND ONLY FOR THE SAME VIRTUAL TUPLE (ID-67). The skip is keyed on NativeBindCanBeSkippedFor: + // the surface's own creation bind is adopted by the FIRST tuple, an identical tuple skips, + // and a DIFFERENT tuple - a second MobileGL context onto the same surface - runs the native + // call again even though the driver's triple is the same, because MakeCurrent's seven + // invalidations describe the frontend context that is changing. Red once, two ways: + // make this arm unconditional (ServerLoopTest's C7 control reads 2 native binds at the EGL + // function table instead of 1), or make NativeBindCanBeSkippedFor answer true for any tuple + // (the ID-67 control's different tuple stays at 1 native bind where 2 are required). const Bool nativelyCurrentAlready = MG_Config::Transport != MG_Config::TransportMode::Monolith && m_eglSurfaceInitialized && m_eglSurface == draw && - DirectGLES::IsBackendContextCurrentOnThisThread(); + DirectGLES::IsBackendContextCurrentOnThisThread() && + NativeBindCanBeSkippedFor(dpy, draw, read, ctx); if (!nativelyCurrentAlready && !DirectGLES::MakeCurrent()) { + NoteNativeContextGone(); return false; } + NoteNativeBoundFor(dpy, draw, read, ctx); #else if (!DirectGLES::MakeCurrent()) { return false; @@ -1003,6 +1086,9 @@ namespace MobileGL::MG_Backend::DirectGLES { if (!BackendObject::MakeEGLCurrent(dpy, draw, read, ctx)) { (void)DirectGLES::ReleaseCurrent(); +#if MOBILEGL_BUILD_DISAGGREGATED + NoteNativeContextGone(); +#endif return false; } return true; @@ -1020,12 +1106,18 @@ namespace MobileGL::MG_Backend::DirectGLES { void BackendObject_DirectGLES::ReleaseEGLResources() { const std::lock_guard lock(m_eglStateMutex); DestroyEGLContext(); +#if MOBILEGL_BUILD_DISAGGREGATED + NoteNativeContextGone(); +#endif BackendObject::ReleaseEGLResources(); } void BackendObject_DirectGLES::OnEGLSurfaceReleased(EGLSurface surface) { (void)surface; DestroyEGLContext(); +#if MOBILEGL_BUILD_DISAGGREGATED + NoteNativeContextGone(); +#endif } const RendererInfo& BackendObject_DirectGLES::GetRendererInfo() const { diff --git a/MobileGL/MG_Remote/Server/ServerLoop.cpp b/MobileGL/MG_Remote/Server/ServerLoop.cpp index 2c6187e2..2eb05253 100644 --- a/MobileGL/MG_Remote/Server/ServerLoop.cpp +++ b/MobileGL/MG_Remote/Server/ServerLoop.cpp @@ -213,6 +213,7 @@ namespace MobileGL::MG_Remote::Server { m_parks.store(0, std::memory_order_release); m_nativeBinds.store(0, std::memory_order_release); m_clientReleases.store(0, std::memory_order_release); + m_makeCurrentRepublishes.store(0, std::memory_order_release); m_haveCurrentTuple = false; { const std::lock_guard lock(m_exitMutex); @@ -238,6 +239,12 @@ namespace MobileGL::MG_Remote::Server { Uint64 ServerLoop::ClientReleaseCount() const { return m_clientReleases.load(std::memory_order_acquire); } + Uint64 ServerLoop::MakeCurrentRepublishCount() const { + return m_makeCurrentRepublishes.load(std::memory_order_acquire); + } + void ServerLoop::NoteMakeCurrentRepublished() { + m_makeCurrentRepublishes.fetch_add(1, std::memory_order_acq_rel); + } // C7 / ID-54. A release-current request (the three NO_* markers, exactly IsReleaseCurrentRequest's // test in BackendObject_DirectGLES.cpp) is a ClientRelease the server records but does not @@ -770,15 +777,19 @@ namespace MobileGL::MG_Remote::Server { ok = outcome.ok; if (!ok || !outcome.boundNatively) return MOBILEGL_OK; // R-12, arm (a): the caps snapshot is REPUBLISHED because InitCapabilities has - // now run for real - and ONLY on a real native bind, not on an identical repeat - // (a repeat re-published nothing). DirectGLES has no OnCapsInvalidated producer at - // all, and c0's answer is that a SECOND arrival IS the invalidation - so the - // client's mirror is refreshed with no dev-shaped backend edit and with no - // eleventh MGPipeCallbacks slot (MGPipeCallbacks.h:56-58's static_assert exists to - // make that cost visible). + // now run for real - on every forwarded bind of a tuple this loop did not hold + // (the first, and every DIFFERENT tuple after it), and NEVER on an identical + // repeat (ID-67: a repeat is a native no-op AND publishes nothing, so the client's + // mirror generation does not move and nothing accumulates). DirectGLES has no + // OnCapsInvalidated producer at all, and c0's answer is that a SECOND arrival IS the + // invalidation - so the client's mirror is refreshed with no dev-shaped backend edit + // and with no eleventh MGPipeCallbacks slot (MGPipeCallbacks.h:56-58's static_assert + // exists to make that cost visible). Red once by republishing only on the first + // bind: the ID-67 control's different tuple reads 1 republish where 2 are required. ServerSession* session = ServerSession::Active(); if (session != nullptr && session->Accepted()) { const MobileGLResult published = session->PublishCapsSnapshot(); + if (published == MOBILEGL_OK) ServerLoopInstance().NoteMakeCurrentRepublished(); if (published != MOBILEGL_OK) { MGLOG_E("MG_Remote server: the post-make-current CapsSnapshot could not " "be published (rc=%d); the client's mirror still holds the empty " diff --git a/MobileGL/MG_Remote/Server/ServerLoop.h b/MobileGL/MG_Remote/Server/ServerLoop.h index 226a5e75..628d9ce8 100644 --- a/MobileGL/MG_Remote/Server/ServerLoop.h +++ b/MobileGL/MG_Remote/Server/ServerLoop.h @@ -40,11 +40,15 @@ // surface is CREATED, and once more from the client's first eglMakeCurrent. So (1) // ServerMakeEGLCurrent classifies the request against the tuple it last bound // (ClassifyEglMakeCurrent): an identical repeat is a no-op, and the R-12 republish decision for -// it is "nothing to republish"; a different tuple is a real forwarded bind; a client -// release-current is RECORDED (ClientReleaseCount) and NOT forwarded. And (2) -// BackendObject_DirectGLES::MakeEGLCurrent, under an active transport only, skips the native call -// when the requested draw surface is the one already natively current on this thread -// (IsBackendContextCurrentOnThisThread, which is EGL ground truth). Measured at the EGL function +// it is "nothing to republish" (ID-67: the client's mirror generation must not move); a different +// tuple is a real forwarded bind AND a caps republish; a client release-current is RECORDED +// (ClientReleaseCount) and NOT forwarded. And (2) BackendObject_DirectGLES::MakeEGLCurrent, under +// an active transport only, skips the native call when the requested draw surface is the one +// already natively current on this thread (IsBackendContextCurrentOnThisThread, which is EGL +// ground truth) AND the virtual tuple is the one that bind was for - or the bind is the surface's +// own creation, which the first tuple adopts; a different virtual context onto the same surface +// binds natively again, because MakeCurrent's invalidations describe the frontend context that +// changed (ID-67). Measured at the EGL function // table by ServerLoopTest's C7 control on a real llvmpipe context: surface creation + two // identical make-currents + a client release + a bind after the release = ONE native // eglMakeCurrent, ZERO native releases, and the apply thread still the owner afterwards. So @@ -152,6 +156,11 @@ namespace MobileGL::MG_Remote::Server { // - and the control reads that one at the EGL function table, not here. Uint64 NativeBindCount() const; Uint64 ClientReleaseCount() const; + // ID-67: how many caps snapshots ServerMakeEGLCurrent has re-published (R-12 arm (a)) - one + // per forwarded bind of a tuple it did not hold, never for an identical repeat, so the + // client's mirror generation moves exactly when the server's answers could have. + Uint64 MakeCurrentRepublishCount() const; + void NoteMakeCurrentRepublished(); // The deduped make-current, on the apply thread. Classifies the request (see // ClassifyEglMakeCurrent), forwards a native bind only for a genuinely new tuple, records @@ -232,6 +241,7 @@ namespace MobileGL::MG_Remote::Server { EGLContext m_curCtx = EGL_NO_CONTEXT; std::atomic m_nativeBinds{0}; std::atomic m_clientReleases{0}; + std::atomic m_makeCurrentRepublishes{0}; }; ServerLoop& ServerLoopInstance(); diff --git a/MobileGL/MG_Test/Wire/ServerLoopTest.cpp b/MobileGL/MG_Test/Wire/ServerLoopTest.cpp index 77693034..d12648d6 100644 --- a/MobileGL/MG_Test/Wire/ServerLoopTest.cpp +++ b/MobileGL/MG_Test/Wire/ServerLoopTest.cpp @@ -115,7 +115,10 @@ namespace { sizes.CmdRingBytes = 64ull * 1024; sizes.StageBytes = 64ull * 1024; sizes.ReplyBytes = 64ull * 1024; - sizes.EventRingBytes = 16ull * 1024; + // 1 MiB, not 16 KiB: the EGL cases have the server republish its caps snapshot into the + // event ring several times with no client draining it, and a ring that fills would turn a + // "no republish" assertion into a ring-full one. + sizes.EventRingBytes = 1024ull * 1024; sizes.ReplySlotCount = 8; return sizes; } @@ -1158,6 +1161,9 @@ namespace { MG_External::EGLFunctionsTable counted = egl; counted.eglMakeCurrent = &CountingEglMakeCurrent; MG_Backend::DirectGLES::SetEGLFuncsTable(counted); + // InitSplitRoles step 2: the session answers its caps snapshots from this backend, so + // ServerMakeEGLCurrent's R-12 republish has something to publish (ID-67's control). + Server::ServerSessionInstance().SetBackend(loop.Backend()); if (!Handshake()) return "the session handshake"; if (!StartLoop()) return "ServerLoop::Start"; EGLint major = 0; @@ -1179,6 +1185,8 @@ namespace { void TearDown() { Server::ServerReleaseEGLResources(); Stop(); + // The backend Stop() just destroyed must not be the session's answer for the next case. + Server::ServerSessionInstance().SetBackend(nullptr); MG_State::pGLContext = Move(savedContext); } }; @@ -1300,6 +1308,60 @@ TEST(ServerLoopEglTest, MakeCurrentBindsNativelyOncePerContextAndNeverForwardsAC fixture.TearDown(); } +// ID-67: an IDENTICAL tuple is a native no-op AND publishes no caps snapshot (the client's mirror +// generation must not move, nothing accumulates); a DIFFERENT tuple - a second MobileGL context onto +// the same surface, the same driver triple - is a real native bind AND a republish (R-12 arm (a)), +// so the client adopts it without a pump or a Present. Both halves measured: native binds at the EGL +// table, republishes at ServerMakeEGLCurrent's own tally. Red once, three ways: make the backend's +// skip answer "same tuple" for any tuple (the different tuple stays at 1 native bind), republish on +// a repeat (the repeat reads 2 republishes), republish only on the first bind (the different tuple +// reads 1). +TEST(ServerLoopEglTest, ADifferentTupleBindsNativelyAndRepublishesAnIdenticalRepeatDoesNeither) { + EglServerFixture fixture; + MGL_EGL_BRING_UP_OR_BAIL(fixture); + Server::ServerLoop& loop = Server::ServerLoopInstance(); + const EGLDisplay dpy = EglServerFixture::Dpy(); + const EGLSurface surf = EglServerFixture::Surf(); + const EGLContext ctxA = EglServerFixture::Ctx(); + const EGLContext ctxB = reinterpret_cast(static_cast(0x2)); + ASSERT_EQ(g_nativeEglBinds.load(), 1) << "the surface's own creation did not bind natively exactly once"; + ASSERT_EQ(loop.MakeCurrentRepublishCount(), 0u); + + // The first tuple adopts the creation's bind, and republishes (InitCapabilities has now run). + ASSERT_TRUE(Server::ServerMakeEGLCurrent(dpy, surf, surf, ctxA)); + EXPECT_EQ(g_nativeEglBinds.load(), 1); + EXPECT_EQ(loop.MakeCurrentRepublishCount(), 1u) + << "the first make-current did not republish the caps snapshot (R-12 arm (a))"; + + // Identical: 0 native binds, 0 republishes. + ASSERT_TRUE(Server::ServerMakeEGLCurrent(dpy, surf, surf, ctxA)); + EXPECT_EQ(g_nativeEglBinds.load(), 1) << "an identical repeat bound natively"; + EXPECT_EQ(loop.MakeCurrentRepublishCount(), 1u) + << "an identical repeat republished the caps snapshot: the client's mirror generation moved " + "for nothing and unpumped snapshots accumulate (ID-67)"; + + // Different (a second context onto the same surface): 1 native bind, 1 republish. + ASSERT_TRUE(Server::ServerMakeEGLCurrent(dpy, surf, surf, ctxB)); + EXPECT_EQ(g_nativeEglBinds.load(), 2) + << "a make-current with a DIFFERENT tuple (a second context onto the same surface) did not " + "bind natively: DirectGLES's invalidations for the new frontend context never ran (ID-67)"; + EXPECT_EQ(loop.NativeBindCount(), 2u); + EXPECT_EQ(loop.MakeCurrentRepublishCount(), 2u) + << "a make-current with a different tuple did not republish the caps snapshot; the client " + "would adopt nothing without a pump or a Present (R-12 arm (a), ID-67)"; + + // Identical again, then back to the first: 0 + 0, then 1 + 1. + ASSERT_TRUE(Server::ServerMakeEGLCurrent(dpy, surf, surf, ctxB)); + EXPECT_EQ(g_nativeEglBinds.load(), 2); + EXPECT_EQ(loop.MakeCurrentRepublishCount(), 2u); + ASSERT_TRUE(Server::ServerMakeEGLCurrent(dpy, surf, surf, ctxA)); + EXPECT_EQ(g_nativeEglBinds.load(), 3); + EXPECT_EQ(loop.NativeBindCount(), 3u); + EXPECT_EQ(loop.MakeCurrentRepublishCount(), 3u); + + fixture.TearDown(); +} + // N-3: a destroy-recreate with the SAME handle values must be a real bind again. Before the fix the // tuple survived ServerReleaseEGLResources, the recreated context's first make-current classified as // a RepeatNoOp, nothing ran in the base class (no InitCapabilities, no current-thread record) and no