diff --git a/MobileGL/MG_Test/Pipe/FramebufferEmitTest.cpp b/MobileGL/MG_Test/Pipe/FramebufferEmitTest.cpp index 4a0584ca..dcf3aec3 100644 --- a/MobileGL/MG_Test/Pipe/FramebufferEmitTest.cpp +++ b/MobileGL/MG_Test/Pipe/FramebufferEmitTest.cpp @@ -338,6 +338,63 @@ TEST(FramebufferEmit, AMakeCurrentClearsBothRecordsAndAdvancesTheSerialRatherTha #endif } +// THE TEARDOWN SCOPE DROPS THE OBJECT RECORDS, SO IT MUST DROP EVERY WORKING HANDLE THAT NAMES +// ONE. The two framebuffer records hold eleven MGPSurface::Res naming texture and renderbuffer +// records, and the three unit windows hold entries naming sampler-view, sampler-CSO and texture +// records; a window left standing after the tables are emptied is a set of handles into empty +// tables, which the next resolve either refuses and counts or - on a slot the next context +// re-mints - resolves onto somebody else's record. Deleting any one of the eleven clears in +// MGPipeApplierReleaseObjectRecords leaves this red. +TEST(FramebufferEmit, AReleaseOfTheObjectRecordsAlsoClearsTheWorkingHandlesThatCouldNameThem) { +#if !MOBILEGL_PIPE_PUSH + GTEST_SKIP() << "MOBILEGL_PIPE_PUSH is off: there is no applier in this build"; +#else + ApplierGuard guard; + MGPipeApplySetFramebufferState(FramebufferRecord(MGPipeHandle{4, 1}, MGPipeFramebufferTarget::Both, 100)); + + // The three kVarTail sets, each with one entry naming a record the release is about to + // drop, and each at a non-zero Start so the window itself is visible in the assertions. + MGPBoundView view{}; + view.View = MGPipeHandle{3, 1}; + view.Texture = MGPipeHandle{9, 1}; + view.Unit = 2; + MGPipeApplySetSamplerViews(MGPSamplerViews{2, 1, 0xAAAAu}, &view); + + const MGPipeHandle samplerState{5, 1}; + MGPipeApplyBindSamplerStates(MGPSamplerStates{2, 1, 0xBBBBu}, &samplerState); + + MGPImageView image{}; + image.Res = MGPipeHandle{9, 1}; + image.Unit = 2; + image.InternalFormat = 0x8058u; // GL_RGBA8 + MGPipeApplySetShaderImages(MGPShaderImages{2, 1, 0xCCCCu}, &image); + + ASSERT_EQ(MGPipeApplier().DrawFramebuffer.Color[0].Res, (MGPipeHandle{9, 1})); + ASSERT_EQ(MGPipeApplier().SamplerViewCount, 1u); + ASSERT_EQ(MGPipeApplier().BoundSamplerViews[2].View, (MGPipeHandle{3, 1})); + ASSERT_EQ(MGPipeApplier().SamplerStateCount, 1u); + ASSERT_EQ(MGPipeApplier().BoundSamplerStates[2], samplerState); + ASSERT_EQ(MGPipeApplier().ShaderImageCount, 1u); + ASSERT_EQ(MGPipeApplier().BoundShaderImages[2].Res, (MGPipeHandle{9, 1})); + + MGPipeApplierReleaseObjectRecords(); + + EXPECT_EQ(MGPipeApplier().DrawFramebuffer.Fbo, kMGPipeNullHandle); + EXPECT_EQ(MGPipeApplier().ReadFramebuffer.Fbo, kMGPipeNullHandle); + EXPECT_EQ(MGPipeApplier().DrawFramebuffer.Color[0].Res, kMGPipeNullHandle) + << "a surface handle into an emptied texture table survived the teardown"; + EXPECT_EQ(MGPipeApplier().SamplerViewStart, 0u); + EXPECT_EQ(MGPipeApplier().SamplerViewCount, 0u); + EXPECT_EQ(MGPipeApplier().BoundSamplerViews[2].View, kMGPipeNullHandle); + EXPECT_EQ(MGPipeApplier().SamplerStateStart, 0u); + EXPECT_EQ(MGPipeApplier().SamplerStateCount, 0u); + EXPECT_EQ(MGPipeApplier().BoundSamplerStates[2], kMGPipeNullHandle); + EXPECT_EQ(MGPipeApplier().ShaderImageStart, 0u); + EXPECT_EQ(MGPipeApplier().ShaderImageCount, 0u); + EXPECT_EQ(MGPipeApplier().BoundShaderImages[2].Res, kMGPipeNullHandle); +#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/ResourceEmitTest.cpp b/MobileGL/MG_Test/Pipe/ResourceEmitTest.cpp index 68245677..4d98e252 100644 --- a/MobileGL/MG_Test/Pipe/ResourceEmitTest.cpp +++ b/MobileGL/MG_Test/Pipe/ResourceEmitTest.cpp @@ -1419,6 +1419,21 @@ namespace { const MGPHandleOnly wrongKind = KindHandle(res, MGPipeKind::SamplerCso); ExpectRefusedNaming("resource_destroy {slot=5, gen=1}: the handle names no resource kind", [&wrongKind]() { MGPipeApplyResourceDestroy(wrongKind); }); + + // AND unmap_persistent GIVES THE SAME VERDICT, because it is the only one of the four + // buffer-only calls that carries a discriminator at all. An assertion here is not a + // check: MOBILEGL_ASSERT compiles out at INFO, which is what all three gate builds and + // every shipped build are, so a texture-kinded record used to walk into ResolveResource + // and alias whatever BUFFER holds that slot - which is exactly what the destroy's Fatal + // above exists to stop. The live buffer at slot 5 is what makes the aliasing reachable. + MGPipeApplyResourceCreate(BufferDesc(res, 0, 44)); + MGPipeApplyResourceRespecify(BufferDesc(res, 256, 44), nullptr); + ASSERT_TRUE(MGPipeApplier().Resources[5].Live); + const MGPHandleOnly textureKind = KindHandle(res, MGPipeKind::Texture); + ExpectRefusedNaming("unmap_persistent {slot=5, gen=1}: the persistent donation is the buffer " + "family's and the handle names another kind", + [&textureKind]() { MGPipeApplyUnmapPersistent(textureKind); }); + EXPECT_EQ(MGPipeApplier().RefusedResourceCalls, 0u) << "a corrupt record is not a dropped call and must not be counted as one"; #endif diff --git a/MobileGL/MG_Test/Pipe/TextureEmitTest.cpp b/MobileGL/MG_Test/Pipe/TextureEmitTest.cpp index fba27001..abe3ebb2 100644 --- a/MobileGL/MG_Test/Pipe/TextureEmitTest.cpp +++ b/MobileGL/MG_Test/Pipe/TextureEmitTest.cpp @@ -436,10 +436,12 @@ TEST(TextureEmit, TheSubDataValidatorRefusesALevelABoxAndARegionTheRecordCannotD #endif } -// A respecify redefines the store, so the boxes and rects that describe the level it replaces -// go with it - a box kept across a shrink would have the backend upload past the end of the -// new level. Nothing is lost by it: the frontend entry points that respecify a texture re-mark -// the levels they define. +// A WHOLE-RESOURCE respecify - a null MGPRespecifiedLevel*, which is every glBufferData, +// glBufferStorage, glTexStorage* and texture view - redefines every level at once, so the boxes +// and rects against all of them go with it: a box kept across a shrink would have the backend +// upload past the end of the new level. THE SCOPE IS THE WHOLE POINT: this case proves the +// whole-resource arm ONLY, and its per-level twin below proves that the other arm may not do +// this. TEST(TextureEmit, ARespecifyDropsThePendingUploadsAgainstTheStorageItReplaces) { #if !MOBILEGL_PIPE_PUSH GTEST_SKIP() << "MOBILEGL_PIPE_PUSH is off: there is no applier in this build"; @@ -450,7 +452,8 @@ TEST(TextureEmit, ARespecifyDropsThePendingUploadsAgainstTheStorageItReplaces) { MGPipeApplyResourceCreate(TextureDesc(texture, 0, 55)); MGPipeApplyResourceRespecify(TextureDesc(texture, 64, 55), nullptr); MGPipeApplyResourceSubData(TextureUpload(texture, 0, MGPBox{0, 0, 0, 64, 64, 1}, 0), texels); - ASSERT_EQ(TextureRecordOf(3).PendingUploads.size(), 1u); + MGPipeApplyResourceSubData(TextureUpload(texture, 1, MGPBox{0, 0, 0, 32, 32, 1}, 0), texels); + ASSERT_EQ(TextureRecordOf(3).PendingUploads.size(), 2u); MGPipeApplyResourceRespecify(TextureDesc(texture, 8, 55), nullptr); EXPECT_TRUE(TextureRecordOf(3).PendingUploads.empty()) @@ -459,6 +462,165 @@ TEST(TextureEmit, ARespecifyDropsThePendingUploadsAgainstTheStorageItReplaces) { #endif } +// C1, AND IT IS THE CANONICAL MIP-BUILDING SEQUENCE. A mutable texture defines its levels one +// glTexImage*D at a time, and MG_State's AllocateStorage / MarkStorageDirty are per +// (uploadTarget, level) - so defining level 1 re-marks LEVEL 1 AND NOTHING ELSE. Level 0's +// client dirty flag was cleared at its own emission (D-D5 step 1) and the applier's entry is +// the only thing that still owes those texels, because Espryt's incomplete-texture bail is +// exactly the arm this set exists for. A blanket clear here destroys them silently, in every +// build, with no counter and no log line: this case goes red the moment the level scoping is +// dropped and green with it. +TEST(TextureEmit, ARespecifyOfOneLevelKeepsThePendingUploadsOfTheOthers) { +#if !MOBILEGL_PIPE_PUSH + GTEST_SKIP() << "MOBILEGL_PIPE_PUSH is off: there is no applier in this build"; +#else + ApplierGuard guard; + const MGPipeHandle texture{10, 1}; + const Uint8 texels[4096] = {}; + MGPipeApplyResourceCreate(TextureDesc(texture, 0, 121)); + + // glTexImage2D(level 0, data): the respecify names the level it defines, and the drain then + // emits level 0's shape, which the applier accepts. + const MGPRespecifiedLevel levelZero{kTex2D, 0}; + MGPipeApplyResourceRespecify(TextureDesc(texture, 64, 121), nullptr, &levelZero); + ASSERT_TRUE(MGPipeApplyResourceSubData(TextureUpload(texture, 0, MGPBox{0, 0, 0, 64, 64, 1}, 0), texels)); + // A SECOND FACE OF THE SAME LEVEL, keyed the way the packed Target keys it (ID-12: high + // byte = the cube-face upload target, low byte = the resource target), so what survives is + // a SET and not one lucky entry - and so that the level number alone cannot be what matched. + const Uint16 secondFace = static_cast((1u << 8) | kTex2D); + const MGPRespecifiedLevel faceOfLevelZero{secondFace, 0}; + MGPipeApplyResourceRespecify(TextureDesc(texture, 64, 121), nullptr, &faceOfLevelZero); + MGPSubData otherFace = TextureUpload(texture, 0, MGPBox{0, 0, 0, 64, 64, 1}, 0); + otherFace.Target = secondFace; + ASSERT_TRUE(MGPipeApplyResourceSubData(otherFace, texels)); + ASSERT_EQ(TextureRecordOf(10).PendingUploads.size(), 2u); + + // Espryt BAILS - the texture is not mipmap-complete for its min filter - so both entries + // are still owed when the next GL call arrives. + // + // glTexImage2D(level 1, data): this redefines level 1 of the (kTex2D, *) face only. + const MGPRespecifiedLevel levelOne{kTex2D, 1}; + MGPipeApplyResourceRespecify(TextureDesc(texture, 64, 121), nullptr, &levelOne); + + ASSERT_EQ(TextureRecordOf(10).PendingUploads.size(), 2u) + << "a respecify of level 1 dropped the pending uploads of levels it never redefined - " + "those texels are lost for good, because their dirty flags were cleared at emission"; + EXPECT_EQ(TextureRecordOf(10).PendingUploads[0].UploadTarget, kTex2D); + EXPECT_EQ(TextureRecordOf(10).PendingUploads[0].Level, 0u); + EXPECT_EQ(TextureRecordOf(10).PendingUploads[0].UnionBox.W, 64u); + EXPECT_EQ(TextureRecordOf(10).PendingUploads[1].UploadTarget, secondFace); + + // And the key it DOES name goes, because that level's coordinate system has been replaced. + ASSERT_TRUE(MGPipeApplyResourceSubData(TextureUpload(texture, 1, MGPBox{0, 0, 0, 32, 32, 1}, 0), texels)); + ASSERT_EQ(TextureRecordOf(10).PendingUploads.size(), 3u); + MGPipeApplyResourceRespecify(TextureDesc(texture, 64, 121), nullptr, &levelOne); + ASSERT_EQ(TextureRecordOf(10).PendingUploads.size(), 2u) + << "the level the respecify DOES redefine kept its box across the redefinition"; + for (const auto& entry : TextureRecordOf(10).PendingUploads) { + EXPECT_FALSE(entry.UploadTarget == kTex2D && entry.Level == 1u) + << "the redefined (upload target, level) survived"; + } +#endif +} + +// D-D5 step 1 says the client clears its dirty flag "only for levels whose record the applier +// ACCEPTED", and the call is the only thing that can say so: a dead or stale handle is a +// counted no-op and a corrupt record is a Fatal that deliberately moves NO counter, so in a +// shipped push build a refused upload and an accumulated one are otherwise identical from the +// call site. An emitter that clears on the strength of having emitted loses those texels. +TEST(TextureEmit, TheSubDataCallAnswersWhetherTheRecordWasAcceptedSoTheClientCanClearItsFlag) { +#if !MOBILEGL_PIPE_PUSH + GTEST_SKIP() << "MOBILEGL_PIPE_PUSH is off: there is no applier in this build"; +#else + ApplierGuard guard; + const MGPipeHandle texture{8, 1}; + const Uint8 texels[4096] = {}; + MGPipeApplyResourceCreate(TextureDesc(texture, 0, 131)); + MGPipeApplyResourceRespecify(TextureDesc(texture, 64, 131), nullptr); + + EXPECT_TRUE(MGPipeApplyResourceSubData(TextureUpload(texture, 0, MGPBox{0, 0, 0, 64, 64, 1}, 0), texels)) + << "an accumulated upload answered 'not accepted' and the client would re-send it forever"; + ASSERT_EQ(TextureRecordOf(8).PendingUploads.size(), 1u); + + // A stale generation is the refusal that is NOT a Fatal, so it is the one the answer has to + // carry: the record is gone, the texels were never taken, and the flag may not be cleared. + const Uint64 refusedBefore = MGPipeApplier().RefusedResourceCalls; + EXPECT_FALSE( + MGPipeApplyResourceSubData(TextureUpload(MGPipeHandle{8, 2}, 0, MGPBox{0, 0, 0, 8, 8, 1}, 0), texels)) + << "a refused upload answered 'accepted' and the client would clear a flag nothing owes"; + EXPECT_EQ(MGPipeApplier().RefusedResourceCalls, refusedBefore + 1); + EXPECT_EQ(TextureRecordOf(8).PendingUploads.size(), 1u); + + // The buffer half answers on the same terms, and its acceptance does not depend on a + // backend table being registered - a unit process has none. + const MGPipeHandle buffer{8, 1}; + MGPResourceDesc bufferDesc{}; + bufferDesc.Resource = buffer; + bufferDesc.Target = kMGPipeResourceTargetBuffer; + bufferDesc.Width = 256; + bufferDesc.GlNameForDiag = 132; + MGPipeApplyResourceCreate(bufferDesc); + MGPipeApplyResourceRespecify(bufferDesc, nullptr); + MGPSubData write{}; + write.Res = buffer; + write.Target = kMGPipeResourceTargetBuffer; + ASSERT_TRUE(MGPipeSetSubDataBufferRange(write, 0, 64)); + EXPECT_TRUE(MGPipeApplyResourceSubData(write, texels)); + write.Res = MGPipeHandle{8, 9}; + EXPECT_FALSE(MGPipeApplyResourceSubData(write, texels)); +#endif +} + +// m3. MGPSubData::Target is PACKED - low byte = MGPipeResourceTarget, high byte = the cube-face +// upload target (ID-12) - so MGPipeResourceTarget::Renderbuffer is a perfectly well-formed +// value for it, and without a gate a renderbuffer record would route to TextureResources and +// accumulate a pending upload onto whatever TEXTURE holds that slot. It is the same argument +// ResourceTableForTarget makes for returning null on an unknown enumerator, and the same +// verdict: acting outside the storage the record names is corruption, not a dropped call. +TEST(TextureEmit, ASubDataRecordWhoseResourceTargetNamesNoTextureIsRefusedRatherThanRouted) { +#if !MOBILEGL_PIPE_PUSH + GTEST_SKIP() << "MOBILEGL_PIPE_PUSH is off: there is no applier in this build"; +#else + ApplierGuard guard; + const MGPipeHandle texture{9, 1}; + const Uint8 texels[4096] = {}; + MGPipeApplyResourceCreate(TextureDesc(texture, 0, 141)); + MGPipeApplyResourceRespecify(TextureDesc(texture, 64, 141), nullptr); + MGPipeApplyResourceSubData(TextureUpload(texture, 0, MGPBox{0, 0, 0, 64, 64, 1}, 0), texels); + ASSERT_EQ(TextureRecordOf(9).PendingUploads.size(), 1u); + const Uint64 serialBefore = TextureRecordOf(9).Serial; + const Uint64 refusedBefore = MGPipeApplier().RefusedResourceCalls; + + MGPSubData renderbuffer = TextureUpload(texture, 0, MGPBox{0, 0, 0, 8, 8, 1}, 0); + renderbuffer.Target = static_cast(MGPipeResourceTarget::Renderbuffer); + ExpectRefusedNaming("resource_subdata {slot=9, gen=1}: the record's resource target names no texture " + "to upload into", + [&renderbuffer, &texels]() { MGPipeApplyResourceSubData(renderbuffer, texels); }); + + // And so is a value at or above the catalogue, and so is the buffer target arriving with a + // non-zero upload-target half - which the whole-field buffer test above cannot see. + MGPSubData pastTheCatalogue = renderbuffer; + pastTheCatalogue.Target = static_cast(MGPipeResourceTarget::Count); + ExpectRefusedNaming("resource_subdata {slot=9, gen=1}: the record's resource target names no texture " + "to upload into", + [&pastTheCatalogue, &texels]() { + MGPipeApplyResourceSubData(pastTheCatalogue, texels); + }); + + MGPSubData packedBuffer = renderbuffer; + packedBuffer.Target = static_cast(0x0100u | kMGPipeResourceTargetBuffer); + ExpectRefusedNaming("resource_subdata {slot=9, gen=1}: the record's resource target names no texture " + "to upload into", + [&packedBuffer, &texels]() { MGPipeApplyResourceSubData(packedBuffer, texels); }); + + EXPECT_EQ(TextureRecordOf(9).PendingUploads.size(), 1u) + << "a record naming no texture was accumulated onto the texture holding that slot"; + EXPECT_EQ(TextureRecordOf(9).Serial, serialBefore) << "not one refusal may move the serial"; + EXPECT_EQ(MGPipeApplier().RefusedResourceCalls, refusedBefore) + << "a corrupt record is not a dropped call and must not be counted as one"; +#endif +} + // D-J4, for the kind that made the rule matter: a TEXTURE lives in a share group exactly as a // buffer does, so its record - and the parameters and the pending uploads that ride on it - // outlives a make-current, and only the applier's own teardown takes it.