diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index e29f42ae..8d5a454c 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -830,15 +830,17 @@ jobs: # once inside a mutex critical section. Nothing under these two trees prints to a # stdio stream today - MGLOG_D compiles out in INFO builds and is the only channel # they are allowed to use - so this gate starts with no exceptions, and any addition - # to it needs a reason in the pull request rather than a quiet whitelist entry. + # to it needs a reason in the pull request rather than a quiet whitelist entry. The + # alternation names every stdio spelling, not just the two that were committed: + # fprintf to either stream, printf, puts, and the iostream pair. - name: No stdio instrumentation in MG_Backend or MG_State run: | - if grep -rnE 'fprintf[[:space:]]*\(stderr|(^|[^[:alnum:]_>.])printf[[:space:]]*\(' \ + if grep -rnE 'fprintf[[:space:]]*\((stderr|stdout)|(^|[^[:alnum:]_>.])printf[[:space:]]*\(|(^|[^[:alnum:]_>.:])puts[[:space:]]*\(|std::(cout|cerr)' \ MobileGL/MG_Backend MobileGL/MG_State; then echo "::error::stdio instrumentation found; use MGLOG_D (compiled out in INFO builds)" exit 1 fi - echo "no fprintf(stderr / printf( under MobileGL/MG_Backend or MobileGL/MG_State" + echo "no fprintf(stderr/stdout / printf( / puts( / std::cout|cerr under MobileGL/MG_Backend or MobileGL/MG_State" # Informational: the frontend mutation surface an MGPipe aggregate generation has to # cover. It becomes a gate in P1, when the mapping file exists to diff against. diff --git a/CMakeLists.txt b/CMakeLists.txt index 354cac15..8224159b 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -441,9 +441,14 @@ if (MOBILEGL_BUILD_DISAGGREGATED AND NOT EXISTS "${CMAKE_CURRENT_SOURCE_DIR}/3rdparty/flatbuffers/include/flatbuffers/flatbuffers.h") message(WARNING "MOBILEGL_BUILD_DISAGGREGATED=ON but 3rdparty/flatbuffers/include is missing. " - "Run `git submodule update --init 3rdparty/flatbuffers`. Forcing the option OFF.") - set(MOBILEGL_BUILD_DISAGGREGATED OFF CACHE BOOL - "Build the MG_Remote transport layer (two-process shape)" FORCE) + "Run `git submodule update --init 3rdparty/flatbuffers`. Building without the " + "disaggregated shape for this configure; the cached ON takes effect once the " + "submodule is present.") + # A NORMAL variable, deliberately not `CACHE BOOL ... FORCE`: forcing OFF into the cache + # made the plain re-configure after `git submodule update` stay OFF with no message at + # all. Shadowing the cache entry for this configure only keeps the operator's ON where it + # was, so the next configure - with the submodule there - honours it. + set(MOBILEGL_BUILD_DISAGGREGATED OFF) endif() if (MOBILEGL_BUILD_DISAGGREGATED) diff --git a/MobileGL/ConfigLoader.cpp b/MobileGL/ConfigLoader.cpp index 94558a22..a329008e 100644 --- a/MobileGL/ConfigLoader.cpp +++ b/MobileGL/ConfigLoader.cpp @@ -159,8 +159,11 @@ namespace MobileGL::MG_ConfigLoader { return static_cast(parsedValue); } - // Same contract as QueryEnvUint32, over 64 bits and accepting a 0x prefix: the one - // consumer is a subsystem BITMASK, and a bitmask written in decimal is unreadable. + // Same contract as QueryEnvUint32, over 64 bits and accepting an explicit 0x prefix: the + // one consumer is a subsystem BITMASK, and a bitmask written in decimal is unreadable. + // Decimal otherwise - never strtoull's base 0, whose "leading zero means octal" rule + // silently read MOBILEGL_PIPE_PUSH=010 as 8 - and a '-' anywhere is rejected rather than + // wrapped, which strtoull would otherwise do without complaint (-1 -> every bit set). inline Uint64 QueryEnvUint64(const String& key, Uint64 defaultValue) { auto it = acceptedEnvVariablesMap->find(key); if (it == acceptedEnvVariablesMap->end()) { @@ -168,12 +171,19 @@ namespace MobileGL::MG_ConfigLoader { } const String& value = it->second; + const char* text = value.c_str(); + int base = 10; + if (value.size() > 2 && text[0] == '0' && (text[1] == 'x' || text[1] == 'X')) { + text += 2; + base = 16; + } char* parseEnd = nullptr; errno = 0; - const unsigned long long parsedValue = std::strtoull(value.c_str(), &parseEnd, 0); - if (parseEnd == value.c_str() || *parseEnd != '\0' || errno == ERANGE) { - MGLOG_W("Config: Ignoring invalid env variable %s='%s'; expected an integer (decimal or " - "0x-prefixed), using default %llu", + const bool negative = value.find('-') != String::npos; + const unsigned long long parsedValue = negative ? 0 : std::strtoull(text, &parseEnd, base); + if (negative || parseEnd == text || *parseEnd != '\0' || errno == ERANGE) { + MGLOG_W("Config: Ignoring invalid env variable %s='%s'; expected a non-negative integer " + "(decimal, or 0x-prefixed hexadecimal), using default %llu", key.c_str(), value.c_str(), static_cast(defaultValue)); return defaultValue; } diff --git a/MobileGL/MG_Pipe/MGPipeTypes.h b/MobileGL/MG_Pipe/MGPipeTypes.h index 133fff8f..7d8f73d4 100644 --- a/MobileGL/MG_Pipe/MGPipeTypes.h +++ b/MobileGL/MG_Pipe/MGPipeTypes.h @@ -14,7 +14,9 @@ // Every MGPipe payload (plan B section 4.5). Each one is a flat POD with explicit padding, // carries a static_assert on trivial copyability and one on its exact size, and never -// contains a pointer other than the single MGHostSpan the design isolates on purpose. +// contains a pointer: MGHostSpan, the one shape that changes with the transport, only ever +// rides in a variable tail (draw_vbo's user indices, set_shader_buffers' named-UBO bytes), +// never inline in a fixed payload. // // Sizes are asserted rather than merely documented because the wire records generated from // these structs (generated/PipeWire.inc) are memcpy'd; a field silently changing width is a @@ -194,6 +196,15 @@ namespace MobileGL::MG_Pipe { }; MGP_ASSERT_POD(MGPQueryResultRequest, 16); + // query_timestamp: glGetInteger64v(GL_TIMESTAMP), the synchronous "what time is it on the + // GPU" GLFunctionsTable::GetGpuTimestampNs answers today. The request names nothing; the + // Int64 nanosecond stamp comes back through the reply slot. + struct MGPTimestampRequest { + Uint32 Reserved; + Uint32 Pad0; + }; + MGP_ASSERT_POD(MGPTimestampRequest, 8); + // --------------------------------------------------------------------------------- // CSOs // --------------------------------------------------------------------------------- @@ -409,25 +420,32 @@ namespace MobileGL::MG_Pipe { }; MGP_ASSERT_POD(MGPShaderImages, 16); - // One bound buffer range. Payload is populated for the Uniform class only, and only - // while kCapNeedsHostUboBytes is set (D-B8). + // One bound buffer range: 24 bytes, no inline host span. The named-UBO host bytes a + // backend needs under kCapNeedsHostUboBytes (D-B8) travel as an OPTIONAL second var-tail, + // MGHostSpan[HostSpanCount] behind the ranges, announced by MGPShaderBuffers below. An + // inline span would have cost every SSBO, atomic-counter and XFB range 32 dead bytes, and + // D-B8 says not to freeze that payload's shape before the stage-ubo-named counter has + // produced numbers. struct MGPBufferRange { MGPipeHandle Res; Uint64 Offset; Uint64 Size; - MGHostSpan Payload; }; - MGP_ASSERT_POD(MGPBufferRange, 56); + MGP_ASSERT_POD(MGPBufferRange, 24); - // Var-tail header: MGPBufferRange[Count] follows. + // Var-tail header: MGPBufferRange[Count], then MGHostSpan[HostSpanCount]. HostSpanCount is + // 0, or Count for the Uniform class under kCapNeedsHostUboBytes (a range with nothing to + // ship carries an empty span, so the two arrays stay index-aligned). struct MGPShaderBuffers { Uint32 Class; // Uniform | ShaderStorage | AtomicCounter Uint32 Start; Uint32 Count; Uint32 WritableMask; + Uint32 HostSpanCount; // 0, or Count when the kHostSpan tail is present (D-B8) + Uint32 Pad0; Uint64 ContentHash; }; - MGP_ASSERT_POD(MGPShaderBuffers, 24); + MGP_ASSERT_POD(MGPShaderBuffers, 32); // Var-tail header: MGPBufferRange[Count] then Uint32 offsets[Count]. struct MGPStreamOutputTargets { @@ -470,7 +488,11 @@ namespace MobileGL::MG_Pipe { PixelStoreParameters Pack; }; static_assert(std::is_trivially_copyable_v); - static_assert(sizeof(MGPPixelPackState) == sizeof(PixelStoreParameters)); + // 28 is what PixelStoreParameters measures: two Bools, two bytes of padding, six Ints. + // Asserting against sizeof(PixelStoreParameters) itself was a tautology that could not + // notice the value struct changing width under the wire format. + static_assert(sizeof(MGPPixelPackState) == 28, + "MGPPixelPackState changed size; update the wire format and this assertion"); // Also a shader-variant input: both backends bake these into the synthesized // pass-through control stage. @@ -542,6 +564,15 @@ namespace MobileGL::MG_Pipe { // Carries the union box AND the region list so the SERVER picks the upload shape - the // decision belongs on the side that pays the GPU cost. Mali prices texture upload by // JOB COUNT: ~100 sprite rects against one union box measured +6 ms/frame. + // + // THE BUFFER HALF. With Target == Buffer there is no level and no box, so the destination + // byte range rides in the box's first coordinate and first extent: UnionBox.X is the byte + // offset, UnionBox.W the byte size, Y = Z = 0, H = D = 1, Level = 0, RegionCount = 0, and + // Blob holds exactly Size source bytes. That caps ONE record at a 2^31-1 offset and a + // 2^32-1 size; a range beyond either is split by the emitter - the same rule, and at + // SEG_STAGE's 32 MiB the far tighter one, that the ring's half-capacity bound already + // imposes on it. MGPipeSetSubDataBufferRange / MGPipeSubDataBufferOffset / Size below are + // the only spelling of this convention; nothing else reads the box for a buffer. struct MGPSubData { MGPipeHandle Res; Uint16 Target, Level; @@ -556,6 +587,24 @@ namespace MobileGL::MG_Pipe { }; MGP_ASSERT_POD(MGPSubData, 72); + // Encodes a buffer byte range into the record's box. False, with the record untouched, + // when the range does not fit one record: the emitter has to split it. + inline Bool MGPipeSetSubDataBufferRange(MGPSubData& record, Uint64 offset, Uint64 size) { + if (offset > 0x7FFFFFFFull || size > 0xFFFFFFFFull) { + return false; + } + record.UnionBox = MGPBox{static_cast(offset), 0, 0, static_cast(size), 1, 1}; + record.Level = 0; + record.RegionCount = 0; + return true; + } + inline Uint64 MGPipeSubDataBufferOffset(const MGPSubData& record) { + // A negative X is a corrupt record (the encoder never writes one); read as unsigned + // it lands above the encodable bound, which the applier's bounds gate refuses. + return static_cast(static_cast(record.UnionBox.X)); + } + inline Uint64 MGPipeSubDataBufferSize(const MGPSubData& record) { return record.UnionBox.W; } + // The forward terminator for a server-initiated texture pull (section 7.1). May carry // zero regions - that is how a pull that needs nothing is answered. struct MGPSubDataComplete { diff --git a/MobileGL/MG_Pipe/PipeCalls.def b/MobileGL/MG_Pipe/PipeCalls.def index 05db1f18..469cf3bf 100644 --- a/MobileGL/MG_Pipe/PipeCalls.def +++ b/MobileGL/MG_Pipe/PipeCalls.def @@ -27,22 +27,23 @@ // that the expansion, the two generated tables and this number agree. // // class entries group (as the plan tabulates it) -// kScreen 10 screen: caps 1 + resource 3 + persistent map 2 + fence 4 -// kCtxQuery 6 query object namespace +// kScreen 11 screen: caps 1 + resource 3 + persistent map 2 + fence 4, plus the +// appended server-side fence wait 1 +// kCtxQuery 8 query object namespace 6, plus the appended timestamp pair 2 // kCtxCso 13 CSO create/bind/delete // kCtxState 17 16 of the 17 set_* calls + the temporary set_residual_value_state // kCtxObject 9 set_texture_params (the 17th set_*) + 8 object-scoped transfers // kCtxVerb 13 3 context-reading transfer calls + the 10 commands -// total 68 +// total 71 // // Reconciliation with the plan's headline numbers (section 4.4 / appendix A), because they // do not add up to a set of UNIQUE records and this file has to hold unique records: // - "screen 14" tabulates the fence and query families together with the screen block. // Section 4.3 assigns the query NAMESPACE to the context ("VAO / FBO / XFB object / // query namespaces, the command stream, present"), so the six query calls carry -// kCtxQuery and live in MGPipeContext. Screen keeps 10. The eight EGL lifecycle entry -// points stay virtual functions on pActiveBackendObject and are deliberately NOT calls -// here (section 4.4.1, last row). +// kCtxQuery and live in MGPipeContext. Screen keeps 10 of the plan's (11 with the appended +// FenceWaitServer, below). The eight EGL lifecycle entry points stay virtual functions on +// pActiveBackendObject and are deliberately NOT calls here (section 4.4.1, last row). // - "CSO 15" is create/bind/delete x 5 kinds. Two of those binds are ALSO named in the // set_* catalogue as their array forms - bind_sampler_states and set_sampler_views // (section 4.4.3) - and a call may only exist once, so they are emitted under @@ -54,10 +55,18 @@ // resource_subdata_complete). Eleven is what is emitted; the twelfth is not named // anywhere in the plan. // - "about 74 items" in section 4.1 is the sum of those headline numbers, so it inherits -// the same double counting. 68 unique records is the honest total. +// the same double counting. 68 unique records was the honest total of the plan's own +// catalogue. +// - Three LIVE GLFunctionsTable entries had no carrier in it at all: GetGpuTimestampNs +// (glGetInteger64v(GL_TIMESTAMP), a synchronous server answer), QueryCounterTimestamp +// (glQueryCounter, a one-shot stamp rather than a begin/end pair) and WaitSync (the +// GPU-side wait, which FenceWait's client-side wait does not express). They are +// QueryTimestamp, QueryCounter and FenceWaitServer, APPENDED at the end of the list - +// not slotted into their groups - because the wire opcode is the position, so a record +// that arrives late goes last. 71 unique records. // --------------------------------------------------------------------------------------- -#define MGP_CALL_LIST_DOCUMENTED_COUNT 68 +#define MGP_CALL_LIST_DOCUMENTED_COUNT 71 // clang-format off #define MGP_CALL_LIST(X) \ @@ -137,7 +146,18 @@ X(ResumeStreamOutput, MGPStreamOutputControl, kCtxVerb, kNone) \ X(Flush, MGPFlush, kCtxVerb, kNone) \ X(Present, MGPPresent, kCtxVerb, kNone) \ - X(SetSwapInterval, MGPSwapInterval, kCtxVerb, kOptional) + X(SetSwapInterval, MGPSwapInterval, kCtxVerb, kOptional) \ + /* ---- APPENDED. Opcodes are positional, so a late arrival goes at the END, never into ---- */ \ + /* ---- its group: three live GLFunctionsTable entries the catalogue had no carrier for. ---- */ \ + /* glGetInteger64v(GL_TIMESTAMP) - GetGpuTimestampNs, a synchronous server answer, which */ \ + /* the reply slot carries. The query namespace is the context's (plan 4.3). */ \ + X(QueryTimestamp, MGPTimestampRequest, kCtxQuery, kReplySlot) \ + /* glQueryCounter(GL_TIMESTAMP) - QueryCounterTimestamp, a one-shot stamp into a query */ \ + /* object, NOT a begin/end pair. Kind carries GL_TIMESTAMP. */ \ + X(QueryCounter, MGPQueryDesc, kCtxQuery, kNone) \ + /* glWaitSync - WaitSync, the GPU-side wait, distinct from FenceWait's client-side one. */ \ + /* TimeoutNs is GL_TIMEOUT_IGNORED by contract. */ \ + X(FenceWaitServer, MGPFenceWait, kScreen, kNone) // clang-format on // Explicitly NOT migrated (plan 4.4.6 / appendix A "explicit deletions"): diff --git a/MobileGL/MG_Pipe/PipeFields.def b/MobileGL/MG_Pipe/PipeFields.def index 0aaddd1d..d142ac80 100644 --- a/MobileGL/MG_Pipe/PipeFields.def +++ b/MobileGL/MG_Pipe/PipeFields.def @@ -54,6 +54,9 @@ #define MGP_FIELDS_MGPQueryResultRequest(F) \ F(Query) F(Wait) +#define MGP_FIELDS_MGPTimestampRequest(F) \ + F(Reserved) + #define MGP_FIELDS_MGPRenderStateDesc(F) \ F(Cso) F(BaseCso) F(ChunkMask) F(Blob) @@ -116,10 +119,10 @@ F(Start) F(Count) F(ContentHash) #define MGP_FIELDS_MGPBufferRange(F) \ - F(Res) F(Offset) F(Size) F(Payload) + F(Res) F(Offset) F(Size) #define MGP_FIELDS_MGPShaderBuffers(F) \ - F(Class) F(Start) F(Count) F(WritableMask) F(ContentHash) + F(Class) F(Start) F(Count) F(WritableMask) F(HostSpanCount) F(ContentHash) #define MGP_FIELDS_MGPStreamOutputTargets(F) \ F(Count) F(Generation) F(ContentHash) @@ -219,7 +222,7 @@ // macros; gen_pipe.py reads THIS list to know what to emit. #define MGP_VERIFY_PAYLOAD_LIST(P) \ P(MGPBlobRef) P(MGPRange) P(MGPBox) P(MGPReplySlot) P(MGPStateChunk) P(MGPHandleOnly) P(MGPCaps) \ - P(MGPResourceDesc) P(MGPFenceWait) P(MGPQueryDesc) P(MGPQueryResultRequest) P(MGPRenderStateDesc) \ + P(MGPResourceDesc) P(MGPFenceWait) P(MGPQueryDesc) P(MGPQueryResultRequest) P(MGPTimestampRequest) P(MGPRenderStateDesc) \ P(MGPBindRenderState) P(MGPDynamicState) P(MGPVertexElements) P(MGPSamplerDesc) P(MGPSamplerView) \ P(MGPTextureParams) P(MGPProgramDesc) P(MGPSurface) P(MGPFramebufferState) P(MGPVertexBuffer) \ P(MGPVertexBuffers) P(MGPIndexBuffer) P(MGPIndirectBuffers) P(MGPBoundView) P(MGPSamplerViews) \ diff --git a/MobileGL/MG_Pipe/generated/PipeTables.inc b/MobileGL/MG_Pipe/generated/PipeTables.inc index cb8a602d..072635c4 100644 --- a/MobileGL/MG_Pipe/generated/PipeTables.inc +++ b/MobileGL/MG_Pipe/generated/PipeTables.inc @@ -12,7 +12,7 @@ // Regenerate with `python3 scripts/gen_pipe.py`; CI runs it and diffs the result. // This file is included from MG_Pipe/MGPipe.h inside namespace MobileGL::MG_Pipe. -// share group: 10 calls. A null entry means the backend does not implement this +// share group: 11 calls. A null entry means the backend does not implement this // call and the frontend keeps its own path (plan B section 4.1). struct MGPipeScreen { void (*GetCaps)(const MGPCaps* payload, MGPReplySlot* reply); @@ -25,9 +25,10 @@ struct MGPipeScreen { void (*FenceStatus)(const MGPHandleOnly* payload, MGPReplySlot* reply); void (*FenceWait)(const MGPFenceWait* payload, MGPReplySlot* reply); void (*FenceDestroy)(const MGPHandleOnly* payload); + void (*FenceWaitServer)(const MGPFenceWait* payload); }; -// context: 58 calls. A null entry means the backend does not implement this +// context: 60 calls. A null entry means the backend does not implement this // call and the frontend keeps its own path (plan B section 4.1). struct MGPipeContext { void (*QueryCreate)(const MGPQueryDesc* payload); @@ -88,11 +89,13 @@ struct MGPipeContext { void (*Flush)(const MGPFlush* payload); void (*Present)(const MGPPresent* payload); void (*SetSwapInterval)(const MGPSwapInterval* payload); + void (*QueryTimestamp)(const MGPTimestampRequest* payload, MGPReplySlot* reply); + void (*QueryCounter)(const MGPQueryDesc* payload); }; -inline constexpr SizeT kMGPipeScreenCallCount = 10; -inline constexpr SizeT kMGPipeContextCallCount = 58; -inline constexpr SizeT kMGPipeCallCount = 68; +inline constexpr SizeT kMGPipeScreenCallCount = 11; +inline constexpr SizeT kMGPipeContextCallCount = 60; +inline constexpr SizeT kMGPipeCallCount = 71; // A table that is not exactly its call count of function pointers has grown a // member that no generator knows about. diff --git a/MobileGL/MG_Pipe/generated/PipeThunks.inc b/MobileGL/MG_Pipe/generated/PipeThunks.inc index 70e99931..6a898679 100644 --- a/MobileGL/MG_Pipe/generated/PipeThunks.inc +++ b/MobileGL/MG_Pipe/generated/PipeThunks.inc @@ -288,3 +288,15 @@ inline void MGP_Present(const MGPPresent* payload) { inline void MGP_SetSwapInterval(const MGPSwapInterval* payload) { gMGPipeContext.SetSwapInterval(payload); } + +inline void MGP_QueryTimestamp(const MGPTimestampRequest* payload, MGPReplySlot* reply) { + gMGPipeContext.QueryTimestamp(payload, reply); +} + +inline void MGP_QueryCounter(const MGPQueryDesc* payload) { + gMGPipeContext.QueryCounter(payload); +} + +inline void MGP_FenceWaitServer(const MGPFenceWait* payload) { + gMGPipeScreen.FenceWaitServer(payload); +} diff --git a/MobileGL/MG_Pipe/generated/PipeVerify.inc b/MobileGL/MG_Pipe/generated/PipeVerify.inc index 4b14a829..db652e6e 100644 --- a/MobileGL/MG_Pipe/generated/PipeVerify.inc +++ b/MobileGL/MG_Pipe/generated/PipeVerify.inc @@ -38,6 +38,7 @@ inline Bool MGPipeVerify(const MGPResourceDesc& a, const MGPResourceDesc& b, con inline Bool MGPipeVerify(const MGPFenceWait& a, const MGPFenceWait& b, const char** outField); inline Bool MGPipeVerify(const MGPQueryDesc& a, const MGPQueryDesc& b, const char** outField); inline Bool MGPipeVerify(const MGPQueryResultRequest& a, const MGPQueryResultRequest& b, const char** outField); +inline Bool MGPipeVerify(const MGPTimestampRequest& a, const MGPTimestampRequest& b, const char** outField); inline Bool MGPipeVerify(const MGPRenderStateDesc& a, const MGPRenderStateDesc& b, const char** outField); inline Bool MGPipeVerify(const MGPBindRenderState& a, const MGPBindRenderState& b, const char** outField); inline Bool MGPipeVerify(const MGPDynamicState& a, const MGPDynamicState& b, const char** outField); @@ -113,6 +114,8 @@ struct MGPipeHasFieldVerifier : std::true_type {}; template <> struct MGPipeHasFieldVerifier : std::true_type {}; template <> +struct MGPipeHasFieldVerifier : std::true_type {}; +template <> struct MGPipeHasFieldVerifier : std::true_type {}; template <> struct MGPipeHasFieldVerifier : std::true_type {}; @@ -305,6 +308,11 @@ inline Bool MGPipeVerify(const MGPQueryResultRequest& a, const MGPQueryResultReq return true; } +inline Bool MGPipeVerify(const MGPTimestampRequest& a, const MGPTimestampRequest& b, const char** outField) { + MGP_FIELDS_MGPTimestampRequest(MGP_VERIFY_FIELD) + return true; +} + inline Bool MGPipeVerify(const MGPRenderStateDesc& a, const MGPRenderStateDesc& b, const char** outField) { MGP_FIELDS_MGPRenderStateDesc(MGP_VERIFY_FIELD) return true; @@ -562,4 +570,4 @@ inline Bool MGPipeVerify(const MGPSurfaceInfo& a, const MGPSurfaceInfo& b, const #undef MGP_VERIFY_FIELD -inline constexpr SizeT kMGPipeVerifiedPayloadCount = 62; +inline constexpr SizeT kMGPipeVerifiedPayloadCount = 63; diff --git a/MobileGL/MG_Pipe/generated/PipeWire.inc b/MobileGL/MG_Pipe/generated/PipeWire.inc index 249fa634..fca186f1 100644 --- a/MobileGL/MG_Pipe/generated/PipeWire.inc +++ b/MobileGL/MG_Pipe/generated/PipeWire.inc @@ -112,7 +112,10 @@ enum class MGPWireOp : Uint16 { Flush = 66, Present = 67, SetSwapInterval = 68, - kOpCount = 69, + QueryTimestamp = 69, + QueryCounter = 70, + FenceWaitServer = 71, + kOpCount = 72, }; struct alignas(8) MGPWireRec_GetCaps { @@ -659,6 +662,30 @@ static_assert(sizeof(MGPWireRec_SetSwapInterval) == ((sizeof(MGPWireRecHeader) + sizeof(MGPSwapInterval) + 7u) & ~SizeT(7u)), "MGPWireRec_SetSwapInterval gained padding; the wire format moved"); +struct alignas(8) MGPWireRec_QueryTimestamp { + MGPWireRecHeader Header; + MGPTimestampRequest Payload; +}; +static_assert(sizeof(MGPWireRec_QueryTimestamp) == + ((sizeof(MGPWireRecHeader) + sizeof(MGPTimestampRequest) + 7u) & ~SizeT(7u)), + "MGPWireRec_QueryTimestamp gained padding; the wire format moved"); + +struct alignas(8) MGPWireRec_QueryCounter { + MGPWireRecHeader Header; + MGPQueryDesc Payload; +}; +static_assert(sizeof(MGPWireRec_QueryCounter) == + ((sizeof(MGPWireRecHeader) + sizeof(MGPQueryDesc) + 7u) & ~SizeT(7u)), + "MGPWireRec_QueryCounter gained padding; the wire format moved"); + +struct alignas(8) MGPWireRec_FenceWaitServer { + MGPWireRecHeader Header; + MGPFenceWait Payload; +}; +static_assert(sizeof(MGPWireRec_FenceWaitServer) == + ((sizeof(MGPWireRecHeader) + sizeof(MGPFenceWait) + 7u) & ~SizeT(7u)), + "MGPWireRec_FenceWaitServer gained padding; the wire format moved"); + [[noreturn]] inline void MGPipeWireProtocolFatal(const char* call, Uint64 size, Uint64 remaining) { MGLOG_F("MGPipe: protocol corruption applying %s: size=%llu remaining=%llu", call, static_cast(size), static_cast(remaining)); @@ -885,6 +912,15 @@ inline Bool MGPipeApplyWireRecord(MGPWireOp op, const void* record, Uint64 size, case MGPWireOp::SetSwapInterval: MGP_WIRE_CHECK_BOUNDS(MGPWireRec_SetSwapInterval, "SetSwapInterval"); return false; + case MGPWireOp::QueryTimestamp: + MGP_WIRE_CHECK_BOUNDS(MGPWireRec_QueryTimestamp, "QueryTimestamp"); + return false; + case MGPWireOp::QueryCounter: + MGP_WIRE_CHECK_BOUNDS(MGPWireRec_QueryCounter, "QueryCounter"); + return false; + case MGPWireOp::FenceWaitServer: + MGP_WIRE_CHECK_BOUNDS(MGPWireRec_FenceWaitServer, "FenceWaitServer"); + return false; case MGPWireOp::kInvalid: case MGPWireOp::kOpCount: default: diff --git a/MobileGL/MG_Remote/Transport/Doorbell.cpp b/MobileGL/MG_Remote/Transport/Doorbell.cpp index 7ec1f215..1851f38b 100644 --- a/MobileGL/MG_Remote/Transport/Doorbell.cpp +++ b/MobileGL/MG_Remote/Transport/Doorbell.cpp @@ -20,6 +20,12 @@ #include #endif +// Same fallback as FdPassing.cpp: on macOS / BSD the protection is SO_NOSIGPIPE on the +// socket, set in SocketDoorbell's constructor, not a per-send flag. +#if !defined(_WIN32) && !defined(MSG_NOSIGNAL) +#define MSG_NOSIGNAL 0 +#endif + namespace MobileGL::MG_Remote::Transport { // ----------------------------------------------------------------------- @@ -100,7 +106,16 @@ namespace MobileGL::MG_Remote::Transport { // ----------------------------------------------------------------------- SocketDoorbell::SocketDoorbell(int fd, std::uint8_t code, bool ownsFd) - : m_fd(fd), m_code(code), m_ownsFd(ownsFd) {} + : m_fd(fd), m_code(code), m_ownsFd(ownsFd) { +#if defined(SO_NOSIGPIPE) + // The per-socket form of MSG_NOSIGNAL, on the platforms that lack the per-call one: + // a Notify to a hung-up peer must come back as EPIPE, not as a fatal signal. + if (m_fd >= 0) { + const int one = 1; + (void)::setsockopt(m_fd, SOL_SOCKET, SO_NOSIGPIPE, &one, sizeof(one)); + } +#endif + } SocketDoorbell::~SocketDoorbell() { if (m_ownsFd && m_fd >= 0) { diff --git a/MobileGL/MG_Remote/Transport/FdPassing.cpp b/MobileGL/MG_Remote/Transport/FdPassing.cpp index 22991177..a2dff4c4 100644 --- a/MobileGL/MG_Remote/Transport/FdPassing.cpp +++ b/MobileGL/MG_Remote/Transport/FdPassing.cpp @@ -15,12 +15,21 @@ #if !defined(_WIN32) #include +#include #include #include #include #include #endif +// MSG_NOSIGNAL is Linux (and Android). macOS and the BSDs spell the same protection as the +// SO_NOSIGPIPE socket option, set once per socket at creation (CreateSocketPair below, and +// SocketDoorbell's constructor). With neither, a write to a hung-up peer raises SIGPIPE and +// kills the process instead of returning EPIPE. +#if !defined(_WIN32) && !defined(MSG_NOSIGNAL) +#define MSG_NOSIGNAL 0 +#endif + namespace MobileGL::MG_Remote::Transport::FdPassing { #if defined(_WIN32) @@ -84,6 +93,13 @@ namespace MobileGL::MG_Remote::Transport::FdPassing { MGLOG_E("MG_Remote fd passing: socketpair failed (errno=%d)", errno); return MOBILEGL_ERR_TRANSPORT_CLOSED; } +#if defined(SO_NOSIGPIPE) + // The per-socket form of MSG_NOSIGNAL, on the platforms that lack the per-call one. + for (int fd : fds) { + const int one = 1; + (void)::setsockopt(fd, SOL_SOCKET, SO_NOSIGPIPE, &one, sizeof(one)); + } +#endif outFds[0] = fds[0]; outFds[1] = fds[1]; return MOBILEGL_OK; @@ -241,6 +257,17 @@ namespace MobileGL::MG_Remote::Transport::FdPassing { received[receivedCount++] = fd; } } +#if !defined(MSG_CMSG_CLOEXEC) + // No atomic close-on-exec on receive here (macOS, the BSDs): set it by hand on every + // descriptor that arrived, before anything else can fork. The window between the + // recvmsg and this loop is the platform's, not ours; leaving the flag off altogether + // would hand every shared segment to every child the process ever spawns. + for (int i = 0; i < receivedCount; ++i) { + if (received[i] >= 0) { + (void)::fcntl(received[i], F_SETFD, FD_CLOEXEC); + } + } +#endif const auto closeAll = [&](int keepIndex) { for (int i = 0; i < receivedCount; ++i) { if (i != keepIndex && received[i] >= 0) { diff --git a/MobileGL/MG_Test/Pipe/PipeCatalogueTest.cpp b/MobileGL/MG_Test/Pipe/PipeCatalogueTest.cpp index 3d63eb71..ab9907dd 100644 --- a/MobileGL/MG_Test/Pipe/PipeCatalogueTest.cpp +++ b/MobileGL/MG_Test/Pipe/PipeCatalogueTest.cpp @@ -12,6 +12,8 @@ #include +#include + #include "Includes.h" #include @@ -81,8 +83,8 @@ TEST(PipeCatalogue, GeneratedTablesHoldTheWholeCatalogue) { EXPECT_EQ(kMGPipeContextCallCount, kMGPipeCallCount - ClassCount()); // The per-class counts PipeCalls.def documents in its header. - EXPECT_EQ(ClassCount(), 10u); - EXPECT_EQ(ClassCount(), 6u); + EXPECT_EQ(ClassCount(), 11u); + EXPECT_EQ(ClassCount(), 8u); EXPECT_EQ(ClassCount(), 13u); EXPECT_EQ(ClassCount(), 17u); EXPECT_EQ(ClassCount(), 9u); @@ -123,6 +125,17 @@ TEST(PipeCatalogue, WireOpcodesAreThePositionsInTheCatalogue) { EXPECT_EQ(sizeof(MGPWireRec_SetResidualValueState) % 8, 0u); } +// Records are append-only. The three carriers added after the first cut - for the live +// GLFunctionsTable entries GetGpuTimestampNs, QueryCounterTimestamp and WaitSync - sit at +// the END of the list, after SetSwapInterval, so no opcode the first cut assigned has moved. +TEST(PipeCatalogue, LateArrivalsAreAppendedWithoutRenumbering) { + EXPECT_EQ(static_cast(MGPWireOp::SetSwapInterval), 68); + EXPECT_EQ(static_cast(MGPWireOp::QueryTimestamp), 69); + EXPECT_EQ(static_cast(MGPWireOp::QueryCounter), 70); + EXPECT_EQ(static_cast(MGPWireOp::FenceWaitServer), 71); + EXPECT_EQ(static_cast(MGPWireOp::kOpCount), 72); +} + // A well-formed record passes the applier's bounds gate. P0 has no applier, so "accepted" // is reported as "not applied" rather than "fatal". TEST(PipeCatalogue, ApplierAcceptsAWellFormedRecord) { @@ -218,3 +231,59 @@ TEST(PipeCatalogue, HostSpanResolvesTheMonolithPointer) { EXPECT_EQ(gMGPipeSegmentResolver, nullptr); EXPECT_EQ(MGPipeHostBytes(staged), nullptr); } + +// D-B8: a bound buffer range carries no inline host span. The named-UBO bytes are an +// optional second var-tail announced by HostSpanCount, so the SSBO, atomic-counter and XFB +// ranges - the majority - pay nothing for a payload whose shape is not frozen yet. +TEST(PipeCatalogue, BufferRangeCarriesNoInlineHostSpan) { + static_assert(sizeof(MGPBufferRange) == 24); + static_assert(sizeof(MGPShaderBuffers) == 32); + EXPECT_LT(sizeof(MGPBufferRange), sizeof(MGHostSpan)); + + // The call still declares the span it may carry, so the transport lays the tail out. + Uint32 flags = 0; +#define MGP_FLAGS_OF_SET_SHADER_BUFFERS(Name, Payload, Class, Flags) \ + if (std::strcmp(#Name, "SetShaderBuffers") == 0) flags = static_cast(Flags); + MGP_CALL_LIST(MGP_FLAGS_OF_SET_SHADER_BUFFERS) +#undef MGP_FLAGS_OF_SET_SHADER_BUFFERS + EXPECT_EQ(flags & (kVarTail | kHostSpan), static_cast(kVarTail | kHostSpan)); + + // And the comparator sees the count that announces the tail. + MGPShaderBuffers a{}; + MGPShaderBuffers b{}; + const char* field = nullptr; + EXPECT_TRUE(MGPipeVerify(a, b, &field)); + b.HostSpanCount = 4; + EXPECT_FALSE(MGPipeVerify(a, b, &field)); + EXPECT_STREQ(field, "HostSpanCount"); +} + +// The buffer half of resource_subdata has no level and no box of its own: [offset, size) +// rides in UnionBox.X / UnionBox.W, and only through the two helpers, which also say where +// one record stops and the emitter has to split. +TEST(PipeCatalogue, SubDataBufferRangeRidesInTheUnionBox) { + MGPSubData record{}; + record.Level = 3; + record.RegionCount = 2; + ASSERT_TRUE(MGPipeSetSubDataBufferRange(record, 4096, 65536)); + EXPECT_EQ(record.UnionBox.X, 4096); + EXPECT_EQ(record.UnionBox.W, 65536u); + EXPECT_EQ(record.UnionBox.Y, 0); + EXPECT_EQ(record.UnionBox.Z, 0); + EXPECT_EQ(record.UnionBox.H, 1u); + EXPECT_EQ(record.UnionBox.D, 1u); + EXPECT_EQ(record.Level, 0); + EXPECT_EQ(record.RegionCount, 0u); + EXPECT_EQ(MGPipeSubDataBufferOffset(record), 4096u); + EXPECT_EQ(MGPipeSubDataBufferSize(record), 65536u); + + // The largest range one record expresses... + ASSERT_TRUE(MGPipeSetSubDataBufferRange(record, 0x7FFFFFFFull, 0xFFFFFFFFull)); + EXPECT_EQ(MGPipeSubDataBufferOffset(record), 0x7FFFFFFFull); + EXPECT_EQ(MGPipeSubDataBufferSize(record), 0xFFFFFFFFull); + // ...and beyond it the emitter splits: refused, record untouched. + EXPECT_FALSE(MGPipeSetSubDataBufferRange(record, 0x80000000ull, 1)); + EXPECT_FALSE(MGPipeSetSubDataBufferRange(record, 0, 0x100000000ull)); + EXPECT_EQ(MGPipeSubDataBufferOffset(record), 0x7FFFFFFFull); + EXPECT_EQ(MGPipeSubDataBufferSize(record), 0xFFFFFFFFull); +} diff --git a/MobileGL/MG_Util/Metrics/PipeStats.cpp b/MobileGL/MG_Util/Metrics/PipeStats.cpp index e1212315..165e436c 100644 --- a/MobileGL/MG_Util/Metrics/PipeStats.cpp +++ b/MobileGL/MG_Util/Metrics/PipeStats.cpp @@ -296,7 +296,12 @@ namespace MobileGL::MG_Util::PipeStats { void RecordDrawPayloadBytes(Uint64 bytes) { Bump(g_totalPayloadBuckets[PayloadBucketOf(bytes)], 1); } void OnPresent() { -#ifdef TRACY_ENABLE + // Every frame accumulator is EXCHANGED for zero, and the exchanged value is what gets + // plotted. A read followed by a store(0) would lose any Bump that lands in between - + // buffer and texture staging reach these counters from more than one thread - from + // the plot AND from every frame; an exchange hands every add to exactly one frame. + // Without Tracy the value is taken and dropped: the clear is still the point. + // // One plot per counter, the frame's value. Tracy keeps the series by name, and the // names are the static literals above, which is what TracyPlot requires. A gate is // two series - hits and misses - because the ratio is the deliverable and a miss @@ -305,26 +310,30 @@ namespace MobileGL::MG_Util::PipeStats { // The payload histogram is deliberately NOT plotted: it is a run-total distribution // over draws (section 4.5.7), not a per-frame scalar, and Tracy has no histogram // series. It reaches the operator through the JSON dump. + const auto take = [](Counter& counter) { return counter.exchange(0, std::memory_order_relaxed); }; for (Uint32 i = 0; i < kByteClassCount; ++i) { - TracyPlot(kByteClassNames[i], static_cast(Read(g_frameBytes[i]))); - } - for (Uint32 i = 0; i < kCallClassCount; ++i) { - TracyPlot(kCallClassNames[i], static_cast(Read(g_frameCalls[i]))); - } - for (Uint32 i = 0; i < kGateCount; ++i) { - TracyPlot(kGateHitPlotNames[i], static_cast(Read(g_frameGateHit[i]))); - TracyPlot(kGateMissPlotNames[i], static_cast(Read(g_frameGateMiss[i]))); - } + const Uint64 value = take(g_frameBytes[i]); + (void)value; +#ifdef TRACY_ENABLE + TracyPlot(kByteClassNames[i], static_cast(value)); #endif - for (Uint32 i = 0; i < kByteClassCount; ++i) { - g_frameBytes[i].store(0, std::memory_order_relaxed); } for (Uint32 i = 0; i < kCallClassCount; ++i) { - g_frameCalls[i].store(0, std::memory_order_relaxed); + const Uint64 value = take(g_frameCalls[i]); + (void)value; +#ifdef TRACY_ENABLE + TracyPlot(kCallClassNames[i], static_cast(value)); +#endif } for (Uint32 i = 0; i < kGateCount; ++i) { - g_frameGateHit[i].store(0, std::memory_order_relaxed); - g_frameGateMiss[i].store(0, std::memory_order_relaxed); + const Uint64 hits = take(g_frameGateHit[i]); + const Uint64 misses = take(g_frameGateMiss[i]); + (void)hits; + (void)misses; +#ifdef TRACY_ENABLE + TracyPlot(kGateHitPlotNames[i], static_cast(hits)); + TracyPlot(kGateMissPlotNames[i], static_cast(misses)); +#endif } const Uint64 frames = g_frameCount.fetch_add(1, std::memory_order_relaxed) + 1; if (frames % kSummaryFramePeriod == 0) { diff --git a/scripts/gen_pipe.py b/scripts/gen_pipe.py index e6599abb..8b128798 100644 --- a/scripts/gen_pipe.py +++ b/scripts/gen_pipe.py @@ -24,6 +24,9 @@ from the catalogue (they all consume the same .def). python3 scripts/gen_pipe.py # write the generated files, print the summary python3 scripts/gen_pipe.py --check # fail if regenerating would change anything + +Both modes refuse a catalogue whose call payload has no field list in PipeFields.def: a +payload the G4 comparator cannot see is a payload MOBILEGL_PIPE_VERIFY is blind to. """ import argparse @@ -157,6 +160,17 @@ def parse_calls(): return calls +# The member types the G4 comparator falls back to memcmp for (see gen_verify): the +# MG_State / MG_Backend value structs and MGHostSpan. They are not call payloads and get +# field lists of their own in P0.5. Nothing else may be missing from PipeFields.def. +MEMCMP_FALLBACK_TYPES = { + "RenderStateParameters", + "PixelStoreParameters", + "DynamicBackendParameters", + "MGHostSpan", +} + + def parse_verify_payloads(): text = read(os.path.join(PIPE_DIR, "PipeFields.def")) match = re.search(r"#define MGP_VERIFY_PAYLOAD_LIST\(P\)(.*?)\n\n", text, re.S) @@ -169,6 +183,17 @@ def parse_verify_payloads(): return payloads +def check_call_payloads_have_field_lists(calls, payloads): + """Every payload PipeCalls.def names must have a G4 field list, or the verify comparator + is silently blind to that call. Runs in both modes, --check included.""" + known = set(payloads) + missing = sorted({c.Payload for c in calls + if c.Payload not in known and c.Payload not in MEMCMP_FALLBACK_TYPES}) + if missing: + sys.exit("PipeFields.def: call payload(s) with no field list, so MOBILEGL_PIPE_VERIFY " + "would be blind to them: %s" % ", ".join(missing)) + + def parse_coverage(): text = read(os.path.join(PIPE_DIR, "Coverage.def")) accessors = [] @@ -608,6 +633,7 @@ def main(): calls = parse_calls() payloads = parse_verify_payloads() + check_call_payloads_have_field_lists(calls, payloads) accessors, deltas = parse_coverage() rows = parse_inventory()