[Fix] (Pipe): stop the tracker's context Reset eating the pending base instance the same call is about to read, and clear it on the validate point's no-context exit too

MobileGL/MG_Impl/Pipe/PipeFill.cpp

@@ -795,9 +795,15 @@ namespace MobileGL::MG_Pipe {
 #endif
         MGPipeFillAccess::SetVerb(inputs, MGPipeVerb::kVerbCount);
         // The pending base instance belongs to the verb that was about to run, so leaving
-        // one drops it. The validate point clears it too, after the emission consumed it -
-        // the two together are what make a plain draw after a base-instanced one see 0
-        // again, and neither of them relies on the other being called.
+        // one drops it.
+        //
+        // THIS IS NOT THE CLEAR PRODUCTION RELIES ON, and saying so is better than implying
+        // two independent guarantees where there is one: no GL entry point calls
+        // MGPipeLeaveVerb - grep finds MG_Test/ScopedPipeVerb.h and MG_Test/Pipe/TrackerTest
+        // .cpp and nothing else - so what this line guarantees is that a unit case which
+        // opens a ScopedPipeVerb cannot leak a base instance into the next case. The
+        // production property ("consumed by exactly the verb whose entry point set it, and 0
+        // at every other Update") is held by MGPipeValidateForVerb, on both of its exits.
         MGPipeTrackerInstance().ClearPendingBaseInstance();
     }
 
@@ -1367,7 +1373,15 @@ namespace MobileGL::MG_Pipe {
         MGPipeFillAccess::SetVerb(inputs, verb);
         auto* ctx = LiveContext();
         MGPipeFillAccess::SetIdentity(inputs, ctx);
-        if (ctx == nullptr) return;
+        if (ctx == nullptr) {
+            // The pending base instance belongs to THIS verb, and this exit skips step 3's
+            // clear, so it has to make the same promise here: a base-instanced draw with no
+            // live context is a no-op, but leaving its argument standing would hand it to the
+            // next verb - which, since the tracker's Reset() no longer clears it, is the one
+            // path that could still carry a stale shift across.
+            MGPipeTrackerInstance().ClearPendingBaseInstance();
+            return;
+        }
         const MGPipeVerbClass verbClass = kMGPipeVerbClass[static_cast<SizeT>(verb)];
         const MGPipeFieldMask& mask = kMGPipeClassFieldMask[static_cast<SizeT>(verbClass)];
 

MobileGL/MG_Impl/Pipe/Tracker.h

@@ -364,13 +364,23 @@ namespace MobileGL::MG_Pipe {
         // every bit set, which is what makes the first verb on a fresh context publish a
         // complete state rather than an increment. Deliberately does NOT clear the fire
         // tallies: they are a per-run measurement, not per-context state.
+        //
+        // AND IT DELIBERATELY DOES NOT CLEAR m_pendingBaseInstance. Everything else this
+        // function clears is a LATCH describing what the server was last told; the pending
+        // base instance is THIS CALL'S ARGUMENT, written by the draw entry point one
+        // statement before MGP_FILL and not yet read by anybody. Update() calls Reset() from
+        // inside itself whenever the current GLContext pointer moves, so clearing it here
+        // meant that `eglMakeCurrent(ctxB); glDrawArraysInstancedBaseInstance(..., 7)` put a
+        // BaseInstance of 0 on the wire - one silently mis-shifted instanced draw per context
+        // switch, on the emulation path, with nothing to catch it. The value is cleared by the
+        // verb that consumes it (PipeFill.cpp's step 3, and its no-context early return) and
+        // by MGPipeLeaveVerb, which is where a per-call argument belongs.
         void Reset() {
             std::memset(m_lastPushed, 0, sizeof(m_lastPushed));
             m_renderStateVersion.Reset();
             m_pipelineStateVersion.Reset();
             m_framebufferBind.Reset();
             m_indexSlotVersion.Reset();
-            m_pendingBaseInstance = 0;
             m_pack = PixelStoreParameters{};
             m_patch = PatchTrio{};
             m_staged = RenderStateParameters{};
@@ -432,7 +442,14 @@ namespace MobileGL::MG_Pipe {
         // whose only change is its base instance has to reach the emitter. Set immediately
         // before the fill at the three *BaseInstance draw entry points; CONSUMED and cleared
         // by the validate point once it has been emitted, so a plain draw that follows one
-        // sees 0 again whether or not anything called MGPipeLeaveVerb in between.
+        // sees 0 again.
+        //
+        // THE CLEAR THAT ACTUALLY RUNS IN PRODUCTION IS THE VALIDATE POINT'S. MGPipeLeaveVerb
+        // clears it too, but no GL entry point calls MGPipeLeaveVerb - only MG_Test's
+        // ScopedPipeVerb and TrackerTest do - so the production guarantee is entirely
+        // PipeFill.cpp's, on BOTH of its exits: the end of step 3, and the no-live-context
+        // early return that skips step 3 altogether. Reset() deliberately does not clear it
+        // (see there): it is this call's argument, not a latch.
         void SetPendingBaseInstance(Uint32 baseInstance) { m_pendingBaseInstance = baseInstance; }
         Uint32 PendingBaseInstance() const { return m_pendingBaseInstance; }
         void ClearPendingBaseInstance() { m_pendingBaseInstance = 0; }