diff --git a/MobileGL/Config.h b/MobileGL/Config.h index 37bbe2fe..93dfeabf 100644 --- a/MobileGL/Config.h +++ b/MobileGL/Config.h @@ -335,7 +335,10 @@ namespace MobileGL::MG_Config { // 0x100 vertex input (vertex elements / vertex buffers / index buffer) // 0x200 framebuffer (set_framebuffer_state) - requires 0x400 // 0x400 texture resources (texture + renderbuffer resource_*, - // set_texture_params) - requires 0x80 + // set_texture_params) - requires 0x80 AND 0x800 + // (the built-in sampler CSO a set_texture_params record names is minted by + // the sampler family alone, ID-15; the four rows are MG_Impl/Pipe/PipeFill.cpp's + // kMGPipeP4aFamilyDependencies, mirrored bit for bit by Espryt's resolvers) // 0x800 samplers (sampler CSO, sampler view, set_sampler_views / // bind_sampler_states / set_shader_images) - requires 0x400 // 0x1000 programs (shader CSO, set_draw/dispatch_program, global constants) diff --git a/MobileGL/MG_Impl/Pipe/TextureEmit.h b/MobileGL/MG_Impl/Pipe/TextureEmit.h index 5f27e06f..33dab3e5 100644 --- a/MobileGL/MG_Impl/Pipe/TextureEmit.h +++ b/MobileGL/MG_Impl/Pipe/TextureEmit.h @@ -762,9 +762,10 @@ namespace MobileGL::MG_Pipe { entry.SamplerVersion == samplerVersion && !entry.ForceParamsResync) { return; } - entry.HasParamsLatch = true; - entry.ParamsVersion = paramsVersion; - entry.SamplerVersion = samplerVersion; + // THE LATCH IS TAKEN BELOW, ON ACCEPTANCE (final review m-1, audit F-7) - like the + // sub-data and respecify paths, and unlike v2, which advanced it here and left a + // refused record (no applier record for the handle, the SD-1/SD-3 shape) unsent + // until the next glTexParameter* moved a version. // ID-14 / ID-17: THE BUILT-IN SAMPLER COMES FROM C's CONTENT-ADDRESSED CACHE and is // never minted here. v1 took MGPipeSlots().Acquire(SamplerCso, the SamplerObject's @@ -797,10 +798,25 @@ namespace MobileGL::MG_Pipe { const MGPTextureParams params = MGPipeBuildTextureParams(texture, handle, entry.BuiltinSampler, entry.ForceParamsResync); - entry.ForceParamsResync = false; m_lastParams = params; ++m_paramSets; - MGPipeApplySetTextureParams(params); + // 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. + ++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; } void EmitRenderbufferCreate(RenderbufferObject& renderbuffer) { @@ -946,6 +962,8 @@ namespace MobileGL::MG_Pipe { // Records the applier REFUSED. The dirty flag survives one of these, which is the whole // of D-D5 step 1 - so a case that wants to prove the flag survived asserts on this. Uint64 RefusedSubDataCount() const { return m_refusedSubDatas; } + // set_texture_params records the applier refused; the latch survives one of these (m-1). + Uint64 RefusedParamCount() const { return m_refusedParamSets; } // What create_sampler_state put on the wire on this emitter's behalf, so the csob-blob // accounting does not under-report 100 bytes per built-in sampler mint. set_texture_params // itself returns no byte count - it is not emitted from the validate point's payload @@ -975,6 +993,7 @@ namespace MobileGL::MG_Pipe { void ResetCounters() { m_creates = m_respecifies = m_paramSets = m_subDatas = 0; m_refusedSubDatas = 0; + m_refusedParamSets = 0; m_samplerCsoPayloadBytes = 0; m_deadResolves = 0; } @@ -1276,6 +1295,7 @@ namespace MobileGL::MG_Pipe { Uint64 m_paramSets = 0; Uint64 m_subDatas = 0; Uint64 m_refusedSubDatas = 0; + Uint64 m_refusedParamSets = 0; Uint64 m_samplerCsoPayloadBytes = 0; mutable Uint64 m_deadResolves = 0; }; diff --git a/MobileGL/MG_Pipe/MGPipeTypes.h b/MobileGL/MG_Pipe/MGPipeTypes.h index 8a84f086..5e58c0c6 100644 --- a/MobileGL/MG_Pipe/MGPipeTypes.h +++ b/MobileGL/MG_Pipe/MGPipeTypes.h @@ -1107,10 +1107,12 @@ namespace MobileGL::MG_Pipe { // call that NAMES a level is that level's redefinition whatever the descriptor says - // a non-base level's extent is not a descriptor field - and drops exactly that level // (P4a final review C-1); the client's mask republish passes null on purpose. - // - The stored descriptor's BindMask and ImageBindableHint ARE updated - BindMask is - // sticky and therefore ORed, never replaced - and the twin re-derives its storage - // flags from the new mask on its next sync, recreating backend storage only where the - // backend actually needs it. The record itself is not a request to recreate. + // - The stored descriptor's BindMask and ImageBindableHint ARE updated: the applier + // replaces the descriptor WHOLE with the one the client sent (PipeApply.cpp), and the + // mask in it is the CLIENT's sticky OR (TextureEmit.h's entry, never cleared), so the + // replacement can never lose a bit the record once carried. The twin re-derives its + // storage flags from the new mask on its next sync, recreating backend storage only + // where the backend actually needs it. The record itself is not a request to recreate. // // THE STORAGE-DEFINING FIELD SET, named here so that neither side has to guess and a // later field cannot join it by silence. It is every MGPResourceDesc member except the diff --git a/MobileGL/MG_Pipe/PipeApply.cpp b/MobileGL/MG_Pipe/PipeApply.cpp index e4287f35..7ac25323 100644 --- a/MobileGL/MG_Pipe/PipeApply.cpp +++ b/MobileGL/MG_Pipe/PipeApply.cpp @@ -2479,11 +2479,11 @@ namespace MobileGL::MG_Pipe { record->Gen = gen; } - void MGPipeApplySetTextureParams(const MGPTextureParams& params) { + Bool MGPipeApplySetTextureParams(const MGPTextureParams& params) { // P4a's belt, and FIRST here because this call's first act is a resolution: with no // consumer no texture create was accepted, so resolving would report the absence as // RefusedObjectCalls - the counter that means a seam defect - for the designed state. - if (NoP4aConsumer()) return; + if (NoP4aConsumer()) return false; // ADDRESSED BY RESOURCE AND BY NOTHING ELSE, which is the whole point of the call: a // texture that is only an FBO attachment, only an image-unit binding or only a @@ -2492,7 +2492,7 @@ namespace MobileGL::MG_Pipe { // moment the parameters move, whether or not anything is bound. MGPipeResourceRecord* record = ResolveObject(g_applier.TextureResources, "set_texture_params", params.Res); - if (record == nullptr) return; + if (record == nullptr) return false; // EVERY ITextureObject OWNS A SamplerObject, so the built-in sampler CSO is not // optional and a null handle is not "no sampler" - it is a record that would have the @@ -2505,7 +2505,7 @@ namespace MobileGL::MG_Pipe { " set_texture_params {slot=%u, gen=%u, glName=%u}: the record names no " "built-in sampler CSO, and every texture object owns one", params.Res.Slot, params.Res.Gen, record->Desc.GlNameForDiag); - return; + return false; } // AND THE CSO IT NAMES IS NOT RESOLVED. The sampler subsystem is its own bit and may be // clear while the texture bit is set, so a record that names a CSO this applier has not @@ -2520,6 +2520,7 @@ namespace MobileGL::MG_Pipe { // bytes are CARRIED, never cleared here: the server ORs them into its own flags and // clears its own copy, and the client never clears a server flag. ++record->ParamsSerial; + return true; } // The three of them, and NO STAGE DIMENSION on any of them: MobileGL's texture-unit space diff --git a/MobileGL/MG_Pipe/PipeApply.h b/MobileGL/MG_Pipe/PipeApply.h index 0fc8f065..e736f06b 100644 --- a/MobileGL/MG_Pipe/PipeApply.h +++ b/MobileGL/MG_Pipe/PipeApply.h @@ -1013,7 +1013,14 @@ namespace MobileGL::MG_Pipe { // glCopyImageSubData endpoint carry its parameters at all. params.BuiltinSampler may never // be the null handle - every ITextureObject owns a sampler object - so a null is // Fatal{ProtocolCorruption} rather than "no sampler". - void MGPipeApplySetTextureParams(const MGPTextureParams& params); + // + // Returns true when the record took the parameters (P4a final review m-1, audit F-7): the + // emitter's version latch advances on this answer and on nothing else, the way the + // sub-data and respecify paths latch on theirs, so a refused record - no consumer, no + // record for the handle, a null sampler - is re-sent at the next call rather than at the + // next glTexParameter*. Source-compatible for the same reason the three resource returns + // are: a Bool is ignorable and gen_pipe never parses this header. + Bool MGPipeApplySetTextureParams(const MGPTextureParams& params); // set_sampler_views / bind_sampler_states / set_shader_images: `tail` is hdr.Count entries // starting at hdr.Start, and hdr.Start + hdr.Count above the unit bound is diff --git a/MobileGL/MG_Test/Pipe/TextureEmitTest.cpp b/MobileGL/MG_Test/Pipe/TextureEmitTest.cpp index 6d25b401..cc8874bc 100644 --- a/MobileGL/MG_Test/Pipe/TextureEmitTest.cpp +++ b/MobileGL/MG_Test/Pipe/TextureEmitTest.cpp @@ -223,6 +223,7 @@ TEST(TextureEmit, TheEmitterIsOneNeverDestroyedProcessSingleton) { X(TextureEmit, ADeadTexturesHandleResolvesToNothingAndLeavesTheDrainList) \ X(TextureEmit, ATextureRecycledOntoADeadSlotDoesNotInheritTheDrainEntry) \ X(TextureEmit, ADeadRenderbuffersEntryIsRetiredWithItsSlot) \ + X(TextureEmit, ARefusedParamsRecordDoesNotAdvanceTheLatch) \ X(TextureEmit, ADeadTexturesSamplerViewLatchIsRetiredAtItsDeath) #define MGL_DECLARE_PULL_SKIP(Suite, Name) \ @@ -1498,6 +1499,42 @@ TEST(TextureEmit, ADeadRenderbuffersEntryIsRetiredWithItsSlot) { EXPECT_EQ(Textures().RenderbufferBindMask(successorHandle), 0u); } +// ============================ 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. +TEST(TextureEmit, ARefusedParamsRecordDoesNotAdvanceTheLatch) { + TextureScope scope; + const auto texture = MakeTexture2D(95, 8); + const MGPipeHandle handle = Textures().FindTexture(*texture); + ASSERT_NE(AppliedTexture(handle), nullptr); + + // 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(); + texture->GetSamplerObject()->SetLodBias(0.5f); + const Uint64 paramsBefore = Textures().ParamCount(); + MG_Pipe::MGPipeEmitTextureParams(*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"; + + // 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. + 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"; + EXPECT_EQ(record->Params.LodBias, 0.5f); +} + #endif // MOBILEGL_PIPE_PUSH // =========================================================================================