From 8f1eaafa79c24ec49c0f93536b4162bae9125496 Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Tue, 8 Sep 2026 14:37:29 -0400 Subject: [PATCH] [Test] (Pipe): pin what a Named framebuffer record does and does not move, the stale-generation refusal on the per-object table, the two handles it will not store, and that a respecify which redefines no storage carries the mask without eating the texels --- MobileGL/MG_Test/Pipe/FramebufferEmitTest.cpp | 140 ++++++++++++++++++ MobileGL/MG_Test/Pipe/TextureEmitTest.cpp | 120 +++++++++++++++ 2 files changed, 260 insertions(+) diff --git a/MobileGL/MG_Test/Pipe/FramebufferEmitTest.cpp b/MobileGL/MG_Test/Pipe/FramebufferEmitTest.cpp index 87a4d521..f41ce507 100644 --- a/MobileGL/MG_Test/Pipe/FramebufferEmitTest.cpp +++ b/MobileGL/MG_Test/Pipe/FramebufferEmitTest.cpp @@ -421,6 +421,146 @@ TEST(FramebufferEmit, AReleaseOfTheObjectRecordsAlsoClearsTheWorkingHandlesThatC #endif } +// ID-19's CORRECTION, AND THE CASE THAT SAYS WHAT THE FOURTH TARGET IS FOR. Every DSA entry +// point - BlitNamedFramebuffer and the four ClearNamedFramebuffer* - hands Espryt a framebuffer +// BY NAME, and that framebuffer is very often bound to neither binding. With only the two bound +// records the server had no description of it at all, bound its driver FBO with no attachments +// and cleared or blitted into nothing (esprytobj C-1). A Named record fixes that WITHOUT lying +// about the bindings: the record is written and addressable by handle, and BoundFramebuffer +// does not move. Making the Named arm touch either binding leaves this red. +TEST(FramebufferEmit, ANamedRecordDescribesTheFramebufferItNamesWithoutMovingEitherBinding) { +#if !MOBILEGL_PIPE_PUSH + GTEST_SKIP() << "MOBILEGL_PIPE_PUSH is off: there is no applier in this build"; +#else + ApplierGuard guard; + // TWO DIFFERENT FRAMEBUFFERS ON THE TWO BINDINGS FIRST, so "the bindings did not move" is an + // assertion about values rather than about null. + MGPipeApplySetFramebufferState(FramebufferRecord(MGPipeHandle{4, 1}, MGPipeFramebufferTarget::Draw, 100)); + MGPipeApplySetFramebufferState(FramebufferRecord(MGPipeHandle{5, 2}, MGPipeFramebufferTarget::Read, 200)); + const Uint64 serialBefore = MGPipeApplier().FramebufferSerial; + + MGPFramebufferState named = FramebufferRecord(MGPipeHandle{6, 3}, MGPipeFramebufferTarget::Draw, 300); + named.Target = kMGPipeFramebufferTargetNamed; + named.Color[0].Res = MGPipeHandle{21, 1}; + MGPipeApplySetFramebufferState(named); + + // (a) THE DSA LOOKUP FINDS IT, BY HANDLE, WITH ITS ATTACHMENTS. This is the call package D + // makes at every named blit and clear. + ASSERT_NE(MGPipeApplier().FramebufferRecordFor(MGPipeHandle{6, 3}), nullptr) + << "a framebuffer handed to the server by name has no record, which is the state that " + "clears into a driver framebuffer with no attachments"; + EXPECT_EQ(MGPipeApplier().FramebufferRecordFor(MGPipeHandle{6, 3})->Width, 300u); + EXPECT_EQ(MGPipeApplier().FramebufferRecordFor(MGPipeHandle{6, 3})->Color[0].Res, (MGPipeHandle{21, 1})); + EXPECT_EQ(MGPipeApplier().FramebufferRecordFor(MGPipeHandle{6, 3})->Target, kMGPipeFramebufferTargetNamed); + + // (b) AND NEITHER BINDING MOVED. + ASSERT_NE(MGPipeApplier().DrawFramebuffer(), nullptr); + ASSERT_NE(MGPipeApplier().ReadFramebuffer(), nullptr); + EXPECT_EQ(MGPipeApplier().DrawFramebuffer()->Fbo, (MGPipeHandle{4, 1})) + << "a Named record claimed the draw binding"; + EXPECT_EQ(MGPipeApplier().ReadFramebuffer()->Fbo, (MGPipeHandle{5, 2})) + << "a Named record claimed the read binding"; + EXPECT_EQ(MGPipeApplier().BoundFramebuffer[0], (MGPipeHandle{4, 1})); + EXPECT_EQ(MGPipeApplier().BoundFramebuffer[1], (MGPipeHandle{5, 2})); + + // (c) The serial moves for a Named record too: a twin memoising a framebuffer's attachments + // has to hear that they moved, and whether it is bound is a different question. + EXPECT_EQ(MGPipeApplier().FramebufferSerial, serialBefore + 1); + + // (d) And the same framebuffer can then be BOUND, which moves the binding and restates the + // record - the two targets are not two tables. + MGPipeApplySetFramebufferState(FramebufferRecord(MGPipeHandle{6, 3}, MGPipeFramebufferTarget::Draw, 400)); + ASSERT_NE(MGPipeApplier().DrawFramebuffer(), nullptr); + EXPECT_EQ(MGPipeApplier().DrawFramebuffer()->Fbo, (MGPipeHandle{6, 3})); + EXPECT_EQ(MGPipeApplier().DrawFramebuffer()->Width, 400u); + EXPECT_EQ(MGPipeApplier().ReadFramebuffer()->Fbo, (MGPipeHandle{5, 2})); + EXPECT_EQ(MGPipeApplier().RefusedObjectCalls, 0u); +#endif +} + +// STALE-GENERATION REFUSAL, ON THE ONE TABLE WHOSE OBJECT HAS NO WIRE LIFETIME. A framebuffer is +// never destroyed on the wire, so its slot is simply overwritten by its successor - and until +// that successor describes itself, a handle naming the DEAD one must be refused rather than +// answered with the predecessor's attachments. That answer would be a blit or a clear into +// somebody else's colour buffer. It is LOUD (counted, and logged once) because the only way to +// reach it is an emitter defect, and it is counted APART from RefusedObjectCalls because this is +// a read by the server's own sync path and not a call the applier refused. +TEST(FramebufferEmit, AFramebufferHandleWhoseGenerationHasMovedOnIsRefusedRatherThanAnswered) { +#if !MOBILEGL_PIPE_PUSH + GTEST_SKIP() << "MOBILEGL_PIPE_PUSH is off: there is no applier in this build"; +#else + ApplierGuard guard; + MGPipeApplySetFramebufferState(FramebufferRecord(MGPipeHandle{12, 1}, MGPipeFramebufferTarget::Draw, 100)); + ASSERT_NE(MGPipeApplier().FramebufferRecordFor(MGPipeHandle{12, 1}), nullptr); + const Uint64 staleBefore = MGPipeApplier().StaleFramebufferRecordLookups; + + // The slot has been recycled and the successor has not described itself yet. + EXPECT_EQ(MGPipeApplier().FramebufferRecordFor(MGPipeHandle{12, 2}), nullptr) + << "a handle at a recycled slot was answered with its predecessor's record"; + EXPECT_EQ(MGPipeApplier().StaleFramebufferRecordLookups, staleBefore + 1); + + // Now it does, and the predecessor's handle becomes the stale one - in the other direction. + MGPipeApplySetFramebufferState(FramebufferRecord(MGPipeHandle{12, 2}, MGPipeFramebufferTarget::Draw, 200)); + ASSERT_NE(MGPipeApplier().FramebufferRecordFor(MGPipeHandle{12, 2}), nullptr); + EXPECT_EQ(MGPipeApplier().FramebufferRecordFor(MGPipeHandle{12, 2})->Width, 200u); + EXPECT_EQ(MGPipeApplier().FramebufferRecordFor(MGPipeHandle{12, 1}), nullptr); + EXPECT_EQ(MGPipeApplier().StaleFramebufferRecordLookups, staleBefore + 2); + + // THE TWO SILENT NULLS, and they are silent on purpose. "Nothing is bound to this binding" + // is what a make-current leaves behind and arrives on every draw of a context that has not + // described its framebuffers; "no record at this slot" is what every framebuffer looks like + // before its first set_framebuffer_state. Counting either would bury the one that matters. + EXPECT_EQ(MGPipeApplier().FramebufferRecordFor(kMGPipeNullHandle), nullptr); + EXPECT_EQ(MGPipeApplier().FramebufferRecordFor(MGPipeHandle{99, 1}), nullptr); + EXPECT_EQ(MGPipeApplier().StaleFramebufferRecordLookups, staleBefore + 2) + << "an unbound binding or an undescribed slot was counted as a stale generation"; + EXPECT_EQ(MGPipeApplier().RefusedObjectCalls, 0u) + << "the framebuffer family may never move the object-refusal counter"; +#endif +} + +// THE TWO REFUSALS THE PER-OBJECT TABLE ADDED. The null handle is what "nothing is bound" reads +// as, so a record installed at {0,0} would be answered to every caller asking about an EMPTY +// binding; and Slot is a client-supplied Uint32 that now reaches an allocator, so it takes the +// same bound the five object tables take. Every emitter has a handle for every framebuffer it +// describes - kMGPipeDefaultFramebuffer {0,1} for the default one - so neither value is +// producible by a correct client, which is why both are Fatal rather than counted refusals. +TEST(FramebufferEmit, AFramebufferRecordThatNamesNoUsableHandleIsRefusedRatherThanStored) { +#if !MOBILEGL_PIPE_PUSH + GTEST_SKIP() << "MOBILEGL_PIPE_PUSH is off: there is no applier in this build"; +#else + ApplierGuard guard; + // The positive control first: the DEFAULT framebuffer is slot 0 at generation 1 and is + // perfectly legal, so what follows refuses the null handle and not slot 0. + MGPipeApplySetFramebufferState( + FramebufferRecord(kMGPipeDefaultFramebuffer, MGPipeFramebufferTarget::Both, 128)); + ASSERT_NE(MGPipeApplier().FramebufferRecordFor(kMGPipeDefaultFramebuffer), nullptr); + EXPECT_EQ(MGPipeApplier().FramebufferRecordFor(kMGPipeDefaultFramebuffer)->Width, 128u); + const Uint64 serialBefore = MGPipeApplier().FramebufferSerial; + + MGPFramebufferState nullHandle = FramebufferRecord(kMGPipeNullHandle, MGPipeFramebufferTarget::Draw, 300); + ExpectRefusedNaming("set_framebuffer_state {slot=0, gen=0, target=0}: the record names the null " + "framebuffer handle", + [&nullHandle]() { MGPipeApplySetFramebufferState(nullHandle); }); + + MGPFramebufferState pastTheBound = FramebufferRecord( + MGPipeHandle{kMGPipeMaxFramebufferSlots, 1}, MGPipeFramebufferTarget::Draw, 400); + ExpectRefusedNaming("set_framebuffer_state {slot=65536, gen=1, target=0}: the framebuffer slot is " + "outside the record table's bound", + [&pastTheBound]() { MGPipeApplySetFramebufferState(pastTheBound); }); + static_assert(kMGPipeMaxFramebufferSlots == 65536u, + "the refusal line above names the bound; move both together"); + + EXPECT_EQ(MGPipeApplier().FramebufferSerial, serialBefore) << "a refused record moved the serial"; + ASSERT_NE(MGPipeApplier().DrawFramebuffer(), nullptr); + EXPECT_EQ(MGPipeApplier().DrawFramebuffer()->Fbo, kMGPipeDefaultFramebuffer) + << "a refused record took the draw binding"; + EXPECT_LT(MGPipeApplier().FramebufferRecords.size(), + static_cast(kMGPipeMaxFramebufferSlots)) + << "an out-of-range slot resized the table instead of being refused"; +#endif +} + int main(int argc, char** argv) { // Before anything logs: the logger reads this variable once, on its first write, and // caches the handle. The name carries this process's pid, and the file is removed on the diff --git a/MobileGL/MG_Test/Pipe/TextureEmitTest.cpp b/MobileGL/MG_Test/Pipe/TextureEmitTest.cpp index 75bab9a2..9bdaff8a 100644 --- a/MobileGL/MG_Test/Pipe/TextureEmitTest.cpp +++ b/MobileGL/MG_Test/Pipe/TextureEmitTest.cpp @@ -675,6 +675,126 @@ TEST(TextureEmit, TheTextureRecordAndItsParamsAndPendingUploadsSurviveAMakeCurre #endif } +// ID-18 M4, AND IT IS THE ONE ARM AN IMMUTABLE TEXTURE HAS. A sticky BindMask / +// ImageBindableHint bit has exactly one way onto the wire - a respecify - and glTexStorage2D +// leaves a texture with no further respecify to carry it, so for the canonical order (allocate, +// THEN bind as an image or attach) the hint that exists to prevent a texture re-mint would never +// arrive at all. B therefore republishes the descriptor when the mask moves, and the applier has +// to tell that call apart from a redefinition: it replaces the descriptor and moves the serial, +// and it drops NOTHING - the storage it is against was not replaced, so no level's coordinate +// system moved. A mask change landing between a glTexSubImage2D and the sync that consumes it +// must not eat those texels, which is C1's bug with a different trigger and just as silent. +TEST(TextureEmit, ARespecifyThatRedefinesNoStorageCarriesTheStickyMaskAndKeepsThePendingUploads) { +#if !MOBILEGL_PIPE_PUSH + GTEST_SKIP() << "MOBILEGL_PIPE_PUSH is off: there is no applier in this build"; +#else + ApplierGuard guard; + const MGPipeHandle texture{11, 1}; + const Uint8 texels[4096] = {}; + MGPipeApplyResourceCreate(TextureDesc(texture, 0, 151)); + + // glTexStorage2D: an IMMUTABLE store, which is the whole reason this arm exists. + MGPResourceDesc allocated = TextureDesc(texture, 64, 151); + allocated.Immutable = 1; + allocated.Levels = 1; + allocated.InternalFormat = 0x8058u; // GL_RGBA8 + allocated.BindMask = static_cast(kMGPipeBindSampler); + ASSERT_TRUE(MGPipeApplyResourceRespecify(allocated, nullptr)); + + // glTexSubImage2D: texels whose client-side dirty flag was cleared at THIS emission, so the + // applier's entry is the only thing that still owes them. + ASSERT_TRUE(MGPipeApplyResourceSubData(TextureUpload(texture, 0, MGPBox{0, 0, 0, 64, 64, 1}, 0), texels)); + ASSERT_EQ(TextureRecordOf(11).PendingUploads.size(), 1u); + const Uint64 serialBefore = TextureRecordOf(11).Serial; + + // glBindImageTexture: the mask moves and nothing about the storage does. + MGPResourceDesc masked = allocated; + masked.BindMask = static_cast(allocated.BindMask | kMGPipeBindShaderImage); + masked.ImageBindableHint = 1; + ASSERT_FALSE(MGPipeResourceRespecifyNeedsAck(masked)) + << "a metadata respecify must never ask for a reallocation acknowledgement"; + ASSERT_TRUE(MGPipeApplyResourceRespecify(masked, nullptr)); + + EXPECT_EQ(TextureRecordOf(11).Desc.BindMask, masked.BindMask) + << "the mask this call exists to carry did not reach the record"; + EXPECT_EQ(TextureRecordOf(11).Desc.ImageBindableHint, 1); + ASSERT_EQ(TextureRecordOf(11).PendingUploads.size(), 1u) + << "a respecify that redefined no storage ate the texels standing against it"; + EXPECT_EQ(TextureRecordOf(11).PendingUploads[0].UnionBox.W, 64u); + EXPECT_GT(TextureRecordOf(11).Serial, serialBefore) + << "the serial is the whole publication of a metadata update - the twin re-derives its " + "storage flags from the new mask on the strength of it"; + + // AND THE LEVEL POINTER DOES NOT CHANGE THE ANSWER. This is where ID-18 M4 refines C1: + // C1's rule drops the uploads against the storage a call REPLACES, and a call that replaces + // no storage replaces no level's coordinate system either, whatever level it names. + const MGPRespecifiedLevel levelZero{kTex2D, 0}; + MGPResourceDesc maskedAgain = masked; + maskedAgain.BindMask = static_cast(masked.BindMask | kMGPipeBindRenderTarget); + ASSERT_TRUE(MGPipeApplyResourceRespecify(maskedAgain, nullptr, &levelZero)); + ASSERT_EQ(TextureRecordOf(11).PendingUploads.size(), 1u) + << "a metadata update dropped the level it named"; + EXPECT_EQ(TextureRecordOf(11).Desc.BindMask, maskedAgain.BindMask); + + // THE NEGATIVE CONTROL, in the same case: move ONE storage-defining field and the same call + // is a redefinition again, which takes the level it names with it. + MGPResourceDesc reallocated = maskedAgain; + reallocated.Width = 32; + reallocated.Height = 32; + ASSERT_TRUE(MGPipeApplyResourceRespecify(reallocated, nullptr, &levelZero)); + EXPECT_TRUE(TextureRecordOf(11).PendingUploads.empty()) + << "a 64-wide box survived a redefinition onto a 32-wide level"; + EXPECT_EQ(MGPipeApplier().RefusedResourceCalls, 0u); +#endif +} + +// D-D5 step 1 again, for the two calls that DEFINE the storage an upload lands in (ID-18 M3). +// The emitter cannot see either refusal from its call site: a dead or stale handle is a counted +// no-op and a corrupt record is a Fatal that deliberately moves no counter, so a create or a +// respecify the applier dropped is indistinguishable from one it took. A client that goes on to +// clear a level's dirty flags, or to advance its own descriptor dedupe, on the strength of +// having emitted has lost those texels for good. +TEST(TextureEmit, TheCreateAndRespecifyCallsAnswerWhetherTheRecordWasAccepted) { +#if !MOBILEGL_PIPE_PUSH + GTEST_SKIP() << "MOBILEGL_PIPE_PUSH is off: there is no applier in this build"; +#else + ApplierGuard guard; + const MGPipeHandle texture{12, 1}; + EXPECT_TRUE(MGPipeApplyResourceCreate(TextureDesc(texture, 0, 161))); + EXPECT_TRUE(MGPipeApplyResourceRespecify(TextureDesc(texture, 64, 161), nullptr)); + ASSERT_TRUE(TextureRecordOf(12).Live); + + // THE RESERVED SLOT is refused by the create, and the answer says so. + EXPECT_FALSE(MGPipeApplyResourceCreate(TextureDesc(MGPipeHandle{0, 1}, 0, 162))) + << "resource_create answered accepted for the reserved slot 0"; + + // A STALE GENERATION is the one refusal that is not a Fatal, so it is the only one the + // return can carry, and it is counted on the way out. + const Uint64 refusedBefore = MGPipeApplier().RefusedResourceCalls; + MGPipeHandle recycled = texture; + recycled.Gen = 2; + EXPECT_FALSE(MGPipeApplyResourceRespecify(TextureDesc(recycled, 64, 161), nullptr)) + << "resource_respecify answered accepted for a handle it refused"; + EXPECT_EQ(MGPipeApplier().RefusedResourceCalls, refusedBefore + 1); + EXPECT_EQ(TextureRecordOf(12).Desc.Width, 64u) << "a refused respecify moved the record anyway"; + + // AND THE BUFFER HALF ANSWERS ON THE SAME TERMS WITH NO BACKEND TABLE REGISTERED. Whether a + // backend installed MGPipeResourceOps is a property of the BUILD and not of the record; an + // emitter that read "not accepted" off an unregistered table would re-send a call the + // applier has already taken responsibility for. + MGPResourceDesc buffer{}; + buffer.Resource = MGPipeHandle{13, 1}; + buffer.Target = kMGPipeResourceTargetBuffer; + buffer.GlNameForDiag = 163; + EXPECT_TRUE(MGPipeApplyResourceCreate(buffer)); + buffer.Width = 256; + EXPECT_TRUE(MGPipeApplyResourceRespecify(buffer, nullptr)); + MGPResourceDesc deadBuffer = buffer; + deadBuffer.Resource.Gen = 7; + EXPECT_FALSE(MGPipeApplyResourceRespecify(deadBuffer, nullptr)); +#endif +} + int main(int argc, char** argv) { namespace fs = std::filesystem; const fs::path path =