swung0x48/MobileGL
1
0
Fork 0
mirror of https://github.com/MobileGL-Dev/MobileGL synced 2026-11-07 01:53:24 +09:00
Code Issues Packages Projects Releases Wiki Activity

[Refactor] (Remote, P10 B): the emit table's SetSwapInterval slot is the server forwarder - SetSwapInterval_Unmigrated and the BackendObject_Remote override deleted, class C empty (69 emitted / 0 unmigrated), and a unit control that the slot crosses as one control frame

Browse Source
This commit is contained in:
swung0x48
2026-09-29 04:34:02 -04:00
parent 4373964443
commit 80d205c401
6 changed files with 101 additions and 57 deletions
Download Patch File Download Diff File Expand all files Collapse all files
MobileGL/MG_Remote/Client/BackendObject_Remote.cpp
+4 -10
View File
@@ -32,7 +32,8 @@ namespace MobileGL::MG_Remote::Client {
// ---- the EGL seam ------------------------------------------------------------------
//
// THE NINE EGL VIRTUALS CALL v1's TWELVE FORWARDERS AND NOTHING ELSE. c1 round 1 built
// THE EGL VIRTUALS CALL v1's TWELVE FORWARDERS AND NOTHING ELSE (the swap interval's
// since P10 through the emit table's slot, not an override here). c1 round 1 built
// its own trampolines over ServerLoop's control channel and ServerLoop::Backend(),
// which ran the right driver call on the right thread and was still wrong, because
// three of the twelve do MORE than forward:
@@ -403,15 +404,8 @@ namespace MobileGL::MG_Remote::Client {
return MG_Backend::BackendObject::SwapEGLBuffers(dpy, draw);
}
void BackendObject_Remote::SetEGLSwapInterval(Int interval) {
// OVERRIDDEN BECAUSE THE BASE WOULD FATAL. BackendObject.cpp:402 null-checks
// GetBackendFunctions().SetSwapInterval and calls it when non-null - one of the 41
// null checks R-4 turns into "always supported" - and SetSwapInterval is class C, so
// the base implementation would abort on every eglSwapInterval. The answer is the
// caps-mirror-read rule's general shape: the question "can the presentation path take
// an interval" belongs to the server, so it is asked of the server.
Server::ServerSetEGLSwapInterval(interval);
}
// No SetEGLSwapInterval here any more (P10 B): the base class calls the emit table's slot,
// which is now Server::ServerSetEGLSwapInterval itself (EmitTables.cpp).
void BackendObject_Remote::ReleaseEGLSurface(EGLSurface surface) {
// P12: a released server-owned surface stops taking the server window's geometry.
MobileGL/MG_Remote/Client/BackendObject_Remote.h
+6 -7
View File
@@ -40,11 +40,11 @@
// function-pointer mailbox before it), and then lets the
// base class keep the client-side books.
//
// SetEGLSwapInterval IS THE ONE THAT MUST NOT REACH THE TABLE. The base implementation
// null-checks GetBackendFunctions().SetSwapInterval (BackendObject.cpp:402) - one of the 41
// null checks R-4 turns into "always supported" - and SetSwapInterval is class C, so the base
// implementation would Fatal on every eglSwapInterval. It is overridden to forward instead,
// which is the caps-mirror-read rule's shape for a slot whose answer is "ask the server".
// SetEGLSwapInterval IS NOT OVERRIDDEN (P10 B). It used to be, because the base implementation
// calls GetBackendFunctions().SetSwapInterval (BackendObject.cpp:402) and that slot was the last
// class-C Fatal in the emit table. The slot is now the forwarder the override called
// (Server::ServerSetEGLSwapInterval, EmitTables.cpp), so the base's route IS the forward and a
// second spelling of it here would only be a second place for the two to drift apart.
#pragma once
#include <Includes.h>
@@ -68,14 +68,13 @@ namespace MobileGL::MG_Remote::Client {
const MG_Backend::DynamicBackendParameters& GetDynamicParameters() const override;
BackendType GetBackendType() const override;
// ---- the nine EGL lifecycle virtuals ---------------------------------------------
// ---- the EGL lifecycle virtuals (SetEGLSwapInterval is the base's: see above) -----
Bool InitializeEGLDisplay(EGLDisplay dpy, EGLint* major, EGLint* minor) override;
Bool CreateEGLWindowSurface(EGLSurface surface, const MG_Backend::WindowHandle& handle) override;
Bool ResizeEGLWindowSurface(EGLSurface surface, Uint32 width, Uint32 height) override;
Bool CreateEGLPbufferSurface(EGLSurface surface, EGLint width, EGLint height) override;
Bool MakeEGLCurrent(EGLDisplay dpy, EGLSurface draw, EGLSurface read, EGLContext ctx) override;
Bool SwapEGLBuffers(EGLDisplay dpy, EGLSurface draw) override;
void SetEGLSwapInterval(Int interval) override;
void ReleaseEGLSurface(EGLSurface surface) override;
void ReleaseEGLResources() override;
MobileGL/MG_Remote/Client/EmitTables.cpp
+22 -9
View File
@@ -34,6 +34,8 @@
#include "GpuWritePending.h"
#include "PersistentMapTracker.h"
#include "../Server/ServerLoop.h"
#include <MG_Util/Converters/GLToMG/TextureEnumConverter.h>
#include <MG_Util/Converters/MGToGL/TextureEnumConverter.h>
#include <MG_Util/Debug/Log.h>
@@ -1807,8 +1809,8 @@ namespace MobileGL::MG_Remote::Client {
// per-package ownership assertions below then still hold, the totals stay arithmetic,
// and a slot that changes class without changing the arithmetic is a build break. The
// X-macro shape is kept so the DEFINITION and the ASSIGNMENT cannot drift apart. Three
// slots carry a body the macro cannot (DispatchCompute, DispatchComputeIndirect,
// SetSwapInterval) and are written out by hand below.
// slots carried a body the macro cannot (DispatchCompute, DispatchComputeIndirect,
// SetSwapInterval) and were written out by hand below; all three are class B now.
//
// d1 indexed / instanced / multi-draw / indirect draws -> draw_vbo (59), its
// kDrawIsIndirect tail and its kDrawHasUserIndices span
@@ -1849,7 +1851,8 @@ namespace MobileGL::MG_Remote::Client {
#define MGR_UNMIGRATED_F1_SLOTS(X)
// The wave-3 tail. SetSwapInterval is hand-written below (it is not a GL.* slot).
// The wave-3 tail. Empty since P10 B retired SetSwapInterval (it was not a GL.* slot and
// was never a row here).
#define MGR_UNMIGRATED_TAIL_SLOTS(X)
// The non-void ones, kept apart only because the macro body differs: a [[noreturn]]
@@ -1880,8 +1883,13 @@ namespace MobileGL::MG_Remote::Client {
// hook before their Fatal so that the package flipping them would inherit a call site
// that was already correct, and EmitDispatchCompute / EmitDispatchComputeIndirect above
// are that inheritance - same two calls, same order, the record where the Fatal was.
void SetSwapInterval_Unmigrated(Int) { UnmigratedVerbFatal("SetSwapInterval"); }
//
// THE LAST HAND-WRITTEN STUB, SetSwapInterval_Unmigrated, IS GONE TOO (P10 B), and class C
// is empty. It was never reachable: eglSwapInterval reaches the table only through
// BackendObject::SetEGLSwapInterval, and BackendObject_Remote overrode that to call
// Server::ServerSetEGLSwapInterval - a SurfaceControlFrame on the control channel, which
// crosses processes. The slot now IS that forwarder (assigned in the class-B block), the
// override is deleted, and there is one route from eglSwapInterval to the server.
// The counts, as arithmetic. MGR_COUNT_ONE expands to `+ 1` per row.
#define MGR_COUNT_ONE(Name, Ret, Sig) +1
@@ -1890,9 +1898,8 @@ namespace MobileGL::MG_Remote::Client {
constexpr Uint32 kUnmigratedI1 = 0 MGR_UNMIGRATED_I1_SLOTS(MGR_COUNT_ONE);
constexpr Uint32 kUnmigratedT2 = 0 MGR_UNMIGRATED_T2_SLOTS(MGR_COUNT_ONE);
constexpr Uint32 kUnmigratedF1 = 0 MGR_UNMIGRATED_F1_SLOTS(MGR_COUNT_ONE);
// + SetSwapInterval, written out by hand.
constexpr Uint32 kUnmigratedTail =
0 MGR_UNMIGRATED_TAIL_SLOTS(MGR_COUNT_ONE) MGR_UNMIGRATED_TAIL_VALUE_SLOTS(MGR_COUNT_ONE) + 1;
0 MGR_UNMIGRATED_TAIL_SLOTS(MGR_COUNT_ONE) MGR_UNMIGRATED_TAIL_VALUE_SLOTS(MGR_COUNT_ONE);
#undef MGR_COUNT_ONE
constexpr Uint32 kUnmigratedSlots =
kUnmigratedD1 + kUnmigratedI1 + kUnmigratedT2 + kUnmigratedF1 + kUnmigratedTail;
@@ -1903,7 +1910,9 @@ namespace MobileGL::MG_Remote::Client {
constexpr Uint32 kEmittedSlotsI1 = 7;
constexpr Uint32 kEmittedSlotsT2 = 7;
constexpr Uint32 kEmittedSlotsF1 = 11;
constexpr Uint32 kEmittedSlotsTail = 3; // BlitNamedFramebuffer, both texture readbacks
// BlitNamedFramebuffer, both texture readbacks, and SetSwapInterval (P10 B) - the one
// class-B slot that crosses as a surface control frame rather than a SEG_CMD record.
constexpr Uint32 kEmittedSlotsTail = 4;
constexpr Uint32 kEmittedSlotsSync = 5;
constexpr Uint32 kEmittedSlotsQueries = 11;
constexpr Uint32 kEmittedSlots =
@@ -1938,7 +1947,6 @@ namespace MobileGL::MG_Remote::Client {
MGR_UNMIGRATED_GL_SLOTS(MGR_ASSIGN_UNMIGRATED)
MGR_UNMIGRATED_GL_VALUE_SLOTS(MGR_ASSIGN_UNMIGRATED)
#undef MGR_ASSIGN_UNMIGRATED
table.SetSwapInterval = &SetSwapInterval_Unmigrated;
table.GL.FenceSync = &EmitFenceSync;
table.GL.ClientWaitSync = &EmitClientWaitSync;
@@ -1996,6 +2004,11 @@ namespace MobileGL::MG_Remote::Client {
table.GL.BlitFramebuffer = &EmitBlitFramebuffer;
table.GL.BlitNamedFramebuffer = &EmitBlitNamedFramebuffer;
table.Present = &EmitPresent;
// P10 B: the swap interval is the server's presentation-path question, so the slot
// forwards it there as a SurfaceControlFrame (ServerLoop.cpp's SetSwapInterval arm),
// across processes on spawn and tcp - the forwarder BackendObject_Remote's deleted
// SetEGLSwapInterval override used to call.
table.SetSwapInterval = &Server::ServerSetEGLSwapInterval;
// ---- class B, P5b t2. Assigned AFTER the class-C block above, which is what makes
// the flip a single-line change per slot: the Fatal thunk is overwritten, and a slot
// whose row is removed from MGR_UNMIGRATED_T2_SLOTS but not assigned here would be
MobileGL/MG_Remote/Client/EmitTables.h
+4 -1
View File
@@ -38,7 +38,10 @@
// Present has ZERO MG_Impl call sites: it is reached through EGLImpl.cpp:178 ->
// BackendObject.cpp:396, so mirroring GLImpl will not find it.
// C. Fatal{UnmigratedVerb} - the remaining 64, SetSwapInterval and GetGpuTimestampNs among
// them.
// them. (That was P5's partition. P5b..P10 moved every one of them to B; the last,
// SetSwapInterval, is a forward to the server's control channel since P10, and class C is
// empty. UnmigratedVerbFatal below remains the named refusal of a class-B emitter's
// unrepresentable SHAPE - "<slot>+<QUALIFIER>".)
//
// AND THE RULE R-4 WOULD OTHERWISE BREAK. 41 of the 69 slots are null-checked at their call
// site, and several of those checks are CAPABILITY PROBES, not safety checks - BeginOcclusionQuery
MobileGL/MG_Test/Wire/RemoteClientTest.cpp
+38 -30
View File
@@ -192,10 +192,11 @@ namespace {
// =====================================================================================
TEST(RemoteEmitTable, TheThreeClassesPartitionAllSeventyOneSlots) {
// P5 baseline five + f1 eleven + i1 seven + t2 six emitted slots.
// P5 baseline five + f1 eleven + i1 seven + t2 six emitted slots, ... and since P10 B the
// swap interval: class C is EMPTY, every slot but the two local answers reaches the server.
EXPECT_EQ(LocallyAnsweredSlotCount(), 2u);
EXPECT_EQ(ImplementedVerbCount(), 68u);
EXPECT_EQ(UnmigratedSlotCount(), 1u);
EXPECT_EQ(ImplementedVerbCount(), 69u);
EXPECT_EQ(UnmigratedSlotCount(), 0u);
EXPECT_EQ(LocallyAnsweredSlotCount() + ImplementedVerbCount() + UnmigratedSlotCount(),
kRemoteEmitSlotCount);
}
@@ -333,27 +334,32 @@ TEST(RemoteEmitTable, TheFiveEmittersAreTheOnesTheCensusMeasured) {
}
#if MGTEST_HAVE_FORK
TEST(RemoteEmitTable, AnUnmigratedSlotAbortsAndNamesItself) {
TEST(RemoteEmitTable, AnUnmigratedShapeRefusalAbortsAndNamesItself) {
// THE DEATH TEST ON THE UnmigratedVerbFatal ARM. It asserts the exact wording, not merely
// that the child died: a control that trips on any abort is satisfied by the wrong abort,
// which is one of the three shapes R-16 was written after.
// GetTexImage is the wave-3 tail (CONTRACT-P5B.md §7): no P5b package flips it, so this
// case keeps its subject across the four P5b landings. (It was DrawElements until d1 made
// that a class-B emitter.)
const ChildResult r = RunInChild([] { RemoteEmitTable().SetSwapInterval(1); });
// NO SLOT IS CLASS C ANY MORE: P10 B retired SetSwapInterval, the last one and this case's
// subject until then. The funnel is still the named refusal of a class-B emitter's
// unrepresentable SHAPE (RefuseDrawByName, EmitF1Clear's default arm), and every refusal
// left on it is spelled "<slot>+<QUALIFIER>" - so that is what is driven here, through the
// funnel itself, because every emitter that reaches it needs a session and a context first.
const ChildResult r =
RunInChild([] { UnmigratedVerbFatal("MultiDrawElementsBaseVertex+CLIENT_INDICES"); });
ASSERT_TRUE(DiedOfAbort(r)) << DescribeStatus(r) << "\n" << r.Log;
EXPECT_NE(r.Log.find("Fatal{UnmigratedVerb, \"SetSwapInterval\"}"), std::string::npos) << r.Log;
EXPECT_NE(r.Log.find("Fatal{UnmigratedVerb, \"MultiDrawElementsBaseVertex+CLIENT_INDICES\"}"),
std::string::npos)
<< r.Log;
}
TEST(RemoteEmitTable, EachUnmigratedSlotNamesItsOwnSlot) {
TEST(RemoteEmitTable, EachUnmigratedShapeRefusalNamesItsOwnShape) {
// The half the case above cannot state on its own: that the name in the message is the
// slot's and not a constant. Two different slots, two different names - both from the
// wave-3 tail, for the reason the case above gives.
const ChildResult r = RunInChild([] { RemoteEmitTable().SetSwapInterval(1); });
// caller's and not a constant. Two different shapes, two different names.
const ChildResult r = RunInChild([] { UnmigratedVerbFatal("DrawArraysIndirect+CLIENT_COMMANDS"); });
ASSERT_TRUE(DiedOfAbort(r)) << DescribeStatus(r) << "\n" << r.Log;
EXPECT_NE(r.Log.find("Fatal{UnmigratedVerb, \"SetSwapInterval\"}"), std::string::npos) << r.Log;
EXPECT_EQ(r.Log.find("GetTexImage"), std::string::npos)
<< "the Fatal message names a slot other than the one that was called:\n"
EXPECT_NE(r.Log.find("Fatal{UnmigratedVerb, \"DrawArraysIndirect+CLIENT_COMMANDS\"}"), std::string::npos)
<< r.Log;
EXPECT_EQ(r.Log.find("MultiDrawElementsBaseVertex"), std::string::npos)
<< "the Fatal message names a shape other than the one that was refused:\n"
<< r.Log;
}
@@ -373,13 +379,20 @@ TEST(RemoteEmitTable, AFlippedDrawSlotDemandsASessionRatherThanNamingItselfUnmig
<< r.Log;
}
TEST(RemoteEmitTable, SetSwapIntervalIsClassCAndSaysSo) {
TEST(RemoteEmitTable, SetSwapIntervalIsTheServerForwarderAndNotAFatal) {
// The slot the verb census found by NOT mirroring GLImpl: SetSwapInterval has zero MG_Impl
// call sites and is reached only through the EGL path, so a table built from the 89 GLImpl
// sites would have left it null.
// call sites and is reached only through the EGL path (BackendObject::SetEGLSwapInterval),
// so a table built from the 89 GLImpl sites would have left it null. It was the last class-C
// slot; since P10 B it IS the control-frame forwarder the old BackendObject_Remote override
// called. With no apply loop in this child the forwarder answers NOT_INITIALIZED and returns
// (ServerLoop.cpp, PostSurfaceControlFrameWithCallerLock), so the child EXITS - the P5 stub
// died Fatal{UnmigratedVerb, "SetSwapInterval"} right here. That the frame then reaches a
// running server is ServerLoopTest's TheEmitTableSwapIntervalSlotCrossesAsOneDispatchedFrame.
EXPECT_EQ(reinterpret_cast<const void*>(RemoteEmitTable().SetSwapInterval),
reinterpret_cast<const void*>(&MobileGL::MG_Remote::Server::ServerSetEGLSwapInterval));
const ChildResult r = RunInChild([] { RemoteEmitTable().SetSwapInterval(1); });
ASSERT_TRUE(DiedOfAbort(r)) << DescribeStatus(r) << "\n" << r.Log;
EXPECT_NE(r.Log.find("Fatal{UnmigratedVerb, \"SetSwapInterval\"}"), std::string::npos) << r.Log;
ASSERT_TRUE(WIFEXITED(r.Status) && WEXITSTATUS(r.Status) == 0) << DescribeStatus(r) << "\n" << r.Log;
EXPECT_EQ(r.Log.find("Fatal{UnmigratedVerb"), std::string::npos) << r.Log;
}
#endif // MGTEST_HAVE_FORK
@@ -389,13 +402,11 @@ TEST(RemoteEmitTable, SetSwapIntervalIsClassCAndSaysSo) {
TEST(RemoteEmitTable, TheSevenI1SlotsAreClassBAndAreNotTheFatalThunk) {
// The census's five measured slots plus the two companions that share their rows. Named
// rather than counted, so a table that flipped a DIFFERENT seven is red here and not only
// in the arithmetic. The comparison is against a slot that is still class C: a flipped slot
// and an unflipped one cannot be the same pointer, which is what a forgotten class-B
// assignment would look like (class C is assigned FIRST in BuildRemoteEmitTable precisely so
// that the mistake is loud rather than null).
// in the arithmetic. This case used to compare each slot against a slot that was still
// class C; since P10 B there is none left to compare against, so the class is decided
// behaviourally by EachI1SlotReachesRequireSessionUnderItsOwnName below and this case keeps
// the pointer facts that need no fork.
const MG_Backend::GlobalBackendFunctionsTable& table = RemoteEmitTable();
const void* fatal = reinterpret_cast<const void*>(table.SetSwapInterval);
ASSERT_NE(fatal, nullptr);
const void* const i1[] = {
reinterpret_cast<const void*>(table.GL.BindImageTexture),
reinterpret_cast<const void*>(table.GL.DispatchCompute),
@@ -411,9 +422,6 @@ TEST(RemoteEmitTable, TheSevenI1SlotsAreClassBAndAreNotTheFatalThunk) {
"ShaderStorageBlockBinding"};
for (SizeT i = 0; i < sizeof(i1) / sizeof(i1[0]); ++i) {
EXPECT_NE(i1[i], nullptr) << kNames[i] << " is null";
EXPECT_NE(i1[i], fatal) << kNames[i]
<< " still points at an UnmigratedVerbFatal thunk; i1 flipped it "
"to class B";
}
// The two barrier slots and the two dispatch slots share a WIRE ROW but not an emitter: the
// discriminant (ByRegion / IsIndirect) is set by the emitter, so one thunk for both would
MobileGL/MG_Test/Wire/ServerLoopTest.cpp
+27
View File
@@ -45,6 +45,7 @@
#include <MG_Pipe/PipeApply.h>
#include <MG_Remote/CapsCodec.h>
#include <MG_Remote/Client/ClientSession.h>
#include <MG_Remote/Client/EmitTables.h>
#include <MG_Remote/FatalFunnel.h>
#include <MG_Remote/Protocol/generated/protocol_generated.h>
#include <MG_Remote/Protocol/SurfaceOpCodec.h>
@@ -1199,6 +1200,32 @@ TEST(ServerLoopTest, AVoidForwarderCrossesAsOneDispatchedFrame) {
fixture.Stop();
}
// P10 B: THE EMIT TABLE'S SetSwapInterval SLOT REACHES THE SERVER. It was the last class-C slot
// (Fatal{UnmigratedVerb, "SetSwapInterval"}) and is now the forwarder above, which is what
// BackendObject::SetEGLSwapInterval calls for every eglSwapInterval on the client. The case goes
// through the TABLE, not the forwarder by name, and requires the frame to have crossed: the same
// dispatch counter as the case above. Red once by pointing the slot at a no-op lambda in
// BuildRemoteEmitTable - the counter stays 0 - and with the P5 stub restored the call aborts
// with Fatal{UnmigratedVerb, "SetSwapInterval"}.
TEST(ServerLoopTest, TheEmitTableSwapIntervalSlotCrossesAsOneDispatchedFrame) {
ServerFixture fixture;
ASSERT_TRUE(fixture.Handshake());
ASSERT_TRUE(fixture.StartLoop());
Server::ServerLoop& loop = Server::ServerLoopInstance();
ASSERT_TRUE(fixture.WaitUntilTrulyParked());
ASSERT_EQ(loop.ControlFramesDispatched(), 0u);
const auto slot = MobileGL::MG_Remote::Client::RemoteEmitTable().SetSwapInterval;
ASSERT_NE(slot, nullptr);
slot(1);
EXPECT_EQ(loop.ControlFramesDispatched(), 1u)
<< "the emit table's SetSwapInterval slot did not put exactly one control frame on the "
"server's channel; eglSwapInterval under split would not reach the server";
fixture.Stop();
}
// P5d round 3, package D: THE IDENTITY MUST DIE WITH THE THREAD, and this is the case that says
// so. OnApplyThread() is no longer `m_running && id == mine` - it is one relaxed load of
// Server::Detail::g_applyThreadKey compared against the caller's own thread pointer - so there