[MG_Pipe, MG_Test] (P7): the respecify scope pin asks that a per-level producer cover the range it declares, so verify and split can be in the same build

This commit is contained in:
2026-09-22 20:34:50 -04:00
parent 75a64ce0da
commit a1d17566f7
2 changed files with 216 additions and 17 deletions
+64 -17
View File
@@ -947,27 +947,74 @@ namespace MobileGL::MG_Pipe {
#endif
#if MOBILEGL_PIPE_VERIFY
// P5's pin, the same shape and for the same reason. The respecify SCOPE now has a wire
// carrier (MGPResourceDesc's HasRespecifiedLevel + the pair) and no producer: every
// descriptor P5 builds is whole-resource, and the per-level scope still arrives the old
// way, as the trailing MGPRespecifiedLevel* this function does not look at.
// P5's pin, RELAXED BY P7 WAVE 3 (CONTRACT-P7 §6) - and relaxed rather than deleted,
// because the thing it was guarding is now reachable instead of hypothetical.
//
// The two must not disagree, and when a later package wires the carrier it will set the
// fields at a call site that also still passes the pointer - so the first thing that can
// go wrong is exactly one of the two moving. A verify build refuses to let that arrive
// unannounced, because a descriptor that says "whole resource" while the pointer says
// "level 1" drops every other level's pending upload with nothing saying so.
void PinWholeResourceRespecifyScope(const MGPResourceDesc& desc, MGPipeHandle res, const char* call) {
// WHAT IT USED TO SAY. "The respecify SCOPE has a wire carrier (MGPResourceDesc's
// HasRespecifiedLevel + the pair) and NO PRODUCER: every descriptor P5 builds is
// whole-resource, and the per-level scope still arrives the old way, as the trailing
// MGPRespecifiedLevel* this function does not look at." So any descriptor arriving with
// the carrier set was, by definition, a producer nobody had announced, and a verify
// build refused it.
//
// WHY THAT IS NOW WRONG, AND EXACTLY HOW FAR. The producer landed:
// Wire_Escape_ResourceRespecify (Client/WireTables.cpp:425-433) writes the carrier from
// the pointer through MGPipeSetRespecifiedLevel, and the server codec
// (Wire/PipeWireCodec.cpp:1677-1683) rebuilds the pointer from the carrier. So under a
// split transport EVERY per-level glTexImage*D arrives here with the carrier set, and
// the old pin made verify x split unmeasurable - it fired on the server's apply thread
// during bring-up, before a single verify case could arm:
//
// Fatal{PipeRespecifyScope} resource_respecify {slot=12, gen=0}: the descriptor
// carries a per-level respecify scope (target=258, level=0), and no path in this
// phase may set one
//
// WHAT SURVIVES, AND IT IS THE WHOLE INVARIANT. The pin never cared that the carrier
// was set; it cared that THE TWO HALVES OF THE SCOPE COULD DISAGREE - "the first thing
// that can go wrong is exactly one of the two moving". That hazard did not go away when
// the producer landed, it became LIVE, and it is silent: the applier below drops pending
// uploads by the POINTER, so a descriptor that declares (target, level) while this call
// hands over a null pointer takes the WHOLE-RESOURCE arm and eats every other level's
// texels, and one that hands over a different pair erases the wrong key and keeps the
// one the client stopped owing. Neither shows up as anything but missing pixels, a
// frame later, on whichever level the wire dropped.
//
// So the rule becomes CONTRACT-P7 §6's: A PRODUCER OF A PER-LEVEL SCOPE MUST COVER THE
// RANGE IT DECLARES. Carrier set => the call passes a pointer, and that pointer names
// exactly the declared (uploadTarget, level).
//
// ONE-DIRECTIONAL ON PURPOSE. A whole-resource descriptor says NOTHING about the
// pointer, and must not: that is the monolith shape and it is still the majority of the
// calls in the tree. MG_Impl/Pipe/TextureEmit.h builds a value-initialized descriptor
// (carrier clear, by MGPipeClearRespecifiedLevel's own "the safe default") and states
// the scope in the trailing pointer alone, which is legal, load-bearing and untouched
// here. Checking that direction too would red every monolith glTexImage*D in the tree
// and would be asserting a convention, not an invariant - the pointer is the caller's
// statement, and a caller that has not been converted to the carrier has not lied.
void PinRespecifyScopeCoversItsDeclaration(const MGPResourceDesc& desc,
const MGPRespecifiedLevel* level, MGPipeHandle res,
const char* call) {
if (MGPipeRespecifyIsWholeResource(desc)) return;
const Uint16 declaredTarget = MGPipeRespecifiedUploadTargetOf(desc);
const Uint16 declaredLevel = MGPipeRespecifiedLevelOf(desc);
if (level != nullptr && level->UploadTarget == declaredTarget &&
level->Level == declaredLevel) {
return;
}
MGP_TRIP_WIRE_REPORT("MGPipe: " MGP_TRIP_WIRE_TAG("PipeRespecifyScope")
" %s {slot=%u, gen=%u}: the descriptor carries a per-level respecify "
"scope (target=%u, level=%u), and no path in this phase may set one",
call, res.Slot, res.Gen,
static_cast<unsigned>(MGPipeRespecifiedUploadTargetOf(desc)),
static_cast<unsigned>(MGPipeRespecifiedLevelOf(desc)));
" %s {slot=%u, gen=%u}: the descriptor declares a per-level respecify "
"scope (target=%u, level=%u) and this call covers %s(target=%u, "
"level=%u), so a per-level producer is not covering the range it "
"declares",
call, res.Slot, res.Gen, static_cast<unsigned>(declaredTarget),
static_cast<unsigned>(declaredLevel),
level == nullptr ? "NOTHING " : "",
level == nullptr ? 0u : static_cast<unsigned>(level->UploadTarget),
level == nullptr ? 0u : static_cast<unsigned>(level->Level));
}
#else
void PinWholeResourceRespecifyScope(const MGPResourceDesc&, MGPipeHandle, const char*) {}
void PinRespecifyScopeCoversItsDeclaration(const MGPResourceDesc&, const MGPRespecifiedLevel*,
MGPipeHandle, const char*) {}
#endif
#if MOBILEGL_PIPE_VERIFY
@@ -1995,7 +2042,7 @@ namespace MobileGL::MG_Pipe {
MGPipeResourceRecord* record = ResolveResourceIn(*table, "resource_respecify", desc.Resource);
if (record == nullptr) return false;
PinNoLiveHostWrites(*record, desc.Resource, "resource_respecify");
PinWholeResourceRespecifyScope(desc, desc.Resource, "resource_respecify");
PinRespecifyScopeCoversItsDeclaration(desc, level, desc.Resource, "resource_respecify");
// IS THIS A REDEFINITION AT ALL? Asked BEFORE the descriptor is replaced, because the
// stored one is the only thing there is to compare against (ID-18 M4). See
+152
View File
@@ -1338,6 +1338,158 @@ namespace {
#endif
}
// ------------------------------------------------------------------------------------
// P7 wave 3 (CONTRACT-P7 §6): the respecify SCOPE pin, relaxed and still falsifiable.
//
// The pin used to say "no path in this phase may set a per-level scope on the descriptor",
// which stopped being true the day Wire_Escape_ResourceRespecify started writing the
// carrier: under split EVERY per-level glTexImage*D arrives with it set, and the pin fired
// on the server apply thread during bring-up, before one verify case could arm. It is now
// "a producer of a per-level scope must COVER the range it declares", and these two cases
// are the positive and the negative the contract asks for.
//
// A TEXTURE, NOT A BUFFER, and ScopedResourceOps with it: the scope is a texture concept,
// and every non-buffer row is refused with RefusedNoConsumer unless a backend table is
// registered - so without the scope this case would not reach the pin at all.
MGPResourceDesc Tex2DDesc(MGPipeHandle res, Uint32 extent, Uint32 levels, Uint32 glName) {
MGPResourceDesc desc{};
desc.Resource = res;
desc.Target = static_cast<Uint8>(MGPipeResourceTarget::Tex2D);
desc.InternalFormat = 1;
desc.Width = extent;
desc.Height = extent;
desc.Depth = 1;
desc.ArrayLayers = 1;
desc.Levels = levels;
desc.Samples = 1;
desc.GlNameForDiag = glName;
return desc;
}
// The packed (resource target, upload target) pair a Tex2D level's records carry. 0x0102 -
// and it is the very number the split bring-up's Fatal printed, which is how this case is
// known to be about the same shape.
const Uint16 kTex2DUpload = MGPipePackSubDataTarget(
static_cast<Uint32>(MGPipeResourceTarget::Tex2D),
static_cast<Uint32>(TextureUploadTarget::Texture2D));
MGPSubData TextureWrite(MGPipeHandle res, Uint16 uploadTarget, Uint16 level, Uint32 extent) {
MGPSubData record{};
record.Res = res;
record.Target = uploadTarget;
record.Level = level;
record.UnionBox = MGPBox{0, 0, 0, extent, extent, 1};
record.LevelWidth = extent;
record.LevelHeight = extent;
record.LevelDepth = 1;
return record;
}
Bool HasPendingUpload(Uint32 slot, Uint16 uploadTarget, Uint16 level) {
for (const auto& entry : MGPipeApplier().TextureResources[slot].PendingUploads) {
if (entry.UploadTarget == uploadTarget && entry.Level == level) return true;
}
return false;
}
// THE POSITIVE. A per-level producer that covers its declared range is accepted, and the
// accumulated texels of every OTHER level survive it - which is the thing the pin was
// standing in front of, checked rather than asserted.
TEST(ResourceEmit, APerLevelRespecifyCoveringItsDeclaredRangeKeepsTheOtherLevelsTexels) {
#if !MOBILEGL_PIPE_PUSH
GTEST_SKIP() << "MOBILEGL_PIPE_PUSH is off: there is no applier in this build";
#else
ApplierGuard guard;
ScopedResourceOps consumer;
const MGPipeHandle res{12, 1};
ASSERT_TRUE(MGPipeApplyResourceCreate(Tex2DDesc(res, 4, 2, 91)));
// Two levels with texels the server owes. The canonical sequence the pending set
// exists for: level 0 emitted, nothing uploaded yet, then level 1 redefined.
const Uint8 texels[64] = {};
ASSERT_TRUE(MGPipeApplyResourceSubData(TextureWrite(res, kTex2DUpload, 0, 4), texels, nullptr));
ASSERT_TRUE(MGPipeApplyResourceSubData(TextureWrite(res, kTex2DUpload, 1, 2), texels, nullptr));
ASSERT_TRUE(HasPendingUpload(res.Slot, kTex2DUpload, 0));
ASSERT_TRUE(HasPendingUpload(res.Slot, kTex2DUpload, 1));
// The split shape, both halves in agreement: the carrier says (kTex2DUpload, 1) and the
// trailing pointer the codec rebuilt from it says the same.
MGPResourceDesc perLevel = Tex2DDesc(res, 4, 2, 91);
MGPipeSetRespecifiedLevel(perLevel, kTex2DUpload, 1);
ASSERT_FALSE(MGPipeRespecifyIsWholeResource(perLevel));
const MGPRespecifiedLevel covered{kTex2DUpload, 1};
EXPECT_TRUE(MGPipeApplyResourceRespecify(perLevel, nullptr, &covered));
EXPECT_TRUE(HasPendingUpload(res.Slot, kTex2DUpload, 0))
<< "a per-level respecify may not eat the levels it did not redefine";
EXPECT_FALSE(HasPendingUpload(res.Slot, kTex2DUpload, 1))
<< "the level the call DID redefine has a new coordinate system, so its box goes";
EXPECT_EQ(ReadLog().find("PipeRespecifyScope"), std::string::npos)
<< "the relaxed pin must be silent on a producer that covers its declaration";
#endif
}
// THE NEGATIVE, and it is the whole reason the pin is relaxed rather than deleted. A
// producer that DECLARES a range and does not cover it is still refused by name: a null
// pointer takes the whole-resource arm and eats every other level's texels, and a pointer
// naming a different pair erases the wrong key and keeps the one the client stopped owing.
// Neither is visible as anything but missing pixels one frame later.
TEST(ResourceEmit, APerLevelRespecifyThatDoesNotCoverItsDeclaredRangeIsRefusedByName) {
#if !MOBILEGL_PIPE_PUSH
GTEST_SKIP() << "MOBILEGL_PIPE_PUSH is off: there is no applier in this build";
#elif !MOBILEGL_PIPE_VERIFY
// MOBILEGL_PIPE_VERIFY alone, NOT `POISON || VERIFY`, for PinNoLiveHostWrites' reason
// above: PinRespecifyScopeCoversItsDeclaration is compiled under `#if
// MOBILEGL_PIPE_VERIFY` only, so in a plain split build the wire genuinely is compiled
// out and this case must skip rather than expect a death that cannot happen.
GTEST_SKIP() << "Fatal{PipeRespecifyScope} is a MOBILEGL_PIPE_VERIFY wire and is compiled out here";
#elif !MGTEST_HAVE_FORK
GTEST_SKIP() << "no fork on this platform; the wire's verdict is std::abort()";
#else
ApplierGuard guard;
ScopedResourceOps consumer;
const MGPipeHandle res{13, 2};
ASSERT_TRUE(MGPipeApplyResourceCreate(Tex2DDesc(res, 4, 2, 92)));
struct Drive {
const char* What;
bool HasPointer;
Uint16 PointerTarget;
Uint16 PointerLevel;
const char* Wanted;
};
// Declared: (kTex2DUpload, 1). Covered: nothing, the wrong level, the wrong face.
const Drive drives[] = {
{"a null pointer", false, 0, 0, "covers NOTHING (target=0, level=0)"},
{"the wrong level", true, kTex2DUpload, 0, "covers (target=258, level=0)"},
{"the wrong upload target", true, static_cast<Uint16>(kTex2DUpload + 0x0100u), 1,
"covers (target=514, level=1)"},
};
for (const Drive& drive : drives) {
const ChildResult child = RunInChild([&res, &drive]() {
MGPResourceDesc perLevel = Tex2DDesc(res, 4, 2, 92);
MGPipeSetRespecifiedLevel(perLevel, kTex2DUpload, 1);
const MGPRespecifiedLevel named{drive.PointerTarget, drive.PointerLevel};
MGPipeApplyResourceRespecify(perLevel, nullptr,
drive.HasPointer ? &named : nullptr);
});
EXPECT_TRUE(DiedOfAbort(child))
<< drive.What << ": " << DescribeStatus(child) << "; log: " << child.Log;
const std::string named =
"Fatal{PipeRespecifyScope} resource_respecify {slot=13, gen=2}";
EXPECT_NE(child.Log.find(named), std::string::npos)
<< drive.What << ": wanted \"" << named << "\"; log: " << child.Log;
EXPECT_NE(child.Log.find("declares a per-level respecify scope (target=258, level=1)"),
std::string::npos)
<< drive.What << ": the refusal must print the DECLARED range; log: " << child.Log;
EXPECT_NE(child.Log.find(drive.Wanted), std::string::npos)
<< drive.What << ": wanted \"" << drive.Wanted << "\"; log: " << child.Log;
}
#endif
}
// M-D. The slot is the one number in the family that reaches an ALLOCATOR, so it is
// policed like every other: a slot outside the table's bound is Fatal{ProtocolCorruption}
// and never a resize. Removing the bound turns this case into a multi-gigabyte allocation.