From 8c458cd59499c8571e66d21d4149afaa930e56bd Mon Sep 17 00:00:00 2001 From: rereview Date: Wed, 9 Sep 2026 00:41:46 -0400 Subject: [PATCH] [Fix, Test] (clientfb): heal a texture's record from its first set_texture_params - a texture born while the family was not live (the context's default textures are constructed before the backend registers its consumer) has no record, so the application's first glTexParameter* on texture 0 was refused, silently latched away before m-1 and the retrace census's one residual after it went loud; the params path now publishes the create, the storage if the texture has any, and then the parameters, the respecify path's own self-heal shape --- MobileGL/MG_Impl/Pipe/TextureEmit.h | 46 +++++++++--- MobileGL/MG_Test/Pipe/TextureEmitTest.cpp | 86 ++++++++++++++++++----- 2 files changed, 106 insertions(+), 26 deletions(-) diff --git a/MobileGL/MG_Impl/Pipe/TextureEmit.h b/MobileGL/MG_Impl/Pipe/TextureEmit.h index 33dab3e5..b973c3bd 100644 --- a/MobileGL/MG_Impl/Pipe/TextureEmit.h +++ b/MobileGL/MG_Impl/Pipe/TextureEmit.h @@ -802,21 +802,49 @@ namespace MobileGL::MG_Pipe { ++m_paramSets; // Not behind MGPipeTextureRecordsReachTheApplier() (see its comment): the call is // dispatched whenever this emitter runs, so the answer is always a real one. - if (!MGPipeApplySetTextureParams(params)) { - // Refused - a record the applier does not hold, or no consumer. Nothing latched: - // the same versions re-send at the next call, and the self-healing create the - // next respecify carries is what gives the record back. Loud for the reason the - // sub-data refusal is loud. + Bool accepted = MGPipeApplySetTextureParams(params); + if (!accepted) { + // THE SELF-HEAL, the respecify path's shape, and the parameters are the one + // publication that may be a texture's FIRST: the context's default textures are + // constructed before the backend registers its consumer, so no create ever went + // out for them, and the application's first glTexParameter* on texture 0 found + // no record (the retrace census's residual once this refusal went loud). A + // create with no storage gives the record its identity, the storage follows if + // the texture has any (a respecify against the create's descriptor is never + // deduped away), and the parameters land on the record that now exists. The + // same repair covers the served context's teardown scope, where the records are + // dropped while the objects live on. One retry, never a loop. + const MGPResourceDesc healDesc = MGPipeBuildTextureResourceDesc( + texture, handle, entry.BindMask, /*storageDefined=*/false, kMGPipeNullHandle, + kMGPipeNullHandle, 0, 0); + NoteDesc(healDesc, /*isCreate=*/true); + PublishCreate(MGPipeKind::Texture, handle, entry, healDesc); + const auto* mipmap = MG_State::GLState::AsMipmapTexture(&texture); + const Bool hasStorage = mipmap != nullptr + ? mipmap->GetMipmapLevelCount() > 0 + : texture.GetStorageType() == MobileGL::TextureStorageType::Buffer; + if (hasStorage) { + // Can grow the table (a view's owner is acquired inside): no Entry& is held + // across it - `entry` is re-fetched below. + EmitResourceRespecify(texture, MGPipeTextureRespecifyScope::WholeResource, 0, 0); + } + accepted = MGPipeApplySetTextureParams(params); + } + Entry& latched = EntryFor(m_textures, handle); + if (!accepted) { + // Refused on its merits (a null built-in sampler, no consumer). Nothing latched: + // the same versions re-send at the next call. Loud for the reason the sub-data + // refusal is loud. ++m_refusedParamSets; MGLOG_E_ONCE("MGPipe: set_texture_params for texture %u {slot=%u, gen=%u} was refused; the " "latch is not taken and the parameters are re-sent at the next call", texture.GetExternalIndex(), handle.Slot, handle.Gen); return; } - entry.HasParamsLatch = true; - entry.ParamsVersion = paramsVersion; - entry.SamplerVersion = samplerVersion; - entry.ForceParamsResync = false; + latched.HasParamsLatch = true; + latched.ParamsVersion = paramsVersion; + latched.SamplerVersion = samplerVersion; + latched.ForceParamsResync = false; } void EmitRenderbufferCreate(RenderbufferObject& renderbuffer) { diff --git a/MobileGL/MG_Test/Pipe/TextureEmitTest.cpp b/MobileGL/MG_Test/Pipe/TextureEmitTest.cpp index cc8874bc..6c04f426 100644 --- a/MobileGL/MG_Test/Pipe/TextureEmitTest.cpp +++ b/MobileGL/MG_Test/Pipe/TextureEmitTest.cpp @@ -224,7 +224,8 @@ TEST(TextureEmit, TheEmitterIsOneNeverDestroyedProcessSingleton) { X(TextureEmit, ATextureRecycledOntoADeadSlotDoesNotInheritTheDrainEntry) \ X(TextureEmit, ADeadRenderbuffersEntryIsRetiredWithItsSlot) \ X(TextureEmit, ARefusedParamsRecordDoesNotAdvanceTheLatch) \ - X(TextureEmit, ADeadTexturesSamplerViewLatchIsRetiredAtItsDeath) + X(TextureEmit, ADeadTexturesSamplerViewLatchIsRetiredAtItsDeath) \ + X(TextureEmit, ATextureBornBeforeTheConsumerRegisteredGetsItsRecordFromItsFirstParamsPublication) #define MGL_DECLARE_PULL_SKIP(Suite, Name) \ TEST(Suite, Name) { GTEST_SKIP() << "compiled only under MOBILEGL_PIPE_PUSH"; } @@ -1502,39 +1503,90 @@ TEST(TextureEmit, ADeadRenderbuffersEntryIsRetiredWithItsSlot) { // ============================ final review m-1 (audit F-7) ============================ // // set_texture_params LATCHES ON ACCEPTANCE, like the sub-data and respecify paths. A record the -// applier refused (it holds nothing for the handle) used to advance the version latch anyway, -// so the parameters were not re-sent until the next glTexParameter* moved a version. +// applier refused used to advance the version latch anyway, so the parameters were not re-sent +// until the next glTexParameter* moved a version. A refusal for a missing record is HEALED now +// (the case after this one), so the property is driven through a refusal on the merits: with no +// backend consumer the applier's belt refuses the parameters AND the healing create, and the +// emitter is driven directly (the contract hook would not even emit without the consumer). TEST(TextureEmit, ARefusedParamsRecordDoesNotAdvanceTheLatch) { TextureScope scope; const auto texture = MakeTexture2D(95, 8); const MGPipeHandle handle = Textures().FindTexture(*texture); ASSERT_NE(AppliedTexture(handle), nullptr); + const Uint64 serialBefore = AppliedTexture(handle)->ParamsSerial; - // The served context's teardown scope: every object record is dropped while the frontend - // objects live on. A parameter then moves (a LOD write on the built-in sampler, which the - // format setter's earlier publication did not carry) and its set_texture_params is refused. - MGPipeApplierReleaseObjectRecords(); + // A parameter moves (a LOD write on the built-in sampler, which the format setter's earlier + // publication did not carry) while no consumer is registered: refused, and not healable. texture->GetSamplerObject()->SetLodBias(0.5f); const Uint64 paramsBefore = Textures().ParamCount(); - MG_Pipe::MGPipeEmitTextureParams(*texture); + { + ScopedNoResourceOps noConsumer; + Textures().EmitTextureParams(*texture); + } EXPECT_EQ(Textures().ParamCount(), paramsBefore + 1) << "the record was not even emitted"; EXPECT_EQ(Textures().RefusedParamCount(), 1u) << "the emitter did not see the refusal"; + EXPECT_EQ(AppliedTexture(handle)->ParamsSerial, serialBefore) << "the refused record moved the serial"; - // The record comes back through the self-healing create the next respecify carries. - texture->AllocateStorage(TextureUploadTarget::Texture2D, 0, MipmapInput{IntVec3{16, 16, 1}, 16 * 16 * 4}); - const MGPipeResourceRecord* record = AppliedTexture(handle); - ASSERT_NE(record, nullptr); - ASSERT_EQ(record->ParamsSerial, 0u); - - // The same parameters, no version moved: with the latch taken on the REFUSED call this - // returns early and the record never learns them. + // The consumer is back and the same parameters, no version moved, are published again: + // with the latch taken on the REFUSED call this returns early and the record never learns + // the LOD write. MG_Pipe::MGPipeEmitTextureParams(*texture); EXPECT_EQ(Textures().ParamCount(), paramsBefore + 2) << "a refused set_texture_params advanced the latch, so the parameters are not re-sent"; - EXPECT_EQ(record->ParamsSerial, 1u) << "the record never learned the LOD write"; + const MGPipeResourceRecord* record = AppliedTexture(handle); + ASSERT_NE(record, nullptr); + EXPECT_EQ(record->ParamsSerial, serialBefore + 1) << "the record never learned the LOD write"; EXPECT_EQ(record->Params.LodBias, 0.5f); } +// THE RETRACE CENSUS's ONE RESIDUAL after m-1 went loud: a texture born while the family was +// not live - the context's default textures are constructed before the backend registers its +// consumer - has no record, and when the application's first glTexParameter* lands on it +// (texture 0) the record is refused and, before m-1, silently latched away for ever. The +// params path now heals the record the way the respecify path does: a create with no storage +// for the identity, the storage itself if the texture has any, then the parameters. +TEST(TextureEmit, ATextureBornBeforeTheConsumerRegisteredGetsItsRecordFromItsFirstParamsPublication) { + TextureScope scope; + SharedPtr texture; + { + ScopedNoResourceOps noConsumer; + texture = MakeTexture2D(88, 8); // born, formatted and allocated with no consumer: no create + } + const MGPipeHandle handle = Textures().FindTexture(*texture); + ASSERT_FALSE(MGPipeHandleIsNull(handle)); + ASSERT_EQ(AppliedTexture(handle), nullptr) << "the case needs a texture the applier never heard of"; + ASSERT_FALSE(MGPipeHandleIsPublished(MGPipeKind::Texture, handle)); + + // glTexParameterf(GL_TEXTURE_LOD_BIAS) on it, with the consumer present now. + texture->GetSamplerObject()->SetLodBias(0.25f); + MG_Pipe::MGPipeEmitTextureParams(*texture); + EXPECT_EQ(Textures().RefusedParamCount(), 0u) + << "the parameters of a texture born before the consumer were refused instead of healing its record"; + const MGPipeResourceRecord* record = AppliedTexture(handle); + ASSERT_NE(record, nullptr) << "no record was healed"; + EXPECT_TRUE(MGPipeHandleIsPublished(MGPipeKind::Texture, handle)); + EXPECT_EQ(record->Desc.Width, 8u) << "the healed record carries no storage although the texture has some"; + EXPECT_EQ(record->Desc.Levels, 1u); + EXPECT_EQ(record->ParamsSerial, 1u); + EXPECT_EQ(record->Params.LodBias, 0.25f); + // And a texture with NO storage at all - the default texture's shape - heals to a record + // with the identity only, which is what its parameters need and all a create says. + const auto bare = MakeShared(87); + { + // its create went out with the consumer present, so take the record away again to model + // a birth the applier never saw + MGPipeApplierReleaseObjectRecords(); + } + bare->GetSamplerObject()->SetLodBias(0.75f); + MG_Pipe::MGPipeEmitTextureParams(*bare); + const MGPipeHandle bareHandle = Textures().FindTexture(*bare); + const MGPipeResourceRecord* bareRecord = AppliedTexture(bareHandle); + ASSERT_NE(bareRecord, nullptr) << "a storage-less texture's parameters healed no record"; + EXPECT_EQ(bareRecord->Desc.Width, 0u); + EXPECT_EQ(bareRecord->Params.LodBias, 0.75f); + EXPECT_EQ(Textures().RefusedParamCount(), 0u); +} + #endif // MOBILEGL_PIPE_PUSH // =========================================================================================