diff --git a/MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp b/MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp index 4c780454..dbb790e5 100644 --- a/MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp +++ b/MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp @@ -13232,9 +13232,9 @@ namespace MobileGL::MG_Backend::DirectGLES { // // WHAT THE SKIP LOSES IS BOUNDED BY TWO FATALS, which is the whole reason it is allowed // to be a skip rather than a port: a later glGetTexImage of the destination served from - // the shadow is class C wave 3 (Fatal{UnmigratedVerb, "GetTexImage"}, P9) and a texture - // re-mint that re-uploads the level is Fatal{UnmigratedEmulation, "texture-remint-pull"} - // (Managers.cpp:5634). Neither can silently read the un-mirrored shadow. + // the shadow is class C wave 3 (Fatal{UnmigratedVerb, "GetTexImage"}, P9), and a texture + // re-mint under a transport reads the GPU level itself (RequireImageBindableStorageByHandle, + // P9 W2) rather than any shadow. Neither can silently read the un-mirrored shadow. // // BEHIND #if MOBILEGL_BUILD_DISAGGREGATED so the pull build's code does not move (G1), // and the arm is the TRANSPORT and not the build - build-split runs its unit and @@ -14952,10 +14952,10 @@ namespace MobileGL::MG_Backend::DirectGLES { // Session::Fail entirely, which is the funnel the census gate watches. // // THE FUNNEL KEEPS ITS TEETH FOR THE OTHER SITES: MGPipeUnmigratedEmulation still - // aborts for generate-mipmap-storage, generate-mipmap-cpu-fallback, - // generate-mipmap-cpu-filter and texture-remint-pull, which really do reach into a - // client address space. Only this call site is retired, and only because its own - // premise did not hold. + // aborts for generate-mipmap-storage, generate-mipmap-cpu-fallback and + // generate-mipmap-cpu-filter, which really do reach into a client address space. Only + // this call site is retired here, and only because its own premise did not hold + // (texture-remint-pull went the same way in P9 W2, for the same reason). if (MG_Config::Transport != MG_Config::TransportMode::Monolith) { if (shadow == nullptr) { MGLOG_E_ONCE("GetTexImage: no level shadow on this side for target=0x%x level=%d, and under an " diff --git a/MobileGL/MG_Backend/DirectGLES/DirectGLES.h b/MobileGL/MG_Backend/DirectGLES/DirectGLES.h index 94d72f68..f206d10e 100644 --- a/MobileGL/MG_Backend/DirectGLES/DirectGLES.h +++ b/MobileGL/MG_Backend/DirectGLES/DirectGLES.h @@ -32,6 +32,12 @@ namespace MobileGL::MG_Backend::DirectGLES { Bool ReadTextureLevelTight(GLuint texture, TextureTarget target, TextureUploadTarget uploadTarget, TextureInternalFormat logicalFormat, GLint level, const IntVec3& logicalExtent, Bool sourceUsesImageCarrier, GLenum format, GLenum type, Vector& bytes); + // Whether the NATIVE texture currently holds `level` of `uploadTarget` at `logicalExtent` + // (glGetTexLevelParameteriv). False for a level the driver never allocated - one defined after + // the texture's last sync - so a re-mint does not read what is not there. A query the driver + // refuses answers true, keeping the caller on its readback path. + Bool NativeTextureLevelHasExtent(GLuint texture, TextureTarget target, TextureUploadTarget uploadTarget, + GLint level, const IntVec3& logicalExtent); #endif void ClearBufferfi(GLenum buffer, GLint drawbuffer, GLfloat depth, GLint stencil); void ClearBufferfv(GLenum buffer, GLint drawbuffer, const GLfloat* value); diff --git a/MobileGL/MG_Backend/DirectGLES/Managers.cpp b/MobileGL/MG_Backend/DirectGLES/Managers.cpp index f298226e..17e0d561 100644 --- a/MobileGL/MG_Backend/DirectGLES/Managers.cpp +++ b/MobileGL/MG_Backend/DirectGLES/Managers.cpp @@ -6303,6 +6303,18 @@ namespace MobileGL::MG_Backend::DirectGLES { m_forceTextureParamsResync = true; return; } + // P9 W2 (notes/p9/W2-REMINT.md): EVERY BYTE THIS RE-MINT REPLAYS IS ALREADY ON THE + // SERVER. The GPU image holds what the GPU wrote (render targets, clears, copies, + // generated mips, image stores) and the staged store holds what the client uploaded; + // the client is never asked, so no pull protocol exists. Reading only the store would + // be wrong: a GPU-written level's store run is stale or absent - both host corpus + // re-mints (iris-photon, iris-derivative) are render targets with no staged bytes. + const Bool hadBackendStorage = m_isInitialized; + // The R-16 control for W2-b's fallback below: a readback the driver refuses cannot be + // produced on the host (llvmpipe renders every image format), so the test lane forces + // one. Read by the SERVER, like the MGITEST_MAGMA_FORCE_* knobs. + static const Bool s_forceReadbackFailure = + std::getenv("MGITEST_ESPRYT_FORCE_REMINT_READBACK_FAILURE") != nullptr; auto& store = MG_Remote::Server::ServerStagedTexture(); const Uint64 key = MG_Remote::Server::StagedTextureStore::KeyForHandle(res); const auto target = BufferImpl::StagedTextureTargetForPipeTarget(record.Desc.Target); @@ -6331,16 +6343,46 @@ namespace MobileGL::MG_Backend::DirectGLES { const Bool pendingWhole = pending && (pending->Regions.empty() ? coversWhole(pending->UnionBox) : std::any_of(pending->Regions.begin(), pending->Regions.end(), coversWhole)); - if (m_isInitialized && !(covered && pendingWhole)) { + // W2-a: ONLY A LEVEL THE DRIVER ACTUALLY HOLDS IS READ BACK. The store's + // Defined-ness follows the client's respecifies, the driver's follows this + // twin's last sync, and the two differ for a level defined (null data, or null + // data plus a partial upload) after that sync: the driver has no such level, so + // the GPU cannot have written it. A covered level then replays its staged run + // (the client's whole-level shadow, pending boxes included) and an uncovered one + // is undefined content with nothing to preserve. Reading it anyway was + // Fatal{ResourceUnavailable, "image-promotion-readback"} on legal GL. + const Bool nativeLevel = + m_isInitialized && + NativeTextureLevelHasExtent(m_backendTextureId, target, uploadTarget, + static_cast(level), extent); + if (nativeLevel && !(covered && pendingWhole)) { Vector snapshot; - if (!ReadTextureLevelTight(m_backendTextureId, target, uploadTarget, format, - static_cast(level), extent, /*sourceUsesImageCarrier=*/false, - canonicalFormat, canonicalType, snapshot)) { + const Bool readBack = + !s_forceReadbackFailure && + ReadTextureLevelTight(m_backendTextureId, target, uploadTarget, format, + static_cast(level), extent, + /*sourceUsesImageCarrier=*/false, canonicalFormat, + canonicalType, snapshot); + if (!readBack && !covered) { + // The GPU holds this level, the driver will not hand it back, and no + // CPU byte of it exists anywhere: the one case with no honest answer. MGLOG_F("MGPipe: Fatal{ResourceUnavailable, \"image-promotion-readback\"} {%u,%u} level=%u", res.Slot, res.Gen, level); std::abort(); } - if (pending) { + if (!readBack) { + // W2-b: the driver refused the readback (a format it cannot attach + // to a framebuffer - SNORM without EXT_render_snorm, 16-bit norm + // without EXT_texture_norm16, float without EXT_color_buffer_float) + // but the store covers the level. Its staged run is exactly what the + // monolith re-mint replays; texels the GPU wrote since that upload + // are not preserved, which is monolith's answer too. + MGLOG_W_ONCE("MGPipe: image promotion of texture {%u,%u} level %u: the driver " + "refused the readback (internal format %u); replaying the level's " + "staged bytes instead (remint-readback-fallback)", + res.Slot, res.Gen, level, record.Desc.InternalFormat); + } + if (readBack && pending) { // Native contents include prior draws/image // writes; the pending stage owns ONLY its boxes. // Merge those boxes before adopting a whole-level @@ -6375,7 +6417,9 @@ namespace MobileGL::MG_Backend::DirectGLES { merge(box.X, box.Y, box.Z, box.W, box.H, box.D); } } - store.Adopt(key, targetCode, levelCode, extent, snapshot.data(), snapshot.size()); + if (readBack) { + store.Adopt(key, targetCode, levelCode, extent, snapshot.data(), snapshot.size()); + } } // Undefined contents have no byte run to preserve. A // first allocation can still be image-bindable without @@ -6389,6 +6433,12 @@ namespace MobileGL::MG_Backend::DirectGLES { } } } + // W2-d: `trp=` counts a re-mint of storage the backend ALREADY held, on this arm as + // on the frontend one (which counts in RequireImageBindableStorage). The keep path + // above is not a re-mint and returns before this; a first allocation holds nothing. + if (hadBackendStorage && MG_Util::PipeStats::Enabled()) { + MG_Util::PipeStats::AddCalls(MG_Util::PipeStats::CallClass::TextureRemintPulls, 1); + } } #else (void)res; @@ -6412,10 +6462,10 @@ namespace MobileGL::MG_Backend::DirectGLES { return; } #if MOBILEGL_PIPE_PUSH - // Whether this transition re-mints storage that ALREADY EXISTED on the backend: that - // is the remint PULL (the levels below are replayed from the client's shadow to fill - // the new carrier), and it is what ROADMAP open question 2 counts. A texture reaching - // here uninitialised is allocated image-bindable up front and pulls nothing. + // Whether this transition re-mints storage that ALREADY EXISTED on the backend (the + // levels below are replayed from the client's shadow to fill the new carrier): what + // `trp=` counts. A texture reaching here uninitialised is allocated image-bindable up + // front and replays nothing. const Bool hadBackendStorage = m_isInitialized; #endif m_imageBindableStorageRequired = true; @@ -6431,28 +6481,22 @@ namespace MobileGL::MG_Backend::DirectGLES { // glGetTexImage, a draw that samples it, an FBO attach - which is why it survived so // long: the scenario that binds the image immediately after uploading never sees it. // - // P4a (D-M): THIS RE-DIRTY IS AN EMULATION THAT CANNOT SURVIVE A SPLIT, and it is - // the head of ARCHITECTURE.md:299-308's one new stall class. The server is reaching - // BACK into the client's own dirty model to ask for texels it does not hold; under - // split there is no client address space to reach into and the levels have to be - // PULLED across the reverse channel. P4a supplies mitigation 1 - the prevention - // half, MGPResourceDesc::ImageBindableHint on every create and respecify, so a - // texture that has ever been image-bound is allocated in the carrier from the start - // and never reaches this path - and NAMES the site. Mitigations 2-4 (the async pull, - // the bounded retention and the ResourceSubDataComplete terminator) and - // TextureRemintPullScenario are P9's, and P4a must not build half a terminator. + // THIS IS THE MONOLITH ARM ONLY. Under a transport the re-mint is the server's own + // (RequireImageBindableStorageByHandle: the GPU level read back, the staged store for + // what the client uploaded), and nothing selects this function there - the image + // units sync from the record, the metadata respecify calls the by-handle overload, and + // a frontend object reaching the loop below would meet the layer-1 guard + // (Fatal{RoleViolation, "texture-legacy-arm"}) first. P9 W2 retired the + // `texture-remint-pull` marker this comment used to describe: no pull protocol exists + // because no byte of a re-mint has to come from the client (notes/p9/W2-REMINT.md). // - // In monolith the code below keeps running exactly as it does today: the Fatal is a - // split-only arm and the monolith body of MGPipeUnmigratedEmulation is a no-op. + // KNOWN MONOLITH DEFECT, NOT FIXED HERE (W2-REMINT.md §6): replaying the client's shadow + // loses whatever the GPU wrote into the level since the upload - a render target, a + // clear, a generated mip - because the shadow never saw it. The split arm reads the GPU. // - // THE MARKER IS RAISED WHERE A LEVEL IS ACTUALLY REPLAYED, not on entry (review - // N-4). ImageBindableHint IS the prevention half of mitigation 1, and D's own - // metadata-respecify arm makes the hint's ARRIVAL the trigger for entering this - // function - so a glBindImageTexture on a texture whose storage is not yet defined - // reaches here, finds no defined level, replays nothing, and would nevertheless have - // counted (and, under split, aborted at) the very emulation the hint exists to - // prevent. What the marker is about is stated in its own comment above: reaching - // BACK into the client's dirty model for texels. No texels owed, nothing to mark. + // THE COUNT IS TAKEN WHERE A LEVEL IS ACTUALLY REPLAYED, not on entry (review N-4): a + // glBindImageTexture on a texture whose storage is not yet defined reaches here, finds + // no defined level and replays nothing. #if MOBILEGL_PIPE_PUSH // P4a (review M-1): ON THE HANDLE ARM THE RE-DIRTY HAS TO REACH THE APPLIER'S SET, // not only the frontend's model. The client cleared its own flags when it emitted @@ -6465,7 +6509,7 @@ namespace MobileGL::MG_Backend::DirectGLES { // Both models are written, not one: the frontend's because the legacy arm reads it // and because the client's own NoteLevelDirty hook rides on it, the applier's // because that is what this arm consumes. - Bool markedRemintPull = false; + Bool countedRemint = false; const MG_Pipe::MGPipeHandle rearmRes = TextureResourceSubsystemEnabled() ? [&]() { @@ -6485,41 +6529,6 @@ namespace MobileGL::MG_Backend::DirectGLES { "pending set - they would be allocated empty", stateTextureObject->GetExternalIndex()); } -#endif -#if MOBILEGL_BUILD_DISAGGREGATED - // P5c (gt): under an active transport the replay loop below reaches the layer-1 - // texture guard (MarkStorageDirty on a frontend object is Fatal{RoleViolation, - // "texture-legacy-arm"}) BEFORE the N-4 marker it owns - so the marker is hoisted - // here, ahead of the first guarded contact, with its semantics unchanged: raised - // only when a level would actually be replayed. The predicate is answered by the - // server's own staged-texture store rather than the frontend's extent walk: a - // Defined level has a non-zero extent and bytes on the frontend (the two conditions - // the loop tests), and an undefined one reads {0,0,0} and is skipped. A texture with - // no applier record (rearmRes null, the MGLOG_E_ONCE arm above) falls through to the - // guard, which names the same violation one level down. - if (MG_Config::Transport != MG_Config::TransportMode::Monolith && - !MG_Pipe::MGPipeHandleIsNull(rearmRes)) { - const auto* rearmRecord = PipeTextureRecordForHandle(rearmRes); - if (rearmRecord != nullptr) { - auto& rearmStore = MG_Remote::Server::ServerStagedTexture(); - const Uint64 rearmKey = MG_Remote::Server::StagedTextureStore::KeyForHandle(rearmRes); - Bool anyLevelWouldReplay = false; - for (const auto& uploadTarget : - BufferImpl::StagedUploadTargetsForPipeTarget(rearmRecord->Desc.Target)) { - for (Uint32 level = 0; level < rearmRecord->Desc.Levels; ++level) { - if (rearmStore.IsLevelDefined(rearmKey, static_cast(uploadTarget), - static_cast(level))) { - anyLevelWouldReplay = true; - break; - } - } - if (anyLevelWouldReplay) break; - } - if (anyLevelWouldReplay) { - MG_Pipe::MGPipeUnmigratedEmulation("texture-remint-pull"); - } - } - } #endif if (auto* mipmapObject = MG_State::GLState::AsMipmapTexture(stateTextureObject.get())) { const auto levelCount = mipmapObject->GetMipmapLevelCount(); @@ -6530,13 +6539,11 @@ namespace MobileGL::MG_Backend::DirectGLES { if (mipmapObject->GetMipmapByteSize(uploadTarget, level) == 0) continue; mipmapObject->MarkStorageDirty(uploadTarget, level, true); #if MOBILEGL_PIPE_PUSH - // N-4: one level owed is what makes this an unmigrated emulation. Once - // per transition, not once per level: the marker names the SITE. - if (!markedRemintPull) { - MG_Pipe::MGPipeUnmigratedEmulation("texture-remint-pull"); - markedRemintPull = true; - // THE COUNTER BEHIND ROADMAP OPEN QUESTION 2 (final review M-A): one per - // transition that replays a level of storage the backend already held. + // THE COUNTER BEHIND ROADMAP OPEN QUESTION 2 (final review M-A): once per + // transition that replays a level of storage the backend already held, + // not once per level. + if (!countedRemint) { + countedRemint = true; if (hadBackendStorage && MG_Util::PipeStats::Enabled()) { MG_Util::PipeStats::AddCalls(MG_Util::PipeStats::CallClass::TextureRemintPulls, 1); } diff --git a/MobileGL/MG_Backend/DirectGLES/Managers.h b/MobileGL/MG_Backend/DirectGLES/Managers.h index 296f17be..82a1b9f7 100644 --- a/MobileGL/MG_Backend/DirectGLES/Managers.h +++ b/MobileGL/MG_Backend/DirectGLES/Managers.h @@ -1963,7 +1963,10 @@ namespace MobileGL::MG_Backend::DirectGLES { // allocation, or capture the old native contents, merge pending // upload regions and re-arm the server's whole-level replay set. // First allocations use staged bytes directly. No frontend dirty - // flag, allocator or pixel-store object participates. + // flag, allocator or pixel-store object participates. P9 W2: a level + // the driver does not hold (defined after the last sync) is never read + // back, and a readback the driver refuses falls back to the staged + // level when the store covers it (notes/p9/W2-REMINT.md). void RequireImageBindableStorageByHandle(MG_Pipe::MGPipeHandle res, const MG_Pipe::MGPipeResourceRecord& record); #endif diff --git a/MobileGL/MG_Backend/DirectGLES/WireTextureReadback.inc b/MobileGL/MG_Backend/DirectGLES/WireTextureReadback.inc index 2c7dcaf5..b2baaf5a 100644 --- a/MobileGL/MG_Backend/DirectGLES/WireTextureReadback.inc +++ b/MobileGL/MG_Backend/DirectGLES/WireTextureReadback.inc @@ -280,6 +280,44 @@ return true; } + // P9 W2-a. The store's Defined-ness follows the client's respecifies; the driver's follows the + // twin's last sync. A re-mint asks this before reading a level back, because a level defined + // after that sync (null data, or null data plus a partial upload) is not on the driver yet and + // its FBO attachment is incomplete - which used to end the session at + // Fatal{ResourceUnavailable, "image-promotion-readback"} on legal GL. + Bool NativeTextureLevelHasExtent(GLuint texture, TextureTarget target, TextureUploadTarget uploadTarget, + GLint level, const IntVec3& logicalExtent) { + if (!texture || level < 0 || !g_GLESFuncs.glGetTexLevelParameteriv) return true; + const IntVec3 extent = TextureImpl::GetBackendUploadSize(target, logicalExtent); + const GLenum nativeTarget = TextureImpl::ConvertTextureTargetToBackendGLEnum(target); + GLenum binding = GL_NONE; + switch (nativeTarget) { + case GL_TEXTURE_2D: binding = GL_TEXTURE_BINDING_2D; break; + case GL_TEXTURE_3D: binding = GL_TEXTURE_BINDING_3D; break; + case GL_TEXTURE_2D_ARRAY: binding = GL_TEXTURE_BINDING_2D_ARRAY; break; + case GL_TEXTURE_CUBE_MAP: binding = GL_TEXTURE_BINDING_CUBE_MAP; break; + case GL_TEXTURE_CUBE_MAP_ARRAY: binding = GL_TEXTURE_BINDING_CUBE_MAP_ARRAY; break; + default: return true; + } + // A cube's levels are per face; every other target answers for the whole level. + const GLenum queryTarget = nativeTarget == GL_TEXTURE_CUBE_MAP + ? TextureImpl::ConvertTextureUploadTargetToBackendGLEnum(uploadTarget) : nativeTarget; + DrainESErrors(); + GLint previous = 0; + g_GLESFuncs.glGetIntegerv(binding, &previous); + g_GLESFuncs.glBindTexture(nativeTarget, texture); + GLint width = 0, height = 0, depth = 0; + g_GLESFuncs.glGetTexLevelParameteriv(queryTarget, level, GL_TEXTURE_WIDTH, &width); + g_GLESFuncs.glGetTexLevelParameteriv(queryTarget, level, GL_TEXTURE_HEIGHT, &height); + g_GLESFuncs.glGetTexLevelParameteriv(queryTarget, level, GL_TEXTURE_DEPTH, &depth); + const Bool answered = g_GLESFuncs.glGetError() == GL_NO_ERROR; + g_GLESFuncs.glBindTexture(nativeTarget, static_cast(previous)); + if (!answered) return true; + const Bool layered = nativeTarget == GL_TEXTURE_3D || nativeTarget == GL_TEXTURE_2D_ARRAY || + nativeTarget == GL_TEXTURE_CUBE_MAP_ARRAY; + return width == extent.x() && height == extent.y() && (!layered || depth == extent.z()); + } + Bool ReadTextureImageWire(const MG_Pipe::MGPReadbackInfo& info, Vector& bytes) { const auto* record = PipeTextureRecordForHandle(info.Res); if (!record) return false; diff --git a/MobileGL/MG_Impl/Pipe/TextureEmit.h b/MobileGL/MG_Impl/Pipe/TextureEmit.h index b970c6cb..9ed12cc6 100644 --- a/MobileGL/MG_Impl/Pipe/TextureEmit.h +++ b/MobileGL/MG_Impl/Pipe/TextureEmit.h @@ -233,9 +233,9 @@ namespace MobileGL::MG_Pipe { desc.StorageKind = MGPipeTextureStorageKindForTarget(texture.GetTarget()); desc.BindMask = bindMask; // STICKY AND FOREVER: everImageBound, the client-side answer MGPipeTypes.h asks for. - // It is the PREVENTION half of the texture-remint stall class - a texture the server - // knows may be image-bound is allocated image-bindable up front, so the re-mint that - // would have to pull its texels back never happens. + // It PREVENTS the texture re-mint - a texture the server knows may be image-bound is + // allocated image-bindable up front, so the server never has to read its levels back + // and re-upload them into a new carrier (P9 W2). desc.ImageBindableHint = (bindMask & kMGPipeBindShaderImage) != 0 ? 1 : 0; if (storageDefined) { const MGPipeTextureExtent extent = MGPipeTextureExtentOf(texture); @@ -702,8 +702,7 @@ namespace MobileGL::MG_Pipe { // mask rides resource_create and every resource_respecify - and an IMMUTABLE texture has // no further respecify, that being what immutable means - so for the canonical order // `glTexStorage2D(...); glBindImageTexture(...)` the applier's record kept - // ImageBindableHint = 0 for ever and the PREVENTION half of the texture-remint stall - // class never fired. So a mask that actually MOVES re-emits the stored descriptor with + // ImageBindableHint = 0 for ever and the re-mint PREVENTION never fired. So a mask that actually MOVES re-emits the stored descriptor with // the new mask: every storage-defining field is byte-identical to what the applier // holds, which is exactly the shape wire applies as a METADATA UPDATE - the descriptor // is replaced, no reallocation is acked, and NO pending upload is dropped, so a mask diff --git a/MobileGL/MG_Pipe/MGPipeCallbacks.h b/MobileGL/MG_Pipe/MGPipeCallbacks.h index 65036006..ce50ed84 100644 --- a/MobileGL/MG_Pipe/MGPipeCallbacks.h +++ b/MobileGL/MG_Pipe/MGPipeCallbacks.h @@ -15,12 +15,20 @@ // The backend -> frontend reverse channel, named (plan B section 7.1). // // Today this traffic is 95 call sites across 17 methods poked directly into frontend -// objects. gallium has no vocabulary for shadow writeback, GPU-write notification, texture -// re-send requests or default-framebuffer geometry, because in Mesa the state tracker and -// the driver share an address space. Naming them as eight callbacks plus one forward -// terminator (MGPipeContext::ResourceSubDataComplete) is the deliberate deviation (D8). +// objects. gallium has no vocabulary for shadow writeback, GPU-write notification or +// default-framebuffer geometry, because in Mesa the state tracker and the driver share an +// address space. Naming them as callbacks is the deliberate deviation (D8). // -// EIGHT, NOT THE TEN PLAN B WROTE. The tenth was OnXfbScatterReady, and it went with the +// SEVEN, NOT THE TEN PLAN B WROTE. Three went, each because nothing produces it. +// +// OnTexturePullRequest, the server asking the client to re-send a texture's levels after +// re-minting it (with op 50 ResourceSubDataComplete as the forward terminator). P9 W2 measured +// that no re-mint needs the client: Espryt's server reads the GPU level back and replays its own +// staged store, Magma copies GPU to GPU, on every arm (notes/p9/W2-REMINT.md). So the callback +// went; op 50 stays in the opcode table as a retired row (opcodes only ever grow) and its decoder +// still declines it. +// +// OnXfbScatterReady went with the // design it belonged to: plan B put the XFB scatter on the CLIENT (the server would hand // back the packed scratch and the client would run the patch loop over its own shadow), so // there had to be a callback that told the client the layout. P5c/P5f went the other way - @@ -31,8 +39,8 @@ // reader of this file has to rule out by hand, and the size assertion below made it look // load-bearing. // -// The ninth was OnLog, deleted in P9 W3 for the same reason: zero producers, zero consumers, -// no EventKind. Plan B pictured the server's log lines as reverse-channel records; they never +// And OnLog, deleted in P9 W3 for the same reason: zero producers, zero consumers, no +// EventKind. Plan B pictured the server's log lines as reverse-channel records; they never // were. They travel the CONTROL plane as LogLine frames (protocol.fbs), written by the server's // logger through its log forward (MG_Remote/Transport/LogForward.h) - which is also where the // severity policy this entry's comment promised ("<= WARN lossy, >= ERROR lossless and rate @@ -50,11 +58,6 @@ namespace MobileGL::MG_Pipe { void (*OnGpuWritten)(MGPipeHandle res, Uint rangeCount, const MGPRange* ranges); void (*OnBufferWriteback)(MGPipeHandle res, Uint64 offset, MGPBlobRef bytes); void (*OnTextureWriteback)(MGPipeHandle res, const MGPBox* box, MGPBlobRef bytes); - // The one new stall class in this design (D-B6): the server recast a texture and - // needs its texels back. The client answers with zero or more ResourceSubData - // records terminated by ResourceSubDataComplete carrying the same pullSerial. - void (*OnTexturePullRequest)(MGPipeHandle res, Uint16 target, Uint16 firstLevel, Uint16 levelCount, - Uint64 pullSerial); // SHAPE ONLY, never bytes: the client owns the CPU shadow and allocates the levels // itself. void (*OnMipLevelsGenerated)(MGPipeHandle res, Uint16 base, Uint16 count); @@ -64,9 +67,9 @@ namespace MobileGL::MG_Pipe { void (*OnCapsInvalidated)(); }; - // Eight, and the count is asserted so a ninth cannot be added without touching the + // Seven, and the count is asserted so an eighth cannot be added without touching the // transport's reverse-channel record table. - inline constexpr SizeT kMGPipeCallbackCount = 8; + inline constexpr SizeT kMGPipeCallbackCount = 7; static_assert(sizeof(MGPipeCallbacks) == kMGPipeCallbackCount * sizeof(void (*)()), "MGPipeCallbacks gained or lost a callback"); diff --git a/MobileGL/MG_Pipe/PipeApply.h b/MobileGL/MG_Pipe/PipeApply.h index ccd0f50f..68fbdc68 100644 --- a/MobileGL/MG_Pipe/PipeApply.h +++ b/MobileGL/MG_Pipe/PipeApply.h @@ -1217,8 +1217,8 @@ namespace MobileGL::MG_Pipe { // BindMask / ImageBindableHint bit reaches the applier only on a respecify, and an // IMMUTABLE texture has no further one - that is what immutable means - so the canonical // order (glTexStorage2D, then glBindImageTexture or an FBO attachment) would leave the - // record's hint at 0 for ever, and the hint is the PREVENTION half of the texture-remint - // stall class. So B re-emits the descriptor when the mask moves, and a record whose + // record's hint at 0 for ever, and the hint is what PREVENTS the texture re-mint (a + // server-side readback and re-upload, P9 W2). So B re-emits the descriptor when the mask moves, and a record whose // STORAGE-DEFINING fields all equal the stored descriptor's is applied as a metadata // update: // diff --git a/MobileGL/MG_Pipe/PipeMutation.h b/MobileGL/MG_Pipe/PipeMutation.h index 1e47a7b8..4382c15b 100644 --- a/MobileGL/MG_Pipe/PipeMutation.h +++ b/MobileGL/MG_Pipe/PipeMutation.h @@ -430,15 +430,16 @@ namespace MobileGL::MG_Pipe { // all, glBindImageTexture's state setter (TextureState.h, MG_State), which may include no // emit header at all. So the note goes through this door, exactly as the birth hooks do. // Nothing produced either bit before the fix round: ImageBindableHint was always 0, the - // metadata respecify (ID-18 M4) had no live trigger, and the remint pull the hint exists to + // metadata respecify (ID-18 M4) had no live trigger, and the re-mint the hint exists to // prevent was neither prevented nor counted. // // UNCONDITIONAL IN A PUSH BUILD, like the mints: the mask is CLIENT state the framebuffer // emitter ORs into whether or not the texture family is on, and the emission a mask move // causes (the metadata respecify) is gated inside the emitter on the family's own pair. void MGPipeNoteTextureBoundAs(MGPipeHandle texture, Uint32 bindBit); - // glBindImageTexture. The hint is the PREVENTION half of the texture-remint stall class - - // a texture the server knows may be image-bound is allocated image-bindable up front - so it + // glBindImageTexture. The hint PREVENTS the texture re-mint (a server-side readback and + // re-upload of every defined level, P9 W2) - a texture the server knows may be image-bound is + // allocated image-bindable up front - so it // has to reach the applier before the texture's first sync, i.e. at the bind itself, not at // the validate point's image walk (which notes it as well, D-A4's letter). void MGPipeNoteTextureImageBound(MG_State::GLState::ITextureObject& texture); diff --git a/MobileGL/MG_Remote/Wire/PipeWireCodec.cpp b/MobileGL/MG_Remote/Wire/PipeWireCodec.cpp index 45ff6250..677f95cb 100644 --- a/MobileGL/MG_Remote/Wire/PipeWireCodec.cpp +++ b/MobileGL/MG_Remote/Wire/PipeWireCodec.cpp @@ -2323,8 +2323,10 @@ namespace MobileGL::MG_Remote::Wire { } case MGPWireOp::ResourceSubDataComplete: - // The forward terminator of a SERVER-initiated texture pull (section 7.1). There - // is no client producer and no applier; the reverse channel is P7/P9. + // RETIRED (P9 W2). It was to be the forward terminator of a SERVER-initiated texture + // pull (section 7.1), and no such pull exists: a re-mint reads the GPU level and the + // server's own staged store (notes/p9/W2-REMINT.md). The row stays because opcodes + // only ever grow; no client emits it and a record that claims it is declined. return false; case MGPWireOp::ResourceFlushRange: diff --git a/MobileGL/MG_State/GLState/TextureState/TextureState.h b/MobileGL/MG_State/GLState/TextureState/TextureState.h index 1d7dc7e9..f7231ac2 100644 --- a/MobileGL/MG_State/GLState/TextureState/TextureState.h +++ b/MobileGL/MG_State/GLState/TextureState/TextureState.h @@ -35,9 +35,9 @@ namespace MobileGL::MG_State::GLState { ++Version; #if MOBILEGL_PIPE_PUSH // P4a D-A4 / final review M-A: the EARLIEST producer of kMGPipeBindShaderImage. The - // ImageBindableHint the bit feeds is the prevention half of the texture-remint stall - // class (a texture the server knows may be image-bound is allocated image-bindable - // up front), so it has to reach the applier before the texture's first sync - at + // ImageBindableHint the bit feeds prevents the texture re-mint (a texture the server + // knows may be image-bound is allocated image-bindable up front, P9 W2), so it has + // to reach the applier before the texture's first sync - at // the bind, not at the next validate point's image walk. Push-only through the // contract's door, like every other hook in this directory (G1). if (Texture) MG_Pipe::MGPipeNoteTextureImageBound(*Texture); diff --git a/MobileGL/MG_Test/Pipe/ImageEmitTest.cpp b/MobileGL/MG_Test/Pipe/ImageEmitTest.cpp index b7b11bf9..e1f107f0 100644 --- a/MobileGL/MG_Test/Pipe/ImageEmitTest.cpp +++ b/MobileGL/MG_Test/Pipe/ImageEmitTest.cpp @@ -614,8 +614,8 @@ void main() { imageStore(img, ivec2(0), vec4(1.0)); } // FINAL REVIEW M-A: glBindImageTexture IS THE EARLIEST PRODUCER OF kMGPipeBindShaderImage - // the bit the ImageBindableHint is derived from - and the emitted image set's walk is D-A4's - // (any texture named in an emitted MGPImageView). The hint is the PREVENTION half of the - // texture-remint stall class: a texture the server knows may be image-bound is allocated + // (any texture named in an emitted MGPImageView). The hint PREVENTS the texture re-mint + // (P9 W2): a texture the server knows may be image-bound is allocated // image-bindable up front, so it has to arrive before the first sync, i.e. at the bind. // Nothing produced the bit before the fix round. TEST(ImageEmit, AnImageBoundTextureIsMarkedShaderImageBoundAtTheBind) { diff --git a/MobileGL/MG_Test/Pipe/PipeCatalogueTest.cpp b/MobileGL/MG_Test/Pipe/PipeCatalogueTest.cpp index 41bba0ad..6c9a7f13 100644 --- a/MobileGL/MG_Test/Pipe/PipeCatalogueTest.cpp +++ b/MobileGL/MG_Test/Pipe/PipeCatalogueTest.cpp @@ -1109,19 +1109,21 @@ TEST(PipeCatalogue, PipelineSubsetMembersArePinned) { "ScissorTestEnabledMask"); } -// The reverse channel is exactly EIGHT callbacks: plan B's section 7.1 wrote ten, and the tenth +// The reverse channel is exactly SEVEN callbacks: plan B's section 7.1 wrote ten, and the tenth // (OnXfbScatterReady) went with the design it belonged to when P5c/P5f moved the XFB scatter to // the server's own staged shadow and its OnBufferWriteback return path. P3b/P4b espryt D1 slice 3 // deleted the declaration; this case is what makes the struct's shrink a measured fact rather // than a claim, since the static_assert beside kMGPipeCallbackCount only proves the two agree. -// P9 W3 deleted the ninth, OnLog, which never had a producer either: the server's log lines are -// LogLine frames on the control plane (MG_Remote/Transport/LogForward.h), not reverse records. +// P9 took two more, neither of which ever had a producer: W2 deleted OnTexturePullRequest (a +// texture re-mint never needs the client's texels - the server reads the GPU level and its own +// staged store, notes/p9/W2-REMINT.md), and W3 deleted OnLog (the server's log lines are LogLine +// frames on the control plane, MG_Remote/Transport/LogForward.h, not reverse records). // // THE CASE NAME STAYS "…HasTenCallbacks" DELIBERATELY. G2/G14 say the ctest name set only ever // grows, so renaming this would delete a name the gate is watching; the count it asserts is what // has to be right, and the comment is where the number lives. TEST(PipeCatalogue, ReverseChannelHasTenCallbacks) { - EXPECT_EQ(kMGPipeCallbackCount, 8u); + EXPECT_EQ(kMGPipeCallbackCount, 7u); EXPECT_EQ(sizeof(MGPipeCallbacks), kMGPipeCallbackCount * sizeof(void (*)())); } @@ -1424,16 +1426,22 @@ TEST(PipeCatalogue, ResourceRespecifyAcksOnlyImmutableStorage) { // say WHICH one was lost. The purity gate greps the count; this says what the count is of. TEST(PipeCatalogue, EveryUnmigratedEmulationIsNamedOnce) { // Every one of these is an emulation that reads or writes CLIENT memory a split server - // would not have: a CPU shadow mirror, a CPU mipmap fallback, a shadow-conversion readback, - // and the re-dirty of already-uploaded levels that a texture re-mint performs. + // would not have: a CPU shadow mirror, a CPU mipmap fallback and a shadow-conversion + // readback. + // + // FOUR, NOT FIVE (P9 W2). The fifth was "texture-remint-pull", the re-dirty of + // already-uploaded levels that RequireImageBindableStorage performs, and the head of the one + // new stall class the design admitted. It went because the stall class did: under a transport + // the re-mint is the server's own (RequireImageBindableStorageByHandle reads the GPU level and + // replays the staged store), the frontend arm that carried the marker is monolith-only, and no + // pull protocol or ResourceSubDataComplete terminator exists (notes/p9/W2-REMINT.md). const char* const kNames[] = { "copy-image-shadow-mirror", // the glCopyImageSubData CPU-shadow mirror "generate-mipmap-storage", // EnsureGenerateMipmapStorageAllocated "generate-mipmap-cpu-fallback", // GenerateThreeChannelFloatMipmapOnCpu "get-tex-image-shadow", // GetTexImageViaShadowConversion - "texture-remint-pull", // RequireImageBindableStorage's re-dirty }; - EXPECT_EQ(std::size(kNames), 5u); + EXPECT_EQ(std::size(kNames), 4u); // No duplicates: two sites sharing a name would make the grepped count and this list // disagree in the one direction nobody would notice. for (SizeT i = 0; i < std::size(kNames); ++i) { @@ -1441,12 +1449,6 @@ TEST(PipeCatalogue, EveryUnmigratedEmulationIsNamedOnce) { EXPECT_STRNE(kNames[i], kNames[j]); } } - // The last one is the head of the only NEW stall class the design admits, and P4a supplies - // exactly one of its four mitigations - prevention, through ImageBindableHint on every - // create and respecify. The async pull, the bounded retention and the - // ResourceSubDataComplete terminator are a later phase's, and P4a must not build half a - // terminator. - EXPECT_STREQ(kNames[4], "texture-remint-pull"); #if MOBILEGL_PIPE_PUSH // In monolith it really is a no-op: calling it changes nothing and returns nothing. The // teeth are a split server's, and the call site is what P8 gives them to. diff --git a/MobileGL/MG_Test/Pipe/TextureEmitTest.cpp b/MobileGL/MG_Test/Pipe/TextureEmitTest.cpp index 79a7ccef..a1ade06b 100644 --- a/MobileGL/MG_Test/Pipe/TextureEmitTest.cpp +++ b/MobileGL/MG_Test/Pipe/TextureEmitTest.cpp @@ -433,8 +433,8 @@ TEST(TextureEmit, AnImageBoundTextureCarriesTheImageBindableHintForever) { EXPECT_EQ(Textures().LastDesc().ImageBindableHint, 0); Textures().NoteTextureBoundAs(handle, kMGPipeBindShaderImage); - // The next respecify carries the hint - and it is the PREVENTION half of the texture-remint - // stall class, so it must never go back to 0 afterwards. + // The next respecify carries the hint - and it is what PREVENTS the texture re-mint, so it + // must never go back to 0 afterwards. texture->AllocateStorage(TextureUploadTarget::Texture2D, 1, MipmapInput{IntVec3{4, 4, 1}, 4 * 4 * 4}); EXPECT_EQ(Textures().LastDesc().ImageBindableHint, 1); texture->AllocateStorage(TextureUploadTarget::Texture2D, 2, MipmapInput{IntVec3{2, 2, 1}, 2 * 2 * 4}); @@ -1348,7 +1348,7 @@ TEST(TextureEmit, EveryDKTwoDependencyRowGatesItsOwnFamilyAndTheMirrorPairsStayL // THE CANONICAL ORDER FOR THE TEXTURES THE HINT WAS WRITTEN FOR: glTexStorage2D, then // glBindImageTexture. An IMMUTABLE texture has no further respecify - that is what immutable // means - so before this the applier's record kept ImageBindableHint = 0 for ever and the -// PREVENTION half of the texture-remint stall class was a no-op for exactly its own target. +// texture re-mint PREVENTION was a no-op for exactly its own target. TEST(TextureEmit, AnImmutableTexturesImageBindableHintReachesTheApplierAfterItsAllocation) { TextureScope scope; const auto texture = MakeTexture2D(18, 32); diff --git a/MobileGL/MG_Util/Metrics/PipeStats.cpp b/MobileGL/MG_Util/Metrics/PipeStats.cpp index 55bcf3d6..a683a1ae 100644 --- a/MobileGL/MG_Util/Metrics/PipeStats.cpp +++ b/MobileGL/MG_Util/Metrics/PipeStats.cpp @@ -609,9 +609,10 @@ namespace MobileGL::MG_Util::PipeStats { line += " sie=" + std::to_string(calls[static_cast(CallClass::ShaderImageEmissions)]); line += " ctu=" + std::to_string(calls[static_cast(CallClass::ClientTextureUploadEmissions)]); - // trp is the texture-remint pull count (ROADMAP open question 2): every one is a texture - // Espryt had already allocated and then had to re-mint image-bindable, replaying its - // levels from the client's shadow, because ImageBindableHint reached it too late. + // trp is the texture re-mint count (ROADMAP open question 2): every one is a texture + // Espryt had already allocated and then had to re-mint image-bindable because + // ImageBindableHint reached it too late - replaying the client's shadow in monolith, the + // GPU level and the staged store on the server under a transport (P9 W2; no pull). line += " trp=" + std::to_string(calls[static_cast(CallClass::TextureRemintPulls)]); // rsd is P7's resident sub-data emission count (OQ-10): one per buffer_subdata_resident // record (opcode 49) the client put on the wire. It is the ONLY observable that diff --git a/MobileGL/MG_Util/Metrics/PipeStats.h b/MobileGL/MG_Util/Metrics/PipeStats.h index 28409eff..9bd8c352 100644 --- a/MobileGL/MG_Util/Metrics/PipeStats.h +++ b/MobileGL/MG_Util/Metrics/PipeStats.h @@ -182,13 +182,15 @@ namespace MobileGL::MG_Util::PipeStats { // hides is ~+6 ms/frame, so an emission-shape divergence has to be a difference of two // numbers rather than something only a GPU can see. ClientTextureUploadEmissions, - // THE TEXTURE-REMINT PULL RATE (ROADMAP open question 2; P4a final review M-A). Counted - // by Espryt once per transition in which a texture that ALREADY HAD backend storage is - // re-minted image-bindable and its defined levels are replayed from the client's shadow - // (RequireImageBindableStorage) - the reach-back a split cannot make (D-M) and the one - // ImageBindableHint exists to prevent. A texture whose hint arrived before its first - // sync is allocated image-bindable up front and never counts. `trp=` on the summary - // line; the number that decides MOBILEGL_PIPE_TEXEL_RETAIN_MB's default. + // THE TEXTURE RE-MINT RATE (ROADMAP open question 2; P4a final review M-A). Counted by + // Espryt once per transition in which a texture that ALREADY HAD backend storage is + // re-minted image-bindable - on the monolith arm by RequireImageBindableStorage (levels + // replayed from the client's shadow), under a transport by + // RequireImageBindableStorageByHandle (the GPU level read back, the staged store for + // pending boxes; an immutable core-format texture it keeps as-is does not count). The + // name still says "pull": P9 W2 found no re-mint needs the client, so none is pulled, + // and the name stays because it is printed and peeked (`trp=`). A texture whose hint + // arrived before its first sync is allocated image-bindable up front and never counts. TextureRemintPulls, // P7 wave 2 package C, OQ-10 (CONTRACT-P7 §5.4). `rsd=` - THE RESIDENT SUB-DATA // EMISSION COUNT: one per `buffer_subdata_resident` record (opcode 49) the CLIENT