mirror of
https://github.com/MobileGL-Dev/MobileGL
synced 2026-09-13 22:58:30 +09:00
[Fix, Test] (MG_Impl, MG_Backend/DirectGLES): DSA by-name texture calls corrupted borrowed-slot memo pairings - process-wide glyph death under Iris
Field report: on Espryt with a BSL Iris pipeline built, every glyph in the game died - HUD, menu labels, even the vanilla title screen after leaving the world - while sprites kept rendering. Captured on-device (FCL apitrace rig), reproduced headlessly on llvmpipe, and pinned with a three-way replay: the same trace renders full text on raw Mesa desktop GL and on Magma, so the stream was intact and the execution was Espryt's. MECHANISM. WithTemporarilyBoundNamedTexture implements the by-name (DSA) texture entry points by binding the named texture onto the active unit's real slot, running the bound-texture code, and restoring - without moving the texture bind generation on either edge. DirectGLES's per-draw texture sync memo keys on that generation and BORROWS the slot pointer, so a memo built for texture A kept passing every key while a by-name call had texture B sitting in the slot: A's backend twin was driven with B's frontend object, and SyncMipmapsToBackend re-specified A's storage with B's shape. In the trace, a by-name upload to a BSL 2048x2048 map while the 16x16 lightmap was bound re-specified the lightmap's GL texture 2048x2048-NULL and back 16x16-NULL. The lightmap exists only as render output - no glTexSubImage2D ever touches it - so it stayed zero forever, and rendertype_text (vertexColor = Color * texelFetch(lightmap, ...)), alpha-discards every glyph. Background quads never sample the lightmap, which is why only text died. FIX, class-level, two layers: - Frontend (shared, closes the same hole for DirectVulkan's generation- keyed memos): the temporary bind and the restore each bump the texture bind generation (only when the slot actually changed), and the restore is an RAII scope guard so a throwing body can no longer leak the temporary binding - a second latent bug of the same class. Deliberately a generation bump and not a touched-unit note: the high-water mark must not chase by-name calls, and a completed bind/restore pair leaves the content epoch unchanged, so the cost is an owner-compare re-walk, not a memo rebuild. - DirectGLES defense in depth: both borrowed-pair memos (g_unitTextureSyncList, g_fboTextureSyncList) record which frontend texture each backend twin was paired with and re-check it before any replay (last in the key conjunction, behind the context-id compare). A stale pairing now costs a list rebuild instead of silent cross-texture storage corruption. Tests, both red with their own layer reverted: TextureTest.NamedTextureCallKeepsUnitBindingAccountingCoherent (the accounting contract) and DirectGLESTextureSync.UnitMemoRefusesToDriveA- TwinFromAnotherTexture (the corrupting sequence shape against a mock GLES table, asserting the resident texture's storage is never re-specified). 595/595 unit at default and with the async kill switch. Replay evidence: the captured BSL ESC-menu trace renders all text through Espryt post-fix, byte-comparable to the Mesa-direct and Magma replays; the no-shaderpack control is unchanged. A trace fixture wiring this scene into CI follows in a separate commit.
This commit is contained in:
@@ -86,17 +86,65 @@ namespace MobileGL::MG_Impl::GLImpl {
|
||||
return false;
|
||||
}
|
||||
|
||||
// DSA emulation: the by-name entry points are implemented by putting the named texture
|
||||
// on the active unit's slot for their target, running the classic bound-texture code,
|
||||
// then putting the previous binding back.
|
||||
//
|
||||
// Both of those binds are REAL changes to "which texture is bound at this unit" for as
|
||||
// long as `fn` runs, so both have to move the texture bind generation. Backends memoise
|
||||
// per-unit work keyed on that generation and BORROW the binding slot (they hold a
|
||||
// pointer to the slot's shared_ptr, not a copy); a slot swap the generation never saw
|
||||
// let such a memo replay texture A's backend twin against texture B now sitting in the
|
||||
// slot - which re-specified A's backend storage with B's shape and silently destroyed
|
||||
// A's GPU-rendered contents (Minecraft's lightmap, blanked by a by-name upload to an
|
||||
// Iris shadow map, which then discarded every glyph).
|
||||
//
|
||||
// The generation is bumped directly rather than through NoteTextureUnitTouched because
|
||||
// the touched-unit HIGH-WATER MARK must NOT move: glActiveTexture does not advance it,
|
||||
// so a DSA-only app would otherwise have every later draw walk up to the highest unit it
|
||||
// ever aimed a by-name call at. Not advancing it is also sufficient - a unit above the
|
||||
// mark is outside every memo's coverage and outside the epoch walk, so nothing can
|
||||
// observe the transient swap there; at or below it, the bump is exactly what makes the
|
||||
// epoch re-derive. Bumping only on a real change keeps the very common redundant case (a
|
||||
// by-name call on the texture already bound to the active unit) free.
|
||||
//
|
||||
// The restore is a scope guard because `fn` can throw (the unsupported-state paths use
|
||||
// THROW_EXCEPTION): leaking the temporary binding would leave the wrong texture bound to
|
||||
// a live unit for the rest of the context's life.
|
||||
template <typename Fn>
|
||||
void WithTemporarilyBoundNamedTexture(const SharedPtr<MG_State::GLState::ITextureObject>& textureObject,
|
||||
Fn&& fn) {
|
||||
if (!textureObject) return;
|
||||
|
||||
auto& activeUnit = MG_State::pGLContext->GetTextureUnitObject(MG_State::pGLContext->GetActiveTextureUnit());
|
||||
const Int activeUnitIndex = MG_State::pGLContext->GetActiveTextureUnit();
|
||||
auto& activeUnit = MG_State::pGLContext->GetTextureUnitObject(activeUnitIndex);
|
||||
auto& bindingSlot = activeUnit.GetBindingSlot(textureObject->GetTarget());
|
||||
const auto previousBinding = bindingSlot.GetBoundObject();
|
||||
bindingSlot.Bind(textureObject);
|
||||
|
||||
using SlotType = std::remove_reference_t<decltype(bindingSlot)>;
|
||||
class ScopedSlotRestore {
|
||||
public:
|
||||
ScopedSlotRestore(SlotType& slot, SharedPtr<MG_State::GLState::ITextureObject> previous)
|
||||
: m_slot(slot), m_previous(Move(previous)) {}
|
||||
~ScopedSlotRestore() {
|
||||
if (m_slot.Bind(m_previous)) {
|
||||
MG_State::pGLContext->BumpTextureBindGeneration();
|
||||
}
|
||||
}
|
||||
ScopedSlotRestore(const ScopedSlotRestore&) = delete;
|
||||
ScopedSlotRestore& operator=(const ScopedSlotRestore&) = delete;
|
||||
|
||||
private:
|
||||
SlotType& m_slot;
|
||||
SharedPtr<MG_State::GLState::ITextureObject> m_previous;
|
||||
};
|
||||
|
||||
if (bindingSlot.Bind(textureObject)) {
|
||||
MG_State::pGLContext->BumpTextureBindGeneration();
|
||||
}
|
||||
ScopedSlotRestore restore(bindingSlot, previousBinding);
|
||||
|
||||
fn(MG_Util::ConvertTextureTargetToGLEnum(textureObject->GetTarget()));
|
||||
bindingSlot.Bind(previousBinding);
|
||||
}
|
||||
|
||||
SizeT ComputeTextureStorageByteSize(TextureInternalFormat textureInternalFormat, GLsizei width, GLsizei height,
|
||||
|
||||
Reference in New Issue
Block a user