From 7ffa4998ab2be3d9ecb2604635dedff8c5705bc9 Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Wed, 16 Sep 2026 11:49:41 -0400 Subject: [PATCH 1/9] [Feat] (Remote): migrate f1 clear copy and mipmap verbs under inproc --- MobileGL/MG_IntegrationTest/CMakeLists.txt | 16 + .../Scenarios/F1WireScenario.cpp | 222 +++++++++++++ MobileGL/MG_Remote/Client/EmitTables.cpp | 153 +++++++-- MobileGL/MG_Remote/Server/PipeApplier.cpp | 37 ++- MobileGL/MG_Test/Wire/RemoteClientTest.cpp | 302 +++++++++++++++++- 5 files changed, 697 insertions(+), 33 deletions(-) create mode 100644 MobileGL/MG_IntegrationTest/Scenarios/F1WireScenario.cpp diff --git a/MobileGL/MG_IntegrationTest/CMakeLists.txt b/MobileGL/MG_IntegrationTest/CMakeLists.txt index af59f21a..4438e24d 100644 --- a/MobileGL/MG_IntegrationTest/CMakeLists.txt +++ b/MobileGL/MG_IntegrationTest/CMakeLists.txt @@ -2010,3 +2010,19 @@ if (MOBILEGL_BUILD_DISAGGREGATED) check "${CMAKE_CTEST_COMMAND}" "${CMAKE_BINARY_DIR}") set_tests_properties(SplitLogPaths.PrivateAndDistinct PROPERTIES LABELS "integration-split") endif() + +# f1: split-only source keeps the monolith registry unchanged. +if (MOBILEGL_BUILD_DISAGGREGATED) + target_sources(MobileGLIntegrationTest PRIVATE Scenarios/F1WireScenario.cpp) + foreach(f1Slot ClearBufferfi ClearBufferfv ClearBufferiv ClearBufferuiv ClearNamedFramebufferfi ClearNamedFramebufferfv ClearNamedFramebufferiv ClearNamedFramebufferuiv CopyTexImage2D CopyTexSubImage2D GenerateMipmap) + gtest_discover_tests(MobileGLIntegrationTest + TEST_PREFIX "DirectGLES.Split.F1." + TEST_FILTER "F1WireScenario.${f1Slot}Pixels" + DISCOVERY_TIMEOUT 30 + PROPERTIES + LABELS "integration-split" + TIMEOUT ${MGL_ITEST_TIMEOUT} + ENVIRONMENT "${MGL_ITEST_GLES_SPLIT_ENVIRONMENT}\;MOBILEGL_LOG_FILE_PATH=${CMAKE_CURRENT_BINARY_DIR}/p5b-f1-${f1Slot}.log" + ) + endforeach() +endif() diff --git a/MobileGL/MG_IntegrationTest/Scenarios/F1WireScenario.cpp b/MobileGL/MG_IntegrationTest/Scenarios/F1WireScenario.cpp new file mode 100644 index 00000000..40cc64d3 --- /dev/null +++ b/MobileGL/MG_IntegrationTest/Scenarios/F1WireScenario.cpp @@ -0,0 +1,222 @@ +// f1 split-only pixel controls: every result depends on the migrated verb. +#include "../Harness/ScenarioFixture.h" +#include "../Harness/SplitRuntimePeek.h" +#include +#ifdef GLAPI +#undef GLAPI +#endif +#define GL_GLEXT_PROTOTYPES +#include +#include +#undef GL_GLEXT_PROTOTYPES + +namespace MGITest { +namespace { +class F1WireScenario : public ScenarioTest { +protected: + GLuint fbo = 0, texture = 0; + void SetUp() override { + ScenarioTest::SetUp(); + if (!Ready()) return; + const auto why = SplitRuntimeSkipReason(); + if (!why.empty()) GTEST_SKIP() << why; + glGenFramebuffers(1, &fbo); + glBindFramebuffer(GL_FRAMEBUFFER, fbo); + glGenTextures(1, &texture); + glBindTexture(GL_TEXTURE_2D, texture); + glDisable(GL_SCISSOR_TEST); + glColorMask(GL_TRUE, GL_TRUE, GL_TRUE, GL_TRUE); + glDepthMask(GL_TRUE); + glStencilMask(~0u); + } + void TearDown() override { + if (!Ready()) return; + glBindFramebuffer(GL_FRAMEBUFFER, 0); + if (fbo) glDeleteFramebuffers(1, &fbo); + if (texture) glDeleteTextures(1, &texture); + ScenarioTest::TearDown(); + } + void Attach(GLenum format, GLenum attachment = GL_COLOR_ATTACHMENT0, int levels = 1) { + glTexStorage2D(GL_TEXTURE_2D, levels, format, 8, 8); + glFramebufferTexture2D(GL_FRAMEBUFFER, attachment, GL_TEXTURE_2D, texture, 0); + if (attachment == GL_DEPTH_STENCIL_ATTACHMENT) { glDrawBuffer(GL_NONE); glReadBuffer(GL_NONE); } + ASSERT_EQ(glCheckFramebufferStatus(GL_FRAMEBUFFER), GLenum(GL_FRAMEBUFFER_COMPLETE)) << "F1.setup.framebuffer"; + } +}; +} + +TEST_F(F1WireScenario, ClearBufferfvPixels) { + // Red once (executed, reverted): zero the clear record values; F1.ClearBufferfv.pixels fails. + if (!Ready()) return; + Attach(GL_RGBA8); + const GLfloat value[4] = {0.25f, 0.5f, 0.75f, 1.0f}; + const auto before = PeekSplitRuntime().emitSeq; + glClearBufferfv(GL_COLOR, 0, value); + ASSERT_GT(PeekSplitRuntime().emitSeq, before) << "F1.ClearBufferfv.wire"; + GLubyte pixel[4]{}; + glReadPixels(2, 3, 1, 1, GL_RGBA, GL_UNSIGNED_BYTE, pixel); + ASSERT_EQ(FirstGLError(), GLenum(GL_NO_ERROR)) << "F1.ClearBufferfv.error"; + const int expected[4] = {64, 128, 191, 255}; + for (int i = 0; i < 4; ++i) EXPECT_NEAR(pixel[i], expected[i], 1) << "F1.ClearBufferfv.pixels"; +} + +TEST_F(F1WireScenario, ClearNamedFramebufferfvPixels) { + // Red once (executed, reverted): zero the clear record values; F1.ClearNamedFramebufferfv.pixels fails. + if (!Ready()) return; + Attach(GL_RGBA8); + const GLfloat value[4] = {0.25f, 0.5f, 0.75f, 1.0f}; + const auto before = PeekSplitRuntime().emitSeq; + glClearNamedFramebufferfv(fbo, GL_COLOR, 0, value); + ASSERT_GT(PeekSplitRuntime().emitSeq, before) << "F1.ClearNamedFramebufferfv.wire"; + GLubyte pixel[4]{}; + glReadPixels(2, 3, 1, 1, GL_RGBA, GL_UNSIGNED_BYTE, pixel); + ASSERT_EQ(FirstGLError(), GLenum(GL_NO_ERROR)) << "F1.ClearNamedFramebufferfv.error"; + const int expected[4] = {64, 128, 191, 255}; + for (int i = 0; i < 4; ++i) EXPECT_NEAR(pixel[i], expected[i], 1) << "F1.ClearNamedFramebufferfv.pixels"; +} + +TEST_F(F1WireScenario, ClearBufferivPixels) { + // Red once (executed, reverted): zero the clear record values; F1.ClearBufferiv.pixels fails. + if (!Ready()) return; + Attach(GL_RGBA32I); + const GLint value[4] = {-37, 19, -11, 5}; + const auto before = PeekSplitRuntime().emitSeq; + glClearBufferiv(GL_COLOR, 0, value); + ASSERT_GT(PeekSplitRuntime().emitSeq, before) << "F1.ClearBufferiv.wire"; + GLint pixel[4]{}; + glReadPixels(2, 3, 1, 1, GL_RGBA_INTEGER, GL_INT, pixel); + ASSERT_EQ(FirstGLError(), GLenum(GL_NO_ERROR)) << "F1.ClearBufferiv.error"; + for (int i = 0; i < 4; ++i) EXPECT_EQ(pixel[i], value[i]) << "F1.ClearBufferiv.pixels"; +} + +TEST_F(F1WireScenario, ClearNamedFramebufferivPixels) { + // Red once (executed, reverted): zero the clear record values; F1.ClearNamedFramebufferiv.pixels fails. + if (!Ready()) return; + Attach(GL_RGBA32I); + const GLint value[4] = {-37, 19, -11, 5}; + const auto before = PeekSplitRuntime().emitSeq; + glClearNamedFramebufferiv(fbo, GL_COLOR, 0, value); + ASSERT_GT(PeekSplitRuntime().emitSeq, before) << "F1.ClearNamedFramebufferiv.wire"; + GLint pixel[4]{}; + glReadPixels(2, 3, 1, 1, GL_RGBA_INTEGER, GL_INT, pixel); + ASSERT_EQ(FirstGLError(), GLenum(GL_NO_ERROR)) << "F1.ClearNamedFramebufferiv.error"; + for (int i = 0; i < 4; ++i) EXPECT_EQ(pixel[i], value[i]) << "F1.ClearNamedFramebufferiv.pixels"; +} + +TEST_F(F1WireScenario, ClearBufferuivPixels) { + // Red once (executed, reverted): zero the clear record values; F1.ClearBufferuiv.pixels fails. + if (!Ready()) return; + Attach(GL_RGBA32UI); + const GLuint value[4] = {37, 19, 11, 5}; + const auto before = PeekSplitRuntime().emitSeq; + glClearBufferuiv(GL_COLOR, 0, value); + ASSERT_GT(PeekSplitRuntime().emitSeq, before) << "F1.ClearBufferuiv.wire"; + GLuint pixel[4]{}; + glReadPixels(2, 3, 1, 1, GL_RGBA_INTEGER, GL_UNSIGNED_INT, pixel); + ASSERT_EQ(FirstGLError(), GLenum(GL_NO_ERROR)) << "F1.ClearBufferuiv.error"; + for (int i = 0; i < 4; ++i) EXPECT_EQ(pixel[i], value[i]) << "F1.ClearBufferuiv.pixels"; +} + +TEST_F(F1WireScenario, ClearNamedFramebufferuivPixels) { + // Red once (executed, reverted): zero the clear record values; F1.ClearNamedFramebufferuiv.pixels fails. + if (!Ready()) return; + Attach(GL_RGBA32UI); + const GLuint value[4] = {37, 19, 11, 5}; + const auto before = PeekSplitRuntime().emitSeq; + glClearNamedFramebufferuiv(fbo, GL_COLOR, 0, value); + ASSERT_GT(PeekSplitRuntime().emitSeq, before) << "F1.ClearNamedFramebufferuiv.wire"; + GLuint pixel[4]{}; + glReadPixels(2, 3, 1, 1, GL_RGBA_INTEGER, GL_UNSIGNED_INT, pixel); + ASSERT_EQ(FirstGLError(), GLenum(GL_NO_ERROR)) << "F1.ClearNamedFramebufferuiv.error"; + for (int i = 0; i < 4; ++i) EXPECT_EQ(pixel[i], value[i]) << "F1.ClearNamedFramebufferuiv.pixels"; +} + +TEST_F(F1WireScenario, ClearBufferfiPixels) { + // Red once (executed, reverted): zero the clear record values; F1.ClearBufferfi.pixels fails. + if (!Ready()) return; + Attach(GL_DEPTH24_STENCIL8, GL_DEPTH_STENCIL_ATTACHMENT); + const auto before = PeekSplitRuntime().emitSeq; + glClearBufferfi(GL_DEPTH_STENCIL, 0, 0.375f, 91); + ASSERT_GT(PeekSplitRuntime().emitSeq, before) << "F1.ClearBufferfi.wire"; + GLuint pixel = 0; + glReadPixels(2, 3, 1, 1, GL_DEPTH_STENCIL, GL_UNSIGNED_INT_24_8, &pixel); + ASSERT_EQ(FirstGLError(), GLenum(GL_NO_ERROR)) << "F1.ClearBufferfi.error"; + EXPECT_EQ(pixel & 255u, 91u) << "F1.ClearBufferfi.pixels"; + EXPECT_NEAR(double(pixel >> 8) / 16777215.0, 0.375, 0.00001) << "F1.ClearBufferfi.pixels"; +} + +TEST_F(F1WireScenario, ClearNamedFramebufferfiPixels) { + // Red once (executed, reverted): zero the clear record values; F1.ClearNamedFramebufferfi.pixels fails. + if (!Ready()) return; + Attach(GL_DEPTH24_STENCIL8, GL_DEPTH_STENCIL_ATTACHMENT); + const auto before = PeekSplitRuntime().emitSeq; + glClearNamedFramebufferfi(fbo, GL_DEPTH_STENCIL, 0, 0.375f, 91); + ASSERT_GT(PeekSplitRuntime().emitSeq, before) << "F1.ClearNamedFramebufferfi.wire"; + GLuint pixel = 0; + glReadPixels(2, 3, 1, 1, GL_DEPTH_STENCIL, GL_UNSIGNED_INT_24_8, &pixel); + ASSERT_EQ(FirstGLError(), GLenum(GL_NO_ERROR)) << "F1.ClearNamedFramebufferfi.error"; + EXPECT_EQ(pixel & 255u, 91u) << "F1.ClearNamedFramebufferfi.pixels"; + EXPECT_NEAR(double(pixel >> 8) / 16777215.0, 0.375, 0.00001) << "F1.ClearNamedFramebufferfi.pixels"; +} + +TEST_F(F1WireScenario, CopyTexImage2DPixels) { + // Red once (executed, reverted): omit the copy sink call; F1.CopyTexImage2D.pixels fails. + if (!Ready()) return; + Attach(GL_RGBA8); + glClearColor(0.25f, 0.5f, 0.75f, 1.0f); + glClear(GL_COLOR_BUFFER_BIT); + GLuint destination = 0; + glGenTextures(1, &destination); + glBindTexture(GL_TEXTURE_2D, destination); + + const auto before = PeekSplitRuntime().emitSeq; + glCopyTexImage2D(GL_TEXTURE_2D, 0, GL_RGBA8, 2, 3, 4, 4, 0); + ASSERT_GT(PeekSplitRuntime().emitSeq, before) << "F1.CopyTexImage2D.wire"; + glFramebufferTexture2D(GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0, GL_TEXTURE_2D, destination, 0); + GLubyte pixel[4]{}; + glReadPixels(1, 1, 1, 1, GL_RGBA, GL_UNSIGNED_BYTE, pixel); + ASSERT_EQ(FirstGLError(), GLenum(GL_NO_ERROR)) << "F1.CopyTexImage2D.error"; + const int expected[4] = {64, 128, 191, 255}; + for (int i = 0; i < 4; ++i) EXPECT_NEAR(pixel[i], expected[i], 1) << "F1.CopyTexImage2D.pixels"; + glDeleteTextures(1, &destination); +} + +TEST_F(F1WireScenario, CopyTexSubImage2DPixels) { + // Red once (executed, reverted): omit the copy sink call; F1.CopyTexSubImage2D.pixels fails. + if (!Ready()) return; + Attach(GL_RGBA8); + glClearColor(0.25f, 0.5f, 0.75f, 1.0f); + glClear(GL_COLOR_BUFFER_BIT); + GLuint destination = 0; + glGenTextures(1, &destination); + glBindTexture(GL_TEXTURE_2D, destination); + glTexStorage2D(GL_TEXTURE_2D, 1, GL_RGBA8, 4, 4); + const auto before = PeekSplitRuntime().emitSeq; + glCopyTexSubImage2D(GL_TEXTURE_2D, 0, 1, 1, 2, 3, 2, 2); + ASSERT_GT(PeekSplitRuntime().emitSeq, before) << "F1.CopyTexSubImage2D.wire"; + glFramebufferTexture2D(GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0, GL_TEXTURE_2D, destination, 0); + GLubyte pixel[4]{}; + glReadPixels(1, 1, 1, 1, GL_RGBA, GL_UNSIGNED_BYTE, pixel); + ASSERT_EQ(FirstGLError(), GLenum(GL_NO_ERROR)) << "F1.CopyTexSubImage2D.error"; + const int expected[4] = {64, 128, 191, 255}; + for (int i = 0; i < 4; ++i) EXPECT_NEAR(pixel[i], expected[i], 1) << "F1.CopyTexSubImage2D.pixels"; + glDeleteTextures(1, &destination); +} + +TEST_F(F1WireScenario, GenerateMipmapPixels) { + // Red once (executed, reverted): omit the mipmap sink call; F1.GenerateMipmap.pixels fails. + if (!Ready()) return; + Attach(GL_RGBA8, GL_COLOR_ATTACHMENT0, 4); + glClearColor(0.25f, 0.5f, 0.75f, 1.0f); + glClear(GL_COLOR_BUFFER_BIT); + const auto before = PeekSplitRuntime().emitSeq; + glGenerateMipmap(GL_TEXTURE_2D); + ASSERT_GT(PeekSplitRuntime().emitSeq, before) << "F1.GenerateMipmap.wire"; + glFramebufferTexture2D(GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0, GL_TEXTURE_2D, texture, 2); + GLubyte pixel[4]{}; + glReadPixels(1, 1, 1, 1, GL_RGBA, GL_UNSIGNED_BYTE, pixel); + ASSERT_EQ(FirstGLError(), GLenum(GL_NO_ERROR)) << "F1.GenerateMipmap.error"; + const int expected[4] = {64, 128, 191, 255}; + for (int i = 0; i < 4; ++i) EXPECT_NEAR(pixel[i], expected[i], 1) << "F1.GenerateMipmap.pixels"; +} +} // namespace MGITest diff --git a/MobileGL/MG_Remote/Client/EmitTables.cpp b/MobileGL/MG_Remote/Client/EmitTables.cpp index 6af52e8e..edae5dc2 100644 --- a/MobileGL/MG_Remote/Client/EmitTables.cpp +++ b/MobileGL/MG_Remote/Client/EmitTables.cpp @@ -39,6 +39,8 @@ #include #include "WireTables.h" +#include +#include namespace MobileGL::MG_Remote::Client { @@ -174,6 +176,123 @@ namespace MobileGL::MG_Remote::Client { nullptr, 0, nullptr); } + + // ---- f1: verbatim clear/copy/mipmap records (CONTRACT-P5B §2) ---- + void EmitF1Clear(const char* slot, MG_Pipe::MGPipeHandle fbo, GLenum buffer, + GLint drawbuffer, Uint8 valueClass, const void* value, + GLfloat depth = 0, GLint stencil = 0) { + auto& session = RequireSession(slot); + BeforeReadOnlyVerb(); + MG_Pipe::MGPClear record{}; + record.Fbo = fbo; + record.DrawBufferIndex = drawbuffer; + record.ValueClass = valueClass; + switch (buffer) { + case GL_COLOR: + record.Kind = MG_Pipe::kMGPipeClearKindColor; + std::memcpy(record.ColorValue, value, sizeof(record.ColorValue)); + break; + case GL_DEPTH: + record.Kind = MG_Pipe::kMGPipeClearKindDepth; + std::memcpy(&record.DepthValue, value, sizeof(record.DepthValue)); + break; + case GL_STENCIL: + record.Kind = MG_Pipe::kMGPipeClearKindStencil; + std::memcpy(&record.StencilValue, value, sizeof(record.StencilValue)); + break; + case GL_DEPTH_STENCIL: + record.Kind = MG_Pipe::kMGPipeClearKindDepthStencil; + record.DepthValue = depth; + record.StencilValue = stencil; + break; + default: UnmigratedVerbFatal(slot); + } + session.EmitAndWait(MG_Pipe::MGPWireOp::Clear, &record, sizeof(record), + nullptr, 0, nullptr, 0, nullptr); + } + + void EmitClearBufferfv(GLenum buffer, GLint drawbuffer, const GLfloat* value) { + EmitF1Clear("ClearBufferfv", MG_Pipe::kMGPipeNullHandle, buffer, drawbuffer, + MG_Pipe::kMGPipeClearValueClassFloat, value); + } + void EmitClearNamedFramebufferfv(const SharedPtr& fbo, + GLenum buffer, GLint drawbuffer, const GLfloat* value) { + EmitF1Clear("ClearNamedFramebufferfv", MG_Pipe::MGPipeFramebufferEmitter::HandleFor(*fbo), + buffer, drawbuffer, MG_Pipe::kMGPipeClearValueClassFloat, value); + } + + void EmitClearBufferiv(GLenum buffer, GLint drawbuffer, const GLint* value) { + EmitF1Clear("ClearBufferiv", MG_Pipe::kMGPipeNullHandle, buffer, drawbuffer, + MG_Pipe::kMGPipeClearValueClassInt, value); + } + void EmitClearNamedFramebufferiv(const SharedPtr& fbo, + GLenum buffer, GLint drawbuffer, const GLint* value) { + EmitF1Clear("ClearNamedFramebufferiv", MG_Pipe::MGPipeFramebufferEmitter::HandleFor(*fbo), + buffer, drawbuffer, MG_Pipe::kMGPipeClearValueClassInt, value); + } + + void EmitClearBufferuiv(GLenum buffer, GLint drawbuffer, const GLuint* value) { + EmitF1Clear("ClearBufferuiv", MG_Pipe::kMGPipeNullHandle, buffer, drawbuffer, + MG_Pipe::kMGPipeClearValueClassUint, value); + } + void EmitClearNamedFramebufferuiv(const SharedPtr& fbo, + GLenum buffer, GLint drawbuffer, const GLuint* value) { + EmitF1Clear("ClearNamedFramebufferuiv", MG_Pipe::MGPipeFramebufferEmitter::HandleFor(*fbo), + buffer, drawbuffer, MG_Pipe::kMGPipeClearValueClassUint, value); + } + + void EmitClearBufferfi(GLenum buffer, GLint drawbuffer, GLfloat depth, GLint stencil) { + EmitF1Clear("ClearBufferfi", MG_Pipe::kMGPipeNullHandle, buffer, drawbuffer, + MG_Pipe::kMGPipeClearValueClassFloat, nullptr, depth, stencil); + } + void EmitClearNamedFramebufferfi(const SharedPtr& fbo, + GLenum buffer, GLint drawbuffer, GLfloat depth, GLint stencil) { + EmitF1Clear("ClearNamedFramebufferfi", MG_Pipe::MGPipeFramebufferEmitter::HandleFor(*fbo), + buffer, drawbuffer, MG_Pipe::kMGPipeClearValueClassFloat, nullptr, depth, stencil); + } + const SharedPtr& F1BoundTexture(GLenum target) { + auto& ctx = *MG_State::pGLContext; + return ctx.GetTextureUnitObject(ctx.GetActiveTextureUnit()) + .GetBindingSlot(MG_Util::ConvertGLEnumToTextureTarget(target)).GetBoundObject(); + } + void EmitF1Copy(GLenum target, GLint level, GLenum format, GLint x, GLint y, + GLsizei width, GLsizei height, GLint xoffset, GLint yoffset, Bool subImage) { + auto& session = RequireSession(subImage ? "CopyTexSubImage2D" : "CopyTexImage2D"); + BeforeReadOnlyVerb(); + MG_Pipe::MGPCopyFromFramebuffer record{}; + record.Dst = MG_Pipe::MGPipeTextureEmitterInstance().FindTexture(*F1BoundTexture(target)); + record.Target = static_cast(target); + record.Level = level; + record.InternalFormat = format; + record.X = x; record.Y = y; + record.Width = width; record.Height = height; + record.XOffset = xoffset; record.YOffset = yoffset; + record.SubImage = subImage; + session.EmitAndWait(MG_Pipe::MGPWireOp::CopyFramebufferToTexture, &record, sizeof(record), + nullptr, 0, nullptr, 0, nullptr); + } + void EmitCopyTexImage2D(GLenum target, GLint level, GLenum format, GLint x, GLint y, + GLsizei width, GLsizei height, GLint) { + EmitF1Copy(target, level, format, x, y, width, height, 0, 0, false); + } + void EmitCopyTexSubImage2D(GLenum target, GLint level, GLint xoffset, GLint yoffset, + GLint x, GLint y, GLsizei width, GLsizei height) { + EmitF1Copy(target, level, 0, x, y, width, height, xoffset, yoffset, true); + } + void EmitGenerateMipmap(GLenum target) { + auto& session = RequireSession("GenerateMipmap"); + BeforeReadOnlyVerb(); + const auto& texture = F1BoundTexture(target); + MG_Pipe::MGPMipPlan record{}; + record.Res = MG_Pipe::MGPipeTextureEmitterInstance().FindTexture(*texture); + record.Target = static_cast(target); + record.BaseLevel = texture->GetLevelRange().x(); + const auto* mipmap = dynamic_cast(texture.get()); + record.LevelCount = mipmap ? mipmap->GetMipmapLevelCount() : 0; + session.EmitAndWait(MG_Pipe::MGPWireOp::GenerateMipmap, &record, sizeof(record), + nullptr, 0, nullptr, 0, nullptr); + } + void EmitDrawArrays(GLenum mode, GLint first, GLsizei count) { ClientSession& session = RequireSession("DrawArrays"); BeforeDrawVerb(); @@ -539,22 +658,7 @@ namespace MobileGL::MG_Remote::Client { X(BindTransformFeedback, void, (GLuint)) \ X(DeleteTransformFeedback, void, (GLuint)) -#define MGR_UNMIGRATED_F1_SLOTS(X) \ - X(ClearBufferfi, void, (GLenum, GLint, GLfloat, GLint)) \ - X(ClearBufferfv, void, (GLenum, GLint, const GLfloat*)) \ - X(ClearBufferuiv, void, (GLenum, GLint, const GLuint*)) \ - X(ClearBufferiv, void, (GLenum, GLint, const GLint*)) \ - X(ClearNamedFramebufferfv, void, \ - (const SharedPtr&, GLenum, GLint, const GLfloat*)) \ - X(ClearNamedFramebufferfi, void, \ - (const SharedPtr&, GLenum, GLint, GLfloat, GLint)) \ - X(ClearNamedFramebufferiv, void, \ - (const SharedPtr&, GLenum, GLint, const GLint*)) \ - X(ClearNamedFramebufferuiv, void, \ - (const SharedPtr&, GLenum, GLint, const GLuint*)) \ - X(CopyTexImage2D, void, (GLenum, GLint, GLenum, GLint, GLint, GLsizei, GLsizei, GLint)) \ - X(CopyTexSubImage2D, void, (GLenum, GLint, GLint, GLint, GLint, GLint, GLsizei, GLsizei)) \ - X(GenerateMipmap, void, (GLenum)) +#define MGR_UNMIGRATED_F1_SLOTS(X) // The wave-3 tail. SetSwapInterval is hand-written below (it is not a GL.* slot). #define MGR_UNMIGRATED_TAIL_SLOTS(X) \ @@ -646,7 +750,7 @@ namespace MobileGL::MG_Remote::Client { constexpr Uint32 kEmittedSlotsD1 = 0; constexpr Uint32 kEmittedSlotsI1 = 0; constexpr Uint32 kEmittedSlotsT2 = 0; - constexpr Uint32 kEmittedSlotsF1 = 0; + constexpr Uint32 kEmittedSlotsF1 = 11; constexpr Uint32 kEmittedSlots = kEmittedSlotsP5 + kEmittedSlotsD1 + kEmittedSlotsI1 + kEmittedSlotsT2 + kEmittedSlotsF1; constexpr Uint32 kLocallyAnsweredSlots = 2; // GetIntegeri_v, IsTimerQuerySupported @@ -660,7 +764,7 @@ namespace MobileGL::MG_Remote::Client { static_assert(kUnmigratedT2 + kEmittedSlotsT2 == 7, "t2 owns the 7 XFB/tessellation slots"); static_assert(kUnmigratedF1 + kEmittedSlotsF1 == 11, "f1 owns the 11 clear/copy/mip slots"); static_assert(kUnmigratedTail == 20, "the wave-3 tail is 20 slots and no P5b package owns one"); - static_assert(kUnmigratedSlots == 64, "CONTRACT-P5.md §7 class C is 64 slots at the P5b contract commit"); + static_assert(kUnmigratedSlots + kEmittedSlots == 69, "class B and C own 69 slots"); static_assert(kLocallyAnsweredSlots + kEmittedSlots + kUnmigratedSlots == kRemoteEmitSlotCount, "the three classes no longer partition the 71 slots"); @@ -698,6 +802,19 @@ namespace MobileGL::MG_Remote::Client { table.GL.BlitFramebuffer = &EmitBlitFramebuffer; table.Present = &EmitPresent; + // ---- f1 ---- + table.GL.ClearBufferfi = &EmitClearBufferfi; + table.GL.ClearBufferfv = &EmitClearBufferfv; + table.GL.ClearBufferiv = &EmitClearBufferiv; + table.GL.ClearBufferuiv = &EmitClearBufferuiv; + table.GL.ClearNamedFramebufferfi = &EmitClearNamedFramebufferfi; + table.GL.ClearNamedFramebufferfv = &EmitClearNamedFramebufferfv; + table.GL.ClearNamedFramebufferiv = &EmitClearNamedFramebufferiv; + table.GL.ClearNamedFramebufferuiv = &EmitClearNamedFramebufferuiv; + table.GL.CopyTexImage2D = &EmitCopyTexImage2D; + table.GL.CopyTexSubImage2D = &EmitCopyTexSubImage2D; + table.GL.GenerateMipmap = &EmitGenerateMipmap; + return table; } diff --git a/MobileGL/MG_Remote/Server/PipeApplier.cpp b/MobileGL/MG_Remote/Server/PipeApplier.cpp index cae264b6..e49cb2db 100644 --- a/MobileGL/MG_Remote/Server/PipeApplier.cpp +++ b/MobileGL/MG_Remote/Server/PipeApplier.cpp @@ -73,14 +73,16 @@ namespace MobileGL::MG_Remote::Server { if (table == nullptr) return false; const MG_Backend::GLFunctionsTable& gl = table->GL; - // THE FBO HANDLE IS NOT RESOLVED HERE, AND THAT IS THE RULING RATHER THAN AN OMISSION. - // MGPClear::Fbo names the framebuffer the clear belongs to, but the BINDING is already - // server state: set_framebuffer_state (op 33) arrives ahead of the clear and the - // applier has bound it. Re-resolving the handle to a frontend FramebufferObject here - // would need the SharedPtr the four ClearNamedFramebuffer* entries take - a frontend - // heap reference that table 2 lists as one of the six fields with no wire carrier. So - // P5 clears THE BOUND FRAMEBUFFER, which for the reduced path (default FBO) is exactly - // right, and the named form is P7's along with the handle it needs. + // Named records precede the verb; bound-form backends re-sync the live draw binding. + if (!MG_Pipe::MGPipeHandleIsNull(clear.Fbo) && + clear.Fbo != MG_Pipe::MGPipeApplier().BoundFramebuffer[0]) { + const char* slot = clear.Kind == kMGPClearKindDepthStencil ? "ClearNamedFramebufferfi+UNBOUND" : + clear.ValueClass == kMGPClearValueClassInt ? "ClearNamedFramebufferiv+UNBOUND" : + clear.ValueClass == kMGPClearValueClassUint ? "ClearNamedFramebufferuiv+UNBOUND" : + "ClearNamedFramebufferfv+UNBOUND"; + MGLOG_F("MGPipe: Fatal{UnmigratedVerb, \"%s\"}", slot); + std::abort(); + } switch (clear.Kind) { case kMGPClearKindWhole: if (gl.Clear == nullptr) return false; @@ -415,12 +417,25 @@ namespace MobileGL::MG_Remote::Server { // ---- f1 ---- Bool ServerVerbSink::OnGenerateMipmap(const MG_Pipe::MGPMipPlan& plan) { - (void)plan; - ServerUnmigratedVerbFatal("GenerateMipmap"); + const auto* table = Table("GenerateMipmap"); + if (table == nullptr || table->GL.GenerateMipmap == nullptr) return false; + table->GL.GenerateMipmap(plan.Target); + return true; } Bool ServerVerbSink::OnCopyFramebufferToTexture(const MG_Pipe::MGPCopyFromFramebuffer& copy) { - ServerUnmigratedVerbFatal(copy.SubImage ? "CopyTexSubImage2D" : "CopyTexImage2D"); + const auto* table = Table(copy.SubImage ? "CopyTexSubImage2D" : "CopyTexImage2D"); + if (table == nullptr) return false; + if (copy.SubImage) { + if (table->GL.CopyTexSubImage2D == nullptr) return false; + table->GL.CopyTexSubImage2D(copy.Target, copy.Level, copy.XOffset, copy.YOffset, + copy.X, copy.Y, copy.Width, copy.Height); + } else { + if (table->GL.CopyTexImage2D == nullptr) return false; + table->GL.CopyTexImage2D(copy.Target, copy.Level, copy.InternalFormat, + copy.X, copy.Y, copy.Width, copy.Height, 0); + } + return true; } // ----------------------------------------------------------------------------------- diff --git a/MobileGL/MG_Test/Wire/RemoteClientTest.cpp b/MobileGL/MG_Test/Wire/RemoteClientTest.cpp index acef2348..14f4e533 100644 --- a/MobileGL/MG_Test/Wire/RemoteClientTest.cpp +++ b/MobileGL/MG_Test/Wire/RemoteClientTest.cpp @@ -175,8 +175,8 @@ TEST(RemoteEmitTable, TheThreeClassesPartitionAllSeventyOneSlots) { // table itself reports with - which is also what t1's arming condition reads - rather than // recomputed here, so a table that lost an emitter cannot look like one that never had it. EXPECT_EQ(LocallyAnsweredSlotCount(), 2u); - EXPECT_EQ(ImplementedVerbCount(), 5u); - EXPECT_EQ(UnmigratedSlotCount(), 64u); + EXPECT_EQ(ImplementedVerbCount(), 16u); + EXPECT_EQ(UnmigratedSlotCount(), 53u); EXPECT_EQ(LocallyAnsweredSlotCount() + ImplementedVerbCount() + UnmigratedSlotCount(), kRemoteEmitSlotCount); } @@ -240,9 +240,9 @@ TEST(RemoteEmitTable, AnUnmigratedSlotAbortsAndNamesItself) { TEST(RemoteEmitTable, EachUnmigratedSlotNamesItsOwnSlot) { // 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. - const ChildResult r = RunInChild([] { RemoteEmitTable().GL.GenerateMipmap(0x0DE1); }); + const ChildResult r = RunInChild([] { RemoteEmitTable().GL.GetTexImage(GL_TEXTURE_2D, 0, GL_RGBA, GL_UNSIGNED_BYTE, nullptr); }); ASSERT_TRUE(DiedOfAbort(r)) << DescribeStatus(r) << "\n" << r.Log; - EXPECT_NE(r.Log.find("Fatal{UnmigratedVerb, \"GenerateMipmap\"}"), std::string::npos) << r.Log; + EXPECT_NE(r.Log.find("Fatal{UnmigratedVerb, \"GetTexImage\"}"), std::string::npos) << r.Log; EXPECT_EQ(r.Log.find("DrawElements"), std::string::npos) << "the Fatal message names a slot other than the one that was called:\n" << r.Log; @@ -847,6 +847,300 @@ TEST(RemoteReadback, AReplyIsScatteredOnlyWhenItIsOkAndExactlyTheReadsExtent) { } #include "RemoteClientControls.inc" +#include +#include +#include + +// f1: the installed emitters are decoded by a peer on the apply thread. +#if MGTEST_HAVE_FORK +namespace { +struct F1Peer : Codec::WireVerbSink { + MGPClear clear{}; + MGPCopyFromFramebuffer copy{}; + MGPMipPlan mip{}; + unsigned calls = 0; + Bool OnClear(const MGPClear& v) override { clear = v; ++calls; return true; } + Bool OnCopyFramebufferToTexture(const MGPCopyFromFramebuffer& v) override { copy = v; ++calls; return true; } + Bool OnGenerateMipmap(const MGPMipPlan& v) override { mip = v; ++calls; return true; } + void Install() { + if (Srv::ServerLoopInstance().RunOnApplyThread([](void* self) { + auto& decoder = Srv::ServerSessionInstance().Applier().*PeerMember(DecoderTag{}); + decoder.SetVerbSink(static_cast(self)); + return MOBILEGL_OK; + }, this) != MOBILEGL_OK) ::_exit(82); + } +}; +} + +TEST(RemoteF1, ClearBufferfvFieldsCross) { + // Red once (executed, reverted): zero the emitted clear values; F1.ClearBufferfv.fields fails. + const auto child = RunInChild([] { + StartControlSession(); + F1Peer peer; peer.Install(); + const GLfloat value[4] = {1, 7, 13, 23}; + + RemoteEmitTable().GL.ClearBufferfv(GL_COLOR, 3, value); + const auto& r = peer.clear; + if (peer.calls != 1 || r.Kind != kMGPipeClearKindColor || r.ValueClass != kMGPipeClearValueClassFloat || + r.DrawBufferIndex != 3 || std::memcmp(r.ColorValue, value, sizeof(value)) != 0 || + !MGPipeHandleIsNull(r.Fbo)) ::_exit(101); + ClientSessionInstance().Stop(); + }); + EXPECT_TRUE(WIFEXITED(child.Status) && WEXITSTATUS(child.Status) == 0) + << "F1.ClearBufferfv.fields: " << DescribeStatus(child) << child.Log; +} + +TEST(RemoteF1, ClearNamedFramebufferfvFieldsCross) { + // Red once (executed, reverted): zero the emitted clear values; F1.ClearNamedFramebufferfv.fields fails. + const auto child = RunInChild([] { + StartControlSession(); + F1Peer peer; peer.Install(); + const GLfloat value[4] = {1, 7, 13, 23}; + const auto fbo = MakeShared(73); + RemoteEmitTable().GL.ClearNamedFramebufferfv(fbo, GL_COLOR, 3, value); + const auto& r = peer.clear; + if (peer.calls != 1 || r.Kind != kMGPipeClearKindColor || r.ValueClass != kMGPipeClearValueClassFloat || + r.DrawBufferIndex != 3 || std::memcmp(r.ColorValue, value, sizeof(value)) != 0 || + r.Fbo != MGPipeFramebufferEmitter::HandleFor(*fbo)) ::_exit(101); + ClientSessionInstance().Stop(); + }); + EXPECT_TRUE(WIFEXITED(child.Status) && WEXITSTATUS(child.Status) == 0) + << "F1.ClearNamedFramebufferfv.fields: " << DescribeStatus(child) << child.Log; +} + +TEST(RemoteF1, ClearBufferivFieldsCross) { + // Red once (executed, reverted): zero the emitted clear values; F1.ClearBufferiv.fields fails. + const auto child = RunInChild([] { + StartControlSession(); + F1Peer peer; peer.Install(); + const GLint value[4] = {1, 7, 13, 23}; + + RemoteEmitTable().GL.ClearBufferiv(GL_COLOR, 3, value); + const auto& r = peer.clear; + if (peer.calls != 1 || r.Kind != kMGPipeClearKindColor || r.ValueClass != kMGPipeClearValueClassInt || + r.DrawBufferIndex != 3 || std::memcmp(r.ColorValue, value, sizeof(value)) != 0 || + !MGPipeHandleIsNull(r.Fbo)) ::_exit(101); + ClientSessionInstance().Stop(); + }); + EXPECT_TRUE(WIFEXITED(child.Status) && WEXITSTATUS(child.Status) == 0) + << "F1.ClearBufferiv.fields: " << DescribeStatus(child) << child.Log; +} + +TEST(RemoteF1, ClearNamedFramebufferivFieldsCross) { + // Red once (executed, reverted): zero the emitted clear values; F1.ClearNamedFramebufferiv.fields fails. + const auto child = RunInChild([] { + StartControlSession(); + F1Peer peer; peer.Install(); + const GLint value[4] = {1, 7, 13, 23}; + const auto fbo = MakeShared(73); + RemoteEmitTable().GL.ClearNamedFramebufferiv(fbo, GL_COLOR, 3, value); + const auto& r = peer.clear; + if (peer.calls != 1 || r.Kind != kMGPipeClearKindColor || r.ValueClass != kMGPipeClearValueClassInt || + r.DrawBufferIndex != 3 || std::memcmp(r.ColorValue, value, sizeof(value)) != 0 || + r.Fbo != MGPipeFramebufferEmitter::HandleFor(*fbo)) ::_exit(101); + ClientSessionInstance().Stop(); + }); + EXPECT_TRUE(WIFEXITED(child.Status) && WEXITSTATUS(child.Status) == 0) + << "F1.ClearNamedFramebufferiv.fields: " << DescribeStatus(child) << child.Log; +} + +TEST(RemoteF1, ClearBufferuivFieldsCross) { + // Red once (executed, reverted): zero the emitted clear values; F1.ClearBufferuiv.fields fails. + const auto child = RunInChild([] { + StartControlSession(); + F1Peer peer; peer.Install(); + const GLuint value[4] = {1, 7, 13, 23}; + + RemoteEmitTable().GL.ClearBufferuiv(GL_COLOR, 3, value); + const auto& r = peer.clear; + if (peer.calls != 1 || r.Kind != kMGPipeClearKindColor || r.ValueClass != kMGPipeClearValueClassUint || + r.DrawBufferIndex != 3 || std::memcmp(r.ColorValue, value, sizeof(value)) != 0 || + !MGPipeHandleIsNull(r.Fbo)) ::_exit(101); + ClientSessionInstance().Stop(); + }); + EXPECT_TRUE(WIFEXITED(child.Status) && WEXITSTATUS(child.Status) == 0) + << "F1.ClearBufferuiv.fields: " << DescribeStatus(child) << child.Log; +} + +TEST(RemoteF1, ClearNamedFramebufferuivFieldsCross) { + // Red once (executed, reverted): zero the emitted clear values; F1.ClearNamedFramebufferuiv.fields fails. + const auto child = RunInChild([] { + StartControlSession(); + F1Peer peer; peer.Install(); + const GLuint value[4] = {1, 7, 13, 23}; + const auto fbo = MakeShared(73); + RemoteEmitTable().GL.ClearNamedFramebufferuiv(fbo, GL_COLOR, 3, value); + const auto& r = peer.clear; + if (peer.calls != 1 || r.Kind != kMGPipeClearKindColor || r.ValueClass != kMGPipeClearValueClassUint || + r.DrawBufferIndex != 3 || std::memcmp(r.ColorValue, value, sizeof(value)) != 0 || + r.Fbo != MGPipeFramebufferEmitter::HandleFor(*fbo)) ::_exit(101); + ClientSessionInstance().Stop(); + }); + EXPECT_TRUE(WIFEXITED(child.Status) && WEXITSTATUS(child.Status) == 0) + << "F1.ClearNamedFramebufferuiv.fields: " << DescribeStatus(child) << child.Log; +} + +TEST(RemoteF1, ClearBufferfiFieldsCross) { + // Red once (executed, reverted): zero the emitted clear values; F1.ClearBufferfi.fields fails. + const auto child = RunInChild([] { + StartControlSession(); F1Peer peer; peer.Install(); + + RemoteEmitTable().GL.ClearBufferfi(GL_DEPTH_STENCIL, 0, 0.375f, 91); + const auto& r = peer.clear; + if (peer.calls != 1 || r.Kind != kMGPipeClearKindDepthStencil || r.DrawBufferIndex != 0 || + r.DepthValue != 0.375f || r.StencilValue != 91 || + !MGPipeHandleIsNull(r.Fbo)) ::_exit(101); + ClientSessionInstance().Stop(); + }); + EXPECT_TRUE(WIFEXITED(child.Status) && WEXITSTATUS(child.Status) == 0) + << "F1.ClearBufferfi.fields: " << DescribeStatus(child) << child.Log; +} + +TEST(RemoteF1, ClearNamedFramebufferfiFieldsCross) { + // Red once (executed, reverted): zero the emitted clear values; F1.ClearNamedFramebufferfi.fields fails. + const auto child = RunInChild([] { + StartControlSession(); F1Peer peer; peer.Install(); + const auto fbo = MakeShared(73); + RemoteEmitTable().GL.ClearNamedFramebufferfi(fbo, GL_DEPTH_STENCIL, 0, 0.375f, 91); + const auto& r = peer.clear; + if (peer.calls != 1 || r.Kind != kMGPipeClearKindDepthStencil || r.DrawBufferIndex != 0 || + r.DepthValue != 0.375f || r.StencilValue != 91 || + r.Fbo != MGPipeFramebufferEmitter::HandleFor(*fbo)) ::_exit(101); + ClientSessionInstance().Stop(); + }); + EXPECT_TRUE(WIFEXITED(child.Status) && WEXITSTATUS(child.Status) == 0) + << "F1.ClearNamedFramebufferfi.fields: " << DescribeStatus(child) << child.Log; +} + +namespace { +SharedPtr F1Texture() { + MG_State::pGLContext = MakeUnique(); + auto tex = MakeShared(91); + tex->SetInternalFormat(TextureInternalFormat::RGBA8); + for (Uint level = 0; level != 3; ++level) { + const Int size = 8 >> level; + tex->AllocateStorage(TextureUploadTarget::Texture2D, level, + MG_State::GLState::MipmapInput{IntVec3{size, size, 1}, static_cast(size * size * 4)}); + } + tex->SetBaseLevel(1); + MGPipeTextureEmitterInstance().AcquireTexture(tex->GetLifetimeId(), tex.get()); + MG_State::pGLContext->GetTextureUnitObject(0).GetBindingSlot(TextureTarget::Texture2D).Bind(tex); + return tex; +} +} + +TEST(RemoteF1, CopyTexImage2DFieldsCross) { + // Red once (executed, reverted): increment the emitted copy level; F1.CopyTexImage2D.fields fails. + const auto child = RunInChild([] { + StartControlSession(); F1Peer peer; peer.Install(); const auto tex = F1Texture(); + RemoteEmitTable().GL.CopyTexImage2D(GL_TEXTURE_2D, 2, GL_RGBA8, -3, 4, 11, 13, 0); + const auto& r = peer.copy; + if (peer.calls != 1 || r.Dst != MGPipeTextureEmitterInstance().FindTexture(*tex) || + MGPipeHandleIsNull(r.Dst) || r.Target != GL_TEXTURE_2D || r.Level != 2 || + r.InternalFormat != GL_RGBA8 || r.X != -3 || r.Y != 4 || r.Width != 11 || r.Height != 13 || + r.XOffset != 0 || r.YOffset != 0 || r.SubImage != 0) ::_exit(101); + ClientSessionInstance().Stop(); + }); + EXPECT_TRUE(WIFEXITED(child.Status) && WEXITSTATUS(child.Status) == 0) + << "F1.CopyTexImage2D.fields: " << DescribeStatus(child) << child.Log; +} + +TEST(RemoteF1, CopyTexSubImage2DFieldsCross) { + // Red once (executed, reverted): increment the emitted copy level; F1.CopyTexSubImage2D.fields fails. + const auto child = RunInChild([] { + StartControlSession(); F1Peer peer; peer.Install(); const auto tex = F1Texture(); + RemoteEmitTable().GL.CopyTexSubImage2D(GL_TEXTURE_2D, 2, 5, 7, -3, 4, 11, 13); + const auto& r = peer.copy; + if (peer.calls != 1 || r.Dst != MGPipeTextureEmitterInstance().FindTexture(*tex) || + MGPipeHandleIsNull(r.Dst) || r.Target != GL_TEXTURE_2D || r.Level != 2 || + r.InternalFormat != 0 || r.X != -3 || r.Y != 4 || r.Width != 11 || r.Height != 13 || + r.XOffset != 5 || r.YOffset != 7 || r.SubImage != 1) ::_exit(101); + ClientSessionInstance().Stop(); + }); + EXPECT_TRUE(WIFEXITED(child.Status) && WEXITSTATUS(child.Status) == 0) + << "F1.CopyTexSubImage2D.fields: " << DescribeStatus(child) << child.Log; +} + +TEST(RemoteF1, GenerateMipmapFieldsCross) { + // Red once (executed, reverted): increment the emitted base level; F1.GenerateMipmap.fields fails. + const auto child = RunInChild([] { + StartControlSession(); F1Peer peer; peer.Install(); const auto tex = F1Texture(); + RemoteEmitTable().GL.GenerateMipmap(GL_TEXTURE_2D); + const auto& r = peer.mip; + if (peer.calls != 1 || r.Res != MGPipeTextureEmitterInstance().FindTexture(*tex) || + MGPipeHandleIsNull(r.Res) || r.Target != GL_TEXTURE_2D || r.BaseLevel != 1 || r.LevelCount != 3) + ::_exit(101); + ClientSessionInstance().Stop(); + }); + EXPECT_TRUE(WIFEXITED(child.Status) && WEXITSTATUS(child.Status) == 0) + << "F1.GenerateMipmap.fields: " << DescribeStatus(child) << child.Log; +} +#endif + + + +#if MGTEST_HAVE_FORK + +TEST(RemoteF1, UnboundNamedfvRefusesByName) { + // Red once (executed, reverted): disable the named-FBO refusal; its exact Fatal disappears. + const auto child = RunInChild([] { + CapsPeer backend; + Srv::ServerVerbSink sink; + sink.SetBackend(&backend); + MGPClear r{}; + r.Fbo = {701, 1}; + r.Kind = kMGPipeClearKindColor; + r.ValueClass = kMGPipeClearValueClassFloat; + sink.OnClear(r); + }); + ExpectNamedAbort(child, "Fatal{UnmigratedVerb, \"ClearNamedFramebufferfv+UNBOUND\"}"); +} + +TEST(RemoteF1, UnboundNamedivRefusesByName) { + // Red once (executed, reverted): disable the named-FBO refusal; its exact Fatal disappears. + const auto child = RunInChild([] { + CapsPeer backend; + Srv::ServerVerbSink sink; + sink.SetBackend(&backend); + MGPClear r{}; + r.Fbo = {701, 1}; + r.Kind = kMGPipeClearKindColor; + r.ValueClass = kMGPipeClearValueClassInt; + sink.OnClear(r); + }); + ExpectNamedAbort(child, "Fatal{UnmigratedVerb, \"ClearNamedFramebufferiv+UNBOUND\"}"); +} + +TEST(RemoteF1, UnboundNameduivRefusesByName) { + // Red once (executed, reverted): disable the named-FBO refusal; its exact Fatal disappears. + const auto child = RunInChild([] { + CapsPeer backend; + Srv::ServerVerbSink sink; + sink.SetBackend(&backend); + MGPClear r{}; + r.Fbo = {701, 1}; + r.Kind = kMGPipeClearKindColor; + r.ValueClass = kMGPipeClearValueClassUint; + sink.OnClear(r); + }); + ExpectNamedAbort(child, "Fatal{UnmigratedVerb, \"ClearNamedFramebufferuiv+UNBOUND\"}"); +} + +TEST(RemoteF1, UnboundNamedfiRefusesByName) { + // Red once (executed, reverted): disable the named-FBO refusal; its exact Fatal disappears. + const auto child = RunInChild([] { + CapsPeer backend; + Srv::ServerVerbSink sink; + sink.SetBackend(&backend); + MGPClear r{}; + r.Fbo = {701, 1}; + r.Kind = kMGPipeClearKindDepthStencil; + r.ValueClass = kMGPipeClearValueClassFloat; + sink.OnClear(r); + }); + ExpectNamedAbort(child, "Fatal{UnmigratedVerb, \"ClearNamedFramebufferfi+UNBOUND\"}"); +} +#endif int main(int argc, char** argv) { namespace fs = std::filesystem; From 734287ac56e4b0f9f5fec74f167d64418671aee0 Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Wed, 16 Sep 2026 12:09:55 -0400 Subject: [PATCH 2/9] [Feat] (MG_Remote, P5b): emit i1 seven image/compute/barrier/copy-image/storage-block slots as class B and give each ServerVerbSink body the backend call the contract names --- MobileGL/MG_Remote/Client/EmitTables.cpp | 302 +++++++++++++++++++--- MobileGL/MG_Remote/Server/PipeApplier.cpp | 135 +++++++++- MobileGL/MG_Remote/Server/PipeApplier.h | 15 ++ 3 files changed, 404 insertions(+), 48 deletions(-) diff --git a/MobileGL/MG_Remote/Client/EmitTables.cpp b/MobileGL/MG_Remote/Client/EmitTables.cpp index 6af52e8e..1fdf4425 100644 --- a/MobileGL/MG_Remote/Client/EmitTables.cpp +++ b/MobileGL/MG_Remote/Client/EmitTables.cpp @@ -10,8 +10,8 @@ // // THE PARTITION IS CONTRACT-P5.md §7's AND IS NOT RE-DERIVED HERE (R-15, ID-12): // class A 2 slots answered locally from the caps mirror, never emitted, never Fatal -// class B 5 slots emitted -// class C 64 slots Fatal{UnmigratedVerb, ""} +// class B 5 slots emitted (+ 7 flipped by P5b package i1, below) +// class C 64 slots Fatal{UnmigratedVerb, ""} (- the same 7) // The three counts are static_asserted to sum to kRemoteEmitSlotCount below, so a slot that // changes class without changing the arithmetic is a build break rather than a behaviour // change nobody reviewed. @@ -34,6 +34,13 @@ #include #include +// P5b i1: the emitters below name a texture's, a buffer's and a program's HANDLE beside the GL +// arguments (rule D). The handle comes from the client's own slot allocator, which is +// MG_Impl/Pipe's - the same table MG_Impl/Pipe/ImageEmit.h's set_shader_images reads, so the +// two records name one identity rather than two. +#include +#include +#include #include #include @@ -428,6 +435,231 @@ namespace MobileGL::MG_Remote::Client { session.PumpControlPlane(); } + // ============================================================================= + // CLASS B - P5b package i1: image bind, compute, barriers, copy-image, SSBO block + // (MG_Remote/CONTRACT-P5B.md §2 i1). Seven slots, five wire rows. + // ============================================================================= + // + // RULE D, WHICH IS WHY THESE ARE SHORT. A P5b verb crosses AS THE CALL: the record + // carries the GL arguments verbatim - the enums as tokens, the GL names the backend + // keys on - beside the handle the P7/P8 form will dispatch on instead, and the server's + // ServerVerbSink reproduces the backend call the monolith makes. The backend keeps + // reading the frontend state it reads today through the BARRIER-PULLED fields of its + // verb class, which the record's own verb stamp is what makes legal. So a migration is + // one emitter here plus one sink body there, and nothing in the backend moves. + // + // THE HANDLES ARE LOOKED UP, NEVER MINTED, and that is a ruling (i1-v1 §4). The sinks + // below dispatch on the GL NAME - that is the whole point of carrying it - so a handle + // is carried for P7's sake only. FindByLifetimeId answers the handle the resource + // subsystem has already published for this object and kMGPipeNullHandle when it has + // published none; Acquire would MINT one here instead, at a call site that emits no + // create record, and the server would then be handed an identity it has never seen. + // A null Res/Src/Dst/ShaderCso therefore means "no handle published yet", which is a + // true statement, rather than a slot nobody allocated. + + MG_Pipe::MGPipeHandle PublishedTextureHandle( + const SharedPtr& texture) { + if (!texture) return MG_Pipe::kMGPipeNullHandle; + return MG_Pipe::MGPipeSlots().FindByLifetimeId(MG_Pipe::MGPipeKind::Texture, + texture->GetLifetimeId()); + } + + // glBindImageTexture. Emitted AT THE CALL, after the frontend has written the unit's + // ImageTextureBinding and MGP_FILL(BindImageTexture) has run - the record's verb + // boundary is what makes the server's read of that binding legal. set_shader_images + // (the draw-prep set) still travels at the next validate, untouched: that record + // describes a resolved unit for the draw, this one reproduces a call. + // + // NO PRE-VERB HOOK, AND THAT IS DELIBERATE. b1's two hooks describe "the work the + // record is ABOUT TO START" - PushPersistentMapsBeforeVerb publishes bytes an + // application wrote through a coherent map, MarkGpuWrites* builds the GPU-write set. + // A bind starts no shader and reads no buffer; the dispatch that later reads this image + // is the verb that carries both hooks, and running them here as well would push the + // same maps twice per dispatch and inflate b1's per-row counters. + void EmitBindImageTexture(GLuint unit, GLuint texture, GLint level, GLboolean layered, + GLint layer, GLenum access, GLenum format) { + ClientSession& session = RequireSession("BindImageTexture"); + + MG_Pipe::MGPImageBind record{}; + // The unit's binding is the frontend's and has just been written by the caller, so + // the texture this record names is the one the server's SyncImageTextureBinding + // will pull for the same unit. Read from MG_State::pGLContext and NOT through + // MGB_CTX: on this side of a split MGB_CTX is gPipeInputs, which is the SERVER's + // view, and the client asking it a question is how the two halves come to disagree. + if (MG_State::pGLContext != nullptr) { + record.Res = PublishedTextureHandle( + MG_State::pGLContext->GetImageTextureBinding(static_cast(unit)).Texture); + } + record.Unit = static_cast(unit); + // The GL name the application passed, verbatim - what the ES slot is handed as + // `texture` and currently ignores. Never an identity (ARCHITECTURE 4.2.1). + record.GlName = static_cast(texture); + record.Level = static_cast(level); + record.Layer = static_cast(layer); + // THE GL ACCESS TOKEN, not MGPImageView::Access's three-value encoding (table 0's + // MGPImageBind::Access row). Two records, two jobs. + record.Access = static_cast(access); + record.Format = static_cast(format); + record.Layered = layered != GL_FALSE ? 1 : 0; + session.EmitAndWait(MG_Pipe::MGPWireOp::BindShaderImage, &record, sizeof(record), + nullptr, 0, nullptr, 0, nullptr); + } + + // glDispatchCompute. THE HOOK ORDER IS b1's AND IS INHERITED FROM THE CLASS-C STUB + // VERBATIM: push the persistent maps (they produce resource_subdata records that must + // precede the verb on SEG_CMD), then the dispatch mark walk, then the record. The stub + // carried both calls before its Fatal precisely so that the package which flipped this + // slot would inherit a call site that was already correct. + void EmitDispatchCompute(GLuint numGroupsX, GLuint numGroupsY, GLuint numGroupsZ) { + ClientSession& session = RequireSession("DispatchCompute"); + PushPersistentMapsBeforeVerb(); + MarkGpuWritesForDispatch(); + + MG_Pipe::MGPGridInfo record{}; + record.GridX = static_cast(numGroupsX); + record.GridY = static_cast(numGroupsY); + record.GridZ = static_cast(numGroupsZ); + // Block* STAY 0 IN P5b (contract i1): the local size is a link artifact the backend + // reads from its own program, and a client-minted copy would be a second statement + // of it. P7's Magma may fill it from the reflection archive. + record.IndirectBuffer = MG_Pipe::kMGPipeNullHandle; + record.IndirectOffset = 0; + record.IsIndirect = 0; + session.EmitAndWait(MG_Pipe::MGPWireOp::LaunchGrid, &record, sizeof(record), nullptr, 0, + nullptr, 0, nullptr); + } + + void EmitDispatchComputeIndirect(GLintptr indirect) { + ClientSession& session = RequireSession("DispatchComputeIndirect"); + PushPersistentMapsBeforeVerb(); + MarkGpuWritesForDispatch(); + + MG_Pipe::MGPGridInfo record{}; + // The counts come from the GL_DISPATCH_INDIRECT_BUFFER, so the three grid fields are + // 0 and IsIndirect is what says so; the sink dispatches on it and calls + // glDispatchComputeIndirect with the offset the application spelled. + record.IsIndirect = 1; + record.IndirectOffset = static_cast(indirect); + record.IndirectBuffer = MG_Pipe::kMGPipeNullHandle; + if (MG_State::pGLContext != nullptr) { + const auto& bound = + MG_State::pGLContext + ->GetBufferBindingSlot(::MobileGL::BufferTarget::DispatchIndirect) + .GetBoundObject(); + if (bound) { + record.IndirectBuffer = MG_Pipe::MGPipeSlots().FindByLifetimeId( + MG_Pipe::MGPipeKind::Buffer, bound->GetLifetimeId()); + } + } + session.EmitAndWait(MG_Pipe::MGPWireOp::LaunchGrid, &record, sizeof(record), nullptr, 0, + nullptr, 0, nullptr); + } + + // glMemoryBarrier / glMemoryBarrierByRegion. The bits cross VERBATIM: the frontend has + // already validated them and already folds glTextureBarrier onto the same field + // (GL_Drawing.cpp:920-937), and Espryt's atomic-counter lowering - the counter bit + // implying the storage bit - stays inside the backend where the reason for it lives + // (DirectGLES.cpp:8837). A client that pre-lowered would be answering a driver question + // from the wrong side and the two arms would stop being byte-identical. + // + // No pre-verb hook: a barrier orders memory the GPU already holds. It starts no shader + // and reads no mapped buffer. + void EmitMemoryBarrier(GLbitfield barriers) { + ClientSession& session = RequireSession("MemoryBarrier"); + MG_Pipe::MGPMemoryBarrier record{}; + record.Bits = static_cast(barriers); + record.ByRegion = 0; + session.EmitAndWait(MG_Pipe::MGPWireOp::MemoryBarrier, &record, sizeof(record), nullptr, + 0, nullptr, 0, nullptr); + } + + void EmitMemoryBarrierByRegion(GLbitfield barriers) { + ClientSession& session = RequireSession("MemoryBarrierByRegion"); + MG_Pipe::MGPMemoryBarrier record{}; + record.Bits = static_cast(barriers); + record.ByRegion = 1; + session.EmitAndWait(MG_Pipe::MGPWireOp::MemoryBarrier, &record, sizeof(record), nullptr, + 0, nullptr, 0, nullptr); + } + + // glCopyImageSubData -> resource_copy_region (53), which P5b rules is glCopyImageSubData + // ONLY (contract §6.4; the framebuffer-sourced copies are f1's row 76). + void EmitCopyImageSubData(const MG_Backend::CopyImageEndpoint& src, GLenum srcTarget, + GLint srcLevel, GLint srcX, GLint srcY, GLint srcZ, + const MG_Backend::CopyImageEndpoint& dst, GLenum dstTarget, + GLint dstLevel, GLint dstX, GLint dstY, GLint dstZ, + GLsizei srcWidth, GLsizei srcHeight, GLsizei srcDepth) { + ClientSession& session = RequireSession("CopyImageSubData"); + + // ID-57's SHAPE: REFUSED BY NAME, BEFORE ANY EMISSION. GL 4.6 core 18.3.2 accepts + // GL_RENDERBUFFER as either endpoint, and an endpoint is a sum type for exactly that + // reason - but no sticky forward hands out a renderbuffer object, so the sink has no + // way to rebuild one from a name, and none was measured. P7 is where the backend + // takes handles and this arm becomes ordinary. The sink refuses the same shape by + // the same name if a record ever reaches it (defence on both sides of one wire). + if (src.IsRenderbuffer() || dst.IsRenderbuffer()) { + UnmigratedVerbFatal("CopyImageSubData+RENDERBUFFER"); + } + + MG_Pipe::MGPCopyRegion record{}; + record.Src = PublishedTextureHandle(src.Texture); + record.Dst = PublishedTextureHandle(dst.Texture); + // The GL names beside the handles: the key MGB_CTX->GetTextureObject(name) takes on + // the far side (a BARRIER-PULLED sticky forward, counted in `rsp`, retired by P7). + record.SrcGlName = + src.Texture ? static_cast(src.Texture->GetExternalIndex()) : 0u; + record.DstGlName = + dst.Texture ? static_cast(dst.Texture->GetExternalIndex()) : 0u; + // The GL targets verbatim, in a Uint16 - every GL texture target fits one. NOT + // MGPipeResourceTarget: the sink only ever forwards these to a slot that takes GL + // enums, and the tree has no resource-target -> GL-enum inverse to spend on them. + record.SrcTarget = static_cast(srcTarget); + record.DstTarget = static_cast(dstTarget); + record.SrcLevel = static_cast(srcLevel); + record.DstLevel = static_cast(dstLevel); + // SrcBox is {srcX, srcY, srcZ, w, h, d}: the source origin AND the extent, which is + // one extent for both endpoints (GL spells the copy's size once). + record.SrcBox = MG_Pipe::MGPBox{srcX, srcY, srcZ, static_cast(srcWidth), + static_cast(srcHeight), + static_cast(srcDepth)}; + record.DstX = dstX; + record.DstY = dstY; + record.DstZ = dstZ; + session.EmitAndWait(MG_Pipe::MGPWireOp::ResourceCopyRegion, &record, sizeof(record), + nullptr, 0, nullptr, 0, nullptr); + } + + // glShaderStorageBlockBinding -> set_storage_block_binding (75), the ONE content-carrying + // row P5b adds. The block is NAMED, not indexed, because the application's index is the + // frontend interface-query enumeration's and no backend shares that index space + // (BackendObject.h:216-221) - the name is the one coordinate all three agree on. + void EmitShaderStorageBlockBinding(GLuint program, const GLchar* storageBlockName, + GLuint storageBlockBinding) { + ClientSession& session = RequireSession("ShaderStorageBlockBinding"); + // The backend slot's own first line (DirectGLES.cpp:9201), kept here so a null name + // never becomes a zero-size blob - which rule A forbids spelling at all. + if (storageBlockName == nullptr) return; + + MG_Pipe::MGPStorageBlockBinding record{}; + record.GlName = static_cast(program); + record.Binding = static_cast(storageBlockBinding); + record.ShaderCso = MG_Pipe::kMGPipeNullHandle; + if (MG_State::pGLContext != nullptr) { + const auto& programObject = MG_State::pGLContext->GetProgramObject(program); + if (programObject) { + record.ShaderCso = MG_Pipe::MGPipeSlots().FindByLifetimeId( + MG_Pipe::MGPipeKind::ShaderCso, programObject->GetLifetimeId()); + } + } + // Size = strlen + 1: THE NUL TRAVELS (contract table 0's block-name row). The + // decoder re-terminates into a bounded local and refuses a run whose last byte is + // not NUL, so the two sides agree on where the name ends. + const Uint64 nameBytes = static_cast(std::strlen(storageBlockName)) + 1ull; + record.Name = session.Encoder().StageBytes(storageBlockName, nameBytes); + session.EmitAndWait(MG_Pipe::MGPWireOp::SetStorageBlockBinding, &record, sizeof(record), + nullptr, 0, nullptr, 0, nullptr); + } + // ============================================================================= // CLASS A - answered locally from the caps mirror (R-15). NO RECORD, EVER. // ============================================================================= @@ -467,7 +699,8 @@ namespace MobileGL::MG_Remote::Client { } // ============================================================================= - // CLASS C - Fatal{UnmigratedVerb}. 64 slots: 63 in GLFunctionsTable + SetSwapInterval. + // CLASS C - Fatal{UnmigratedVerb}. 57 slots: 56 in GLFunctionsTable + SetSwapInterval + // (64 at the P5b contract commit, less the seven package i1 flipped to class B). // ============================================================================= // // PARTITIONED BY THE P5b PACKAGE THAT OWNS THE FLIP (MG_Remote/CONTRACT-P5B.md, @@ -518,17 +751,12 @@ namespace MobileGL::MG_Remote::Client { X(DrawElementsIndirect, void, (GLenum, GLenum, const void*)) \ X(DrawArraysIndirect, void, (GLenum, const void*)) - // DispatchCompute and DispatchComputeIndirect are i1's too; they are hand-written below - // because they carry b1's dispatch hook before the Fatal. -#define MGR_UNMIGRATED_I1_SLOTS(X) \ - X(BindImageTexture, void, (GLuint, GLuint, GLint, GLboolean, GLint, GLenum, GLenum)) \ - X(CopyImageSubData, void, \ - (const MG_Backend::CopyImageEndpoint&, GLenum, GLint, GLint, GLint, GLint, \ - const MG_Backend::CopyImageEndpoint&, GLenum, GLint, GLint, GLint, GLint, GLsizei, GLsizei, \ - GLsizei)) \ - X(MemoryBarrier, void, (GLbitfield)) \ - X(MemoryBarrierByRegion, void, (GLbitfield)) \ - X(ShaderStorageBlockBinding, void, (GLuint, const GLchar*, GLuint)) + // i1 HAS LANDED: the list is EMPTY and all seven slots are class B (the five emitters + // above plus the two compute ones). It is kept as an empty macro rather than deleted so + // that MGR_UNMIGRATED_GL_SLOTS' union, kUnmigratedI1's arithmetic and the ownership + // static_assert below all keep their shape - and so the next package to need a row here + // (a P5b wave-3 image/compute slot) has the partition to put it in. +#define MGR_UNMIGRATED_I1_SLOTS(X) #define MGR_UNMIGRATED_T2_SLOTS(X) \ X(PatchParameteri, void, (GLenum, GLint)) \ @@ -607,31 +835,18 @@ namespace MobileGL::MG_Remote::Client { MGR_UNMIGRATED_GL_VALUE_SLOTS(MGR_DEFINE_UNMIGRATED) #undef MGR_DEFINE_UNMIGRATED - // THE TWO COMPUTE SLOTS CARRY b1's DISPATCH HOOK BEFORE THE FATAL, and this is stated - // rather than hidden. MarkGpuWritesForDispatch() belongs immediately before the - // dispatch record, and the dispatch record is class C until i1 lands - so the call site - // is here, in the right place, and is UNREACHABLE-IN-EFFECT: the abort follows it. The - // package that moves DispatchCompute into class B (i1: launch_grid, opcode 60) replaces - // the Fatal and inherits a call site that is already correct rather than discovering - // that the mark walk was never wired. - void DispatchCompute_Unmigrated(GLuint, GLuint, GLuint) { - PushPersistentMapsBeforeVerb(); - MarkGpuWritesForDispatch(); - UnmigratedVerbFatal("DispatchCompute"); - } - void DispatchComputeIndirect_Unmigrated(GLintptr) { - PushPersistentMapsBeforeVerb(); - MarkGpuWritesForDispatch(); - UnmigratedVerbFatal("DispatchComputeIndirect"); - } + // THE TWO COMPUTE SLOTS' class-C stubs are GONE (P5b i1): they carried b1's dispatch + // 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 counts, as arithmetic. MGR_COUNT_ONE expands to `+ 1` per row. #define MGR_COUNT_ONE(Name, Ret, Sig) +1 constexpr Uint32 kUnmigratedD1 = 0 MGR_UNMIGRATED_D1_SLOTS(MGR_COUNT_ONE); - // + DispatchCompute, DispatchComputeIndirect, written out by hand. - constexpr Uint32 kUnmigratedI1 = 0 MGR_UNMIGRATED_I1_SLOTS(MGR_COUNT_ONE) + 2; + // i1 landed: the list is empty and the two hand-written compute stubs are gone with it. + 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. @@ -644,7 +859,9 @@ namespace MobileGL::MG_Remote::Client { // The emitted counts, PER OWNER. P5's five are c1's; each P5b package raises its own. constexpr Uint32 kEmittedSlotsP5 = 5; // Clear, DrawArrays, ReadPixels, Blit, Present constexpr Uint32 kEmittedSlotsD1 = 0; - constexpr Uint32 kEmittedSlotsI1 = 0; + // P5b i1: BindImageTexture, DispatchCompute, DispatchComputeIndirect, MemoryBarrier, + // MemoryBarrierByRegion, CopyImageSubData, ShaderStorageBlockBinding. + constexpr Uint32 kEmittedSlotsI1 = 7; constexpr Uint32 kEmittedSlotsT2 = 0; constexpr Uint32 kEmittedSlotsF1 = 0; constexpr Uint32 kEmittedSlots = @@ -660,7 +877,11 @@ namespace MobileGL::MG_Remote::Client { static_assert(kUnmigratedT2 + kEmittedSlotsT2 == 7, "t2 owns the 7 XFB/tessellation slots"); static_assert(kUnmigratedF1 + kEmittedSlotsF1 == 11, "f1 owns the 11 clear/copy/mip slots"); static_assert(kUnmigratedTail == 20, "the wave-3 tail is 20 slots and no P5b package owns one"); - static_assert(kUnmigratedSlots == 64, "CONTRACT-P5.md §7 class C is 64 slots at the P5b contract commit"); + // 64 at the P5b contract commit, MINUS the seven i1 flipped. Each landing package lowers + // this line by its own kEmittedSlots*; the invariant that never moves is the partition + // below, which stays 71 whoever lands next. + static_assert(kUnmigratedSlots == 64 - kEmittedSlotsI1, + "class C is 64 slots at the P5b contract commit less the seven i1 flipped"); static_assert(kLocallyAnsweredSlots + kEmittedSlots + kUnmigratedSlots == kRemoteEmitSlotCount, "the three classes no longer partition the 71 slots"); @@ -677,8 +898,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.GL.DispatchCompute = &DispatchCompute_Unmigrated; - table.GL.DispatchComputeIndirect = &DispatchComputeIndirect_Unmigrated; table.SetSwapInterval = &SetSwapInterval_Unmigrated; // ---- class A @@ -698,6 +917,15 @@ namespace MobileGL::MG_Remote::Client { table.GL.BlitFramebuffer = &EmitBlitFramebuffer; table.Present = &EmitPresent; + // ---- class B, P5b package i1 (kEmittedSlotsI1 = 7) + table.GL.BindImageTexture = &EmitBindImageTexture; + table.GL.DispatchCompute = &EmitDispatchCompute; + table.GL.DispatchComputeIndirect = &EmitDispatchComputeIndirect; + table.GL.MemoryBarrier = &EmitMemoryBarrier; + table.GL.MemoryBarrierByRegion = &EmitMemoryBarrierByRegion; + table.GL.CopyImageSubData = &EmitCopyImageSubData; + table.GL.ShaderStorageBlockBinding = &EmitShaderStorageBlockBinding; + return table; } diff --git a/MobileGL/MG_Remote/Server/PipeApplier.cpp b/MobileGL/MG_Remote/Server/PipeApplier.cpp index cae264b6..320a4149 100644 --- a/MobileGL/MG_Remote/Server/PipeApplier.cpp +++ b/MobileGL/MG_Remote/Server/PipeApplier.cpp @@ -355,31 +355,144 @@ namespace MobileGL::MG_Remote::Server { // in the same words), the call, and a tally the lane can assert moved. // ----------------------------------------------------------------------------------- - // ---- i1 ---- + // ---- i1 ---- (MG_Remote/CONTRACT-P5B.md §2 i1; landed by package p5b/i1) + // + // RULE D IN FIVE BODIES. Each reproduces the backend call the monolith makes, from the + // record and from server state, and NOTHING ELSE: the backend goes on reading the frontend + // fields it reads today through the BARRIER-PULLED entries of its verb class, which the + // verb stamp PipeApplier::ApplyOne put up before this sink ran is exactly what makes legal. + // That is why none of these touches a backend file and why the monolith path is byte + // identical - and it is also the honest statement of the debt, which `rsp` counts. + Bool ServerVerbSink::OnLaunchGrid(const MG_Pipe::MGPGridInfo& grid) { - (void)grid; - ServerUnmigratedVerbFatal(grid.IsIndirect ? "DispatchComputeIndirect" : "DispatchCompute"); + const MG_Backend::GlobalBackendFunctionsTable* table = Table("launch_grid"); + if (table == nullptr) return false; + const MG_Backend::GLFunctionsTable& gl = table->GL; + // The compute program is NOT named by this record and must not be: it is + // GetProgramForDispatch, GetProgramForDraw's twin, which the backend pulls inside its + // own PrepareForCompute (DirectGLES.cpp:5779). i1 is what puts compute on the path, so + // the field moves FATAL -> BARRIER_PULLED in FieldOwnership.def (contract §6.9, the + // one row this package is granted). Block* are 0 on the wire for the same reason: the + // local size is a link artifact the backend reads from its own program. + if (grid.IsIndirect != 0) { + if (gl.DispatchComputeIndirect == nullptr) return false; + // IndirectBuffer travels for P7's sake; the BINDING is server state, put there by + // the set_buffer_bindings record that preceded this one, exactly as OnClear's Fbo + // is not re-resolved here. glDispatchComputeIndirect takes only the offset. + gl.DispatchComputeIndirect(static_cast(grid.IndirectOffset)); + } else { + if (gl.DispatchCompute == nullptr) return false; + gl.DispatchCompute(static_cast(grid.GridX), static_cast(grid.GridY), + static_cast(grid.GridZ)); + } + ++m_dispatches; + return true; } Bool ServerVerbSink::OnMemoryBarrier(const MG_Pipe::MGPMemoryBarrier& barrier) { - ServerUnmigratedVerbFatal(barrier.ByRegion ? "MemoryBarrierByRegion" : "MemoryBarrier"); + const MG_Backend::GlobalBackendFunctionsTable* table = Table("memory_barrier"); + if (table == nullptr) return false; + const MG_Backend::GLFunctionsTable& gl = table->GL; + // THE BITS GO OVER VERBATIM AND ARE LOWERED HERE BY NOBODY. Espryt's atomic-counter + // lowering - the counter bit implying the storage bit, because glslang lowers every + // atomic_uint onto a storage block - lives inside its own MemoryBarrier + // (DirectGLES.cpp:8837) and is a statement about the DRIVER. Repeating it on this side + // would make the split arm and the monolith arm two different calls. + if (barrier.ByRegion != 0) { + if (gl.MemoryBarrierByRegion == nullptr) return false; + gl.MemoryBarrierByRegion(static_cast(barrier.Bits)); + } else { + if (gl.MemoryBarrier == nullptr) return false; + gl.MemoryBarrier(static_cast(barrier.Bits)); + } + ++m_memoryBarriers; + return true; } Bool ServerVerbSink::OnResourceCopyRegion(const MG_Pipe::MGPCopyRegion& copy) { - (void)copy; - ServerUnmigratedVerbFatal("CopyImageSubData"); + const MG_Backend::GlobalBackendFunctionsTable* table = Table("resource_copy_region"); + if (table == nullptr) return false; + if (table->GL.CopyImageSubData == nullptr) return false; + + // REFUSED BY NAME ON BOTH SIDES OF ONE WIRE. The client refuses a renderbuffer endpoint + // before it emits (ID-57's shape, EmitTables.cpp), and this is the same refusal for a + // record that reached here anyway: no sticky forward hands out a RenderbufferObject, so + // there is no honest way to build the endpoint, and guessing an empty one would copy + // nothing and say it copied. + if (copy.SrcTarget == GL_RENDERBUFFER || copy.DstTarget == GL_RENDERBUFFER) { + ServerUnmigratedVerbFatal("CopyImageSubData+RENDERBUFFER"); + } + + // THE TWO ENDPOINTS ARE REBUILT FROM THE GL NAMES, through the BARRIER-PULLED sticky + // forward GetTextureObject(name) - `rsp` counts every one of these and P7 is what + // retires them by making the backend take the handles that travel beside the names. + MG_Backend::CopyImageEndpoint src{}; + MG_Backend::CopyImageEndpoint dst{}; + src.Texture = MG_Pipe::gPipeInputs.GetTextureObject(static_cast(copy.SrcGlName)); + dst.Texture = MG_Pipe::gPipeInputs.GetTextureObject(static_cast(copy.DstGlName)); + if (!src.Exists() || !dst.Exists()) { + // The monolith's own answer to this, in its own words (DirectGLES.cpp:9067 + // "source or destination image failed to sync; declining the copy"): the frontend + // validator is what keeps it unreachable and what reports the INVALID_VALUE the + // application is owed. A decline here is a real answer, not a silent success. + MGLOG_E_ONCE("MG_Remote server: resource_copy_region named texture(s) %u -> %u that " + "the frontend no longer holds; declining the copy", + static_cast(copy.SrcGlName), + static_cast(copy.DstGlName)); + return false; + } + + // SrcBox is {origin, extent} and the extent is the copy's, spelled once by GL for both + // endpoints; the destination contributes only its origin. + table->GL.CopyImageSubData(src, static_cast(copy.SrcTarget), + static_cast(copy.SrcLevel), copy.SrcBox.X, copy.SrcBox.Y, + copy.SrcBox.Z, dst, static_cast(copy.DstTarget), + static_cast(copy.DstLevel), copy.DstX, copy.DstY, + copy.DstZ, static_cast(copy.SrcBox.W), + static_cast(copy.SrcBox.H), + static_cast(copy.SrcBox.D)); + ++m_imageCopies; + return true; } Bool ServerVerbSink::OnBindShaderImage(const MG_Pipe::MGPImageBind& bind) { - (void)bind; - ServerUnmigratedVerbFatal("BindImageTexture"); + const MG_Backend::GlobalBackendFunctionsTable* table = Table("bind_shader_image"); + if (table == nullptr) return false; + if (table->GL.BindImageTexture == nullptr) return false; + // THE SAME CALL IS RIGHT FOR BOTH BACKENDS, which is why the record carries the whole + // argument list although neither reads all of it today: Espryt ignores everything but + // Unit and syncs that unit from the barrier-pulled GetImageTextureBinding + // (DirectGLES.cpp:9154, :2471), and Magma's slot is a no-op (DirectVulkan.cpp:665). The + // arguments travel because rule D says a verb crosses as the CALL, and because P7 is + // what makes the backend read them instead of pulling. + table->GL.BindImageTexture(static_cast(bind.Unit), static_cast(bind.GlName), + static_cast(bind.Level), + bind.Layered != 0 ? GL_TRUE : GL_FALSE, + static_cast(bind.Layer), + static_cast(bind.Access), + static_cast(bind.Format)); + ++m_imageBinds; + return true; } Bool ServerVerbSink::OnSetStorageBlockBinding(const MG_Pipe::MGPStorageBlockBinding& binding, const char* name) { - (void)binding; - (void)name; - ServerUnmigratedVerbFatal("ShaderStorageBlockBinding"); + const MG_Backend::GlobalBackendFunctionsTable* table = Table("set_storage_block_binding"); + if (table == nullptr) return false; + if (table->GL.ShaderStorageBlockBinding == nullptr) return false; + if (name == nullptr) return false; + // The NAME is the one coordinate the application, the frontend and both backends agree + // on (BackendObject.h:216-221), which is why the row carries a blob rather than the + // application's block INDEX. `name` points into the decoder's bounded local and is + // valid for this call only (rule C); the backend slot copies what it needs. + // + // Both backends resolve the PROGRAM through the barrier-pulled GetProgramObject(GlName) + // / TryGetDirectVulkanProgram - `rsp` again, retired by P9. ShaderCso travels beside the + // name for the phase that dispatches on it. + table->GL.ShaderStorageBlockBinding(static_cast(binding.GlName), name, + static_cast(binding.Binding)); + ++m_storageBlockBindings; + return true; } // ---- t2 ---- diff --git a/MobileGL/MG_Remote/Server/PipeApplier.h b/MobileGL/MG_Remote/Server/PipeApplier.h index 43b09c1b..3b7af121 100644 --- a/MobileGL/MG_Remote/Server/PipeApplier.h +++ b/MobileGL/MG_Remote/Server/PipeApplier.h @@ -153,6 +153,16 @@ namespace MobileGL::MG_Remote::Server { Uint64 Presents() const { return m_presents; } Uint64 LastPresentSerial() const { return m_lastPresentSerial; } Uint64 ReadbackBytes() const { return m_readbackBytes; } + // ---- P5b package i1's tallies. R-16: a probe may not arm against a stub, and on a + // split build "the scenario passed" is also what a scenario that never left the + // monolith path looks like - so the lane asserts the number that only this sink can + // move. One per wire ROW, not per GL slot: launch_grid carries both dispatch entry + // points and memory_barrier both barrier ones, and the sink is where they separate. + Uint64 ImageBinds() const { return m_imageBinds; } + Uint64 Dispatches() const { return m_dispatches; } + Uint64 MemoryBarriers() const { return m_memoryBarriers; } + Uint64 ImageCopies() const { return m_imageCopies; } + Uint64 StorageBlockBindings() const { return m_storageBlockBindings; } // ID-49's tight-size control reads this: the scratch a read_pixels grew to. It must equal // the tight w*h*bpp extent of the read, never the client's DstSize - a scratch sized from // DstSize is exactly the heap overflow codex 1 found, one field over. @@ -169,6 +179,11 @@ namespace MobileGL::MG_Remote::Server { Uint64 m_presents = 0; Uint64 m_lastPresentSerial = 0; Uint64 m_readbackBytes = 0; + Uint64 m_imageBinds = 0; + Uint64 m_dispatches = 0; + Uint64 m_memoryBarriers = 0; + Uint64 m_imageCopies = 0; + Uint64 m_storageBlockBindings = 0; // ReadPixels' destination. The pixels go into the reply slot, but GLFunctionsTable:: // ReadPixels writes into a caller buffer, so one staging vector per session sits // between them. Grown, never shrunk, and never handed out past the call. From b82c63a9ec0704a4e6f153cbb8d2f18c5b146417 Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Wed, 16 Sep 2026 12:10:01 -0400 Subject: [PATCH 3/9] [Feat] (MG_Pipe, DirectGLES, P5b): move GetProgramForDispatch FATAL to BARRIER_PULLED now that compute crosses, and skip the copy-image shadow mirror under a non-monolith transport --- MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp | 21 +++++++++++++++++++ MobileGL/MG_Pipe/FieldOwnership.def | 14 ++++++++++--- .../MG_Pipe/generated/PipeFieldOwnership.inc | 8 +++---- 3 files changed, 36 insertions(+), 7 deletions(-) diff --git a/MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp b/MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp index 5225c987..101adccd 100644 --- a/MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp +++ b/MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp @@ -8988,6 +8988,27 @@ namespace MobileGL::MG_Backend::DirectGLES { auto* srcMipmap = MG_State::GLState::AsMipmapTexture(srcEndpoint.Texture.get()); auto* dstMipmap = MG_State::GLState::AsMipmapTexture(dstEndpoint.Texture.get()); if (!srcMipmap || !dstMipmap) return; +#if MOBILEGL_BUILD_DISAGGREGATED + // P5b package i1's ONE backend edit, and the contract names this site + // (MG_Remote/CONTRACT-P5B.md §2 i1 "the copy-image-shadow-mirror emulation", ruling + // §6.7). Migrating glCopyImageSubData moves the first blocker off the client's + // Fatal{UnmigratedVerb} and onto the Fatal below, on every Espryt copy between two + // textures with CPU shadows - so the ruling is: UNDER A REAL TRANSPORT THE SERVER SKIPS + // THE MIRROR, and the client-side mirror ROADMAP P8 names ("CopyImage 镜像搬到 client") + // stays P8's. + // + // WHAT THE SKIP LOSES IS BOUNDED BY TWO FATALS, which is the whole reason it is allowed + // to be a skip rather than a port: a later glGetTexImage of the destination served from + // the shadow is class C wave 3 (Fatal{UnmigratedVerb, "GetTexImage"}, P9) and a texture + // re-mint that re-uploads the level is Fatal{UnmigratedEmulation, "texture-remint-pull"} + // (Managers.cpp:5634). Neither can silently read the un-mirrored shadow. + // + // BEHIND #if MOBILEGL_BUILD_DISAGGREGATED so the pull build's code does not move (G1), + // and the arm is the TRANSPORT and not the build - build-split runs its unit and + // integration-gpu lanes under MOBILEGL_TRANSPORT=monolith, where this mirror is on an + // ordinary correct path and must still run. + if (MG_Config::Transport != MG_Config::TransportMode::Monolith) return; +#endif #if MOBILEGL_PIPE_PUSH // P4a (D-M). glCopyImageSubData's CPU-shadow mirror copies the source level's shadow // rows into the DESTINATION's shadow so a later readback of the destination sees what diff --git a/MobileGL/MG_Pipe/FieldOwnership.def b/MobileGL/MG_Pipe/FieldOwnership.def index beeca7c4..9cccb3ec 100644 --- a/MobileGL/MG_Pipe/FieldOwnership.def +++ b/MobileGL/MG_Pipe/FieldOwnership.def @@ -105,6 +105,14 @@ /* DirectGLES.cpp:4497, PrepareForDraw, the second unconditional pointer read of every draw. */ \ X(GetProgramForDraw, BARRIER_PULLED, "P8 (Espryt), P7 (Magma)", \ "frontend SharedPtr; the record carries a handle") \ + /* P5b package i1 (CONTRACT-P5B.md §6.9, the one row the contract grants a package). This */ \ + /* was FATAL "reachable only from kDispatch; there is no compute on the reduced path" - and */ \ + /* i1 IS what puts compute on the path: launch_grid (60) now crosses and the backend's */ \ + /* PrepareForCompute pulls this inside it (DirectGLES.cpp:5779, VulkanRenderer.cpp:7327). */ \ + /* It is GetProgramForDraw's twin in every respect, so it takes its class and its retiring */ \ + /* phases. Overturned by: nothing in P5b; P7/P8 retire both rows together. */ \ + X(GetProgramForDispatch, BARRIER_PULLED, "P7 (Magma), P8 (Espryt)", \ + "frontend SharedPtr; the record carries a handle") \ /* The XFB six. XFB itself is off the reduced path, but kDraw's may-read mask carries all */ \ /* six and the draw walk reads them regardless - which is exactly the case a field census */ \ /* taken from "what the scenario does" rather than from the mask would miss. */ \ @@ -132,13 +140,13 @@ X(GetPixelStoreParameters, APPLIER_DERIVED, "-", \ "set_pixel_pack_state; the applier writes m_pixelStore[0] (PipeApply.cpp:1373)") \ \ - /* ---- FATAL: three non-sticky fields, each off the reduced path for a checkable reason --- */ \ + /* ---- FATAL: two non-sticky fields, each off the reduced path for a checkable reason ----- */ \ + /* Three until P5b: GetProgramForDispatch moved up to BARRIER_PULLED when package i1 put */ \ + /* compute on the path (CONTRACT-P5B.md §6.9). */ \ X(GetBoundTransformFeedbackName, FATAL, "-", \ "DEAD: read by no backend since the D21 rekey (PipeInputs.h:232-234)") \ X(GetTransformFeedbackPausedPrimitiveCounter, FATAL, "-", \ "reachable only from class kQuery, which the reduced path never enters") \ - X(GetProgramForDispatch, FATAL, "-", \ - "reachable only from kDispatch; there is no compute on the reduced path") \ \ /* ---- the seven sticky forwards, as FIELD rows ---- */ \ /* They have no storage, so a read of the FIELD is a call of the FORWARD; the field row and */ \ diff --git a/MobileGL/MG_Pipe/generated/PipeFieldOwnership.inc b/MobileGL/MG_Pipe/generated/PipeFieldOwnership.inc index 8e2e6faa..44cdf75b 100644 --- a/MobileGL/MG_Pipe/generated/PipeFieldOwnership.inc +++ b/MobileGL/MG_Pipe/generated/PipeFieldOwnership.inc @@ -69,7 +69,7 @@ inline constexpr MGPipeFieldOwnership kMGPipeFieldOwnership[kMGPipeInputFieldCou MGPipeFieldOwnership::kRecordSupplied, // GetPolygonOffsetFactor MGPipeFieldOwnership::kRecordSupplied, // GetPolygonOffsetUnits MGPipeFieldOwnership::kRecordSupplied, // GetPrimitiveRestartIndex - MGPipeFieldOwnership::kFatal, // GetProgramForDispatch + MGPipeFieldOwnership::kBarrierPulled, // GetProgramForDispatch MGPipeFieldOwnership::kBarrierPulled, // GetProgramForDraw MGPipeFieldOwnership::kBarrierPulled, // GetProgramObject MGPipeFieldOwnership::kRecordSupplied, // GetProvokingVertexMode @@ -136,7 +136,7 @@ inline constexpr const char* kMGPipeFieldRetiringPhase[kMGPipeInputFieldCount] = "-", // GetPolygonOffsetFactor "-", // GetPolygonOffsetUnits "-", // GetPrimitiveRestartIndex - "-", // GetProgramForDispatch + "P7 (Magma), P8 (Espryt)", // GetProgramForDispatch "P8 (Espryt), P7 (Magma)", // GetProgramForDraw "P9", // GetProgramObject "-", // GetProvokingVertexMode @@ -298,6 +298,6 @@ inline constexpr SizeT kMGPipeVerbBoundaryExemptCount = 3; // The class sizes, as constants a test can pin without recounting the table. inline constexpr SizeT kMGPipeRecordSuppliedFieldCount = 32; inline constexpr SizeT kMGPipeApplierDerivedFieldCount = 1; -inline constexpr SizeT kMGPipeBarrierPulledFieldCount = 27; -inline constexpr SizeT kMGPipeFatalFieldCount = 3; +inline constexpr SizeT kMGPipeBarrierPulledFieldCount = 28; +inline constexpr SizeT kMGPipeFatalFieldCount = 2; static_assert(kMGPipeRecordSuppliedFieldCount + kMGPipeApplierDerivedFieldCount + kMGPipeBarrierPulledFieldCount + kMGPipeFatalFieldCount == kMGPipeInputFieldCount, "the four class sizes do not partition the field set"); From a72a96d85f88b31768b0390ae9588b85eb342784 Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Wed, 16 Sep 2026 12:10:09 -0400 Subject: [PATCH 4/9] [Test] (MG_Test, MG_IntegrationTest, P5b): pin i1 seven class-B slots and the served compute program, and arm 24 inproc lane entries whose results only exist if the records crossed --- MobileGL/MG_IntegrationTest/CMakeLists.txt | 49 ++++++++ .../Harness/SplitLogPaths.cmake.in | 9 ++ MobileGL/MG_Test/Pipe/FieldOwnershipTest.cpp | 44 ++++++- MobileGL/MG_Test/Wire/RemoteClientTest.cpp | 109 +++++++++++++++++- 4 files changed, 202 insertions(+), 9 deletions(-) diff --git a/MobileGL/MG_IntegrationTest/CMakeLists.txt b/MobileGL/MG_IntegrationTest/CMakeLists.txt index af59f21a..9fa27748 100644 --- a/MobileGL/MG_IntegrationTest/CMakeLists.txt +++ b/MobileGL/MG_IntegrationTest/CMakeLists.txt @@ -1935,6 +1935,55 @@ if (MOBILEGL_BUILD_DISAGGREGATED) ENVIRONMENT "${MGL_ITEST_GLES_SPLIT_ENVIRONMENT}" ) + # ---- P5b package i1 (MG_Remote/CONTRACT-P5B.md §2 i1) -------------------------------- + # + # THE LANE CASE THAT PROVES THE RECORDS CROSSED, and it is a RESULT and not a probe (R-16: + # a probe may not arm against a stub). Each of these four scenarios asserts a value that + # only exists if the verb ran on the apply thread - the texel a compute shader stored + # through an image unit, the word glCopyImageSubData moved, the counter an atomic add left + # behind, the block a program pipeline rebound - so a sink that DECLINED, or a client that + # fell through to the driver, is red by the number it reads back and not by a tally. + # + # Before i1 every one of these aborted at the client's Fatal{UnmigratedVerb} on the first + # glBindImageTexture / glDispatchCompute / glCopyImageSubData / glShaderStorageBlockBinding; + # all four are green under inproc now (the census: 13 + 6 + 11 + 3 DirectGLES entries). + # RED ONCE BY DOING X: return false from ServerVerbSink::OnLaunchGrid before the + # gl.DispatchCompute call and UnboundImageDescriptor / AtomicCounter / ProgramPipeline all + # read back their initial values. + # + # THE FILTERS ARE NARROWED TO THE CASES THAT ACTUALLY EMIT, and that is ScenarioFixture's + # rule rather than a convenience: an armed `DirectGLES.Split.` case must move the client + # encoder's record ordinal (ScenarioFixture.h:86), because a lane whose workload produced no + # record is the "resolved the transport and then fell through to the driver" shape that + # every pixel assertion is blind to. ProgramPipelineScenario's other nine cases are pure + # name/state cases that draw nothing, so the whole-scenario filter would arm them and they + # would be red for having nothing to say. The two named here are the ones that were + # Fatal{UnmigratedVerb, "ShaderStorageBlockBinding"} on the c0b head. + # + # One block per scenario, following the three above: a `:`-separated multi-pattern + # TEST_FILTER is unproven through gtest_discover_tests' flat PROPERTIES forwarding. + set(MGL_SPLIT_I1_SCENARIOS + UnboundImageDescriptorScenario # bind_shader_image (72) + launch_grid (60), 13 cases + CopyImageLayeredScenario # resource_copy_region (53), 6 cases + AtomicCounterScenario # launch_grid (60) + memory_barrier (61), 3 cases + ProgramPipelineScenario) # set_storage_block_binding (75), the 2 storage-block cases + set(MGL_SPLIT_I1_FILTER_UnboundImageDescriptorScenario "UnboundImageDescriptorScenario.*") + set(MGL_SPLIT_I1_FILTER_CopyImageLayeredScenario "CopyImageLayeredScenario.*") + set(MGL_SPLIT_I1_FILTER_AtomicCounterScenario "AtomicCounterScenario.*") + set(MGL_SPLIT_I1_FILTER_ProgramPipelineScenario "ProgramPipelineScenario.*StorageBlock*") + foreach(mglItestI1Scenario IN LISTS MGL_SPLIT_I1_SCENARIOS) + gtest_discover_tests(MobileGLIntegrationTest + TEST_PREFIX "DirectGLES.Split." + TEST_LIST "MGL_SPLIT_I1_${mglItestI1Scenario}_TESTS" + TEST_FILTER "${MGL_SPLIT_I1_FILTER_${mglItestI1Scenario}}" + DISCOVERY_TIMEOUT 30 + PROPERTIES + LABELS "integration-gpu\;integration-split" + TIMEOUT ${MGL_ITEST_TIMEOUT} + ENVIRONMENT "${MGL_ITEST_GLES_SPLIT_ENVIRONMENT}" + ) + endforeach() + # The split half of the counting pair. MGITEST_PERSISTENT_MAP_ARM=emulated is R-6: under split # the adopt tier is pinned at T2, the resource owner declines every acquisition and the client # pushes the mapping's dirty blocks - so pmap must be non-zero and mpr must be the monolith diff --git a/MobileGL/MG_IntegrationTest/Harness/SplitLogPaths.cmake.in b/MobileGL/MG_IntegrationTest/Harness/SplitLogPaths.cmake.in index 532ba453..5ae4a1ce 100644 --- a/MobileGL/MG_IntegrationTest/Harness/SplitLogPaths.cmake.in +++ b/MobileGL/MG_IntegrationTest/Harness/SplitLogPaths.cmake.in @@ -11,4 +11,13 @@ foreach(scenario @MGL_SPLIT_SMALL_RING_SCENARIOS@) "MOBILEGL_LOG_FILE_PATH=@CMAKE_CURRENT_BINARY_DIR@/split-logs/${entry}.log") endforeach() endforeach() +# P5b package i1's four scenarios, the same shape as the small-ring loop above: every +# DirectGLES.Split. entry owns exactly one absolute log path and shares it with nobody, which +# is what SplitLogPaths.PrivateAndDistinct checks over the discovered set. +foreach(scenario @MGL_SPLIT_I1_SCENARIOS@) + foreach(entry IN LISTS MGL_SPLIT_I1_${scenario}_TESTS) + set_tests_properties("${entry}" PROPERTIES ENVIRONMENT + "MOBILEGL_LOG_FILE_PATH=@CMAKE_CURRENT_BINARY_DIR@/split-logs/${entry}.log") + endforeach() +endforeach() # PersistentMapArm retains its existing private path and RESOURCE_LOCK: b1 reads it. diff --git a/MobileGL/MG_Test/Pipe/FieldOwnershipTest.cpp b/MobileGL/MG_Test/Pipe/FieldOwnershipTest.cpp index 068e9080..039a3b10 100644 --- a/MobileGL/MG_Test/Pipe/FieldOwnershipTest.cpp +++ b/MobileGL/MG_Test/Pipe/FieldOwnershipTest.cpp @@ -254,13 +254,23 @@ TEST_F(FieldOwnershipTest, TheReducedPathsUnmigratedFieldsAreAllAccountedFor) { EXPECT_EQ(MGPipeFieldOwnershipOf(MGPipeInputField::GetPixelStoreParameters, 1u), MGPipeFieldOwnership::kFatal); - // The three off the reduced path, each for its own checkable reason. + // The two off the reduced path, each for its own checkable reason. THREE UNTIL P5b: the + // third was GetProgramForDispatch, FATAL because "there is no compute on the reduced path", + // and package i1 is what put compute on the path (CONTRACT-P5B.md §6.9). It is asserted + // below in its new class rather than deleted from this case, because a field that quietly + // left the FATAL list is exactly what this case exists to catch. EXPECT_EQ(MGPipeFieldOwnershipOf(MGPipeInputField::GetBoundTransformFeedbackName), MGPipeFieldOwnership::kFatal); EXPECT_EQ(MGPipeFieldOwnershipOf(MGPipeInputField::GetTransformFeedbackPausedPrimitiveCounter), MGPipeFieldOwnership::kFatal); + // P5b i1: launch_grid (60) crosses, the backend's PrepareForCompute pulls the compute + // program inside it (DirectGLES.cpp:5779), and the field takes GetProgramForDraw's class + // and its retiring phases - so it is a measured DEBT now, not a defect. EXPECT_EQ(MGPipeFieldOwnershipOf(MGPipeInputField::GetProgramForDispatch), - MGPipeFieldOwnership::kFatal); + MGPipeFieldOwnership::kBarrierPulled); + EXPECT_EQ(MGPipeFieldOwnershipOf(MGPipeInputField::GetProgramForDispatch), + MGPipeFieldOwnershipOf(MGPipeInputField::GetProgramForDraw)) + << "GetProgramForDispatch is GetProgramForDraw's twin and must share its class"; } TEST_F(FieldOwnershipTest, TheSevenStickyForwardsAgreeWithTheirFieldRows) { @@ -516,17 +526,43 @@ TEST_F(FieldOwnershipTest, StrictErrorsAlsoPromotesTheStickyForwards) { // A FATAL-class read aborts whatever the knob says: no carrier, and the reduced path never // reads it, so it is a real defect rather than a debt. +// P5b i1: the exemplar MOVED. This case used GetProgramForDispatch, which is BARRIER-PULLED +// from i1 on (a debt the server serves, not an abort), so it would now assert that a served +// read aborts - green for the wrong reason at best. GetTransformFeedbackPausedPrimitiveCounter +// is the same statement with a field that is still FATAL: reachable only from class kQuery, +// which the reduced path never enters. +// RED ONCE BY DOING X: put GetProgramForDispatch back in the FATAL block of FieldOwnership.def +// and TheFieldOwnershipTableIsTheContractsTableRow's new kBarrierPulled expectation goes red by +// name; swap the field below for GetProgramForDispatch and THIS case goes red instead, because +// a barrier-pulled read under a stamp does not abort. TEST_F(FieldOwnershipTest, AFatalClassReadAbortsEvenWithoutStrictErrors) { const ChildResult r = RunInChild([] { MGPipeServerStampVerbBoundary(MGPipeVerb::DrawArrays); - (void)gPipeInputs.GetProgramForDispatch(); + (void)gPipeInputs.GetTransformFeedbackPausedPrimitiveCounter(); }); ASSERT_TRUE(DiedOfAbort(r)) << DescribeStatus(r) << "\n" << r.Log; - EXPECT_NE(r.Log.find("Fatal{UnmigratedPipeInput, \"GetProgramForDispatch@DrawArrays\"}"), + EXPECT_NE(r.Log.find("Fatal{UnmigratedPipeInput, " + "\"GetTransformFeedbackPausedPrimitiveCounter@DrawArrays\"}"), std::string::npos) << r.Log; } +// And the field that LEFT the FATAL class is served rather than fatal, under the verb that made +// it reachable. This is i1's half of the §6.9 grant made checkable: a dispatch stamp plus a read +// of the compute program must NOT abort, which is precisely the statement "compute is on the +// path now". RED ONCE BY DOING X: revert the FieldOwnership.def row to FATAL and this child +// aborts with Fatal{UnmigratedPipeInput, "GetProgramForDispatch@DispatchCompute"}. +TEST_F(FieldOwnershipTest, TheComputeProgramIsServedUnderADispatchStampFromP5bOn) { + const ChildResult r = RunInChild([] { + MGPipeServerStampVerbBoundary(MGPipeVerb::DispatchCompute); + (void)gPipeInputs.GetProgramForDispatch(); + MGPipeServerClearVerbBoundary(); + }); + EXPECT_TRUE(ExitedWith(r, 0)) << DescribeStatus(r) << "\n" << r.Log; + EXPECT_EQ(r.Log.find("Fatal{UnmigratedPipeInput, \"GetProgramForDispatch"), std::string::npos) + << r.Log; +} + // The argument-keyed row: the pack half is answerable and the unpack half is not, and the // difference is the accessor's own argument rather than a second field id. Splitting the field // into two ids would have made the unpack half BARRIER-PULLED - silently served - which is diff --git a/MobileGL/MG_Test/Wire/RemoteClientTest.cpp b/MobileGL/MG_Test/Wire/RemoteClientTest.cpp index acef2348..238807fe 100644 --- a/MobileGL/MG_Test/Wire/RemoteClientTest.cpp +++ b/MobileGL/MG_Test/Wire/RemoteClientTest.cpp @@ -171,12 +171,21 @@ namespace { // ===================================================================================== TEST(RemoteEmitTable, TheThreeClassesPartitionAllSeventyOneSlots) { - // CONTRACT-P5.md §7: 2 answered locally + 5 emitted + 64 Fatal. Read from the functions the - // table itself reports with - which is also what t1's arming condition reads - rather than - // recomputed here, so a table that lost an emitter cannot look like one that never had it. + // CONTRACT-P5.md §7: 2 answered locally + 5 emitted + 64 Fatal at the P5b contract commit. + // Read from the functions the table itself reports with - which is also what t1's arming + // condition reads - rather than recomputed here, so a table that lost an emitter cannot look + // like one that never had it. + // + // P5b package i1 (CONTRACT-P5B.md §2 i1) flipped SEVEN slots C -> B: BindImageTexture, + // DispatchCompute, DispatchComputeIndirect, MemoryBarrier, MemoryBarrierByRegion, + // CopyImageSubData, ShaderStorageBlockBinding. So the two numbers that move are 5 -> 12 and + // 64 -> 57, and the SUM below is the invariant that does not move whichever package lands + // next. RED ONCE BY DOING X: comment out `table.GL.MemoryBarrier = &EmitMemoryBarrier;` in + // BuildRemoteEmitTable and this case stays green while NoSlotIsNull goes red - which is why + // the per-package count is asserted here and the null walk is a separate case. EXPECT_EQ(LocallyAnsweredSlotCount(), 2u); - EXPECT_EQ(ImplementedVerbCount(), 5u); - EXPECT_EQ(UnmigratedSlotCount(), 64u); + EXPECT_EQ(ImplementedVerbCount(), 12u); + EXPECT_EQ(UnmigratedSlotCount(), 57u); EXPECT_EQ(LocallyAnsweredSlotCount() + ImplementedVerbCount() + UnmigratedSlotCount(), kRemoteEmitSlotCount); } @@ -257,6 +266,49 @@ TEST(RemoteEmitTable, SetSwapIntervalIsClassCAndSaysSo) { EXPECT_NE(r.Log.find("Fatal{UnmigratedVerb, \"SetSwapInterval\"}"), std::string::npos) << r.Log; } +#endif // MGTEST_HAVE_FORK + +// ---- P5b package i1 (MG_Remote/CONTRACT-P5B.md §2 i1) ------------------------------------- + +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). + const MG_Backend::GlobalBackendFunctionsTable& table = RemoteEmitTable(); + const void* fatal = reinterpret_cast(table.GL.GetTexImage); // wave-3 tail, class C + ASSERT_NE(fatal, nullptr); + const void* const i1[] = { + reinterpret_cast(table.GL.BindImageTexture), + reinterpret_cast(table.GL.DispatchCompute), + reinterpret_cast(table.GL.DispatchComputeIndirect), + reinterpret_cast(table.GL.MemoryBarrier), + reinterpret_cast(table.GL.MemoryBarrierByRegion), + reinterpret_cast(table.GL.CopyImageSubData), + reinterpret_cast(table.GL.ShaderStorageBlockBinding), + }; + static const char* const kNames[] = {"BindImageTexture", "DispatchCompute", + "DispatchComputeIndirect", "MemoryBarrier", + "MemoryBarrierByRegion", "CopyImageSubData", + "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 + // carry the wrong one. + EXPECT_NE(i1[3], i1[4]) << "MemoryBarrier and MemoryBarrierByRegion share memory_barrier (61) " + "but must set opposite ByRegion values"; + EXPECT_NE(i1[1], i1[2]) << "DispatchCompute and DispatchComputeIndirect share launch_grid (60) " + "but must set opposite IsIndirect values"; +} + +#if MGTEST_HAVE_FORK TEST(RemoteEmitTable, AClassBSlotWithNoSessionAbortsRatherThanFallingThrough) { // The other half of "no slot may fall through to the driver". With no ClientSession the // emitter has nowhere to put the record, and the one thing it may not do is return quietly: @@ -265,6 +317,53 @@ TEST(RemoteEmitTable, AClassBSlotWithNoSessionAbortsRatherThanFallingThrough) { ASSERT_TRUE(DiedOfAbort(r)) << DescribeStatus(r) << "\n" << r.Log; EXPECT_NE(r.Log.find("Fatal{NoClientSession, \"Clear\"}"), std::string::npos) << r.Log; } + +TEST(RemoteEmitTable, EachI1SlotReachesRequireSessionUnderItsOwnName) { + // The behavioural half: an i1 slot is class B, so with no ClientSession it reaches + // RequireSession and aborts Fatal{NoClientSession, ""} - NOT Fatal{UnmigratedVerb} + // (which would mean the flip never happened) and NOT quietly (which is the split lane + // running monolith and going green, R-4). The name in the message is the slot's own, which + // is the half a single case could not state. + // + // RED ONCE BY DOING X: put `X(MemoryBarrier, void, (GLbitfield))` back in + // MGR_UNMIGRATED_I1_SLOTS and drop `table.GL.MemoryBarrier = &EmitMemoryBarrier;` - the + // MemoryBarrier arm below then finds Fatal{UnmigratedVerb, "MemoryBarrier"} instead. + const ChildResult barrier = RunInChild([] { RemoteEmitTable().GL.MemoryBarrier(0x2000); }); + ASSERT_TRUE(DiedOfAbort(barrier)) << DescribeStatus(barrier) << "\n" << barrier.Log; + EXPECT_NE(barrier.Log.find("Fatal{NoClientSession, \"MemoryBarrier\"}"), std::string::npos) + << barrier.Log; + EXPECT_EQ(barrier.Log.find("Fatal{UnmigratedVerb"), std::string::npos) + << "MemoryBarrier is class B from P5b i1 on:\n" + << barrier.Log; + + const ChildResult dispatch = RunInChild([] { RemoteEmitTable().GL.DispatchCompute(1, 1, 1); }); + ASSERT_TRUE(DiedOfAbort(dispatch)) << DescribeStatus(dispatch) << "\n" << dispatch.Log; + EXPECT_NE(dispatch.Log.find("Fatal{NoClientSession, \"DispatchCompute\"}"), std::string::npos) + << dispatch.Log; + + const ChildResult bind = RunInChild( + [] { RemoteEmitTable().GL.BindImageTexture(0, 1, 0, GL_FALSE, 0, 0x88BA, 0x8058); }); + ASSERT_TRUE(DiedOfAbort(bind)) << DescribeStatus(bind) << "\n" << bind.Log; + EXPECT_NE(bind.Log.find("Fatal{NoClientSession, \"BindImageTexture\"}"), std::string::npos) + << bind.Log; + + const ChildResult ssbo = RunInChild( + [] { RemoteEmitTable().GL.ShaderStorageBlockBinding(1, "Blk", 2); }); + ASSERT_TRUE(DiedOfAbort(ssbo)) << DescribeStatus(ssbo) << "\n" << ssbo.Log; + EXPECT_NE(ssbo.Log.find("Fatal{NoClientSession, \"ShaderStorageBlockBinding\"}"), + std::string::npos) + << ssbo.Log; + + const ChildResult copy = RunInChild([] { + const MG_Backend::CopyImageEndpoint src{}; + const MG_Backend::CopyImageEndpoint dst{}; + RemoteEmitTable().GL.CopyImageSubData(src, 0x0DE1, 0, 0, 0, 0, dst, 0x0DE1, 0, 0, 0, 0, 1, + 1, 1); + }); + ASSERT_TRUE(DiedOfAbort(copy)) << DescribeStatus(copy) << "\n" << copy.Log; + EXPECT_NE(copy.Log.find("Fatal{NoClientSession, \"CopyImageSubData\"}"), std::string::npos) + << copy.Log; +} #endif // MGTEST_HAVE_FORK // ===================================================================================== From c240557fe0ce7f557b55ea21ad61028acec892bc Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Wed, 16 Sep 2026 12:23:57 -0400 Subject: [PATCH 5/9] [Feat] (MG_Pipe, MG_Remote, P5b/t2): add kCapBackendOwnsXfbCapture, publish it from the server table EndTransformFeedback slot, and answer the capture-ownership probe from it --- MobileGL/MG_Backend/Init.cpp | 18 +++++++++++++++- .../MG_Impl/GLImpl/Drawing/GL_Drawing.cpp | 16 +++++++++++++- MobileGL/MG_Pipe/MGPipeTypes.h | 12 +++++++++++ MobileGL/MG_Remote/CapsCodec.cpp | 8 ++++--- MobileGL/MG_Remote/Client/SlotCaps.h | 21 ++++++++++++++++++- 5 files changed, 69 insertions(+), 6 deletions(-) diff --git a/MobileGL/MG_Backend/Init.cpp b/MobileGL/MG_Backend/Init.cpp index ba4d648b..8a8fb04f 100644 --- a/MobileGL/MG_Backend/Init.cpp +++ b/MobileGL/MG_Backend/Init.cpp @@ -150,7 +150,23 @@ namespace MobileGL::MG_Backend { // kCapNeedsHostIndexBytes and kCapNeedsHostUboBytes must be 0 for the whole of P5 // by ruling - they are the only two things that ask for an MGHostSpan, and 0 is // what keeps every one of them out of the first IPC frame (contract table 0). - session.SetCapabilityBits(0); + // + // P5b t2 (CONTRACT-P5B.md §6.5) PUBLISHES THE ONE BIT P5b ADDS, and this is the + // only place that can: the question kCapBackendOwnsXfbCapture answers is "does the + // SERVER's backend own the transform-feedback capture", and the server's table is + // visible here and nowhere on the client. It is read straight off the table + // ServerLoop::CreateBackend just built - Espryt registers XfbImpl::EndTransformFeedback + // (BackendObject_DirectGLES.cpp:1458) and Magma registers no XFB slot at all - so the + // bit is a statement about THIS backend rather than about a build option, which is + // what makes it survive a backend switch. The client reads it through + // MGL_BACKEND_SLOT_CAP at GL_Drawing.cpp's FixupGsStripCaptureOrder. + Uint64 capBits = 0; + if (const MG_Backend::BackendObject* serverBackend = loop.Backend(); + serverBackend != nullptr && + serverBackend->GetBackendFunctions().GL.EndTransformFeedback != nullptr) { + capBits |= MG_Pipe::kCapBackendOwnsXfbCapture; + } + session.SetCapabilityBits(capBits); session.SetBackend(loop.Backend()); // 3. the handshake, the four segments, and - at its end - the apply thread. diff --git a/MobileGL/MG_Impl/GLImpl/Drawing/GL_Drawing.cpp b/MobileGL/MG_Impl/GLImpl/Drawing/GL_Drawing.cpp index 4b289545..232d80fb 100644 --- a/MobileGL/MG_Impl/GLImpl/Drawing/GL_Drawing.cpp +++ b/MobileGL/MG_Impl/GLImpl/Drawing/GL_Drawing.cpp @@ -15,6 +15,12 @@ #if MOBILEGL_BUILD_DISAGGREGATED #include #endif +// CONTRACT-P5.md §7 / ID-14: a null check on a GLFunctionsTable slot may not survive into the +// client under split - it becomes a caps-mirror read. SlotCaps.h carries the rule and the test +// that decides which of its two spellings a site takes; in a pull build both expand to exactly +// the check they replaced. P5b t2 converts ONE site in this file - the capture-ownership probe +// in FixupGsStripCaptureOrder - for the reason CONTRACT-P5B.md §6.5 gives. +#include #include "../Getter/GL_Getter.h" namespace MobileGL::MG_Impl::GLImpl { @@ -1287,7 +1293,15 @@ namespace MobileGL::MG_Impl::GLImpl { // Only Vulkan-order captures need this. A backend that runs the capture on its // own GL/ES driver (it owns the span, hence the EndTransformFeedback entry) has // already produced GL's vertex order, and reordering it again would corrupt it. - if (MG_Backend::gBackendFunctionsTable.GL.EndTransformFeedback != nullptr) { + // + // P5b t2 (CONTRACT-P5B.md §6.5): UNDER SPLIT THE TABLE THIS USED TO ASK IS THE CLIENT'S + // EMIT TABLE, whose EndTransformFeedback slot t2 just made non-null for every server - + // so the raw null check would answer "the backend owns the capture" even against Magma, + // which registers no XFB slot at all, and would skip a reorder Magma needs. The question + // is about the SERVER's table, so it is answered from the bit the server publishes. + // Under monolith (and in a pull build) this expands to the null check it replaced, + // character for character. + if (MGL_BACKEND_SLOT_CAP(EndTransformFeedback, MG_Pipe::kCapBackendOwnsXfbCapture)) { return; } if (program == nullptr || !program->HasGsTriangleStripCaptureFixup() || inputPrimitives == 0) { diff --git a/MobileGL/MG_Pipe/MGPipeTypes.h b/MobileGL/MG_Pipe/MGPipeTypes.h index 0c3ec042..c64e919f 100644 --- a/MobileGL/MG_Pipe/MGPipeTypes.h +++ b/MobileGL/MG_Pipe/MGPipeTypes.h @@ -121,6 +121,18 @@ namespace MobileGL::MG_Pipe { // The server packs named uniform blocks into its own ring and therefore needs the // host bytes of a set_shader_buffers(Uniform) range (D-B8). kCapNeedsHostUboBytes = 1ull << 8, + // P5b t2 (CONTRACT-P5B.md §6.5), the one cap bit P5b adds. The SERVER's backend owns + // the transform-feedback capture, i.e. its own table registers EndTransformFeedback. + // FixupGsStripCaptureOrder (GL_Drawing.cpp:1290) asks that question to decide whether + // the CLIENT must reorder the captured records into GL's vertex order, and it used to + // ask it of gBackendFunctionsTable.GL.EndTransformFeedback - which under split is the + // client's EMIT table, where the slot is non-null the moment t2 installs an emitter, + // for every server. A client talking to Espryt would then be right by accident and a + // client talking to Magma (which registers no XFB slot at all, so the sink DECLINES) + // would skip a reorder Magma needs and hand the application a silently corrupt capture + // buffer. So the answer is the SERVER's table, published as a bit and read through + // MGL_BACKEND_SLOT_CAP. The first of the "XFB span family" bits SlotCaps.h predicted. + kCapBackendOwnsXfbCapture = 1ull << 9, }; struct MGPCaps { diff --git a/MobileGL/MG_Remote/CapsCodec.cpp b/MobileGL/MG_Remote/CapsCodec.cpp index 051bd501..b6d841e7 100644 --- a/MobileGL/MG_Remote/CapsCodec.cpp +++ b/MobileGL/MG_Remote/CapsCodec.cpp @@ -44,9 +44,11 @@ namespace MobileGL::MG_Remote { - // The consumer mask may not collide with the MGPCapBits below it. kCapNeedsHostUboBytes - // is 1<<8 today; this asserts the gap stays a gap rather than trusting the comment. - static_assert((static_cast(MG_Pipe::kCapNeedsHostUboBytes) & kMGCapsConsumerMask) == 0, + // The consumer mask may not collide with the MGPCapBits below it. The HIGHEST allocated + // feature bit is kCapBackendOwnsXfbCapture, 1<<9 (P5b t2 raised it from + // kCapNeedsHostUboBytes' 1<<8); this asserts the gap stays a gap rather than trusting the + // comment, so it has to name whichever bit is currently the top one. + static_assert((static_cast(MG_Pipe::kCapBackendOwnsXfbCapture) & kMGCapsConsumerMask) == 0, "an MGPCapBit has grown into CallMask's consumer block (bits 32..47)"); static_assert(MGCapsServerConsumes(MGCapsConsumerBits(MG_Pipe::kMGPipeSubsystemResources), MG_Pipe::kMGPipeSubsystemResources), diff --git a/MobileGL/MG_Remote/Client/SlotCaps.h b/MobileGL/MG_Remote/Client/SlotCaps.h index 63bcf4b0..ca50ba04 100644 --- a/MobileGL/MG_Remote/Client/SlotCaps.h +++ b/MobileGL/MG_Remote/Client/SlotCaps.h @@ -75,11 +75,30 @@ // GL_Drawing.cpp:844 PatchParameteri. "Absent" means the patch size is never set and every // tessellation draw silently uses the previous one. Class C; it aborts by name. // +// P5b t2 CLOSED THE FIRST OF THOSE TWO, AND THE OTHER SIX SITES NEEDED NOTHING (CONTRACT-P5B.md +// §2 t2, §6.5). The six SPAN sites - :1274 Begin, :1371 End, :1420 Pause, :1435 Resume, :1641 +// Delete, :1673 Bind - are guards over a slot that is now class B in the client's table, so they +// simply call the emitter; their `if (const auto f = ...)` shape is left exactly as it was, +// because under split the slot is non-null and under monolith nothing moved. THE PROBE AT :1290 +// IS THE ONE THAT HAD TO CHANGE, and it is the reason this header said the XFB span family would +// be the first to need a bit: it is not a guard on a call, it is a QUESTION ABOUT THE BACKEND +// asked of a table that under split belongs to the client. It now reads +// MGL_BACKEND_SLOT_CAP(EndTransformFeedback, kCapBackendOwnsXfbCapture), the bit the server sets +// from ITS table in MG_Backend/Init.cpp's InitSplitRoles. Espryt registers the slot and answers +// yes; Magma registers no XFB slot, answers no, and the client keeps reordering for it exactly +// as it does under monolith. GL_Drawing.cpp:844's PatchParameteri stays a plain guard for the +// same reason as the six: the slot is class B now, so "absent" never arises. +// +// The one XFB slot still class C is DeleteTransformFeedback, which CONTRACT-P5B.md gives no row +// (unmeasured); :1641's guard therefore still reaches Fatal{UnmigratedVerb} by name, which is +// the outcome R-4 asks for. +// // WHAT THIS HEADER DELIBERATELY DOES NOT DO. It does not touch the 28 unguarded slots: those // have no probe to convert, and calling one reaches Fatal{UnmigratedVerb, ""} by name, // which is R-4's intent. And it does not invent a cap bit - a new MGPCapBit is an // MGPipeTypes.h edit and that file is c0's, so a family that needs one goes through the -// integrator (the XFB span family is the first that will). +// integrator (the XFB span family is the first that will). It did: P5b's contract granted t2 +// exactly that one bit, kCapBackendOwnsXfbCapture (CONTRACT-P5B.md §6.5, §8), and t2 added it. // // G1: in a build without MOBILEGL_BUILD_DISAGGREGATED both macros expand to the null check the // site already had, so the pull build's code generation is unchanged. From dd293203ca1adbe5969d0c755abd9bc43d06180d Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Wed, 16 Sep 2026 12:23:57 -0400 Subject: [PATCH 6/9] [Feat] (MG_Remote, P5b/t2): emit the four stream-output span rows, the XFB object bind and the patch parameter, and give each ServerVerbSink body its backend call --- MobileGL/MG_Remote/Client/EmitTables.cpp | 164 ++++++++++++++++++++-- MobileGL/MG_Remote/Server/PipeApplier.cpp | 91 ++++++++++-- MobileGL/MG_Remote/Server/PipeApplier.h | 21 ++- 3 files changed, 254 insertions(+), 22 deletions(-) diff --git a/MobileGL/MG_Remote/Client/EmitTables.cpp b/MobileGL/MG_Remote/Client/EmitTables.cpp index 6af52e8e..d04b5a2e 100644 --- a/MobileGL/MG_Remote/Client/EmitTables.cpp +++ b/MobileGL/MG_Remote/Client/EmitTables.cpp @@ -16,6 +16,12 @@ // changes class without changing the arithmetic is a build break rather than a behaviour // change nobody reviewed. // +// P5b MOVES SLOTS FROM C TO B, ONE PACKAGE AT A TIME (MG_Remote/CONTRACT-P5B.md §7). The three +// numbers above are the partition AT THE P5b CONTRACT COMMIT and they are the ones the contract +// states; the arithmetic below is what the tree currently has, and the per-package ownership +// assertions say which package moved which slot. On this head t2 has landed: class B is 5 + 6 +// and class C is 58. +// // THE PRE-VERB HOOKS RUN BEFORE THE RECORD, NEVER AFTER (b1, ID-18). PushPersistentMapsBeforeVerb // publishes the bytes an application wrote through a coherent map with no API call at all, and // MarkGpuWritesForDraw builds the conservative GPU-write set the client now owns. Both describe @@ -428,6 +434,132 @@ namespace MobileGL::MG_Remote::Client { session.PumpControlPlane(); } + // ============================================================================= + // CLASS B - P5b package t2: the transform-feedback spans, the XFB object bind and the + // tessellation patch parameter (MG_Remote/CONTRACT-P5B.md §2 t2). + // ============================================================================= + // + // SIX SLOTS, SIX ROWS, AND NOT ONE OF THEM STARTS A SHADER. Every one of the six is a + // control call - it opens, closes, pauses, resumes or re-targets a capture span, or sets + // the patch size the next tessellation draw uses - so each takes BeforeReadOnlyVerb(), + // which is CONTRACT-P5B.md §4's rule for "a verb that reads buffers but starts no + // shader" naming the XFB and patch controls by hand. The GPU-WRITE MARK FOR THE CAPTURE + // TARGETS IS NOT TAKEN HERE and that is deliberate: it belongs at the END of the span, + // after the record and before GLContext::EndTransformFeedback clears the live bindings, + // which is exactly where b1 already put it (GL_Drawing.cpp's + // MarkEndTransformFeedbackCaptureTargets). Taking it here as well would mark the same + // buffers twice and taking it INSTEAD of there would mark nothing. + // + // RULE D (CONTRACT-P5B.md §0): each record carries the GL arguments the frontend handed + // the backend slot and nothing that is a READING of them. The capture program, the + // capture-buffer bindings, the patch state and the bound XFB object all stay + // BARRIER-PULLED - the server's backend reads the client's gPipeInputs fill of the + // moment, which MGP_FILL at each call site has just written and the verb barrier holds + // still (R-1). That is why these are six two-line emitters and not an XFB protocol. + // + // WHAT MUST HAVE CROSSED BEFORE begin_stream_output, since it is the ordering question + // this package was asked: the capture buffers' own resource records (emitted at their + // own call sites through the resource family, long before this point), the program + // (the CSO/program family, likewise), and the buffer BINDINGS - which do not cross as a + // record at all, because set_stream_output_targets (39) has no producer and no consumer + // and CONTRACT-P5B.md §2 rules it NOT required for t2: under the barrier the server's + // StartPendingTransformFeedback reads them through the kXfbSpan/kDraw pulls + // (GetTransformFeedbackProgram, GetBufferBindingPoint). Producing that row is P9's. + + void EmitBeginTransformFeedback(GLenum primitiveMode) { + ClientSession& session = RequireSession("BeginTransformFeedback"); + BeforeReadOnlyVerb(); + + MG_Pipe::MGPStreamOutputBegin record{}; + // The GL token verbatim (contract table 0's "GL enums on the wire"): the sink hands + // it to the backend slot that takes it, and nothing between here and there reads it. + record.PrimitiveMode = static_cast(primitiveMode); + session.EmitAndWait(MG_Pipe::MGPWireOp::BeginStreamOutput, &record, sizeof(record), + nullptr, 0, nullptr, 0, nullptr); + } + + void EmitEndTransformFeedback() { + ClientSession& session = RequireSession("EndTransformFeedback"); + BeforeReadOnlyVerb(); + + MG_Pipe::MGPXfbAccounting record{}; + // THE ACCOUNTING IS THE CLIENT'S OWN AND IT IS INFORMATIONAL ON THIS SIDE OF THE + // WIRE: end_stream_output's backend call takes no arguments, and the three numbers + // are what the frontend has counted over this span (CONTRACT-P5B.md §2 t2, the + // companions row). They travel because the row has carried them since P4a and + // because they are what a server-side scatter would need when P9 lands one; the + // sink today calls GL.EndTransformFeedback() and reads none of them. Read here, + // BEFORE GLContext::EndTransformFeedback resets the counters at the call site. + const auto& context = *MG_State::pGLContext; + record.CapturedVertices = context.GetTransformFeedbackCapturedVertices(); + record.PrimitivesWritten = context.GetTransformFeedbackPrimitiveCounter(); + record.PrimitiveMode = static_cast(context.GetTransformFeedbackPrimitiveMode()); + session.EmitAndWait(MG_Pipe::MGPWireOp::EndStreamOutput, &record, sizeof(record), + nullptr, 0, nullptr, 0, nullptr); + } + + void EmitPauseTransformFeedback() { + ClientSession& session = RequireSession("PauseTransformFeedback"); + BeforeReadOnlyVerb(); + + // Reserved IS zero and the contract says so (MGPStreamOutputControl{Reserved = 0}). + // The row exists to BE the verb boundary - the stamp the server puts up before the + // sink runs - not to carry anything. + MG_Pipe::MGPStreamOutputControl record{}; + record.Reserved = 0; + session.EmitAndWait(MG_Pipe::MGPWireOp::PauseStreamOutput, &record, sizeof(record), + nullptr, 0, nullptr, 0, nullptr); + } + + void EmitResumeTransformFeedback() { + ClientSession& session = RequireSession("ResumeTransformFeedback"); + BeforeReadOnlyVerb(); + + MG_Pipe::MGPStreamOutputControl record{}; + record.Reserved = 0; + session.EmitAndWait(MG_Pipe::MGPWireOp::ResumeStreamOutput, &record, sizeof(record), + nullptr, 0, nullptr, 0, nullptr); + } + + void EmitBindTransformFeedback(GLuint name) { + ClientSession& session = RequireSession("BindTransformFeedback"); + BeforeReadOnlyVerb(); + + MG_Pipe::MGPStreamOutputBind record{}; + // THE GL NAME IS NOT AN IDENTITY (ARCHITECTURE 4.2.1) and is carried anyway, because + // it is the key the backend has always used: Espryt indexes its driver objects by it + // (XfbImpl::g_xfbObjects[name], DirectGLES.cpp:1401) and generates the ES object on + // first bind. Name 0 is the default object, which is why the field is not a handle. + record.GlName = static_cast(name); + // Beside it, the identity that WILL dispatch: the frontend's per-object lifetime id, + // process-wide and never reused, which is what survives glGenTransformFeedbacks + // recycling a name. Read AFTER GLContext::BindTransformFeedbackObject at the call + // site, so it is the id of the object being bound and not of the previous one. + record.LifetimeId = MG_State::pGLContext->GetBoundTransformFeedbackLifetimeId(); + session.EmitAndWait(MG_Pipe::MGPWireOp::BindStreamOutput, &record, sizeof(record), + nullptr, 0, nullptr, 0, nullptr); + } + + void EmitPatchParameteri(GLenum pname, GLint value) { + ClientSession& session = RequireSession("PatchParameteri"); + BeforeReadOnlyVerb(); + + MG_Pipe::MGPPatchParameter record{}; + // GL_PATCH_VERTICES is the only pname that reaches a backend slot - the frontend + // answers GL_PATCH_DEFAULT_*_LEVEL itself and bakes those into the synthesized + // control stage - and the frontend has already rejected every other pname with + // INVALID_ENUM before this call (GL_Drawing.cpp's PatchParameteri). Carried verbatim + // so the sink reproduces the call rather than a reading of it. + record.Pname = static_cast(pname); + record.Value = static_cast(value); + // set_patch_state (43) STILL TRAVELS, at the next validate, and that is not a + // duplicate: it is the applier's working-block copy, this is the driver push Espryt + // does AT THE CALL (DirectGLES.cpp:8760), and both pushes happen today on the + // monolith path too (CONTRACT-P5B.md §2 t2). + session.EmitAndWait(MG_Pipe::MGPWireOp::PatchParameter, &record, sizeof(record), + nullptr, 0, nullptr, 0, nullptr); + } + // ============================================================================= // CLASS A - answered locally from the caps mirror (R-15). NO RECORD, EVER. // ============================================================================= @@ -530,13 +662,14 @@ namespace MobileGL::MG_Remote::Client { X(MemoryBarrierByRegion, void, (GLbitfield)) \ X(ShaderStorageBlockBinding, void, (GLuint, const GLchar*, GLuint)) + // t2 LANDED (CONTRACT-P5B.md §2 t2): the three measured slots - BeginTransformFeedback + // (95 lane entries), PatchParameteri (43), BindTransformFeedback (2) - and the three + // companions that share their rows are class B now and live in the block above. + // DeleteTransformFeedback is the one that stays: it has NO ROW in P5b, by ruling and + // not by omission (unmeasured; the driver object leaks on the server until P9's XFB + // namespace work, and a bind of name 0 is what the backend does on delete of the bound + // one, DirectGLES.cpp:1422). It therefore still aborts by its own name. #define MGR_UNMIGRATED_T2_SLOTS(X) \ - X(PatchParameteri, void, (GLenum, GLint)) \ - X(BeginTransformFeedback, void, (GLenum)) \ - X(EndTransformFeedback, void, ()) \ - X(PauseTransformFeedback, void, ()) \ - X(ResumeTransformFeedback, void, ()) \ - X(BindTransformFeedback, void, (GLuint)) \ X(DeleteTransformFeedback, void, (GLuint)) #define MGR_UNMIGRATED_F1_SLOTS(X) \ @@ -645,7 +778,9 @@ namespace MobileGL::MG_Remote::Client { constexpr Uint32 kEmittedSlotsP5 = 5; // Clear, DrawArrays, ReadPixels, Blit, Present constexpr Uint32 kEmittedSlotsD1 = 0; constexpr Uint32 kEmittedSlotsI1 = 0; - constexpr Uint32 kEmittedSlotsT2 = 0; + // t2: BeginTransformFeedback, EndTransformFeedback, PauseTransformFeedback, + // ResumeTransformFeedback, BindTransformFeedback, PatchParameteri. + constexpr Uint32 kEmittedSlotsT2 = 6; constexpr Uint32 kEmittedSlotsF1 = 0; constexpr Uint32 kEmittedSlots = kEmittedSlotsP5 + kEmittedSlotsD1 + kEmittedSlotsI1 + kEmittedSlotsT2 + kEmittedSlotsF1; @@ -660,7 +795,10 @@ namespace MobileGL::MG_Remote::Client { static_assert(kUnmigratedT2 + kEmittedSlotsT2 == 7, "t2 owns the 7 XFB/tessellation slots"); static_assert(kUnmigratedF1 + kEmittedSlotsF1 == 11, "f1 owns the 11 clear/copy/mip slots"); static_assert(kUnmigratedTail == 20, "the wave-3 tail is 20 slots and no P5b package owns one"); - static_assert(kUnmigratedSlots == 64, "CONTRACT-P5.md §7 class C is 64 slots at the P5b contract commit"); + // 64 at the P5b contract commit, minus the six t2 flipped. CONTRACT-P5.md §7's number + // is the one above, not this one; this is the arithmetic after t2 and it moves again + // for every package that lands. + static_assert(kUnmigratedSlots == 58, "class C is 58 slots after t2's six"); static_assert(kLocallyAnsweredSlots + kEmittedSlots + kUnmigratedSlots == kRemoteEmitSlotCount, "the three classes no longer partition the 71 slots"); @@ -697,6 +835,16 @@ namespace MobileGL::MG_Remote::Client { table.GL.ReadPixels = &EmitReadPixels; table.GL.BlitFramebuffer = &EmitBlitFramebuffer; table.Present = &EmitPresent; + // ---- 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 + // NULL and caught by RemoteEmitTable.NoSlotIsNull rather than silently skipped. + table.GL.BeginTransformFeedback = &EmitBeginTransformFeedback; + table.GL.EndTransformFeedback = &EmitEndTransformFeedback; + table.GL.PauseTransformFeedback = &EmitPauseTransformFeedback; + table.GL.ResumeTransformFeedback = &EmitResumeTransformFeedback; + table.GL.BindTransformFeedback = &EmitBindTransformFeedback; + table.GL.PatchParameteri = &EmitPatchParameteri; return table; } diff --git a/MobileGL/MG_Remote/Server/PipeApplier.cpp b/MobileGL/MG_Remote/Server/PipeApplier.cpp index cae264b6..0467aa60 100644 --- a/MobileGL/MG_Remote/Server/PipeApplier.cpp +++ b/MobileGL/MG_Remote/Server/PipeApplier.cpp @@ -382,35 +382,104 @@ namespace MobileGL::MG_Remote::Server { ServerUnmigratedVerbFatal("ShaderStorageBlockBinding"); } - // ---- t2 ---- + // ---- t2 ---- (MG_Remote/CONTRACT-P5B.md §2 t2) + // + // SIX BODIES, SIX BACKEND CALLS, NO STATE OF THEIR OWN. Rule D: the record IS the call, and + // everything the backend reads around it - the capture program, the capture-buffer + // bindings, the bound XFB object, the patch state - it reads from gPipeInputs through its + // verb class's BARRIER-PULLED fields, which the client filled at the call site and the + // verb barrier holds still (R-1). That is why none of these touches m_backend beyond + // Table() and why not one of them caches anything across records. + // + // A NULL SLOT DECLINES, AND THE DECLINE IS THE MONOLITH'S ANSWER IN THE SAME WORDS. Magma + // (DirectVulkan) registers NO XFB slot and no PatchParameteri at all + // (BackendObject_DirectVulkan.cpp), and under monolith the frontend's own + // `if (const auto f = table.GL.X)` guard simply skips the call; `return false` here is that + // same skip, reported to DecodeAndApply as "this build did not apply it" rather than as a + // crash or as a silent success. Contract §2 t2 says so for PatchParameteri by name. + Bool ServerVerbSink::OnBeginStreamOutput(const MG_Pipe::MGPStreamOutputBegin& begin) { - (void)begin; - ServerUnmigratedVerbFatal("BeginTransformFeedback"); + const MG_Backend::GlobalBackendFunctionsTable* table = Table("begin_stream_output"); + if (table == nullptr) return false; + if (table->GL.BeginTransformFeedback == nullptr) return false; + // Espryt's Begin only ARMS the span (DirectGLES.cpp:1212-1220: primitiveMode, pending, + // targets cleared); the driver glBeginTransformFeedback happens in the tail of the next + // PrepareForDraw (StartPendingTransformFeedback, :1224), where the capture program and + // the buffer bindings are read through the pulls. So this record's effect is not + // visible until a DRAW crosses - which is why an XFB scenario whose draw is still class + // C moves its first blocker to that draw rather than rendering. + table->GL.BeginTransformFeedback(static_cast(begin.PrimitiveMode)); + ++m_streamOutputSpans; + return true; } Bool ServerVerbSink::OnEndStreamOutput(const MG_Pipe::MGPXfbAccounting& accounting) { + const MG_Backend::GlobalBackendFunctionsTable* table = Table("end_stream_output"); + if (table == nullptr) return false; + if (table->GL.EndTransformFeedback == nullptr) return false; + // THE THREE ACCOUNTING FIELDS ARE NOT READ, AND THAT IS THE RULING RATHER THAN AN + // OMISSION. glEndTransformFeedback takes no arguments; the numbers are the CLIENT's own + // per-span accounting (contract §2 t2's companions row) and the client is where they are + // consumed - by the primitive queries and by the capture-capacity clamp. A server that + // second-guessed them from its own driver would be publishing a second answer to a + // question the frontend already answers, and the second answer is the one that goes + // stale. They cross because the row has carried them since P4a and because P9's + // server-side scatter is what will need them. (void)accounting; - ServerUnmigratedVerbFatal("EndTransformFeedback"); + table->GL.EndTransformFeedback(); + ++m_streamOutputSpans; + return true; } Bool ServerVerbSink::OnPauseStreamOutput(const MG_Pipe::MGPStreamOutputControl& control) { - (void)control; - ServerUnmigratedVerbFatal("PauseTransformFeedback"); + const MG_Backend::GlobalBackendFunctionsTable* table = Table("pause_stream_output"); + if (table == nullptr) return false; + if (table->GL.PauseTransformFeedback == nullptr) return false; + (void)control; // Reserved, and the contract says it is 0. + table->GL.PauseTransformFeedback(); + ++m_streamOutputControls; + return true; } Bool ServerVerbSink::OnResumeStreamOutput(const MG_Pipe::MGPStreamOutputControl& control) { + const MG_Backend::GlobalBackendFunctionsTable* table = Table("resume_stream_output"); + if (table == nullptr) return false; + if (table->GL.ResumeTransformFeedback == nullptr) return false; (void)control; - ServerUnmigratedVerbFatal("ResumeTransformFeedback"); + table->GL.ResumeTransformFeedback(); + ++m_streamOutputControls; + return true; } Bool ServerVerbSink::OnBindStreamOutput(const MG_Pipe::MGPStreamOutputBind& bind) { - (void)bind; - ServerUnmigratedVerbFatal("BindTransformFeedback"); + const MG_Backend::GlobalBackendFunctionsTable* table = Table("bind_stream_output"); + if (table == nullptr) return false; + if (table->GL.BindTransformFeedback == nullptr) return false; + // THE GL NAME IS THE ARGUMENT, NOT THE LifetimeId BESIDE IT. Espryt keys its driver + // objects by the GL name (XfbImpl::g_xfbObjects[name], DirectGLES.cpp:1401) and creates + // the ES object on first bind; passing the lifetime id would index a map that has never + // heard of it and silently create a second driver object per bind. The lifetime id + // travels as the identity P7/P9 will dispatch on once the XFB namespace has a wire + // lifetime of its own - it has no reader on this side today, and pretending otherwise + // by folding it into the key is exactly the "a GL name is never an identity" confusion + // the contract's GlName row is written against. + table->GL.BindTransformFeedback(static_cast(bind.GlName)); + ++m_streamOutputBinds; + return true; } Bool ServerVerbSink::OnPatchParameter(const MG_Pipe::MGPPatchParameter& patch) { - (void)patch; - ServerUnmigratedVerbFatal("PatchParameteri"); + const MG_Backend::GlobalBackendFunctionsTable* table = Table("patch_parameter"); + if (table == nullptr) return false; + // Magma registers no PatchParameteri: it compiles the patch size into its synthesized + // control stage from set_patch_state instead, so the DECLINE below is the whole of the + // right answer for that backend and not a gap (contract §2 t2). + if (table->GL.PatchParameteri == nullptr) return false; + // Pname is GL_PATCH_VERTICES and the frontend has already rejected every other spelling + // with INVALID_ENUM before the record was built, so this is a forward and not a switch. + table->GL.PatchParameteri(static_cast(patch.Pname), static_cast(patch.Value)); + ++m_patchParameters; + return true; } // ---- f1 ---- diff --git a/MobileGL/MG_Remote/Server/PipeApplier.h b/MobileGL/MG_Remote/Server/PipeApplier.h index 43b09c1b..0dd0d553 100644 --- a/MobileGL/MG_Remote/Server/PipeApplier.h +++ b/MobileGL/MG_Remote/Server/PipeApplier.h @@ -121,9 +121,9 @@ namespace MobileGL::MG_Remote::Server { // i1 OnLaunchGrid ("DispatchCompute"), OnMemoryBarrier, OnResourceCopyRegion // ("CopyImageSubData"), OnBindShaderImage ("BindImageTexture"), // OnSetStorageBlockBinding ("ShaderStorageBlockBinding") - // t2 OnBeginStreamOutput / OnEndStreamOutput / OnPauseStreamOutput / - // OnResumeStreamOutput ("*TransformFeedback"), OnBindStreamOutput - // ("BindTransformFeedback"), OnPatchParameter ("PatchParameteri") + // t2 LANDED. OnBeginStreamOutput / OnEndStreamOutput / OnPauseStreamOutput / + // OnResumeStreamOutput / OnBindStreamOutput / OnPatchParameter are real bodies + // now: the backend call the contract names, the null-slot DECLINE, a tally. // f1 OnGenerateMipmap, OnCopyFramebufferToTexture ("CopyTexImage2D" / // "CopyTexSubImage2D"), and OnClear's four non-Whole kinds (live already) // d1 OnDrawVbo above: the indirect tail, the user-index span, NumDraws > 1 and the @@ -158,6 +158,17 @@ namespace MobileGL::MG_Remote::Server { // DstSize is exactly the heap overflow codex 1 found, one field over. Uint64 ReadbackScratchBytes() const { return static_cast(m_readbackScratch.size()); } + // P5b t2's tallies, for the same reason the five above exist (R-16): under split "the + // scenario passed" is also what a scenario that ran entirely on the monolith path looks + // like, so a lane that wants to say the XFB spans CROSSED has to read a counter the + // server moved. Spans counts Begin and End together - they are one span and a lane that + // saw only one of them has a bug the two-counter version would have hidden behind a + // sum; controls counts Pause and Resume; binds and patch parameters count their own. + Uint64 StreamOutputSpans() const { return m_streamOutputSpans; } + Uint64 StreamOutputControls() const { return m_streamOutputControls; } + Uint64 StreamOutputBinds() const { return m_streamOutputBinds; } + Uint64 PatchParameters() const { return m_patchParameters; } + private: const MG_Backend::GlobalBackendFunctionsTable* Table(const char* verb) const; @@ -169,6 +180,10 @@ namespace MobileGL::MG_Remote::Server { Uint64 m_presents = 0; Uint64 m_lastPresentSerial = 0; Uint64 m_readbackBytes = 0; + Uint64 m_streamOutputSpans = 0; + Uint64 m_streamOutputControls = 0; + Uint64 m_streamOutputBinds = 0; + Uint64 m_patchParameters = 0; // ReadPixels' destination. The pixels go into the reply slot, but GLFunctionsTable:: // ReadPixels writes into a caller buffer, so one staging vector per session sits // between them. Grown, never shrunk, and never handed out past the call. From c831ab03902362eed39ef2a4e388be6f26d5ac92 Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Wed, 16 Sep 2026 12:23:57 -0400 Subject: [PATCH 7/9] [Test] (MG_Test, MG_IntegrationTest, P5b/t2): pin the span order and the accounting fields, the six flipped slots by name, and run the tessellation-capture and buffer-reuse scenarios under inproc --- MobileGL/MG_IntegrationTest/CMakeLists.txt | 47 ++++++++ .../Harness/SplitLogPaths.cmake.in | 5 +- MobileGL/MG_Test/Wire/PipeWireCodecTest.cpp | 107 ++++++++++++++++++ MobileGL/MG_Test/Wire/RemoteClientTest.cpp | 98 +++++++++++++++- 4 files changed, 254 insertions(+), 3 deletions(-) diff --git a/MobileGL/MG_IntegrationTest/CMakeLists.txt b/MobileGL/MG_IntegrationTest/CMakeLists.txt index af59f21a..fb69e532 100644 --- a/MobileGL/MG_IntegrationTest/CMakeLists.txt +++ b/MobileGL/MG_IntegrationTest/CMakeLists.txt @@ -1935,6 +1935,53 @@ if (MOBILEGL_BUILD_DISAGGREGATED) ENVIRONMENT "${MGL_ITEST_GLES_SPLIT_ENVIRONMENT}" ) + # ---- P5b t2: the transform-feedback spans, the XFB object bind, the patch parameter ------- + # + # MG_Remote/CONTRACT-P5B.md §2 t2. The three P5 lanes above are the REDUCED PATH's targets + # (a clear, a triangle, a persistent map); these two are the first lane entries whose GREEN + # DEPENDS ON A t2 RECORD HAVING CROSSED AND BEEN APPLIED BY THE BACKEND, which is the only + # statement a package flipping a verb can make that a unit case cannot: + # + # TessellationXfbCaptureScenario covers BOTH halves of t2 in one workload. Every case + # calls glPatchParameteri(GL_PATCH_VERTICES, n) and then draws GL_PATCHES into a + # transform-feedback capture, and asserts the CAPTURED BYTES. A patch_parameter (73) + # that did not reach the server tessellates at the previous patch size and the capture + # is the wrong length; a begin/end_stream_output (62/63) that did not reach it leaves + # the buffer holding the scenario's poison value, which is what those cases print. + # XfbCaptureBufferReuseScenario is the span family alone, across four buffer lifetimes + # (a buffer per span, one immutable-storage buffer, one respecified buffer, a + # respecification that changes the capture size). It is the case that would notice a + # span whose END crossed but whose BEGIN did not, because the second span's bytes + # would be the first span's. + # + # Both are DirectGLES only, deliberately, and the reason is measured rather than assumed: + # under Magma the capture is written into the server's resident slice and there is no route + # back, because MG_Backend/Init.cpp's ConsumedSubsystemsFor(DirectVulkan) withholds + # kMGPipeSubsystemResources, so BufferObject::SyncGpuWrites() emits no resource_readback. + # Every DirectVulkan XFB capture entry in the inproc census fails on exactly that (t2-v1.md + # §"the 51"), and it retires with P7 (Magma's resource family) / P9 (the readback carrier) - + # not here. A DirectVulkan arm of this lane would be red for a reason t2 cannot fix. + gtest_discover_tests(MobileGLIntegrationTest + TEST_PREFIX "DirectGLES.Split." + TEST_LIST MGL_SPLIT_T2_TESS_TESTS + TEST_FILTER "TessellationXfbCaptureScenario.*" + DISCOVERY_TIMEOUT 30 + PROPERTIES + LABELS "integration-gpu\;integration-split" + TIMEOUT ${MGL_ITEST_TIMEOUT} + ENVIRONMENT "${MGL_ITEST_GLES_SPLIT_ENVIRONMENT}" + ) + gtest_discover_tests(MobileGLIntegrationTest + TEST_PREFIX "DirectGLES.Split." + TEST_LIST MGL_SPLIT_T2_XFB_TESTS + TEST_FILTER "XfbCaptureBufferReuseScenario.*" + DISCOVERY_TIMEOUT 30 + PROPERTIES + LABELS "integration-gpu\;integration-split" + TIMEOUT ${MGL_ITEST_TIMEOUT} + ENVIRONMENT "${MGL_ITEST_GLES_SPLIT_ENVIRONMENT}" + ) + # The split half of the counting pair. MGITEST_PERSISTENT_MAP_ARM=emulated is R-6: under split # the adopt tier is pinned at T2, the resource owner declines every acquisition and the client # pushes the mapping's dirty blocks - so pmap must be non-zero and mpr must be the monolith diff --git a/MobileGL/MG_IntegrationTest/Harness/SplitLogPaths.cmake.in b/MobileGL/MG_IntegrationTest/Harness/SplitLogPaths.cmake.in index 532ba453..757addcb 100644 --- a/MobileGL/MG_IntegrationTest/Harness/SplitLogPaths.cmake.in +++ b/MobileGL/MG_IntegrationTest/Harness/SplitLogPaths.cmake.in @@ -1,7 +1,10 @@ # Included by CTest after all GoogleTest discovery files (ID-53). # CTest appends ENVIRONMENT here; keep all previously discovered lane settings. file(MAKE_DIRECTORY "@CMAKE_CURRENT_BINARY_DIR@/split-logs") -foreach(entry IN LISTS MGL_SPLIT_CLEAR_TESTS MGL_SPLIT_TRIANGLE_TESTS MGL_SPLIT_PMAP_TESTS) +# P5b t2's two lanes ride the same rule: one private log path per entry, or +# SplitLogPaths.PrivateAndDistinct is red for them (ID-53). +foreach(entry IN LISTS MGL_SPLIT_CLEAR_TESTS MGL_SPLIT_TRIANGLE_TESTS MGL_SPLIT_PMAP_TESTS + MGL_SPLIT_T2_TESS_TESTS MGL_SPLIT_T2_XFB_TESTS) set_tests_properties("${entry}" PROPERTIES ENVIRONMENT "MOBILEGL_LOG_FILE_PATH=@CMAKE_CURRENT_BINARY_DIR@/split-logs/${entry}.log") endforeach() diff --git a/MobileGL/MG_Test/Wire/PipeWireCodecTest.cpp b/MobileGL/MG_Test/Wire/PipeWireCodecTest.cpp index 7b909290..f846033c 100644 --- a/MobileGL/MG_Test/Wire/PipeWireCodecTest.cpp +++ b/MobileGL/MG_Test/Wire/PipeWireCodecTest.cpp @@ -1204,6 +1204,113 @@ TEST_F(PipeWireCodecTest, BindStreamOutputReachesTheSink) { EXPECT_EQ(wire.Sink().StreamOutputBinds[0].LifetimeId, 0x1234567890ull); } +// ===================================================================================== +// P5b t2 (MG_Remote/CONTRACT-P5B.md §2 t2). c0b's cases above round-trip each row once; these +// pin the fields t2's EMITTERS actually fill and the one ordering property the span family has. +// ===================================================================================== + +TEST_F(PipeWireCodecTest, EndStreamOutputCarriesAllThreeAccountingFieldsAndNotJustTheVertices) { + // t2's emitter fills all three from the frontend's own per-span accounting + // (GetTransformFeedbackCapturedVertices / GetTransformFeedbackPrimitiveCounter / + // GetTransformFeedbackPrimitiveMode) and the row above asserts only CapturedVertices, so a + // codec that dropped either of the other two - or an emitter that left them zero - reads + // green there. THE THREE ARE DELIBERATELY DIFFERENT NUMBERS: with 300/300 a swap of the two + // 64-bit fields is invisible. + // + // Red once by making the recorder above push a default-constructed MGPXfbAccounting{} + // instead of the one the decoder handed it - the shape of a seam that loses the payload: + // "end_stream_output lost the primitives-written half of its accounting". + Wire2 wire; + MGPXfbAccounting end{}; + end.CapturedVertices = 21; + end.PrimitivesWritten = 7; + end.PrimitiveMode = 0x0000; // GL_POINTS - and 0 is a legal primitive mode, not "unset" + ASSERT_NE(wire.Encoder().EncodeRecord(MGPWireOp::EndStreamOutput, &end, sizeof(end)), kInvalidSeq); + bool applied = false; + ASSERT_TRUE(wire.PumpOne(&applied)); + EXPECT_TRUE(applied); + ASSERT_EQ(wire.Sink().Ends.size(), 1u); + EXPECT_EQ(wire.Sink().Ends[0].CapturedVertices, 21u); + EXPECT_EQ(wire.Sink().Ends[0].PrimitivesWritten, 7u) + << "end_stream_output lost the primitives-written half of its accounting"; + EXPECT_EQ(wire.Sink().Ends[0].PrimitiveMode, 0u) + << "GL_POINTS is 0 and a row that treats 0 as 'no mode' has made a legal capture " + "indistinguishable from an unfilled record"; +} + +TEST_F(PipeWireCodecTest, BindStreamOutputOfNameZeroIsTheDefaultObjectAndNotAnAbsentOne) { + // THE ONE NAME t2 CANNOT TREAT AS "NOTHING". CONTRACT-P5B.md gives DeleteTransformFeedback + // no row, and the backend's answer to deleting the bound object is a bind of NAME 0 + // (DirectGLES.cpp:1422) - so a row that folded 0 into kMGPipeNullHandle, or a sink that read + // 0 as "no object", would silently stop rebinding the default object and leave the driver + // bound to a deleted one. The lifetime id beside it is 0 too here, which is the frontend's + // seed value for the default object, so this case also pins that a wholly-zero record is + // legal and applies rather than being refused as unfilled. + // + // Red once by making the sink's OnBindStreamOutput refuse GlName == 0 (return false): the + // EXPECT_TRUE(applied) below failed, which is the shape the wrong reading would take. + Wire2 wire; + MGPStreamOutputBind bind{}; + bind.GlName = 0; + bind.LifetimeId = 0; + ASSERT_NE(wire.Encoder().EncodeRecord(MGPWireOp::BindStreamOutput, &bind, sizeof(bind)), kInvalidSeq); + bool applied = false; + ASSERT_TRUE(wire.PumpOne(&applied)); + EXPECT_TRUE(applied) << "a bind of the default transform-feedback object did not apply"; + ASSERT_EQ(wire.Sink().StreamOutputBinds.size(), 1u); + EXPECT_EQ(wire.Sink().StreamOutputBinds[0].GlName, 0u); +} + +TEST_F(PipeWireCodecTest, AWholeCaptureSpanReachesTheSinkInTheOrderTheClientEmittedIt) { + // THE PROPERTY A PER-ROW ROUND TRIP CANNOT STATE. A capture span is five calls whose MEANING + // is their order - bind the object, open the span, pause it, resume it, close it - and every + // one of them rides its own row with no sequence field of its own to check. The decoder's + // ordering is the ring's, so this is a pin on the seam rather than a new guarantee: if a + // later change ever batches or reorders records per row, a capture reordered into + // bind/begin/end/pause/resume applies five records, sets `applied` five times, and leaves + // every per-row case above green while the capture is destroyed. + // + // Red once by encoding the Pause AFTER the End: the interleaved-order EXPECT below failed on + // Pauses being 0 at the point the End was seen. + Wire2 wire; + MGPStreamOutputBind bind{}; + bind.GlName = 3; + bind.LifetimeId = 0x5150ull; + MGPStreamOutputBegin begin{}; + begin.PrimitiveMode = 0x0004; // GL_TRIANGLES + MGPStreamOutputControl control{}; + MGPXfbAccounting end{}; + end.CapturedVertices = 9; + end.PrimitivesWritten = 3; + end.PrimitiveMode = 0x0004; + + ASSERT_NE(wire.Encoder().EncodeRecord(MGPWireOp::BindStreamOutput, &bind, sizeof(bind)), kInvalidSeq); + ASSERT_NE(wire.Encoder().EncodeRecord(MGPWireOp::BeginStreamOutput, &begin, sizeof(begin)), kInvalidSeq); + ASSERT_NE(wire.Encoder().EncodeRecord(MGPWireOp::PauseStreamOutput, &control, sizeof(control)), kInvalidSeq); + ASSERT_NE(wire.Encoder().EncodeRecord(MGPWireOp::ResumeStreamOutput, &control, sizeof(control)), kInvalidSeq); + ASSERT_NE(wire.Encoder().EncodeRecord(MGPWireOp::EndStreamOutput, &end, sizeof(end)), kInvalidSeq); + + // Pumped ONE AT A TIME with the sink read between pumps, which is what makes this a + // statement about order rather than about totals: after the third record the pause must + // have happened and the end must NOT have. + bool applied = false; + ASSERT_TRUE(wire.PumpOne(&applied)); + EXPECT_EQ(wire.Sink().StreamOutputBinds.size(), 1u) << "the object bind did not come first"; + EXPECT_EQ(wire.Sink().Begins.size(), 0u); + ASSERT_TRUE(wire.PumpOne(&applied)); + EXPECT_EQ(wire.Sink().Begins.size(), 1u) << "the span did not open second"; + ASSERT_TRUE(wire.PumpOne(&applied)); + EXPECT_EQ(wire.Sink().Pauses, 1u) << "the pause did not arrive third"; + EXPECT_EQ(wire.Sink().Ends.size(), 0u) + << "the span closed before it was paused - the capture's order did not survive the wire"; + ASSERT_TRUE(wire.PumpOne(&applied)); + EXPECT_EQ(wire.Sink().Resumes, 1u); + ASSERT_TRUE(wire.PumpOne(&applied)); + ASSERT_EQ(wire.Sink().Ends.size(), 1u); + EXPECT_EQ(wire.Sink().Ends[0].PrimitivesWritten, 3u); + EXPECT_EQ(wire.Decoder().AppliedSeq(), 5u); +} + TEST_F(PipeWireCodecTest, SetStorageBlockBindingCarriesItsNameAsAStagedBlob) { // i1: the ONE string on the wire. Size is strlen + 1 - the NUL travels - and the decoder // hands the sink a pointer that dies with the call. diff --git a/MobileGL/MG_Test/Wire/RemoteClientTest.cpp b/MobileGL/MG_Test/Wire/RemoteClientTest.cpp index acef2348..385a90bb 100644 --- a/MobileGL/MG_Test/Wire/RemoteClientTest.cpp +++ b/MobileGL/MG_Test/Wire/RemoteClientTest.cpp @@ -174,13 +174,107 @@ TEST(RemoteEmitTable, TheThreeClassesPartitionAllSeventyOneSlots) { // CONTRACT-P5.md §7: 2 answered locally + 5 emitted + 64 Fatal. Read from the functions the // table itself reports with - which is also what t1's arming condition reads - rather than // recomputed here, so a table that lost an emitter cannot look like one that never had it. + // + // P5b MOVES THE SECOND AND THIRD NUMBERS, ONE PACKAGE AT A TIME (CONTRACT-P5B.md §7). On + // this head t2 has landed its six XFB / patch slots, so it is 2 + 11 + 58. The SUM is what + // actually has to hold, and it is asserted separately below for that reason; the two moving + // numbers are spelled out anyway so a package that flips a slot without owning it has to + // edit this line and say so. EXPECT_EQ(LocallyAnsweredSlotCount(), 2u); - EXPECT_EQ(ImplementedVerbCount(), 5u); - EXPECT_EQ(UnmigratedSlotCount(), 64u); + EXPECT_EQ(ImplementedVerbCount(), 11u) << "P5's five class-B verbs plus t2's six"; + EXPECT_EQ(UnmigratedSlotCount(), 58u) << "64 at the P5b contract commit, minus t2's six"; EXPECT_EQ(LocallyAnsweredSlotCount() + ImplementedVerbCount() + UnmigratedSlotCount(), kRemoteEmitSlotCount); } +TEST(RemoteEmitTable, TheSixXfbAndPatchSlotsAreNonNullAndDistinct) { + // P5b t2 (CONTRACT-P5B.md §2 t2), the half that needs no fork: the six slots exist and are + // six DIFFERENT functions. Six identical pointers would be one emitter assigned six times, + // which is how a copy-paste flip loses five records and still passes every count. + // + // Red once by assigning `table.GL.PauseTransformFeedback = &EmitResumeTransformFeedback;` + // in BuildRemoteEmitTable - the exact copy-paste this guards: "t2 slots 2 and 3 are one + // function". + const MG_Backend::GlobalBackendFunctionsTable& table = RemoteEmitTable(); + const void* const six[] = { + reinterpret_cast(table.GL.BeginTransformFeedback), + reinterpret_cast(table.GL.EndTransformFeedback), + reinterpret_cast(table.GL.PauseTransformFeedback), + reinterpret_cast(table.GL.ResumeTransformFeedback), + reinterpret_cast(table.GL.BindTransformFeedback), + reinterpret_cast(table.GL.PatchParameteri), + }; + for (SizeT i = 0; i < 6; ++i) { + EXPECT_NE(six[i], nullptr) << "t2 slot " << i << " is null"; + for (SizeT j = i + 1; j < 6; ++j) { + EXPECT_NE(six[i], six[j]) << "t2 slots " << i << " and " << j << " are one function"; + } + } + // And the one XFB slot t2 does NOT flip is still there to be Fatal - CONTRACT-P5B.md gives + // DeleteTransformFeedback no row (unmeasured), so it must not have been swept up. + EXPECT_NE(table.GL.DeleteTransformFeedback, nullptr); +} + +#if MGTEST_HAVE_FORK +TEST(RemoteEmitTable, EachXfbAndPatchSlotIsClassBAndDemandsASessionByItsOwnName) { + // P5b t2, THE HALF THAT DECIDES THE CLASS. A pointer comparison cannot tell a class-B + // emitter from a class-C thunk - each unmigrated slot gets its own generated function, so + // every slot in the table is already a distinct non-null address. What distinguishes them is + // WHAT THEY SAY when called with no ClientSession: an emitter reaches RequireSession and + // dies Fatal{NoClientSession, ""}; a thunk dies Fatal{UnmigratedVerb, ""}. Both + // strings are asserted, because a case that only looked for the first would be satisfied by + // a build where every one of these had been flipped by accident. + // + // Red once by SWAPPING the Pause and Resume assignments in BuildRemoteEmitTable - the two + // emitters with the same signature, so the swap compiles and neither is orphaned (the first + // attempt redirected one slot at another's emitter and the build failed on the signature + // and on -Wunused-function, which is a control that did not run). The child called through + // PauseTransformFeedback died Fatal{NoClientSession, "ResumeTransformFeedback"} and this + // case failed with "PauseTransformFeedback did not reach the class-B emitter's session + // demand", so the string really is the slot's own name and not a shared constant. + struct Slot { + const char* Name; + void (*Call)(); + }; + static const Slot kSlots[] = { + {"BeginTransformFeedback", [] { RemoteEmitTable().GL.BeginTransformFeedback(0x0004); }}, + {"EndTransformFeedback", [] { RemoteEmitTable().GL.EndTransformFeedback(); }}, + {"PauseTransformFeedback", [] { RemoteEmitTable().GL.PauseTransformFeedback(); }}, + {"ResumeTransformFeedback", [] { RemoteEmitTable().GL.ResumeTransformFeedback(); }}, + {"BindTransformFeedback", [] { RemoteEmitTable().GL.BindTransformFeedback(0); }}, + {"PatchParameteri", [] { RemoteEmitTable().GL.PatchParameteri(0x8E72, 3); }}, + }; + for (const Slot& slot : kSlots) { + const ChildResult r = RunInChild([&slot] { slot.Call(); }); + ASSERT_TRUE(DiedOfAbort(r)) << slot.Name << ": " << DescribeStatus(r) << "\n" << r.Log; + EXPECT_NE(r.Log.find(std::string("Fatal{NoClientSession, \"") + slot.Name + "\"}"), + std::string::npos) + << slot.Name << " did not reach the class-B emitter's session demand:\n" + << r.Log; + EXPECT_EQ(r.Log.find("Fatal{UnmigratedVerb"), std::string::npos) + << slot.Name << " is still class C:\n" + << r.Log; + } +} + +TEST(RemoteEmitTable, DeleteTransformFeedbackHasNoRowAndStillAbortsByItsOwnName) { + // CONTRACT-P5B.md §2 t2 and c0b-v1.md §6: the seventh slot in t2's ownership block gets NO + // row in P5b - it is unmeasured, the driver object leaks on the server until P9's XFB + // namespace work, and a bind of name 0 is what the backend does on delete of the bound one. + // That is a RULING, so it is pinned rather than left to be re-derived from a count: a later + // round that gives it a row has to delete this case and say why. + // + // Red once by assigning `table.GL.DeleteTransformFeedback = &EmitBindTransformFeedback;` in + // BuildRemoteEmitTable - a package sweeping the whole XFB family into class B: the child + // died Fatal{NoClientSession, "BindTransformFeedback"} and the UnmigratedVerb expectation + // failed. + const ChildResult r = RunInChild([] { RemoteEmitTable().GL.DeleteTransformFeedback(7); }); + ASSERT_TRUE(DiedOfAbort(r)) << DescribeStatus(r) << "\n" << r.Log; + EXPECT_NE(r.Log.find("Fatal{UnmigratedVerb, \"DeleteTransformFeedback\"}"), std::string::npos) + << r.Log; +} +#endif // MGTEST_HAVE_FORK + TEST(RemoteEmitTable, NoSlotIsNull) { // R-4's whole rule, asserted over the STRUCT rather than over the list that built it. 91 // MG_Impl sites call through this table directly; a null slot is 91 potential null calls, From 6fa6a925bdf91ba7e84d7f4ca3fde3e928086583 Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Wed, 16 Sep 2026 12:34:33 -0400 Subject: [PATCH 8/9] [Fix] (CI): make split controls mandatory and isolate runner evidence --- .github/workflows/test.yml | 40 +++++++++---------- .../Harness/split_log_paths.py | 8 ++++ MobileGL/MG_Test/Wire/c1f_redcheck.py | 19 ++++++--- scripts/ci/census_junit.py | 24 +++++++++++ scripts/ci/control_smoke_test.sh | 17 +++++--- scripts/ci/junit_tally.py | 13 ++++-- scripts/ci/redcheck_control_smoke_test.sh | 2 +- scripts/ci/retrace_drop_draw_control.sh | 10 +++++ scripts/ci/retrace_pull_library_control.sh | 19 ++++++++- scripts/ci/split_negative_controls.sh | 13 +++--- .../ci/testdata/split_private_log_smoke.sh | 3 +- scripts/ci/testdata/stub_ctest.sh | 21 +++++++++- scripts/p3a_untouched_regions.sh | 5 ++- scripts/p4a_untouched_regions.sh | 5 ++- 14 files changed, 148 insertions(+), 51 deletions(-) create mode 100644 scripts/ci/census_junit.py diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 521bb04b..e6371b80 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -962,6 +962,8 @@ jobs: # workflow passed that option. They had never been compiled by CI, let alone run. - name: Unit tests on the split runtime working-directory: build-split + env: + MOBILEGL_ITEST_REQUIRE_GPU: "1" run: ctest --output-on-failure -L unit --no-tests=error -j "$(nproc)" # --no-tests=error is half the gate, exactly as in integration-verify: the integration-split @@ -979,13 +981,11 @@ jobs: run: | ulimit -c unlimited sudo sysctl -w kernel.core_pattern='/tmp/core.%e.%p' - ctest --output-on-failure -L integration-split --no-tests=error -j 4 + ctest --output-on-failure -L integration-split --no-tests=error -j 4 --output-junit "${RUNNER_TEMP}/split-baseline.xml" + python3 ../scripts/ci/junit_tally.py "${RUNNER_TEMP}/split-baseline.xml" --require-split-ran - # ARCHITECTURE.md:521 asks for `ctest -L integration-gpu` to be name-for-name identical - # between the monolith and the split arm of the SAME build - the G2 shape extended to a - # third arm. The knob goes in the JOB environment rather than in a ctest property, for the - # reason the MOBILEGL_ESPRYT_DISABLE_INVALIDATE_FLUSH precedent in `integration` gives: a - # property would override it and the arm would not be an arm. + # ID-65 supersedes broad inproc status parity: class-C aborts and named wrong-answer + # debts are recorded. Monolith, integration-split and the controls remain hard gates. # # The entries that name MOBILEGL_TRANSPORT in their OWN property (the Split. lanes) keep # their value in both passes, which is correct: they are the split family in both arms and @@ -1007,14 +1007,11 @@ jobs: fi echo "integration-gpu entries in the split build: ${count}" MOBILEGL_TRANSPORT=monolith ctest --output-on-failure -L integration-gpu --no-tests=error -j 4 --output-junit "${RUNNER_TEMP}/arm-monolith.xml" - MOBILEGL_TRANSPORT=inproc ctest --output-on-failure -L integration-gpu --no-tests=error -j 4 --output-junit "${RUNNER_TEMP}/arm-inproc.xml" - # NAME **AND STATUS**, and it is the comparison this step claimed to make and did not - # (review finding N-3): the first version wrote a names file and never read it, and - # `--output-on-failure` treats a SKIPPED test as not-a-failure - so the very failure - # ARCHITECTURE.md:521 is about, "an inproc arm that skipped forty entries the monolith arm - # ran", was invisible here and caught only by the local gate. `ctest -N` cannot see it - # either: this is one build directory, so the two arms have identical name lists by - # construction and the difference is entirely in what each entry DID. + # ID-65: broad inproc is a recorded debt census. Reduced split + controls gate below. + inproc_rc=0 + MOBILEGL_TRANSPORT=inproc ctest --output-on-failure -L integration-gpu --no-tests=error -j 4 --output-junit "${RUNNER_TEMP}/arm-inproc.xml" || inproc_rc=$? + python3 ../scripts/ci/census_junit.py "${RUNNER_TEMP}/arm-inproc.xml" "${inproc_rc}" >> "${GITHUB_STEP_SUMMARY}" + # Retain the per-name status delta as evidence, not as the reduced-path gate. python3 - "${RUNNER_TEMP}/arm-monolith.xml" "${RUNNER_TEMP}/arm-inproc.xml" <<'PY' import sys, xml.etree.ElementTree as ET def rows(path): @@ -1031,13 +1028,10 @@ jobs: diff = sorted(set(a) ^ set(b)) + sorted(n for n in set(a) & set(b) if a[n] != b[n]) if diff: for name in diff[:40]: - print(f"::error::{name}: monolith={a.get(name, '')} inproc={b.get(name, '')}") - print(f"::error::the monolith and inproc arms of ctest -L integration-gpu differ on " - f"{len(diff)} entries. ARCHITECTURE.md:521 requires them identical name for name " - f"AND status; an entry that SKIPPED on one arm and ran on the other is the " - f"failure this compares for, and it is not a failure to --output-on-failure.") - raise SystemExit(1) - print(f"the two arms agree on all {len(a)} entries, name and status") + print(f"{name}: monolith={a.get(name, '')} inproc={b.get(name, '')}") + print(f"Recorded ID-65 census: {len(diff)} name/status differences; not a parity gate") + else: + print(f"the two arms agree on all {len(a)} entries, name and status") PY # THE RUNTIME HALF OF "THIS IS REALLY A SPLIT BUILD". The build-level nm check in @@ -1095,6 +1089,7 @@ jobs: # and FAILED as evidence the lane was live, so the controls could be measured against a # baseline that was already red. - name: Negative controls - the verb barrier and the persistent-map push must be load-bearing + if: ${{ !cancelled() }} working-directory: build-split env: MOBILEGL_ITEST_REQUIRE_GPU: "1" @@ -1109,6 +1104,8 @@ jobs: path: | build-split/MobileGL/MG_IntegrationTest/*.log* build-split/MobileGL/MG_IntegrationTest/split-logs/*.log + ${{ runner.temp }}/arm-inproc.xml + ${{ runner.temp }}/arm-monolith.xml if-no-files-found: warn - name: Upload core dumps @@ -1978,6 +1975,7 @@ jobs: env: CONTROL_TMPDIR: ${{ runner.temp }} LIBRARY_LOG: ${{ matrix.case }}/${{ matrix.backend }}/output/mobilegl.log + FROZEN_LIBRARY: ${{ github.workspace }}/build-linux/libMobileGL.so run: >- bash "${GITHUB_WORKSPACE}/scripts/ci/retrace_drop_draw_control.sh" '${{ matrix.case }}' '${{ matrix.backend }}' diff --git a/MobileGL/MG_IntegrationTest/Harness/split_log_paths.py b/MobileGL/MG_IntegrationTest/Harness/split_log_paths.py index 689717e0..c082f73d 100644 --- a/MobileGL/MG_IntegrationTest/Harness/split_log_paths.py +++ b/MobileGL/MG_IntegrationTest/Harness/split_log_paths.py @@ -68,6 +68,14 @@ def main(): for n in selected for c in by_name[n]) if not_failed: raise ValueError(f"{label} control: {not_failed} selected entries did not fail") + elif mode == "assertion": + cases = ET.parse(sys.argv[4]).getroot().findall(".//testcase") + by_name = {case.get("name"): case for case in cases} + for name in selected: + case = by_name.get(name) + output = "" if case is None else " ".join(" ".join(case.itertext()).split()) + if not re.search(sys.argv[5], output): + raise ValueError(f"E3(a) FAILED: {name} red lacks its persistent-map push diagnostic") elif mode == "evidence": missing = [] label = sys.argv[5] if len(sys.argv) > 5 else "" diff --git a/MobileGL/MG_Test/Wire/c1f_redcheck.py b/MobileGL/MG_Test/Wire/c1f_redcheck.py index 602bb93b..1535277f 100644 --- a/MobileGL/MG_Test/Wire/c1f_redcheck.py +++ b/MobileGL/MG_Test/Wire/c1f_redcheck.py @@ -69,7 +69,7 @@ def cases(): [SUITE+'BoundPackBufferOffsetReadRefusesByName'], 'RemoteClientTest'), ('codex12-repeat-skip', BACKEND, replace('if (draw != EGL_NO_SURFACE && ctx != EGL_NO_CONTEXT) {', 'if (false) {'), - [SUITE+'RepeatedMakeCurrentAdoptsRepublishedCapsWithoutAPumpOrPresent'], 'RemoteClientTest'), + [SUITE+'ADifferentTupleMakeCurrentIsAdoptedWithoutAPumpOrPresent'], 'RemoteClientTest'), ('BlobMissing-optional-to-required', WIRE, replace('record.Blob = StageOptional(session, blobBytes, blobByteCount);', 'record.Blob = StageRequired(session, "SetDynamicState", blobBytes, blobByteCount);'), @@ -122,13 +122,20 @@ def main(): binary = ROOT / 'build-split/MobileGL/MG_Test' / ('Pipe' if target == 'PipeCatalogueTest' else 'Wire') / target build = ['cmake', '--build', 'build-split', '-j', '24', '--target', target] run = [str(binary), '--gtest_filter='+':'.join(names)] + # ID-67: both replacement cases must execute green before/after this mutation. + # Suppressing client adoption only reddens the different-tuple case; an identical + # tuple correctly republishes nothing and must remain green under the perturbation. + green_names = names + if label == 'codex12-repeat-skip': + green_names = names + [SUITE+'AnIdenticalRepeatedMakeCurrentRepublishesNothing'] + green_run = [str(binary), '--gtest_filter='+':'.join(green_names)] print('\n=== '+label+' ===', flush=True) try: brc, out = command(build) if brc: raise RuntimeError('baseline build failed\n'+out) - rc, out = command(run) - if rc or any('[ OK ] '+name+' (' not in out for name in names): + rc, out = command(green_run) + if rc or any('[ OK ] '+name+' (' not in out for name in green_names): raise RuntimeError('baseline not green\n'+out) path.write_text(mutate(original.decode())) brc, out = command(build) @@ -146,12 +153,12 @@ def main(): finally: path.write_bytes(original) brc, out = command(build) - rc, out = command(run) if brc == 0 else (brc, out) - if rc or any('[ OK ] '+name+' (' not in out for name in names): + rc, out = command(green_run) if brc == 0 else (brc, out) + if rc or any('[ OK ] '+name+' (' not in out for name in green_names): print('RESTORE-FAIL\n'+out, flush=True) failures.append(label+' restore') else: - print('RESTORED GREEN: '+', '.join(names), flush=True) + print('RESTORED GREEN: '+', '.join(green_names), flush=True) print('FAILED_CONTROLS='+repr(failures), flush=True) return bool(failures) diff --git a/scripts/ci/census_junit.py b/scripts/ci/census_junit.py new file mode 100644 index 00000000..2518780d --- /dev/null +++ b/scripts/ci/census_junit.py @@ -0,0 +1,24 @@ +#!/usr/bin/env python3 +"""Summarize the indebted broad lane without turning debt into a hard gate.""" +import collections +import sys +import xml.etree.ElementTree as ET + +cases = ET.parse(sys.argv[1]).getroot().findall('.//testcase') +if not cases: + sys.exit('Census FAILED: no executed test records') +counts = collections.Counter() +for case in cases: + failure = case.find('failure') + if failure is not None: + reason = failure.get('message', '') + status = 'aborted' if 'aborted' in reason.lower() else 'failed' + elif case.find('skipped') is not None: + status = 'skipped' + else: + status = 'passed' + counts[status] += 1 +print('### Broad inproc census (ID-65: recorded debt, not a gate)') +print(f'CTest exit: {sys.argv[2]}; total: {len(cases)}') +print('\n| passed | skipped | aborted | failed |\n|---:|---:|---:|---:|') +print('| ' + ' | '.join(str(counts[k]) for k in ('passed', 'skipped', 'aborted', 'failed')) + ' |') diff --git a/scripts/ci/control_smoke_test.sh b/scripts/ci/control_smoke_test.sh index 89e27b80..324ae64d 100644 --- a/scripts/ci/control_smoke_test.sh +++ b/scripts/ci/control_smoke_test.sh @@ -56,10 +56,15 @@ run_split() { # $1 = STUB_MODE run_retrace() { # $1 = STUB_MODE cd "${WORK}" || return 127 mkdir -p "${WORK}/OpenRA" + local rc=0 env -i PATH="${STUB_DIR}:/usr/bin:/bin" STUB_MODE="$1" \ CTEST=ctest CONTROL_TMPDIR="${WORK}/tmp-$1" \ PULL_LIBRARY="${WORK}/pull.so" FROZEN_LIBRARY="${WORK}/frozen.so" \ - bash "${HERE}/retrace_pull_library_control.sh" OpenRA DirectGLES + bash "${HERE}/retrace_pull_library_control.sh" OpenRA DirectGLES || rc=$? + cmp -s "${WORK}/frozen.so" "${WORK}/split.so" || { + echo 'F6 FAILED: pull control did not restore the split library'; return 1; + } + return "${rc}" } run_drop_draw() { # $1 = STUB_MODE @@ -67,7 +72,7 @@ run_drop_draw() { # $1 = STUB_MODE mkdir -p "${WORK}/OpenRA" env -i PATH="${STUB_DIR}:/usr/bin:/bin" STUB_MODE="$1" \ CTEST=ctest CONTROL_TMPDIR="${WORK}/tmp-$1" \ - LIBRARY_LOG="${WORK}/tmp-$1/mobilegl.log" \ + FROZEN_LIBRARY="${WORK}/frozen.so" LIBRARY_LOG="${WORK}/tmp-$1/mobilegl.log" \ bash "${HERE}/retrace_drop_draw_control.sh" OpenRA DirectGLES } @@ -80,8 +85,8 @@ expect PASSED "the scenarios' own diagnostic" -- run_split evidenc expect FAILED "the knob leaves the selection green" -- run_split green # The arming counter's half of the finding: a baseline that is already red cannot arm anything. expect FAILED "the baseline is already red" -- run_split red-baseline -# The disarmed lane, which is a legitimate exit 0 while c1/s1/v1 are landing. -expect PASSED "every split entry skipped (lane not armed)" -- run_split all-skipped +# P5 is complete: losing the runtime implementation must no longer disarm the gate. +expect FAILED "every split entry skipped (implementation lost)" -- run_split all-skipped echo echo "=== the retrace lane's pull-library control (scripts/ci/retrace_pull_library_control.sh)" @@ -93,7 +98,9 @@ else echo "no cc available; the retrace half of this smoke test needs one" >&2 exit 1 fi -: > "${WORK}/frozen.so" +printf '%s\n' 'int MG_Remote_stub(void) { return 1; }' > "${WORK}/split.c" +cc -shared -fPIC -o "${WORK}/frozen.so" "${WORK}/split.c" || exit 1 +cp "${WORK}/frozen.so" "${WORK}/split.so" # THE FINDING, part (b): a regex matching no tests. --no-tests=error exits non-zero and the old # control read that as "the pull library turned it red". diff --git a/scripts/ci/junit_tally.py b/scripts/ci/junit_tally.py index e1a68dbe..f36ef87f 100755 --- a/scripts/ci/junit_tally.py +++ b/scripts/ci/junit_tally.py @@ -20,9 +20,11 @@ import sys import xml.etree.ElementTree as ET -def tally(path): +def tally(path, split_only=False): passed = failed = skipped = 0 for case in ET.parse(path).getroot().iter('testcase'): + if split_only and not case.get('name', '').startswith('DirectGLES.Split.'): + continue if case.find('failure') is not None or case.find('error') is not None: failed += 1 elif case.find('skipped') is not None or case.get('status') in ('notrun', 'disabled'): @@ -33,15 +35,18 @@ def tally(path): def main(): - if len(sys.argv) != 2: - print("usage: junit_tally.py ", file=sys.stderr) + if len(sys.argv) not in (2, 3) or (len(sys.argv) == 3 and sys.argv[2] != '--require-split-ran'): + print("usage: junit_tally.py [--require-split-ran]", file=sys.stderr) return 2 try: - passed, failed, skipped = tally(sys.argv[1]) + passed, failed, skipped = tally(sys.argv[1], split_only=len(sys.argv) == 3) except Exception as exc: # a malformed file is not "zero of everything" print(f"junit_tally: cannot parse {sys.argv[1]}: {exc}", file=sys.stderr) return 1 print(f"{passed} {failed} {skipped}") + if len(sys.argv) == 3 and (passed == 0 or failed): + print('split baseline FAILED: no successful split runtime entries or an already-red selection', file=sys.stderr) + return 1 return 0 diff --git a/scripts/ci/redcheck_control_smoke_test.sh b/scripts/ci/redcheck_control_smoke_test.sh index 043ccc8a..61e71489 100755 --- a/scripts/ci/redcheck_control_smoke_test.sh +++ b/scripts/ci/redcheck_control_smoke_test.sh @@ -55,7 +55,7 @@ split, retrace, dropdraw = sys.argv[1], sys.argv[2], sys.argv[3] # other two catching the smoke test's "unrelated failure" case, and the case never flips - which # is what this script measured the first time the perturbation actually applied. rules = [ - (split, [('LINE', 'grep -qE "${evidence}"', ('if ! ', 'elif ! ')), + (split, [('LINE', '"${log_helper}" assertion', ('if ! ', 'elif ! ')), ('SUBST', '"[A-Za-z_][A-Za-z_0-9]*"\\}\' || exit 1', '"[A-Za-z_][A-Za-z_0-9]*"\\}\' || true'), ('SUBST', '"${private_evidence}" "${name}" || exit 1', '"${private_evidence}" "${name}" || true')]), diff --git a/scripts/ci/retrace_drop_draw_control.sh b/scripts/ci/retrace_drop_draw_control.sh index c7597c0c..5198b1f6 100644 --- a/scripts/ci/retrace_drop_draw_control.sh +++ b/scripts/ci/retrace_drop_draw_control.sh @@ -50,6 +50,14 @@ CTEST="${CTEST:-ctest}" CONTROL_TMPDIR="${CONTROL_TMPDIR:-${RUNNER_TEMP:-/tmp}}" LIBRARY_LOG="${LIBRARY_LOG:-${CASE}/${BACKEND}/output/mobilegl.log}" mkdir -p "${CONTROL_TMPDIR}" +FROZEN_LIBRARY="${FROZEN_LIBRARY:?FROZEN_LIBRARY must name the split library the replay loads}" +symbols=$(nm --defined-only "${FROZEN_LIBRARY}") || exit 1 +remote_count=$(printf '%s\n' "${symbols}" | grep -ic MG_Remote || true) +echo "draw-drop control library: ${FROZEN_LIBRARY}: MG_Remote=${remote_count}" +if [ "${remote_count}" -lt 1 ]; then + echo '::error::draw-drop control requires a split library: MG_Remote=0' + exit 1 +fi selector="^MobileGLTraceReplay\.${CASE}\.${BACKEND}$" @@ -66,6 +74,8 @@ restore_good_output() { echo "restored the verified run's output over the control's" fi } +trap restore_good_output EXIT +trap 'exit 130' INT TERM matched=$("${CTEST}" -N -R "${selector}" | grep -cE '^ *Test *#[0-9]+:') if [ "${matched}" -lt 1 ]; then diff --git a/scripts/ci/retrace_pull_library_control.sh b/scripts/ci/retrace_pull_library_control.sh index 4f45f47f..89ce9036 100644 --- a/scripts/ci/retrace_pull_library_control.sh +++ b/scripts/ci/retrace_pull_library_control.sh @@ -69,6 +69,18 @@ restore_good_output() { fi } +# Restore the exact split library on success, failure, and interruption. The following +# draw-drop control uses this same frozen path. +saved_library=$(mktemp "${CONTROL_TMPDIR}/split-library.XXXXXX") || exit 1 +cp -p "${FROZEN_LIBRARY}" "${saved_library}" || exit 1 +restore_control() { + cp -p "${saved_library}" "${FROZEN_LIBRARY}" + rm -f "${saved_library}" + restore_good_output +} +trap restore_control EXIT +trap 'exit 130' INT TERM + # HOLE 1: COUNT THE SELECTION FIRST. `--no-tests=error` turns an empty selection into a non-zero # exit, which is indistinguishable from a working control unless the selection is counted. matched=$("${CTEST}" -N -R "${selector}" | grep -cE '^ *Test *#[0-9]+:') @@ -82,8 +94,11 @@ fi # in. It defines no MG_Remote symbol, so ConfigLoader has no transport parser and # MOBILEGL_TRANSPORT=inproc is accepted and ignored - the exact shape of "the split lane ran # monolith". -cp "${PULL_LIBRARY}" "${FROZEN_LIBRARY}" -if nm --defined-only "${FROZEN_LIBRARY}" | grep -q -i MG_Remote; then +cp "${PULL_LIBRARY}" "${FROZEN_LIBRARY}" || exit 1 +symbols=$(nm --defined-only "${FROZEN_LIBRARY}") || exit 1 +remote_count=$(printf '%s\n' "${symbols}" | grep -ic MG_Remote || true) +echo "pull control library: ${FROZEN_LIBRARY}: MG_Remote=${remote_count}" +if [ "${remote_count}" -ne 0 ]; then restore_good_output echo "::error::the control's own library defines MG_Remote symbols, so it is not a pull build and this control would prove nothing" exit 1 diff --git a/scripts/ci/split_negative_controls.sh b/scripts/ci/split_negative_controls.sh index 95a10d3a..cb3ad70d 100644 --- a/scripts/ci/split_negative_controls.sh +++ b/scripts/ci/split_negative_controls.sh @@ -51,6 +51,7 @@ CONTROL_TMPDIR="${CONTROL_TMPDIR:-${RUNNER_TEMP:-/tmp}}" mkdir -p "${CONTROL_TMPDIR}" junit="${CONTROL_TMPDIR}/isplit.xml" +rm -f "${junit}" log_helper="$(dirname "$0")/../../MobileGL/MG_IntegrationTest/Harness/split_log_paths.py" # Check ownership even while the runtime lane is disarmed and will skip. @@ -87,14 +88,14 @@ echo "split entries - passed: ${baseline_passed}, failed: ${baseline_failed}, sk # A RED BASELINE DISARMS THE CONTROLS RATHER THAN ARMING THEM (review finding 8, second half). # `|| true` plus a "not skipped" counter used to treat a case that ran and FAILED as evidence the # lane was live. Turning an already-red entry red is not a measurement. -if [ "${baseline_failed}" -gt 0 ]; then +if [ "${baseline_failed}" -gt 0 ] || [ "${baseline_rc}" -ne 0 ]; then echo "::error::${baseline_failed} DirectGLES.Split. entries are ALREADY RED with both knobs at their defaults, so neither negative control below can attribute its red to the knob it turns. Fix the lane first; a control measured against a red baseline is not a control. (This used to be swallowed by an unconditional '|| true' and counted as 'the lane is armed'.)" exit 1 fi if [ "${baseline_passed}" -lt 1 ]; then - echo "::warning::every DirectGLES.Split. entry SKIPPED, so neither negative control can fire. The arming condition is a runtime fact - MG_Config::Transport, ClientSession::Active() and ImplementedVerbCount(), read by Harness/SplitRuntimePeek - and it becomes true on the commit that lands the last of c1/s1/v1. This step becomes a gate then, with no edit; it is not a green that asserted anything today." - exit 0 + echo "::error::split baseline FAILED: every DirectGLES.Split. entry SKIPPED; the split implementation did not execute" + exit 1 fi # ---- the controls --------------------------------------------------------------------------- @@ -145,7 +146,7 @@ run_control() { if [ "${evidence}" = "private-barrier-fatal" ]; then python3 "${log_helper}" evidence "${manifest}" "${filter}" \ 'Fatal\{BarrierViolation, "[A-Za-z_][A-Za-z_0-9]*"\}' || exit 1 - elif ! tr -s '[:space:]' ' ' < "${out}" | grep -qE "${evidence}"; then + elif ! python3 "${log_helper}" assertion "${manifest}" "${filter}" "${result}" "${evidence}"; then echo "::error::${name} FAILED: red lacks its persistent-map push diagnostic. Required: ${evidence}" exit 1 fi @@ -179,8 +180,10 @@ run_control "negative control E1 (MOBILEGL_IPC_VERB_BARRIER=0)" \ # * the LIBRARY's own line in the entry's private file, saying the push was disabled by this # knob. It did not exist until ID-65 assigned it (joint-v1.md 3), which is why this control # used to rest on the pixels alone. +# TheMapLandsInTheArmItsLaneDeclares skips by design outside PersistentMapArm. +# Select only the pixel cases; a pre-flight skip in either remains a hard failure. run_control "negative control E3(a) (MOBILEGL_IPC_PERSISTENT_BLOCK_KB=0)" \ - 'DirectGLES\.Split\.(SmallRing\.)?PersistentCoherentMapScenario' \ + 'DirectGLES\.Split\.(SmallRing\.)?PersistentCoherentMapScenario\.(TwoWritesThroughTheCoherentPointerEachReachTheirOwnDraw|AWriteAfterAFrameBoundaryReachesTheNextFramesDraw)$' \ "the SECOND write through the same mapping, announced by nothing|frame 1's write through the SAME mapping, after a Present" \ 'MGPipe: persistent-map push disabled - MOBILEGL_IPC_PERSISTENT_BLOCK_KB=0' \ MOBILEGL_IPC_PERSISTENT_BLOCK_KB=0 diff --git a/scripts/ci/testdata/split_private_log_smoke.sh b/scripts/ci/testdata/split_private_log_smoke.sh index f52c802b..86fa8983 100644 --- a/scripts/ci/testdata/split_private_log_smoke.sh +++ b/scripts/ci/testdata/split_private_log_smoke.sh @@ -7,7 +7,7 @@ trap 'rm -rf "${WORK}"' EXIT cp "${HERE}/testdata/stub_ctest.sh" "${WORK}/ctest" chmod +x "${WORK}/ctest" passes=0 -for mode in missing-fatal stdout-fatal stale-fatal evidence e3-unrelated e3-no-private skipped-selection notrun-selection missing-selection partial-fatal wrong-fatal; do +for mode in missing-fatal stdout-fatal stale-fatal evidence e3-unrelated e3-no-private skipped-selection e3-skipped-selection notrun-selection missing-selection partial-fatal wrong-fatal; do mkdir -p "${WORK}/${mode}" rc=0 STUB_MODE="${mode}" CTEST="${WORK}/ctest" CONTROL_TMPDIR="${WORK}/${mode}" \ @@ -25,6 +25,7 @@ for mode in missing-fatal stdout-fatal stale-fatal evidence e3-unrelated e3-no-p case "${mode}" in e3-no-private) message='no selected private log carries /MGPipe: persistent-map push disabled' ;; skipped-selection) message='SplitLogPaths FAILED: E1 control: the knob killed the pre-flight, not the entry - 1 selected entries skipped' ;; + e3-skipped-selection) message='SplitLogPaths FAILED: E3(a) control: the knob killed the pre-flight, not the entry - 1 selected entries skipped' ;; notrun-selection|missing-selection) message='SplitLogPaths FAILED: E1 control: 1 selected entries did not run' ;; esac if [[ "${mode}" = *-selection ]]; then diff --git a/scripts/ci/testdata/stub_ctest.sh b/scripts/ci/testdata/stub_ctest.sh index ae2739ed..d98efdc6 100755 --- a/scripts/ci/testdata/stub_ctest.sh +++ b/scripts/ci/testdata/stub_ctest.sh @@ -24,7 +24,7 @@ # never says the push was disabled (the half ID-65 added) # green baseline green; the control's own run PASSES (the knob is not load-bearing) # red-baseline the baseline itself has a failed entry -# all-skipped the baseline is entirely skipped (the disarmed lane, a legitimate exit 0) +# all-skipped the baseline is entirely skipped (lost implementation, a hard failure) # retrace-noselect `ctest -N` matches nothing; the run exits 8 the way --no-tests=error does # retrace-unrelated one match; the run fails without naming the transport # retrace-evidence one match; the run fails with run_trace_case.cmake's own sentence @@ -76,10 +76,20 @@ write_junit() { entry=DirectGLES.Split.ClearThenReadPixelsScenario.ClearWithNoDrawIsVisibleToDefaultFramebufferReadPixels [ "${MOBILEGL_IPC_PERSISTENT_BLOCK_KB:-64}" != 0 ] || entry=DirectGLES.Split.PersistentCoherentMapScenario.TwoWritesThroughTheCoherentPointerEachReachTheirOwnDraw body="" + if [ "${MOBILEGL_IPC_PERSISTENT_BLOCK_KB:-64}" = 0 ]; then + case "${mode}" in + evidence|e3-no-private) + body="the SECOND write through the same mapping, announced by nothing" ;; + esac + fi if [ "${mode}" = partial-fatal ] && [ "${MOBILEGL_IPC_VERB_BARRIER:-1}" = 0 ]; then body="${body}" fi case "${mode}" in + e3-skipped-selection) + if [ "${MOBILEGL_IPC_PERSISTENT_BLOCK_KB:-64}" = 0 ]; then + body="" + fi ;; skipped-selection) body="" ;; notrun-selection) body="" ;; missing-selection) body='' ;; @@ -130,7 +140,7 @@ fi # The control's own run. if [ "${MOBILEGL_IPC_VERB_BARRIER:-1}" = 0 ]; then case "${mode}" in - evidence|e3-unrelated|e3-no-private|skipped-selection|notrun-selection|missing-selection|partial-fatal) echo 'Fatal{BarrierViolation, "DrawVbo"}' > "${log}" ;; + evidence|e3-unrelated|e3-no-private|skipped-selection|e3-skipped-selection|notrun-selection|missing-selection|partial-fatal) echo 'Fatal{BarrierViolation, "DrawVbo"}' > "${log}" ;; wrong-fatal) echo 'Fatal{ReplyMissing, "DrawVbo"}' > "${log}" ;; missing-fatal) echo "library setup only; no fatal" > "${log}" ;; stdout-fatal) echo 'Fatal{BarrierViolation, "DrawVbo"}' ;; @@ -144,6 +154,13 @@ if [ "${MOBILEGL_IPC_PERSISTENT_BLOCK_KB:-64}" = 0 ] && [ "${mode}" = evidence ] > "${CONTROL_TMPDIR}/pmap.log" fi case "${mode}" in + e3-skipped-selection) + if [ "${MOBILEGL_IPC_PERSISTENT_BLOCK_KB:-64}" = 0 ]; then + echo 'selected E3 entry ... ***Skipped' + exit 0 + fi + exit 8 + ;; skipped-selection|notrun-selection|missing-selection) echo '1/1 Test #1: selected entry ... ***Skipped' echo '100% tests passed, 0 tests failed out of 1' diff --git a/scripts/p3a_untouched_regions.sh b/scripts/p3a_untouched_regions.sh index 1f97bed7..98289664 100644 --- a/scripts/p3a_untouched_regions.sh +++ b/scripts/p3a_untouched_regions.sh @@ -532,7 +532,8 @@ if [ "${1:-}" = "--self-test" ]; then printf '%s %s\n' \ "0000000000000000000000000000000000000000000000000000000000000000" "$target" \ >> "$WORK_DIR/pinprec.sha" - apply_pinned_shas "$WORK_DIR/pinprec.sha" || exit 2 + # Drive production extraction on a real historical ref whose body differs from the pin. + extract_baseline ff2994d9 pinprec || exit 2 got=$(awk -v n="$target" '$2 == n { print $1 }' "$WORK_DIR/pinprec.sha") if [ "$got" != "$pinned" ]; then say "PIN CONTROL FAILED: a baseline that carried a DIFFERENT sha for $target came out as" @@ -552,7 +553,7 @@ if [ "${1:-}" = "--self-test" ]; then python3 "$PY" extract "$WORK_DIR/pinperturbed.cpp" "$ALL_FUNCTIONS" \ > "$WORK_DIR/pinperturbed.sha" || exit 2 cp -f "$WORK_DIR/pristine.sha" "$WORK_DIR/pinbase.sha" || exit 2 - apply_pinned_shas "$WORK_DIR/pinbase.sha" || exit 2 + extract_baseline HEAD pinbase || exit 2 if compare_lists "$WORK_DIR/pinbase.sha" "$WORK_DIR/pinperturbed.sha" \ "PIN($PINNED_BASELINE_REF)" "one-token-perturbed" 2> "$WORK_DIR/pinperturbed.err"; then say "NEGATIVE CONTROL DID NOT TRIP: one token was inserted into $target's body and the" diff --git a/scripts/p4a_untouched_regions.sh b/scripts/p4a_untouched_regions.sh index fa23391a..928047e6 100644 --- a/scripts/p4a_untouched_regions.sh +++ b/scripts/p4a_untouched_regions.sh @@ -766,7 +766,8 @@ if [ "${1:-}" = "--self-test" ]; then printf '%s %s\n' \ "0000000000000000000000000000000000000000000000000000000000000000" "$target" \ >> "$WORK_DIR/pinprec.sha" - apply_pinned_shas "$WORK_DIR/pinprec.sha" || exit 2 + # Drive production extraction on a real historical ref whose body differs from the pin. + extract_baseline 37da3c3a pinprec || exit 2 got=$(awk -v n="$target" '$2 == n { print $1 }' "$WORK_DIR/pinprec.sha") if [ "$got" != "$pinned" ]; then say "PIN CONTROL FAILED: a baseline that carried a DIFFERENT sha for $target came out as" @@ -789,7 +790,7 @@ if [ "${1:-}" = "--self-test" ]; then "$WORK_DIR/pinperturbed/$(blob_name "$targetSource")" token || exit 2 python3 "$PY" extract "$WORK_DIR/pinperturbed.spec" > "$WORK_DIR/pinperturbed.sha" || exit 2 cp -f "$WORK_DIR/pristine.sha" "$WORK_DIR/pinbase.sha" || exit 2 - apply_pinned_shas "$WORK_DIR/pinbase.sha" || exit 2 + extract_baseline HEAD pinbase || exit 2 if compare_lists "$WORK_DIR/pinbase.sha" "$WORK_DIR/pinperturbed.sha" \ "PIN($PINNED_BASELINE_REF)" "one-token-perturbed" 2> "$WORK_DIR/pinperturbed.err"; then say "NEGATIVE CONTROL DID NOT TRIP: one token was inserted into $target's body and the" From d50183cb1a9855d9d03b590676cd41dc8d17a005 Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Wed, 16 Sep 2026 12:34:33 -0400 Subject: [PATCH 9/9] [Fix] (IntegrationTest): assert actual staging backpressure in the small ring lane --- .../Harness/SplitRuntimePeek.cpp | 11 ++++++ .../Harness/SplitRuntimePeek.h | 7 ++-- .../Harness/WireLedgerChecks.h | 3 ++ .../Scenarios/TriangleScenario.cpp | 15 +++++++ MobileGL/MG_Remote/Client/ClientSession.cpp | 1 + MobileGL/MG_Remote/Server/ServerLoop.cpp | 1 + MobileGL/MG_Remote/Server/ServerLoop.h | 6 +++ MobileGL/MG_Remote/Wire/PipeWireCodec.cpp | 39 ++++++++++++------- MobileGL/MG_Remote/Wire/PipeWireCodec.h | 17 ++++---- MobileGL/MG_Test/Wire/PipeWireCodecTest.cpp | 14 +++++++ 10 files changed, 88 insertions(+), 26 deletions(-) diff --git a/MobileGL/MG_IntegrationTest/Harness/SplitRuntimePeek.cpp b/MobileGL/MG_IntegrationTest/Harness/SplitRuntimePeek.cpp index 7cb8c851..81a3b992 100644 --- a/MobileGL/MG_IntegrationTest/Harness/SplitRuntimePeek.cpp +++ b/MobileGL/MG_IntegrationTest/Harness/SplitRuntimePeek.cpp @@ -19,10 +19,21 @@ #include #include +#include +#include +#include #define MGITEST_SPLIT_RUNTIME_PEEK_LIVE 1 #endif namespace MGITest { + void DelaySplitRetirementForTesting(bool enabled) { +#if defined(MGITEST_SPLIT_RUNTIME_PEEK_LIVE) + MobileGL::MG_Remote::Server::ServerLoopInstance().SetBeforeRetireHookForTesting( + enabled ? +[] { std::this_thread::sleep_for(std::chrono::milliseconds(30)); } : nullptr); +#else + (void)enabled; +#endif + } SplitRuntimeState PeekSplitRuntime() { SplitRuntimeState state; diff --git a/MobileGL/MG_IntegrationTest/Harness/SplitRuntimePeek.h b/MobileGL/MG_IntegrationTest/Harness/SplitRuntimePeek.h index 9451f78e..f4e7ad88 100644 --- a/MobileGL/MG_IntegrationTest/Harness/SplitRuntimePeek.h +++ b/MobileGL/MG_IntegrationTest/Harness/SplitRuntimePeek.h @@ -91,9 +91,8 @@ namespace MGITest { // than asserted: a uniform record stride over a power-of-two ring lands on the // boundary exactly and never straddles it. // - // stageReclaimWaits: SEG_STAGE allocations that only fitted after the encoder reclaimed - // what the server had retired - P5's one real producer wait (see PipeWireCodec.h for - // why the command ring has none while the verb barrier is armed). + // stageReclaimWaits: allocations blocked after immediate reclamation failed; + // already-retired bytes reclaimed lazily do not count as a wait. unsigned long long maxRecordBytes = 0; unsigned long long maxRecordBytesCap = 0; unsigned long long cmdWraps = 0; @@ -103,6 +102,8 @@ namespace MGITest { }; SplitRuntimeState PeekSplitRuntime(); + // Scheduling-only perturbation; never changes a watermark or counter. + void DelaySplitRetirementForTesting(bool enabled); // Empty when this process is a real split run that can be asserted about; otherwise the // reason to GTEST_SKIP() with, naming the first fact that is not true and the package that diff --git a/MobileGL/MG_IntegrationTest/Harness/WireLedgerChecks.h b/MobileGL/MG_IntegrationTest/Harness/WireLedgerChecks.h index 1c5eb512..b14d11af 100644 --- a/MobileGL/MG_IntegrationTest/Harness/WireLedgerChecks.h +++ b/MobileGL/MG_IntegrationTest/Harness/WireLedgerChecks.h @@ -134,6 +134,9 @@ namespace MGITest::WireLedger { "kSmallRingLaneCmdByteTarget, which is sized for the 1 MiB ring this lane " "declares (MGL_ITEST_GLES_SPLIT_SMALL_RING_ENVIRONMENT); a larger ring needs a " "larger workload and is not what this lane is for"; + EXPECT_GE(state.stageReclaimWaits, 1u) + << where << ": exit gate E3(e) - producer NEVER WAITED for staging retirement; " + "lazy reclamation of already-retired bytes is not back-pressure"; ::testing::Test::RecordProperty("ring_wraps", static_cast(state.cmdWraps)); ::testing::Test::RecordProperty("ring_wrap_pads", static_cast(state.cmdWrapPads)); ::testing::Test::RecordProperty("ring_waits", static_cast(state.stageReclaimWaits)); diff --git a/MobileGL/MG_IntegrationTest/Scenarios/TriangleScenario.cpp b/MobileGL/MG_IntegrationTest/Scenarios/TriangleScenario.cpp index 520615c0..490822b4 100644 --- a/MobileGL/MG_IntegrationTest/Scenarios/TriangleScenario.cpp +++ b/MobileGL/MG_IntegrationTest/Scenarios/TriangleScenario.cpp @@ -264,6 +264,21 @@ void main() { oColor = vec4(vColor, 1.0); } // the lane is that IT is the arm with a ring the workload can fill. if (SplitLane::IsSmallRingLane()) { const unsigned long long driven = DriveUntilSmallRingOverruns(); + // Ordinary uploads, each fitting by itself, jointly exceed the lane's 1 MiB + // staging segment. Delay only scheduling between applied and retired: the + // production allocator, not this test, must observe capacity and wait. + GLuint pressureBuffer = 0; + glGenBuffers(1, &pressureBuffer); + glBindBuffer(GL_COPY_WRITE_BUFFER, pressureBuffer); + std::vector upload(768 * 1024, 0x5a); + glBufferData(GL_COPY_WRITE_BUFFER, upload.size(), nullptr, GL_DYNAMIC_DRAW); + DelaySplitRetirementForTesting(true); + glBufferSubData(GL_COPY_WRITE_BUFFER, 0, upload.size(), upload.data()); + upload[0] = 0xa5; + glBufferSubData(GL_COPY_WRITE_BUFFER, 0, upload.size(), upload.data()); + DelaySplitRetirementForTesting(false); + glBindBuffer(GL_COPY_WRITE_BUFFER, 0); + glDeleteBuffers(1, &pressureBuffer); Gl().EndFrame(); WireLedger::ExpectSmallRingWrappedAtLeastOnce( "TriangleScenario.TheSameVboAndVaoRedrawAcrossAFrameBoundary", driven); diff --git a/MobileGL/MG_Remote/Client/ClientSession.cpp b/MobileGL/MG_Remote/Client/ClientSession.cpp index bc0be7d4..72c478fb 100644 --- a/MobileGL/MG_Remote/Client/ClientSession.cpp +++ b/MobileGL/MG_Remote/Client/ClientSession.cpp @@ -516,6 +516,7 @@ namespace MobileGL::MG_Remote::Client { // a live RingProducer over a cursor triple nobody consumes would be the // half-wired shape this session exists not to have. m_encoder = Wire::PipeWireEncoder(control, &m_cmd, nullptr, &m_segments); + m_encoder.SetStageRetirementDoorbell(m_producer.SelfDoorbell()); // ---- 7. the first CapsSnapshot, if the server had a backend to publish one from. // ONE DRAIN, ONE ADOPTER (c1): PumpControlPlane below is the only thing in the client diff --git a/MobileGL/MG_Remote/Server/ServerLoop.cpp b/MobileGL/MG_Remote/Server/ServerLoop.cpp index 6047769f..3ac42680 100644 --- a/MobileGL/MG_Remote/Server/ServerLoop.cpp +++ b/MobileGL/MG_Remote/Server/ServerLoop.cpp @@ -530,6 +530,7 @@ namespace MobileGL::MG_Remote::Server { // a loop that applies and never retires ends the first MOBILEGL_IPC_STAGE_MB of // staging in Fatal{RingOverrun, "SEG_STAGE"} (w1-v1 5). Once per drain batch, not // once per record: retiring LATE is always legal, retiring EARLY never is. + if (const auto hook = m_beforeRetireHook.load(std::memory_order_acquire)) hook(); consumer.RetireThrough(consumer.AppliedSeq()); // LEAVING THE APPLIER (p1's M-5) IS *NOT* DONE HERE. It is done inside // PipeApplier::ApplyOne, before s1's SessionConsumer::ApplyOne publishes appliedSeq diff --git a/MobileGL/MG_Remote/Server/ServerLoop.h b/MobileGL/MG_Remote/Server/ServerLoop.h index 628d9ce8..4681ec93 100644 --- a/MobileGL/MG_Remote/Server/ServerLoop.h +++ b/MobileGL/MG_Remote/Server/ServerLoop.h @@ -146,6 +146,11 @@ namespace MobileGL::MG_Remote::Server { // fail for its own reason. Uint64 DrainedRecords() const; Uint64 ParkCount() const; + // Scheduling perturbation only: the hook runs after application, before retirement. + // Integration tests use it to observe real producer back-pressure from GL uploads. + void SetBeforeRetireHookForTesting(void (*hook)()) { + m_beforeRetireHook.store(hook, std::memory_order_release); + } // C7 / ID-54 diagnostics, read by ServerLoopTest's C7 and N-3 controls. NativeBindCount is // how many times ApplyMakeCurrent FORWARDED a bind to the backend (a tuple it did not @@ -230,6 +235,7 @@ namespace MobileGL::MG_Remote::Server { Uint64 m_affinityMask = 0; std::atomic m_drained{0}; std::atomic m_parks{0}; + std::atomic m_beforeRetireHook{nullptr}; // C7 / ID-54: the (dpy, draw, read, ctx) currently bound on the apply thread. Written and // read ONLY on the apply thread inside ApplyMakeCurrent, so it needs no lock; the two diff --git a/MobileGL/MG_Remote/Wire/PipeWireCodec.cpp b/MobileGL/MG_Remote/Wire/PipeWireCodec.cpp index 3b940914..20974dd8 100644 --- a/MobileGL/MG_Remote/Wire/PipeWireCodec.cpp +++ b/MobileGL/MG_Remote/Wire/PipeWireCodec.cpp @@ -776,10 +776,12 @@ namespace MobileGL::MG_Remote::Wire { std::abort(); } - for (int attempt = 0; attempt < 2; ++attempt) { + bool reclaimed = false; + bool waited = false; + for (;;) { // THE WRAP SKIP MAY ONLY BE CHARGED AGAINST BYTES THAT ARE STILL IN FLIGHT. When // there are none the allocator starts over at offset zero, so a blob the segment - // can hold whole is never refused (see RebaseEmptyStage). On attempt 1 this runs + // can hold whole is never refused (see RebaseEmptyStage). On retry this runs // AFTER ReclaimStagedBytes, which is the case the finding describes: 8 MiB // allocated, then retired, then a 28 MiB request that used to abort. RebaseEmptyStage(); @@ -793,21 +795,28 @@ namespace MobileGL::MG_Remote::Wire { m_stageHead += skip + need; return m_stageBase + at; } - if (attempt == 0) { - // One try at reclaiming what the server has already retired. A second failure - // means the bytes genuinely do not fit, which R-10 says P5 does not chunk and - // must instead prove it never needs to. - // - // AND THIS IS P5'S ONE REAL BACK-PRESSURE EVENT, so it is counted here and - // published as `ringwaits=`. Reaching this line means the producer could not - // place a blob until the CONSUMER had retired earlier ones - the producer's - // progress depended on retiredSeq, which is exactly what R-9's "batching may - // only delay a watermark" is about. Exit gate E3(e)'s small-ring lane exists - // to make it happen at least once; a lane that never reaches it has a ring - // that is small only in its environment block. - ++m_stageReclaimWaits; + if (!reclaimed) { + // Lazy reclamation of already-retired bytes is NOT a producer wait. ReclaimStagedBytes(); + reclaimed = true; + continue; } + if (m_stageRetirementBell == nullptr || m_stageMarkFront == m_stageMarks.size()) break; + const Uint64 pending = m_stageMarks[m_stageMarkFront].Seq; + const auto ready = [&] { + return m_control->retiredSeq.load(std::memory_order_acquire) >= pending; + }; + if (!ready()) { + // The allocation still cannot progress after reclamation. Count this + // blocked allocation once, not each watermark poll or each reclaimed mark. + if (!waited) { ++m_stageReclaimWaits; waited = true; } + if (!m_stageRetirementBell->Wait(m_control->producerParked, ready, 0, 5000)) { + MGLOG_F("MGPipe: Fatal{RetirementWaitFailed, \"SEG_STAGE\"} producer wait " + "ended before the pending allocation retired (shutdown or timeout)"); + std::abort(); + } + } + ReclaimStagedBytes(); } MGLOG_F("MGPipe: Fatal{RingOverrun, \"SEG_STAGE\"} a %llu byte blob does not fit a %llu " "byte staging segment with %llu bytes still in flight (retiredSeq=%llu); P5 " diff --git a/MobileGL/MG_Remote/Wire/PipeWireCodec.h b/MobileGL/MG_Remote/Wire/PipeWireCodec.h index c5005c7c..5cb7b3c8 100644 --- a/MobileGL/MG_Remote/Wire/PipeWireCodec.h +++ b/MobileGL/MG_Remote/Wire/PipeWireCodec.h @@ -44,6 +44,7 @@ #include #include "../Transport/Ring.h" +#include "../Transport/Doorbell.h" namespace MobileGL::MG_Remote::Wire { @@ -311,17 +312,16 @@ namespace MobileGL::MG_Remote::Wire { // this number would have been red for the arithmetic of the record catalogue rather // than for anything about the ring. // - // `StageReclaimWaits()` counts every SEG_STAGE allocation that did not fit until the - // encoder reclaimed the runs the server had already retired - i.e. every time the - // producer's progress depended on the consumer's retiredSeq. That is the honest - // back-pressure reading in P5, and the reason the command ring has none: the verb - // barrier makes EmitAndWait wait for appliedSeq after EVERY record (R-1), so at most - // one record is ever in flight on SEG_CMD and a full command ring is not a wait but a - // Fatal{RingOverrun} (ClientSession.cpp). Publishing a "command ring waits" counter - // that can only ever be zero-or-dead is the decoration this file's counters are not. + // `StageReclaimWaits()` counts allocations blocked on an outstanding retiredSeq + // after immediate reclamation still left insufficient space. Reclaiming bytes + // the consumer had already retired does not increment it. One allocation counts + // once even if it waits for several marks; this is staging, not command-ring pressure. Uint64 CmdWraps() const; Uint64 CmdWrapPads() const; Uint64 StageReclaimWaits() const; + // The live session supplies its shutdown-aware producer doorbell. Standalone codecs + // without a consumer cannot wait for retirement and retain the named refusal. + void SetStageRetirementDoorbell(Transport::Doorbell* bell) { m_stageRetirementBell = bell; } // Bytes this encoder has ever written into SEG_CMD, pad fillers included: the // producer's monotonic head cursor. It is the DENOMINATOR the wrap count only means @@ -365,6 +365,7 @@ namespace MobileGL::MG_Remote::Wire { Uint64 m_cmdWraps = 0; Uint64 m_cmdWrapPads = 0; Uint64 m_stageReclaimWaits = 0; + Transport::Doorbell* m_stageRetirementBell = nullptr; Vector m_stageMarks; SizeT m_stageMarkFront = 0; Uint8* m_stageBase = nullptr; diff --git a/MobileGL/MG_Test/Wire/PipeWireCodecTest.cpp b/MobileGL/MG_Test/Wire/PipeWireCodecTest.cpp index 3bc60de6..c1d212ad 100644 --- a/MobileGL/MG_Test/Wire/PipeWireCodecTest.cpp +++ b/MobileGL/MG_Test/Wire/PipeWireCodecTest.cpp @@ -1595,6 +1595,20 @@ TEST_F(PipeWireCodecTest, ABigProgramArchiveDoesNotGrowItsRecordAtAll) { EXPECT_GE(wire.Encoder().StagedBytesInFlight(), archive.size()); } +TEST_F(PipeWireCodecTest, AlreadyRetiredStagingReclamationIsNotAProducerWait) { + Wire2 wire; + std::vector payload(Wire2::kStageBytes * 3 / 4, 0x5a); + wire.Encoder().StageBytes(payload.data(), payload.size()); + MGPBindRenderState bind{}; + ASSERT_NE(wire.Encoder().EncodeRecord(MGPWireOp::BindRenderState, &bind, sizeof(bind)), kInvalidSeq); + bool applied = false; + ASSERT_TRUE(wire.PumpOne(&applied)); + // Do not explicitly reclaim: the second real allocation must do that itself. + wire.Encoder().StageBytes(payload.data(), payload.size()); + EXPECT_EQ(wire.Encoder().StageReclaimWaits(), 0u) + << "already-retired lazy reclamation is not a producer wait"; +} + TEST_F(PipeWireCodecTest, StagedBytesAreReclaimedOnlyBehindRetiredSeq) { Wire2 wire; const std::uint8_t payload[64] = {};