[Test] (MG_Test): drive the live-host-writes probe through the production chain instead of writing the record by hand, and pin both map edges and the disabled-push control by serial arithmetic

MobileGL/MG_Test/Buffer/SplitBufferTest.cpp

@@ -292,14 +292,47 @@ TEST_F(SplitBufferSet, TheLastBlockIsTheRemainderAndNotAWholeBlock) {
     buffer->ReleaseMemory(false);
 }
 
-// E3(a)'s NEGATIVE CONTROL: 0 disables the push, it does not mean "one unlimited block".
+// THE STATE RECORD ON BOTH EDGES (B-1). The rising edge is what breaks the cycle the probe
+// would otherwise latch: a coherent persistent map behind a static VAO emits NO content record
+// of its own until the push runs, the push runs inside the ensure path, and a draw-clean answer
+// skips that ensure and then latches it. So the map itself has to publish, and the unmap has to
+// publish the retraction - one block each, observable here as exactly one serial bump each.
+TEST_F(SplitBufferSet, BothEdgesOfAWriteMapPublishOneStateRecord) {
+    constexpr SizeT kSize = 4096;
+    auto buffer = MakeBuffer(26u, kSize);
+    EXPECT_FALSE(buffer->HasLiveHostWritesForWire());
+
+    const Uint64 beforeMap = buffer->GetChangeSerial();
+    buffer->AcquireMemoryRange(Range1D{0, kSize},
+                               BufferMappingAccessBit::Write | BufferMappingAccessBit::Persistent);
+    EXPECT_TRUE(buffer->HasLiveHostWritesForWire());
+    EXPECT_EQ(buffer->GetChangeSerial(), beforeMap + 1)
+        << "the rising edge published nothing, so the server cannot know a host writer is live "
+           "until a content record it may never emit";
+
+    const Uint64 beforeUnmap = buffer->GetChangeSerial();
+    buffer->ReleaseMemory(/*landStagedWrites=*/true);
+    EXPECT_FALSE(buffer->HasLiveHostWritesForWire());
+    EXPECT_EQ(buffer->GetChangeSerial(), beforeUnmap + 2)
+        << "the unmap's own NotifyFlushMappedRange plus the falling-edge state record: without "
+           "the second the record stays dirty for the buffer's life";
+}
+
+// E3(a)'s NEGATIVE CONTROL: 0 disables the push, it does not mean "one unlimited block" - and
+// it disables the STATE RECORDS with it. An earlier cut skipped the blocks and still shipped a
+// whole buffer at unmap, so a scenario that unmapped before reading back went green and the
+// control was dead.
 TEST_F(SplitBufferSet, AZeroBlockSizeTurnsThePushOffRatherThanMakingItUnlimited) {
     MG_Config::Ipc.PersistentBlockKb = 0;
     constexpr SizeT kSize = 128u * 1024u;
     auto buffer = MakeBuffer(23u, kSize);
+
+    const Uint64 beforeMap = buffer->GetChangeSerial();
     buffer->AcquireMemoryRange(Range1D{0, kSize},
                                BufferMappingAccessBit::Write | BufferMappingAccessBit::Persistent);
     ASSERT_TRUE(PersistentMapTracker::IsLivePersistentMap(*buffer));
+    EXPECT_EQ(buffer->GetChangeSerial(), beforeMap)
+        << "the rising-edge state record went out with the push disabled";
 
     const Uint64 before = MG_Util::PipeStats::TotalBytes(MG_Util::PipeStats::ByteClass::PersistentMapPush);
     MG_Remote::Client::PushPersistentMapsBeforeVerb();
@@ -307,7 +340,13 @@ TEST_F(SplitBufferSet, AZeroBlockSizeTurnsThePushOffRatherThanMakingItUnlimited)
     EXPECT_EQ(MG_Util::PipeStats::TotalBytes(MG_Util::PipeStats::ByteClass::PersistentMapPush), before)
         << "MOBILEGL_IPC_PERSISTENT_BLOCK_KB=0 must ship nothing, so that "
            "PersistentCoherentMapScenario goes red under it";
-    buffer->ReleaseMemory(false);
+
+    const Uint64 beforeUnmap = buffer->GetChangeSerial();
+    buffer->ReleaseMemory(/*landStagedWrites=*/true);
+    EXPECT_EQ(buffer->GetChangeSerial(), beforeUnmap + 1)
+        << "with the push off the unmap must emit only its own NotifyFlushMappedRange - a "
+           "falling-edge record here delivers a whole buffer the control exists to withhold, "
+           "and a negative control that still delivers the bytes is not a control";
 }
 
 // The set does not keep a pointer to a dead buffer.
@@ -380,6 +419,7 @@ TEST(SplitBufferSet, TheWholeSetIsInertOnTheMonolithPath) { MGL_SPLIT_ONLY_OR_SK
 TEST(SplitBufferSet, MembershipIsSyncPersistentMappedRangesOwnEarlyOutChain) { MGL_SPLIT_ONLY_OR_SKIP(); }
 TEST(SplitBufferSet, ThePushCutsTheMappedSpanIntoBlocksAndMovesPmap) { MGL_SPLIT_ONLY_OR_SKIP(); }
 TEST(SplitBufferSet, TheLastBlockIsTheRemainderAndNotAWholeBlock) { MGL_SPLIT_ONLY_OR_SKIP(); }
+TEST(SplitBufferSet, BothEdgesOfAWriteMapPublishOneStateRecord) { MGL_SPLIT_ONLY_OR_SKIP(); }
 TEST(SplitBufferSet, AZeroBlockSizeTurnsThePushOffRatherThanMakingItUnlimited) { MGL_SPLIT_ONLY_OR_SKIP(); }
 TEST(SplitBufferSet, ADestroyedBufferLeavesTheSet) { MGL_SPLIT_ONLY_OR_SKIP(); }
 TEST(SplitBufferSet, UnderSplitTheWritebackClearsThePendingFlagAndNotTheRequest) { MGL_SPLIT_ONLY_OR_SKIP(); }

MobileGL/MG_Test/SanityTest.cpp

@@ -26,6 +26,11 @@
 #include <MG_State/GLState/FramebufferState/FramebufferObject.h>
 #include <MG_State/GLState/TextureState/TextureState.h>
 #include <MG_Test/ScopedPipeVerb.h>
+#if MOBILEGL_PIPE_PUSH
+// P5 b1: MGPipeResourceTrackerInstance(), so the split probe case can ask the PRODUCTION
+// tracker for a buffer's handle instead of minting one by hand.
+#include <MG_Impl/Pipe/ResourceTracker.h>
+#endif
 #include <MG_Backend/DirectVulkan/Renderer/ProgramFactory.h>
 #include <MG_Backend/DirectVulkan/Renderer/UniformManager.h>
 #include <MG_Backend/DirectVulkan/Renderer/VkRenderPassManager.h>
@@ -4655,15 +4660,18 @@ TEST(DirectGLESBufferDrawProbe, ALiveHostMapKeepsTheHandleArmProbeDirtyBetweenTw
 // P5 b1's half of the case above: THE PIN IS LIFTED, AND THIS IS WHAT REPLACED IT.
 //
 // The case above exists because answering the live-map question from HasLiveHostWrites alone
-// read draw-CLEAN forever. P5 gives that field a producer - MGPSubData::HasLiveHostWrites, set
+// read draw-CLEAN forever. P5 gives that field a producer and retires the last frontend read
-// by MGPipeEmitResourceSubData from BufferObject::HasLiveHostWritesForWire - and with the
+// in IsBufferDrawCleanByHandle under split, because under a spawn there is no frontend object
-// producer in place the last frontend read in IsBufferDrawCleanByHandle retires under split,
+// on that side to ask.
-// because under a spawn there is no frontend object on that side to ask.
 //
-// So the same fixture is driven twice with the transport flipped, and the second half is the
+// EVERYTHING THE CASE OBSERVES IS WRITTEN BY THE PRODUCER, NOT BY THE CASE. A first cut of
-// one that would have been impossible before: the record ALONE has to be able to say both
+// this test set `record.HasLiveHostWrites = true` by hand and therefore passed with the
-// answers. If it could not, this case reads CLEAN in both arms and the regression the case
+// producer deleted - a test that constructs the state it is supposed to be observing cannot
-// above records comes back through the other door.
+// fail for the reason it exists. So the resource subsystem is armed with an empty ops table
+// (MG_Test/Pipe's PushArm shape), the map and the unmap are made through the ordinary
+// frontend entry points, and the record is only ever READ. Delete
+// BufferObject::NotePersistentMapStateChanged's emission, or PipeFill.cpp's
+// `record.HasLiveHostWrites = ...`, and this goes red.
 TEST(DirectGLESBufferDrawProbe, UnderSplitTheRecordAloneAnswersTheLiveHostMapQuestion) {
     using namespace MobileGL;
     using namespace MobileGL::MG_Backend::DirectGLES;
@@ -4676,65 +4684,99 @@ TEST(DirectGLESBufferDrawProbe, UnderSplitTheRecordAloneAnswersTheLiveHostMapQue
     GTEST_SKIP() << "MG_Config::Transport is a constexpr Monolith without the transport built in, "
                     "so the split arm of this probe cannot be entered";
 #else
+    // The subsystem, armed the way MG_Test/Pipe arms it: an EMPTY op table is enough, because
+    // MGPipeResourceSubsystemEnabled() only asks whether one is registered, and every hook this
+    // case reaches is optional.
+    const Uint64 previousPush = MG_Config::Features.PipePush;
+    const auto previousTransport = MG_Config::Transport;
+    const Uint32 previousBlockKb = MG_Config::Ipc.PersistentBlockKb;
+    MG_Pipe::MGPipeResourceOps ops{};
+    MG_Config::Features.PipePush |= MG_Pipe::kMGPipeSubsystemResources;
+    MG_Pipe::MGPipeSetResourceOps(&ops);
+    MG_Config::Transport = MG_Config::TransportMode::InProcess;
+    MG_Config::Ipc.PersistentBlockKb = 64;
+
+    {
+        // The constructor mints the handle and emits resource_create; Respecify emits the
+        // descriptor. Both through the production path.
         auto owner = MakeShared<BufferObject>(0u);
         owner->Respecify(256, nullptr);
-    const MG_Pipe::MGPipeHandle res =
-        MG_Pipe::MGPipeSlots().Acquire(MG_Pipe::MGPipeKind::Buffer, owner->GetLifetimeId());
-    ASSERT_FALSE(MG_Pipe::MGPipeHandleIsNull(res));
 
+        const MG_Pipe::MGPipeHandle res = MG_Pipe::MGPipeResourceTrackerInstance().Find(*owner);
+        ASSERT_FALSE(MG_Pipe::MGPipeHandleIsNull(res));
         auto& applier = MG_Pipe::MGPipeApplier();
-    if (applier.Resources.size() <= static_cast<SizeT>(res.Slot)) {
+        ASSERT_GT(applier.Resources.size(), static_cast<SizeT>(res.Slot));
-        applier.Resources.resize(static_cast<SizeT>(res.Slot) + 1);
-    }
         auto& record = applier.Resources[res.Slot];
-    record = {};
+        ASSERT_TRUE(record.Live) << "resource_create did not reach the applier";
-    record.Gen = res.Gen;
+        ASSERT_EQ(record.Desc.Width, 256u) << "resource_respecify did not reach the applier";
-    record.Live = true;
+        EXPECT_FALSE(record.HasLiveHostWrites) << "nothing maps this buffer yet";
-    record.Desc.Width = 256;
-    record.Serial = 7;
-    record.HasLiveHostWrites = false;
 
         auto& twin = BufferImpl::g_backendBufferResources.GetOrCreate(res);
         twin = MakeShared<BufferImpl::GLESBufferResource>();
         auto* const resource = twin.get();
-    resource->id = 1;
+        resource->id = 1; // a name, never used: this probe issues no GL
         resource->contextGeneration = BufferImpl::CurrentBufferContextGeneration();
         resource->storageInitialized = true;
         resource->storageSize = 256;
+        const auto stampSynced = [&]() {
             resource->syncedChangeSerial = record.Serial;
+            const std::lock_guard<std::mutex> lock(resource->pendingMutex);
+            resource->pendingRanges.clear();
+            resource->pendingResidentWrites.clear();
+        };
+        stampSynced();
 
-    const auto previousTransport = MG_Config::Transport;
+        // The probe is given NO frontend object at all, which is the point: this is the
-    MG_Config::Transport = MG_Config::TransportMode::InProcess;
+        // question a spawned server has to answer, and it has nothing to ask.
+        ASSERT_TRUE(BufferImpl::IsBufferDrawCleanByHandle(res, resource, nullptr))
+            << "the fixture is not clean before the map, so this case cannot isolate the "
+               "live-map question it exists for";
 
-    // The map is live and the object still says so - but the probe may no longer ask it.
+        // ---- map. The RISING EDGE is what has to publish, and nothing else can. -----------
         void* const mapped = owner->AcquireMemoryRange(
             Range1D{0, 256}, BufferMappingAccessBit::Write | BufferMappingAccessBit::Persistent);
         ASSERT_NE(mapped, nullptr);
         ASSERT_TRUE(owner->IsMapped());
-    // AcquireMemoryRange's NotePersistentMapStateChanged may have emitted the falling/rising
+        ASSERT_FALSE(owner->IsBackendPersistentMapped())
-    // edge record; re-stamp the fixture so the only question left is the flag.
+            << "the acquisition was minted, so R-6's decline did not happen and this is the "
-    record.Serial = 9;
+               "adopted arm rather than the emulated one";
-    resource->syncedChangeSerial = record.Serial;
+
-    {
+        EXPECT_TRUE(record.HasLiveHostWrites)
-        const std::lock_guard<std::mutex> lock(resource->pendingMutex);
+            << "the map published nothing the server can see. Without it the probe below answers "
-        resource->pendingRanges.clear();
+               "CLEAN, the ensure path is skipped and then latched (DirectGLES.cpp:691/:697), "
+               "SyncPersistentMappedRange is never reached again, and the frame draws the last "
+               "uploaded bytes for ever with no diagnostic";
+
+        // Absorb the rising-edge record so the ONLY thing left dirty is the flag.
+        stampSynced();
+        EXPECT_FALSE(BufferImpl::IsBufferDrawCleanByHandle(res, resource, nullptr))
+            << "a live host map read draw-CLEAN from the record alone";
+
+        // ---- a write with no API call announcing it, then the per-draw push --------------
+        static_cast<Uint8*>(mapped)[0] = 0x5Au;
+        const Uint64 serialBeforePush = record.Serial;
+        owner->SyncPersistentMappedRange();
+        EXPECT_GT(record.Serial, serialBeforePush)
+            << "the block push emitted nothing, so a write made through the pointer never left "
+               "the client";
+
+        // ---- unmap. The FALLING EDGE has to put it back. ---------------------------------
+        owner->ReleaseMemory(false);
+        EXPECT_FALSE(record.HasLiveHostWrites)
+            << "the unmap published nothing, so the record stays dirty for the buffer's life and "
+               "every later draw re-uploads it";
+        stampSynced();
+        EXPECT_TRUE(BufferImpl::IsBufferDrawCleanByHandle(res, resource, nullptr))
+            << "the probe stayed dirty after the unmap, i.e. it is not the record that is "
+               "being read";
+
+        BufferImpl::g_backendBufferResources.ReleaseByHandle(res);
     }
 
-    record.HasLiveHostWrites = false;
+    MG_Config::Ipc.PersistentBlockKb = previousBlockKb;
-    EXPECT_TRUE(BufferImpl::IsBufferDrawCleanByHandle(res, resource, owner.get()))
-        << "the frontend IsMapped() read did NOT retire under split - it is still what answers, "
-           "and a spawned server has no object to ask";
-
-    record.HasLiveHostWrites = true;
-    EXPECT_FALSE(BufferImpl::IsBufferDrawCleanByHandle(res, resource, owner.get()))
-        << "the record alone cannot say 'a host writer is live', so the question has no answer "
-           "on the server's side of a split at all";
-
     MG_Config::Transport = previousTransport;
-    owner->ReleaseMemory(false);
+    MG_Pipe::MGPipeSetResourceOps(nullptr);
-    BufferImpl::g_backendBufferResources.ReleaseByHandle(res);
+    MG_Config::Features.PipePush = previousPush;
-    MG_Pipe::MGPipeSlots().Free(MG_Pipe::MGPipeKind::Buffer, res);
-    record = {};
 #endif
 }