swung0x48/MobileGL
1
0
Fork 0
mirror of https://github.com/MobileGL-Dev/MobileGL synced 2026-09-09 04:38:30 +09:00
Code Issues Packages Projects Releases Wiki Activity

[Fix] (Pipe): scope a respecify's pending-upload clear to the level it redefines, answer the emitter whether a sub-data record was accepted, and refuse a resource target that names no texture

Browse Source
This commit is contained in:
swung0x48
2026-09-08 15:07:23 -04:00
parent ae1a1c503f
commit c9958075e8
2 changed files with 167 additions and 29 deletions
Download Patch File Download Diff File Expand all files Collapse all files
MobileGL/MG_Pipe/PipeApply.cpp
+100 -27
View File
@@ -502,14 +502,33 @@ namespace MobileGL::MG_Pipe {
}
// A sub-data record's own discriminator, and it is DELIBERATELY NOT the table selector
// above: MGPSubData::Target is the UPLOAD target - a cube face is one, and those are
// not MGPipeResourceTarget enumerators - so the only thing it can be asked is the one
// question that has an answer for every value. kMGPipeResourceTargetBuffer is 0 and no
// texture upload target is, which is the contract the emitter is held to.
// above: MGPSubData::Target carries the UPLOAD target as well - a cube face is one, and
// those are not MGPipeResourceTarget enumerators - so the buffer question is asked of
// the whole field, which is the one question that has an answer for every value.
// kMGPipeResourceTargetBuffer is 0, a buffer has no upload target to pack beside it, and
// no texture emission leaves the whole field 0: that is the contract the emitter is
// held to.
Bool SubDataNamesABuffer(const MGPSubData& record) {
return record.Target == kMGPipeResourceTargetBuffer;
}
// THE VALUE SPACE OF MGPSubData::Target, written down here because the record itself does
// not say it and package B is about to encode cube faces into the same field. It is
// PACKED (integrator ruling ID-12, the DV-3 seam): the LOW byte is the MGPipeResourceTarget
// that owns the storage and the HIGH byte is the TextureUploadTarget the upload names -
// which is what makes the packing necessary at all, since TextureUploadTarget::Texture1D
// is 0 and the bare enumerator would collide with Buffer. Both halves are therefore
// free to take any value their own enum defines and neither may be read without the mask.
//
// MGPipeTypes.h IS SUPPOSED TO OWN THE THREE HELPERS (MGPipePackSubDataTarget /
// MGPipeSubDataResourceTargetOf / MGPipeSubDataUploadTargetOf, contract commit c0c);
// they are NOT on this package's base (feat/disaggregated = c0 + c0b), so this is the
// local decode and it is one line to retire the day c0c lands. The applier still matches
// the WHOLE field for the buffer question above, exactly as ID-12 says it does.
Uint16 SubDataResourceTargetOf(const MGPSubData& record) {
return static_cast<Uint16>(record.Target & 0x00FFu);
}
// WHY A DEAD HANDLE IS NOT A TRIP WIRE HERE, and the bounds faults below are - and
// why it is nonetheless COUNTED rather than silently dropped.
//
@@ -785,9 +804,13 @@ namespace MobileGL::MG_Pipe {
// buffer_subdata_resident differ only in which backend hook takes the bytes and in the
// fact that one of them is allowed to be absent, so a second copy of this arithmetic
// would be a second place to get it wrong.
void ApplyBufferWrite(const char* call, const MGPSubData& record, const void* bytes, Bool resident) {
// RETURNS THE ACCEPTANCE, for D-D5 step 1's reason: the emitter clears its own dirty
// state on the strength of this answer and neither refusal path is otherwise visible
// from the call site (a dead handle is a counted no-op, a corrupt record is a Fatal
// that deliberately moves no counter).
Bool ApplyBufferWrite(const char* call, const MGPSubData& record, const void* bytes, Bool resident) {
MGPipeResourceRecord* stored = ResolveResource(call, record.Res);
if (stored == nullptr) return;
if (stored == nullptr) return false;
const Uint64 offset = MGPipeSubDataBufferOffset(record);
const Uint64 size = MGPipeSubDataBufferSize(record);
@@ -803,7 +826,7 @@ namespace MobileGL::MG_Pipe {
call, record.Res.Slot, record.Res.Gen, stored->Desc.GlNameForDiag,
fault, static_cast<unsigned long long>(offset),
static_cast<unsigned long long>(size), stored->Desc.Width);
return;
return false;
}
PinNoLiveHostWrites(*stored, record.Res, call);
@@ -813,7 +836,12 @@ namespace MobileGL::MG_Pipe {
// draw would re-upload what it had just landed.
++stored->Serial;
if (g_resourceOps == nullptr) return;
// THE RANGE IS LANDED FROM HERE ON, so every path below returns true: whether a
// backend table is registered, and whether it implements the optional resident
// hook, is a property of the BUILD and not of the record. An emitter that read
// "not accepted" off an unregistered table would keep re-sending a write the
// applier has already taken responsibility for.
if (g_resourceOps == nullptr) return true;
if (resident) {
// kOptional, and the frontend already checks the same way for the table this
// one replaces: a backend that does not implement the resident path leaves the
@@ -821,9 +849,10 @@ namespace MobileGL::MG_Pipe {
if (g_resourceOps->SubDataResident != nullptr) {
g_resourceOps->SubDataResident(record.Res, record, bytes);
}
return;
return true;
}
if (g_resourceOps->SubData != nullptr) g_resourceOps->SubData(record.Res, record, bytes);
return true;
}
// The texture half of resource_subdata, and it DISPATCHES TO NOBODY. Nothing in this
@@ -831,10 +860,10 @@ namespace MobileGL::MG_Pipe {
// and Espryt uploads it at its own sync point, out of the accumulated set below. So the
// whole of this function is the gate, the accumulation and the serial - which is also
// why MGPipeResourceOps did not have to grow a member for it.
void ApplyTextureUpload(const MGPSubData& record, const void* bytes, const MGPSubRegion* regions) {
Bool ApplyTextureUpload(const MGPSubData& record, const void* bytes, const MGPSubRegion* regions) {
MGPipeResourceRecord* stored =
ResolveResourceIn(g_applier.TextureResources, "resource_subdata", record.Res);
if (stored == nullptr) return;
if (stored == nullptr) return false;
const char* fault = SubDataTextureFault(record, regions);
// A whole-level upload declares no regions and a non-empty box; a record that
@@ -854,7 +883,7 @@ namespace MobileGL::MG_Pipe {
record.Target, record.Level, record.UnionBox.X, record.UnionBox.Y,
record.UnionBox.Z, record.UnionBox.W, record.UnionBox.H,
record.UnionBox.D, record.RegionCount);
return;
return false;
}
if (!AccumulatePendingUpload(*stored, record, regions)) {
MGP_TRIP_WIRE_REPORT("MGPipe: " MGP_TRIP_WIRE_TAG("ProtocolCorruption")
@@ -863,14 +892,16 @@ namespace MobileGL::MG_Pipe {
"have (%u)",
record.Res.Slot, record.Res.Gen, stored->Desc.GlNameForDiag,
kMGPipeMaxPendingUploads);
return;
return false;
}
// The serial moves for the buffer half's reason: the twin stamps its own synced
// serial from inside the sync that reads this record, so a bump afterwards would
// leave it one mutation behind and the next draw would re-upload what it had just
// landed. The ACCEPTANCE is what the client reads to clear its own dirty flag - the
// record was accumulated, so the texels are the server's now.
// record was accumulated, so the texels are the server's now, and that is the true
// this returns.
++stored->Serial;
return true;
}
// ----------------------------------------------------------------------------
@@ -1455,7 +1486,8 @@ namespace MobileGL::MG_Pipe {
}
}
void MGPipeApplyResourceRespecify(const MGPResourceDesc& desc, const void* initialBytes) {
void MGPipeApplyResourceRespecify(const MGPResourceDesc& desc, const void* initialBytes,
const MGPRespecifiedLevel* level) {
Vector<MGPipeResourceRecord>* table = ResourceTableForTarget(desc.Target);
if (table == nullptr) {
MGP_TRIP_WIRE_REPORT("MGPipe: " MGP_TRIP_WIRE_TAG("ProtocolCorruption")
@@ -1475,14 +1507,38 @@ namespace MobileGL::MG_Pipe {
record->Desc = desc;
++record->Serial;
// A RESPECIFY REDEFINES THE STORE, SO THE PENDING UPLOADS AGAINST THE OLD ONE GO WITH
// IT. They are boxes and rects in a level's coordinate system, and the level that space
// belonged to has just been replaced - a box kept across a shrink would have Espryt
// 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
// (AllocateStorage then MarkStorageDirty), so what is still owed is re-emitted against
// the storage that now exists. A buffer never has one, so this is inert for P3a's half.
record->PendingUploads.clear();
// A RESPECIFY REDEFINES A STORE, SO THE PENDING UPLOADS AGAINST THE STORE IT REPLACES
// GO WITH IT - AND ONLY THOSE. They are boxes and rects in a level's coordinate system
// and that space has just been replaced, so a box kept across a shrink would have
// Espryt upload past the end of the new level. But WHICH storage a respecify replaces
// is the call's to say and the descriptor's to say nothing about:
//
// - `level == nullptr` is a WHOLE-RESOURCE redefinition - every glBufferData /
// glBufferStorage, every glTexStorage* (which defines every level at once), every
// texture view - and every key goes.
// - a non-null `level` is ONE glTexImage*D on a mutable texture, which redefines that
// (uploadTarget, level) AND NOTHING ELSE. Dropping the other keys here would lose
// texels the client no longer owes: MG_State's AllocateStorage / MarkStorageDirty
// are per (uploadTarget, level) too, so the re-mark this call causes covers ITS
// level only, while every other level's dirty flag was cleared at its own emission
// (D-D5 step 1). The canonical sequence that loses them is glTexImage2D(0, data) ->
// draw (the shape is emitted and accumulated; Espryt bails because the texture is
// not yet mipmap-complete, Managers.cpp's incomplete-texture early return, which is
// the arm this set exists for) -> glTexImage2D(1, data), which under a blanket
// clear destroys level 0's entry before anything ever uploaded it.
//
// A buffer never has a pending upload at all, so both arms are inert for P3a's half.
if (level == nullptr) {
record->PendingUploads.clear();
} else {
// The keys are unique by AccumulatePendingUpload's construction - it looks for the
// pair before it appends - so this erases at most one entry and stops.
for (auto it = record->PendingUploads.begin(); it != record->PendingUploads.end(); ++it) {
if (it->UploadTarget != level->UploadTarget || it->Level != level->Level) continue;
record->PendingUploads.erase(it);
break;
}
}
// resource_respecify is the catalogue's only kNeedsAck call, and the per-record half
// of that flag is MGPipeResourceRespecifyNeedsAck(desc): glBufferStorage is a real
@@ -1499,7 +1555,7 @@ namespace MobileGL::MG_Pipe {
}
}
void MGPipeApplyResourceSubData(const MGPSubData& record, const void* bytes,
Bool MGPipeApplyResourceSubData(const MGPSubData& record, const void* bytes,
const MGPSubRegion* regions) {
// ONE CALL, TWO HALVES, and the branch is one comparison. For a buffer the applier
// stores NOTHING per record - the contents are the backend's, and the range is the
@@ -1509,10 +1565,27 @@ namespace MobileGL::MG_Pipe {
if (SubDataNamesABuffer(record)) {
MOBILEGL_ASSERT(regions == nullptr,
"resource_subdata: the buffer half declares no sub-regions and carries none");
ApplyBufferWrite("resource_subdata", record, bytes, /*resident=*/false);
return;
return ApplyBufferWrite("resource_subdata", record, bytes, /*resident=*/false);
}
ApplyTextureUpload(record, bytes, regions);
// AND THE TEXTURE HALF IS NOT "EVERYTHING ELSE". The table selector's own argument -
// acting on the wrong table would have this applier act outside the storage the record
// names - is exactly the argument for refusing a resource-target half that names no
// texture, and Renderbuffer is the reachable one: it is enumerator 10 and therefore a
// perfectly well-formed MGPSubData::Target, so without this a renderbuffer record would
// accumulate a pending upload onto whatever TEXTURE holds slot N in the texture slot
// space. Renderbuffers have no sub-data path at all, so no correct client can produce
// one and this is a protocol fault rather than a dropped call.
const Uint16 resourceTarget = SubDataResourceTargetOf(record);
if (resourceTarget == kMGPipeResourceTargetBuffer ||
resourceTarget == static_cast<Uint16>(MGPipeResourceTarget::Renderbuffer) ||
resourceTarget >= static_cast<Uint16>(MGPipeResourceTarget::Count)) {
MGP_TRIP_WIRE_REPORT("MGPipe: " MGP_TRIP_WIRE_TAG("ProtocolCorruption")
" resource_subdata {slot=%u, gen=%u}: the record's resource target names "
"no texture to upload into (target=%u, resource target=%u)",
record.Res.Slot, record.Res.Gen, record.Target, resourceTarget);
return false;
}
return ApplyTextureUpload(record, bytes, regions);
}
void MGPipeApplyBufferSubDataResident(const MGPSubData& record, const void* bytes) {
MobileGL/MG_Pipe/PipeApply.h
+67 -2
View File
@@ -229,6 +229,22 @@ namespace MobileGL::MG_Pipe {
// cannot be recomputed after emission, so the tracker retains the pre-clear set and
// the comparator compares the emitted (UnionBox, RegionCount, Regions[]) against it
// field by field.
//
// THE SET IS KEYED (UploadTarget, Level) AND EVERY KEY IS INDEPENDENT OF EVERY OTHER.
// That is not a detail: a respecify redefines ONE level when it arrives from
// glTexImage*D (MGPipeApplyResourceRespecify's trailing MGPRespecifiedLevel*), so it
// may only drop that one key - the frontend's AllocateStorage / MarkStorageDirty are
// per (uploadTarget, level) too, and the other levels' dirty flags were cleared at
// THEIR emission, so nothing anywhere still owes them.
//
// THE ACCUMULATED RECT LIST MAY OVERLAP, AND A CONSUMER MUST TOLERATE THAT. Behind one
// level the frontend's own model is pairwise disjoint (MipmapStorage keeps it so), but
// this list CONCATENATES the lists of successive emissions and the applier's gate only
// asks that each rect be inside the record's own union box - so two emissions that
// touch the same texels leave two rects that do. Staging N rects therefore uploads
// those texels twice, which is a cost and never a correctness problem; nothing here
// de-duplicates and nothing downstream may assume "the frontend's model" means disjoint
// once the shapes have been accumulated.
struct PendingUpload {
Uint16 UploadTarget = 0;
Uint16 Level = 0;
@@ -607,6 +623,20 @@ namespace MobileGL::MG_Pipe {
// the backend and the gates compile against, and the records above are what they write
// into; the bodies land in the two commits that follow this one on the same branch.
// The scope of one resource_respecify, and it is an APPLIER-SIDE ARGUMENT and not a wire
// record: it is not in PipeFields.def, it crosses no payload, and the transport reads the
// scope off the call it is replaying rather than off a field. The two members mirror
// MGPipeResourceRecord::PendingUpload's key exactly, which is the only thing the applier
// does with them - so UploadTarget is MGPSubData::Target VERBATIM, the whole packed field
// (ID-12: low byte = MGPipeResourceTarget, high byte = the cube-face upload target), the
// same value the emission of that level put in the record. A per-face respecify therefore
// drops the face it redefines and leaves the other five standing, and a caller that packs
// the pair differently here than it packs it there simply matches nothing.
struct MGPRespecifiedLevel {
Uint16 UploadTarget = 0;
Uint16 Level = 0;
};
// resource_create: mints the record and marks the slot Live. Emitted from the buffer
// object's CONSTRUCTOR, so a resource exists before anything can name it; storage is
// defined lazily by the first respecify and a backend tolerates a resource with none.
@@ -614,7 +644,25 @@ namespace MobileGL::MG_Pipe {
// resource_respecify: replaces the stored descriptor and bumps Serial. `initialBytes` is
// the shadow when desc.HasDefinedContent, else null. kNeedsAck on the call,
// MGPipeResourceRespecifyNeedsAck(desc) per record - only an immutable store acks.
void MGPipeApplyResourceRespecify(const MGPResourceDesc& desc, const void* initialBytes);
//
// P4a: `level` IS THE SCOPE OF THE REDEFINITION, and MGPResourceDesc cannot carry it - the
// descriptor describes the resource, and a mutable texture redefines its levels ONE
// glTexImage*D AT A TIME. Null means "this respecify redefines the WHOLE resource" - every
// glBufferData / glBufferStorage, every glTexStorage*, every texture view - and drops every
// pending upload, which is right because every level's coordinate system has just been
// replaced. Non-null names the single (uploadTarget, level) the call redefines and drops
// ONLY that key: the frontend's AllocateStorage / MarkStorageDirty are per
// (uploadTarget, level) as well (MG_State/GLState/TextureState/TextureObject.h), so a
// glTexImage2D(level 1) re-marks level 1 AND NOTHING ELSE, while the levels already
// emitted had their client dirty flags cleared at THEIR emission (D-D5 step 1) and nothing
// anywhere still owes them. Clearing the whole set here would lose exactly those texels,
// silently, in every build - the loss the server-side set exists to prevent.
//
// Trailing and defaulted for W1's reason: P3a's buffer call site (PipeFill.cpp:691) and
// every existing case compile unchanged. PACKAGE B PASSES THE PAIR IT JUST ALLOCATED at
// every per-level respecify; it has both halves in hand at the AllocateStorage call site.
void MGPipeApplyResourceRespecify(const MGPResourceDesc& desc, const void* initialBytes,
const MGPRespecifiedLevel* level = nullptr);
// resource_subdata, buffer half: the destination range rides in the record's box through
// MGPipeSetSubDataBufferRange, and a false from that helper is where the EMITTER split.
// The applier stores nothing per record - contents are the backend's - and bumps Serial.
@@ -625,7 +673,24 @@ namespace MobileGL::MG_Pipe {
// applier's pending-upload set is (UnionBox, RegionCount, Regions[]) and the verify lane's
// retain mode compares all three. The buffer half declares no regions, so P3a's one call
// site and every existing case are unchanged by the default.
void MGPipeApplyResourceSubData(const MGPSubData& record, const void* bytes,
//
// THE RETURN IS THE ACCEPTANCE SIGNAL D-D5 STEP 1 NAMES: true when the record was stored -
// the buffer half landed its range, or the texture half accumulated the shape onto the
// record - and false when it was refused. THE EMITTER MUST GATE ITS DIRTY-FLAG CLEAR ON IT
// ("only for levels whose record the applier ACCEPTED"), because the two refusal paths are
// otherwise invisible to it: a dead or stale handle is a counted no-op and a corrupt record
// is a Fatal that does NOT move RefusedResourceCalls, so in a shipped push build a refused
// upload and an accumulated one are indistinguishable from the call site. A client that
// clears on the strength of having emitted drops those texels for good.
//
// THE RESOURCE-TARGET HALF OF record.Target PICKS THE HALF. MGPSubData::Target is PACKED
// (ID-12): low byte = MGPipeResourceTarget, high byte = the cube-face upload target. The
// buffer half is the whole field being 0 - the encoding the emitter is held to, since a
// buffer has no upload target - and the texture half additionally requires the low byte to
// name a TEXTURE target: Buffer, Renderbuffer and anything at or above
// MGPipeResourceTarget::Count are Fatal{ProtocolCorruption} rather than an upload onto
// whatever object holds that slot in the texture slot space.
Bool MGPipeApplyResourceSubData(const MGPSubData& record, const void* bytes,
const MGPSubRegion* regions = nullptr);
// buffer_subdata_resident: same shape; `bytes` is the application's staging store and is
// valid for the duration of the call only. The op-table entry may be null.