From 5de6ecd4e14cf1fd29dc594410e464391ff8f3ef Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Sat, 3 Oct 2026 02:58:02 -0400 Subject: [PATCH] [Fix] (SharedImage): uploads into a texture bound to a shared image are written into the image, and a producer's flush sends them first --- MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp | 2 + MobileGL/MG_Backend/DirectGLES/Managers.cpp | 113 +++++++++++++++--- MobileGL/MG_Backend/DirectGLES/Managers.h | 10 ++ MobileGL/MG_Impl/EGLImpl/EGLImpl.cpp | 4 + MobileGL/MG_Impl/Pipe/PipeFill.cpp | 14 +++ MobileGL/MG_Pipe/PipeApply.cpp | 7 +- MobileGL/MG_Pipe/PipeMutation.h | 5 + .../MG_Remote/Server/StagedTextureStore.h | 23 ++++ 8 files changed, 159 insertions(+), 19 deletions(-) diff --git a/MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp b/MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp index d74d900f..245e76c6 100644 --- a/MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp +++ b/MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp @@ -18856,6 +18856,8 @@ namespace MobileGL::MG_Backend::DirectGLES { Bool PublishPendingAccesses() { ReadFrame& frame = CurrentReadFrame(); frame.ImplicitSync = true; + // Sub-data waiting for a sync that nothing of this session would otherwise trigger. + if (t_boundNativeContext != EGL_NO_CONTEXT) TextureImpl::WritePendingSharedImageUploads(); if (frame.Tracker.Pending() == 0) return true; int fence = -1; if (t_boundNativeContext != EGL_NO_CONTEXT && !ExportFence(&fence)) { diff --git a/MobileGL/MG_Backend/DirectGLES/Managers.cpp b/MobileGL/MG_Backend/DirectGLES/Managers.cpp index 60ca7d30..4a2f6662 100644 --- a/MobileGL/MG_Backend/DirectGLES/Managers.cpp +++ b/MobileGL/MG_Backend/DirectGLES/Managers.cpp @@ -8858,27 +8858,12 @@ namespace MobileGL::MG_Backend::DirectGLES { return false; } - // Sub-data aimed at the image is not replayed into it - the image is written by the - // producer's present - but it must still leave the pending set, or the per-draw clean - // gate (IsDrawSyncCleanByRecord) would never close. - const auto dropPendingUploads = [&record] { - if (record.PendingUploads.empty()) return; - MGLOG_W_ONCE("Shared image texture %u: sub-data on an attached shared image is dropped", - record.Desc.GlNameForDiag); - Vector> keys; - keys.reserve(record.PendingUploads.size()); - for (const auto& pending : record.PendingUploads) { - keys.emplace_back(MG_Pipe::MGPipeSubDataUploadTargetOf(pending.UploadTarget), pending.Level); - } - for (const auto& key : keys) ConsumePipeTextureUpload(record.Desc.Resource, key.first, key.second); - }; - // Steady state: bound to this image already (the serial moves with sub-data and with a // metadata respecify, neither of which changes what the name samples). A failed bind is - // retried only once the record moves. + // retried only once the record moves. Pending sub-data is the caller's + // (ReplaySharedImageUploads), after the image's sync state has been acquired. if (m_sharedImageId == imageId && m_isInitialized && (m_sharedImage != nullptr || m_syncedResourceSerial == record.Serial)) { - dropPendingUploads(); m_syncedResourceSerial = record.Serial; return true; } @@ -8893,7 +8878,6 @@ namespace MobileGL::MG_Backend::DirectGLES { // Whatever comes next redefines storage through a re-mint (see the imageId == 0 arm). m_backendStorageImmutable = true; m_syncedResourceSerial = record.Serial; - dropPendingUploads(); const TextureTarget target = BufferImpl::StagedTextureTargetForPipeTarget(record.Desc.Target); if (ConvertTextureTargetToBackendGLEnum(target) != GL_TEXTURE_2D) { @@ -8915,6 +8899,7 @@ namespace MobileGL::MG_Backend::DirectGLES { return true; } m_sharedImage = std::move(image); + m_sharedImageUploadsRgb = false; // The shape the name now has: the descriptor's level 0, and that one level only. m_prevTextureInfo = {static_cast(record.Desc.InternalFormat), record.Desc.Width, @@ -8926,6 +8911,83 @@ namespace MobileGL::MG_Backend::DirectGLES { record.Desc.FixedSampleLocations != 0}; return true; } + + // SUB-DATA ON A SHARED IMAGE IS WRITTEN INTO IT. An X server drawing with glamor uploads + // its clients' pixels (MIT-SHM / PutImage) into window pixmaps whose level 0 is a shared + // image, with glTexSubImage2D: the texels are in the staged store (the client defined the + // level without data, so only the uploaded boxes are covered), and they go into the + // EGLImage-backed name box by box. Every pending entry leaves the set either way, or the + // per-draw clean gate would never close. Called after AcquireForSampling, so the upload is + // ordered after the image's pending reads and writes like any other use. + void BackendTextureObject::ReplaySharedImageUploads(const MG_Pipe::MGPipeResourceRecord& record) { + if (record.PendingUploads.empty()) return; + Vector> keys; + Vector> boxes; + keys.reserve(record.PendingUploads.size()); + for (const auto& pending : record.PendingUploads) { + const Uint16 uploadHalf = MG_Pipe::MGPipeSubDataUploadTargetOf(pending.UploadTarget); + keys.emplace_back(uploadHalf, pending.Level); + if (pending.Level != 0) continue; // an image has one level + if (pending.Regions.empty()) { + boxes.emplace_back(uploadHalf, pending.UnionBox); + } else { + for (const auto& region : pending.Regions) + boxes.emplace_back(uploadHalf, MG_Pipe::MGPBox{region.X, region.Y, region.Z, region.W, region.H, + region.D}); + } + } + if (m_sharedImage != nullptr && !boxes.empty()) { + auto& store = MG_Remote::Server::ServerStagedTexture(); + const Uint64 key = MG_Remote::Server::StagedTextureStore::KeyForHandle(record.Desc.Resource); + Bind(GL_TEXTURE_2D); + ScopedDefaultUnpackState unpackState; + BufferImpl::BindPixelUnpackBufferId(0); + for (const auto& [uploadHalf, box] : boxes) { + Int rowTexels = 0; + // A shared image is RGBA8 to every client that names it (the level the client + // defined over it): four bytes a texel. + const Uint8* bytes = + store.CoveredBoxBytes(key, uploadHalf, 0, box.X, box.Y, box.W, box.H, 4, &rowTexels); + if (bytes == nullptr) { + MGLOG_E_ONCE("Shared image texture %u: an upload of %ux%u at (%d,%d) has no staged texels; " + "not written", + record.Desc.GlNameForDiag, box.W, box.H, box.X, box.Y); + continue; + } + const Uint8* origin = + bytes + (static_cast(box.Y) * static_cast(rowTexels) + static_cast(box.X)) * 4; + if (!m_sharedImageUploadsRgb) { + while (g_GLESFuncs.glGetError() != GL_NO_ERROR) { + } + g_GLESFuncs.glPixelStorei(GL_UNPACK_ROW_LENGTH, rowTexels); + g_GLESFuncs.glTexSubImage2D(GL_TEXTURE_2D, 0, box.X, box.Y, static_cast(box.W), + static_cast(box.H), GL_RGBA, GL_UNSIGNED_BYTE, origin); + g_GLESFuncs.glPixelStorei(GL_UNPACK_ROW_LENGTH, 0); + // An image of an X format (no alpha) is an RGB8 texture to the driver, which + // takes only RGB texels: this image's uploads are converted from now on. + if (g_GLESFuncs.glGetError() != GL_INVALID_OPERATION) continue; + m_sharedImageUploadsRgb = true; + } + Vector rgb(static_cast(box.W) * box.H * 3); + for (Uint32 row = 0; row < box.H; ++row) { + const Uint8* src = origin + static_cast(row) * static_cast(rowTexels) * 4; + Uint8* dst = rgb.data() + static_cast(row) * box.W * 3; + for (Uint32 x = 0; x < box.W; ++x) { + dst[x * 3 + 0] = src[x * 4 + 0]; + dst[x * 3 + 1] = src[x * 4 + 1]; + dst[x * 3 + 2] = src[x * 4 + 2]; + } + } + g_GLESFuncs.glPixelStorei(GL_UNPACK_ALIGNMENT, 1); + g_GLESFuncs.glTexSubImage2D(GL_TEXTURE_2D, 0, box.X, box.Y, static_cast(box.W), + static_cast(box.H), GL_RGB, GL_UNSIGNED_BYTE, rgb.data()); + g_GLESFuncs.glPixelStorei(GL_UNPACK_ALIGNMENT, 4); + } + MGLOG_I_ONCE("Shared image texture %u: sub-data written into its image (%s texels)", + record.Desc.GlNameForDiag, m_sharedImageUploadsRgb ? "RGB" : "RGBA"); + } + for (const auto& [uploadHalf, level] : keys) ConsumePipeTextureUpload(record.Desc.Resource, uploadHalf, level); + } #endif void BackendTextureObject::SyncMipmapsToBackend( @@ -8979,6 +9041,7 @@ namespace MobileGL::MG_Backend::DirectGLES { // a write that landed since the last one is waited for on this context's GPU // before the command this sync is for, and the session's frame notes the read. if (m_sharedImage != nullptr) SharedImageImpl::AcquireForSampling(m_sharedImage); + ReplaySharedImageUploads(*pushedStorage); return; } // P5e (tx2), CONTRACT-P5E ยง5.2 (scout G-S2-2): THE VIEW TEST MOVES BEHIND THE @@ -11248,6 +11311,20 @@ namespace MobileGL::MG_Backend::DirectGLES { g_boundTexturesCache; TwinRegistry g_backendTextureObjects; +#if MOBILEGL_BUILD_DISAGGREGATED + void WritePendingSharedImageUploads() { + auto& applier = MG_Pipe::MGPipeApplier(); + for (SizeT slot = 0; slot < applier.TextureResources.size(); ++slot) { + const auto& record = applier.TextureResources[slot]; + if (!record.Live || record.SharedImageId == 0 || record.PendingUploads.empty()) continue; + const MG_Pipe::MGPipeHandle handle{static_cast(slot), record.Gen}; + auto* twin = g_backendTextureObjects.FindByHandle(handle); + if (twin == nullptr || !*twin) continue; + (*twin)->SyncMipmapsToBackendByHandle(handle); + } + } +#endif + void NoteDriverSideTextureWrite(const SharedPtr& textureObject) { SharedPtr storage = textureObject; // Bounded: GL resolves a view of a view to the original storage, so this takes one step. diff --git a/MobileGL/MG_Backend/DirectGLES/Managers.h b/MobileGL/MG_Backend/DirectGLES/Managers.h index 60619a71..b9999894 100644 --- a/MobileGL/MG_Backend/DirectGLES/Managers.h +++ b/MobileGL/MG_Backend/DirectGLES/Managers.h @@ -2111,6 +2111,10 @@ namespace MobileGL::MG_Backend::DirectGLES { // not go on to allocate or upload. False when the record names no image: a twin that // was bound to one has been told to re-mint, and the normal storage path takes over. Bool SyncSharedImageStorage(const MG_Pipe::MGPipeResourceRecord& record); + // Pending sub-data written into the attached shared image (Managers.cpp). + void ReplaySharedImageUploads(const MG_Pipe::MGPipeResourceRecord& record); + // The attached image takes RGB texels only (an X format): uploads are converted. + Bool m_sharedImageUploadsRgb = false; #endif // Whether this texture's ES storage was minted in an image carrier rather than in the // frontend format's own layout - the readback has to ask, because for a NORMALIZED @@ -2483,6 +2487,12 @@ namespace MobileGL::MG_Backend::DirectGLES { // FBO's texture attachments - onto the backend, through the two borrowed-pair memos // documented at their definitions. Declared here so tests can drive those memos directly. void SyncNeccessaryTextures(); +#if MOBILEGL_BUILD_DISAGGREGATED + // Every texture of this session bound to a shared image with sub-data still pending is + // synced now, so the uploads are in the images before a flush publishes them (an X + // server's PutImage into a window pixmap no later draw of its own samples). + void WritePendingSharedImageUploads(); +#endif extern Array, MG_State::GLState::TextureState::MAX_TEXTURE_IMAGE_UNITS> g_boundTexturesCache; diff --git a/MobileGL/MG_Impl/EGLImpl/EGLImpl.cpp b/MobileGL/MG_Impl/EGLImpl/EGLImpl.cpp index 6a291907..2a5c8d7a 100644 --- a/MobileGL/MG_Impl/EGLImpl/EGLImpl.cpp +++ b/MobileGL/MG_Impl/EGLImpl/EGLImpl.cpp @@ -1458,6 +1458,10 @@ namespace MobileGL::MG_Impl::EGLImpl { ownsServerWindow, std::getenv("MOBILEGL_SHARED_IMAGE_FLUSH_SYNC"))) return; auto* backendObject = MG_Backend::pActiveBackendObject.get(); +#if MOBILEGL_BUILD_DISAGGREGATED + // Uploads still waiting for a validate point go first: they are part of what is published. + MG_Pipe::MGPipeClientDrainTextureUploads(); +#endif if (backendObject != nullptr && !backendObject->FlushSharedImageAccesses()) { MGLOG_E_ONCE("glFlush: this session's shared-image accesses could not be published; another session may " "read them early"); diff --git a/MobileGL/MG_Impl/Pipe/PipeFill.cpp b/MobileGL/MG_Impl/Pipe/PipeFill.cpp index afdefecc..bdd015c5 100644 --- a/MobileGL/MG_Impl/Pipe/PipeFill.cpp +++ b/MobileGL/MG_Impl/Pipe/PipeFill.cpp @@ -3987,6 +3987,20 @@ namespace MobileGL::MG_Pipe { #endif } + void MGPipeClientDrainTextureUploads() { + if (MG_Config::Transport == MG_Config::TransportMode::Monolith) return; + if (MG_Remote::Client::RunsAsTheServerRole()) return; + auto* ctx = LiveContext(); + if (ctx == nullptr) return; + const Uint64 pushMask = MG_Config::Features.PipePush; + if ((pushMask & kMGPipeSubsystemTextureResources) != 0 && + (kMGPipeWiredSubsystems & kMGPipeSubsystemTextureResources) != 0 && + P4aFamilyHasItsConsumer(kMGPipeSubsystemTextureResources) && + P4aFamilyDependenciesAreSet(kMGPipeSubsystemTextureResources, pushMask)) { + (void)MGPipeTextureEmitterInstance().DrainTextureSubData(*ctx); + } + } + #if MOBILEGL_BUILD_DISAGGREGATED // Declared in PipeMutation.h, where the list is argued. THE ORDER: the latch goes first, so // everything after it - a pin's last owner dying, a deferred destroy replayed, an evicted diff --git a/MobileGL/MG_Pipe/PipeApply.cpp b/MobileGL/MG_Pipe/PipeApply.cpp index db0d5699..caa7cddc 100644 --- a/MobileGL/MG_Pipe/PipeApply.cpp +++ b/MobileGL/MG_Pipe/PipeApply.cpp @@ -2371,7 +2371,12 @@ namespace MobileGL::MG_Pipe { // the mask arrives - and by construction only the non-storage fields differ. record->Desc = desc; // A shared image is the storage this call replaces - unless the call replaces none. - if (level != nullptr || !metadataOnly) record->SharedImageId = 0; + if ((level != nullptr || !metadataOnly) && record->SharedImageId != 0) { + MGLOG_D("MGPipe: texture %u leaves shared image %llu: a respecify (%s) redefined its storage", + desc.GlNameForDiag, static_cast(record->SharedImageId), + level != nullptr ? "one level" : "whole resource"); + record->SharedImageId = 0; + } #if MOBILEGL_BUILD_DISAGGREGATED if (level != nullptr && desc.Target != static_cast(MGPipeResourceTarget::TexBuffer)) { // Scope and BufOffset/BufSize are transient respecify carriers, not resource state. diff --git a/MobileGL/MG_Pipe/PipeMutation.h b/MobileGL/MG_Pipe/PipeMutation.h index a08a158a..2e99bfac 100644 --- a/MobileGL/MG_Pipe/PipeMutation.h +++ b/MobileGL/MG_Pipe/PipeMutation.h @@ -266,6 +266,11 @@ namespace MobileGL::MG_Pipe { void MGPipeClientFinish(); // Submit a stream link's published prefix; never wait for appliedSeq. void MGPipeClientFlush(); + // The texture sub-data drain of a validate point, outside one: a shared-image producer's + // glFlush (an X server's PutImage into a window pixmap is followed by no draw of its own) + // has to get its uploads to the server before the flush publishes the images. Gated as the + // validate point's drain is; a no-op on a monolith and on the server role. + void MGPipeClientDrainTextureUploads(); // A FRESH SESSION REPLACES AN ENDED ONE (a device loss recovered, or an eglTerminate / // eglInitialize): every client-side answer to "what does the server hold" describes the diff --git a/MobileGL/MG_Remote/Server/StagedTextureStore.h b/MobileGL/MG_Remote/Server/StagedTextureStore.h index d5622c9e..4e22da8c 100644 --- a/MobileGL/MG_Remote/Server/StagedTextureStore.h +++ b/MobileGL/MG_Remote/Server/StagedTextureStore.h @@ -668,6 +668,29 @@ namespace MobileGL::MG_Remote::Server { site, static_cast(uploadTarget), static_cast(level)); } + // A box of a level for a replay into storage that is not this store's - a shared image, + // whose level was defined with no data and is written in pieces (an X server uploading a + // client's pixels into a window pixmap). The level image and its row length in texels when + // every row of the box is covered; null otherwise. Unlike RequireLevelBytes a partially + // covered level answers, and nothing is fatal: an uncovered box is the caller's to skip. + const Uint8* CoveredBoxBytes(Uint64 key, Uint16 uploadTarget, Uint16 level, Int x, Int y, Uint32 w, Uint32 h, + SizeT texel, Int* rowTexels) const { + if (!m_copies) return nullptr; + const std::lock_guard lock(m_mutex); + const LevelShadow* shadow = FindLevel(key, uploadTarget, level); + if (shadow == nullptr || !shadow->Defined || x < 0 || y < 0 || w == 0 || h == 0 || + x + static_cast(w) > shadow->Extent.x() || y + static_cast(h) > shadow->Extent.y()) + return nullptr; + for (Uint32 row = 0; row < h; ++row) { + const SizeT begin = TexelOffset(shadow->Extent, x, y + static_cast(row), 0, texel); + const SizeT end = begin + static_cast(w) * texel; + if (end > shadow->Bytes.size() || !StagedShadowStore::CoverageHas(shadow->Covered, begin, end)) + return nullptr; + } + *rowTexels = shadow->Extent.x(); + return shadow->Bytes.data(); + } + // Diagnostics the sync path and the unit cases read, so that a check can assert WHAT // HAPPENED rather than that nothing blew up. Bool IsCovered(Uint64 key, Uint16 uploadTarget, Uint16 level) const {