mirror of
https://github.com/MobileGL-Dev/MobileGL
synced 2026-09-10 05:08:31 +09:00
[Fix, Docs] (MG_Pipe, clientfb): latch set_texture_params on acceptance, and two comments - the emitter advanced its version latch before the applier answered, so a refused record (no record for the handle, no consumer) was not re-sent until the next glTexParameter* moved a version (final review m-1, audit F-7): the applier now returns whether the record took the parameters and the emitter latches on true, counting and naming a refusal; Config.h's mask table gains the fourth D-K2 row (0x400 requires 0x800), and MGPipeTypes.h's metadata paragraph says what the applier does (replaces the descriptor whole with the client's sticky OR) instead of "ORed, never replaced"
This commit is contained in:
+4
-1
@@ -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)
|
||||
|
||||
@@ -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;
|
||||
};
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
// =========================================================================================
|
||||
|
||||
Reference in New Issue
Block a user