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

[Feat] (MG_Pipe): give resource_respecify's redefinition scope a wire carrier in the descriptor's two existing pad fields, so a per-level glTexImage cannot cross as a whole-resource redefinition and eat every other level's pending upload

Browse Source
This commit is contained in:
rereview
2026-09-11 13:23:46 -04:00
parent b4bbcc113a
commit 45b759e7a5
3 changed files with 107 additions and 3 deletions
Download Patch File Download Diff File Expand all files Collapse all files
MobileGL/MG_Pipe/MGPipeTypes.h
+75 -2
View File
@@ -300,7 +300,30 @@ namespace MobileGL::MG_Pipe {
Uint32 StorageFlags; // glBufferStorage flags
Uint8 HasDefinedContent; // false after a NULL-data respecify - STORAGE-DEFINING
Uint8 ImageBindableHint; // client-side everImageBound; pre-emptive allocation
Uint16 Pad0;
// THE SCOPE OF A RESPECIFY (P5 R-13.3b, CONTRACT-P5.md table 1 row 19b). Zero means
// "this respecify redefines the WHOLE resource"; non-zero means it redefines exactly
// the (RespecifiedUploadTarget, RespecifiedLevel) pair below and nothing else.
//
// It exists because the applier ALREADY takes that scope - as a trailing
// const MGPRespecifiedLevel* (PipeApply.h:792-795) - and MGPResourceDesc could not
// express it, so it was the second of resource_respecify's two companions with no wire
// carrier. The difference is not cosmetic: a null scope drops EVERY pending upload,
// because every level's coordinate system has just been replaced, while a per-level
// one drops only that key. Crossing without the scope would make every mutable
// per-level glTexImage*D on the far side look like a whole-resource redefinition and
// silently eat the texels of every other level - exactly the loss the server-side
// pending-upload set exists to prevent.
//
// READ IT THROUGH THE THREE HELPERS BELOW, never by touching the fields: the
// presence byte and the pair are one value in three pieces, and an open-coded reader
// that forgets the presence byte reads level 0 of upload target 0 as a real scope.
//
// NOT STORAGE-DEFINING, and not metadata either: it does not describe the resource at
// all, it describes what this CALL replaces. MGPipeResourceRespecifyNeedsAck and the
// storage-defining field set below are unaffected by it, which is why it is not named
// in either.
Uint8 HasRespecifiedLevel;
Uint8 Pad0;
// ImageBindableHint and BindMask above are the two METADATA fields the rule exists
// for: a respecify that moves only them - every storage-defining field equal to the
// stored descriptor - is a metadata update, with no reallocation ack and no
@@ -312,12 +335,62 @@ namespace MobileGL::MG_Pipe {
// of a content hash (section 4.2.1). Widened from the plan's two bytes, which
// cannot hold one.
Uint32 GlNameForDiag;
Uint32 Pad1;
// The pair HasRespecifiedLevel above gates. Byte for byte the two members of
// MGPRespecifiedLevel (PipeApply.h:792-795), in that order, so the applier's existing
// struct and this carrier are one layout - but spelled as two Uint16s rather than as
// that type, because PipeApply.h includes THIS header and not the other way round.
Uint16 RespecifiedUploadTarget;
Uint16 RespecifiedLevel;
MGPipeHandle ViewOf; // storage owner for a texture view
MGPipeHandle BufferForTexBuffer; // texture-buffer backing store
Uint64 BufOffset, BufSize; // kWholeBuffer == ~0, resolved live
};
MGP_ASSERT_POD(MGPResourceDesc, 88);
// The scope fields went into the two existing pads, so the descriptor did not grow and this
// number did not move. That was the deciding argument against giving MGPResourceDesc an
// MGPBlobRef for the OTHER uncarried companion (initialBytes): a blobref is 24 bytes, the
// pads are 6, and it would have taken the struct to 112. The scope needs 5 and fits.
static_assert(offsetof(MGPResourceDesc, RespecifiedUploadTarget) + sizeof(Uint16) ==
offsetof(MGPResourceDesc, RespecifiedLevel),
"the respecify scope pair must stay adjacent and in MGPRespecifiedLevel's order");
// ---- the only supported reads of the respecify scope --------------------------------
//
// Three fields, one value. Open-coding them is how a reader that forgets the presence byte
// turns "whole resource" into "upload target 0, level 0" - a real scope, and the wrong one.
// True when this respecify replaces the whole resource, which is every glBufferData, every
// glBufferStorage, every glTexStorage* and every texture view. The applier drops every
// pending upload for it.
inline constexpr Bool MGPipeRespecifyIsWholeResource(const MGPResourceDesc& desc) {
return desc.HasRespecifiedLevel == 0;
}
// The single (uploadTarget, level) a per-level respecify replaces. Reading either half of a
// whole-resource descriptor is a caller error; both answer 0 so that a misuse is at least
// deterministic rather than whatever the pad happened to hold.
inline constexpr Uint16 MGPipeRespecifiedUploadTargetOf(const MGPResourceDesc& desc) {
return MGPipeRespecifyIsWholeResource(desc) ? Uint16(0) : desc.RespecifiedUploadTarget;
}
inline constexpr Uint16 MGPipeRespecifiedLevelOf(const MGPResourceDesc& desc) {
return MGPipeRespecifyIsWholeResource(desc) ? Uint16(0) : desc.RespecifiedLevel;
}
// The two writers. A producer sets the scope with one call so the presence byte cannot be
// left behind, and clears it with the other; a descriptor built by value-initialization is
// already whole-resource, which is the safe default and the only one P5 produces.
inline constexpr void MGPipeSetRespecifiedLevel(MGPResourceDesc& desc, Uint16 uploadTarget,
Uint16 level) {
desc.HasRespecifiedLevel = 1;
desc.RespecifiedUploadTarget = uploadTarget;
desc.RespecifiedLevel = level;
}
inline constexpr void MGPipeClearRespecifiedLevel(MGPResourceDesc& desc) {
desc.HasRespecifiedLevel = 0;
desc.RespecifiedUploadTarget = 0;
desc.RespecifiedLevel = 0;
}
inline constexpr Uint64 kMGPipeWholeBuffer = ~0ull;
struct MGPFenceWait {
MobileGL/MG_Pipe/PipeApply.cpp
+25
View File
@@ -852,6 +852,30 @@ namespace MobileGL::MG_Pipe {
void PinNoLiveHostWrites(const MGPipeResourceRecord&, MGPipeHandle, const char*) {}
#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.
//
// 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) {
if (MGPipeRespecifyIsWholeResource(desc)) 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)));
}
#else
void PinWholeResourceRespecifyScope(const MGPResourceDesc&, MGPipeHandle, const char*) {}
#endif
// The one gate every content-carrying buffer write goes through. resource_subdata and
// 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
@@ -1628,6 +1652,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");
// 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
MobileGL/MG_Pipe/PipeFields.def
+7 -1
View File
@@ -41,10 +41,16 @@
#define MGP_FIELDS_MGPCaps(F) \
F(Dynamic) F(CallMask) F(FormatCapabilities) F(RendererInfo)
// The three respecify-scope fields are listed like any other member: they occupy what used to be
// Pad0's high byte and Pad1, and the generator's cover check only excludes members actually
// NAMED Pad*, so leaving them out would fail --check rather than pass it. They are also real
// G4 comparands - a verify build that let the scope differ silently would be blind to the one
// thing that decides whether a respecify drops one pending upload or all of them.
#define MGP_FIELDS_MGPResourceDesc(F) \
F(Resource) F(Target) F(StorageKind) F(BindMask) F(InternalFormat) F(Width) F(Height) F(Depth) \
F(ArrayLayers) F(Levels) F(Samples) F(FixedSampleLocations) F(Immutable) F(Usage) F(StorageFlags) \
F(HasDefinedContent) F(ImageBindableHint) F(GlNameForDiag) F(ViewOf) F(BufferForTexBuffer) \
F(HasDefinedContent) F(ImageBindableHint) F(HasRespecifiedLevel) F(GlNameForDiag) \
F(RespecifiedUploadTarget) F(RespecifiedLevel) F(ViewOf) F(BufferForTexBuffer) \
F(BufOffset) F(BufSize)
#define MGP_FIELDS_MGPFenceWait(F) \