From 9524c688caadef76e6a479dc138e6e8d7b3d5c43 Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Tue, 29 Sep 2026 09:06:07 -0400 Subject: [PATCH 1/9] [Fix] (Espryt, Magma): scattered glTexSubImage writes upload exactly their texels - MipmapStorage keeps an exact dirty footprint beside its rect list (a write log once a merge or a fold would widen the list past the writes), Magma always uploads it, Espryt uploads it box by box (through the unpack ring too) once the driver texture may hold GPU writes and merges it at the image-bindable re-mint; the union box, the 3/4 cut-off, the 96-rect fold and a level-spanning union no longer put the stale shadow over GPU-written texels (two 2x2 writes over a GPU clear: 41 of 64 texels red -> 0) --- MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp | 13 ++ MobileGL/MG_Backend/DirectGLES/Managers.cpp | 127 +++++++++---- MobileGL/MG_Backend/DirectGLES/Managers.h | 17 ++ .../Renderer/VkTextureManager.cpp | 42 +++-- .../GLState/TextureState/MipmapStorage.cpp | 174 ++++++++++++++++++ .../GLState/TextureState/MipmapStorage.h | 21 +++ .../TextureState/MipmapUploadTargetArray.h | 5 + .../GLState/TextureState/TextureObject.cpp | 6 + .../GLState/TextureState/TextureObject.h | 14 ++ .../TextureState/TextureObject2DCube.cpp | 5 + .../TextureState/TextureObject2DCube.h | 2 + .../TextureState/TextureObjectView.cpp | 9 + .../GLState/TextureState/TextureObjectView.h | 2 + 13 files changed, 377 insertions(+), 60 deletions(-) diff --git a/MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp b/MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp index dbc42c72..d6ee1ace 100644 --- a/MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp +++ b/MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp @@ -1707,6 +1707,11 @@ namespace MobileGL::MG_Backend::DirectGLES { return; } + // Before the sync: `backendTexture` below is a reference into the registry, which the + // lookup inside the note may reshuffle. + if (imageBinding.Access != GL_READ_ONLY) { + NoteDriverSideTextureWrite(imageBinding.Texture); + } auto& backendTexture = SyncTextureObjectToBackend(imageBinding.Texture, true); const Bool layerable = SupportsLayeredImageBinding(imageBinding.Texture->GetTarget()); const GLboolean layered = layerable ? imageBinding.Layered : GL_FALSE; @@ -5919,6 +5924,7 @@ namespace MobileGL::MG_Backend::DirectGLES { continue; } + TextureImpl::NoteDriverSideTextureWrite(destinationTexture); auto backendSource = TextureImpl::SyncTextureObjectToBackend(sourceTexture); auto backendDestination = TextureImpl::SyncTextureObjectToBackend(destinationTexture); if (!backendSource || !backendDestination) continue; @@ -6635,6 +6641,7 @@ namespace MobileGL::MG_Backend::DirectGLES { const auto& textureObject = MG_State::pGLContext->GetTextureUnitObject((Int)activeTextureUnit) .GetBindingSlot(MG_Util::ConvertGLEnumToTextureTarget(target)) .GetBoundObject(); + if (textureObject) TextureImpl::NoteDriverSideTextureWrite(textureObject); auto* backendTextureSlot = TextureImpl::g_backendTextureObjects.Find(textureObject.get()); if (!backendTextureSlot || !*backendTextureSlot) { MGLOG_E_ONCE("CopyTexSubImage2D: No backend texture found for texture %u.", @@ -6730,6 +6737,7 @@ namespace MobileGL::MG_Backend::DirectGLES { const auto& textureObject = MG_State::pGLContext->GetTextureUnitObject(activeTextureUnit) .GetBindingSlot(MG_Util::ConvertGLEnumToTextureTarget(target)) .GetBoundObject(); + if (textureObject) TextureImpl::NoteDriverSideTextureWrite(textureObject); auto* backendTextureSlot = TextureImpl::g_backendTextureObjects.Find(textureObject.get()); if (!backendTextureSlot || !*backendTextureSlot) { MGLOG_E_ONCE("CopyTexSubImage2D: No backend texture found for texture %u.", @@ -6879,6 +6887,8 @@ namespace MobileGL::MG_Backend::DirectGLES { TextureImpl::SyncTextureObjectToBackend(texture); return; } + // Every path below writes the levels above the base on the driver only. + TextureImpl::NoteDriverSideTextureWrite(texture); auto& backendTexture = TextureImpl::SyncTextureObjectToBackend(texture); if (IsDepthOnlyFormat(texture->GetFormat())) { @@ -7152,6 +7162,9 @@ namespace MobileGL::MG_Backend::DirectGLES { // dereference. The frontend validator is what keeps this unreachable and what reports // the error the application is owed; declining is only how a future gap up there stops // being a crash. See the level guard in VulkanRenderer::CopyImageSubData. + if (!dstEndpoint.IsRenderbuffer() && dstEndpoint.Texture) { + TextureImpl::NoteDriverSideTextureWrite(dstEndpoint.Texture); + } if (!MakeGLESCopyImageEndpoint(srcEndpoint, srcTarget, srcX, srcY, srcZ, src) || !MakeGLESCopyImageEndpoint(dstEndpoint, dstTarget, dstX, dstY, dstZ, dst)) { MGLOG_E_ONCE("%s: source or destination image failed to sync; declining the copy", __func__); diff --git a/MobileGL/MG_Backend/DirectGLES/Managers.cpp b/MobileGL/MG_Backend/DirectGLES/Managers.cpp index f49141cf..82fc9d18 100644 --- a/MobileGL/MG_Backend/DirectGLES/Managers.cpp +++ b/MobileGL/MG_Backend/DirectGLES/Managers.cpp @@ -2787,23 +2787,29 @@ namespace MobileGL::MG_Backend::DirectGLES { } // Replaces one level's shadow bytes with what the driver texture holds for it, then copies - // back the client writes the driver has not been sent yet (the level's pending rects, the - // same boxes a plain sync would upload over the driver's copy). Returns false, leaving the - // shadow untouched, when the driver's copy cannot stand in for the shadow: the client owns - // every byte of the level, the level is stored compressed, the driver has no image at this - // level and extent (defined after the last sync), or the readback cannot be served (a - // multisample image, a format the driver will not attach to a framebuffer). + // back the client writes the driver has not been sent yet - exactly the texels written + // (the level's dirty footprint), not the union box or the rect list, which also span texels + // the client never wrote and which only the driver's copy holds correctly. Returns false, + // leaving the shadow untouched, when the driver's copy cannot stand in for the shadow: the + // client wrote every texel of the level, the level is stored compressed, the driver has no + // image at this level and extent (defined after the last sync), or the readback cannot be + // served (a multisample image, a format the driver will not attach to a framebuffer). static Bool AdoptDriverLevelIntoShadow(GLuint backendTextureId, BackendTextureObject& backendTexture, const MG_State::GLState::ITextureObject& stateTextureObject, MG_State::GLState::TextureObjectMipmap& mipmapObject, TextureUploadTarget uploadTarget, Uint level) { const IntVec3 texelSize = mipmapObject.GetMipmapTexelSize(uploadTarget, level); const IntVec3 extent{texelSize.x(), texelSize.y(), std::max(texelSize.z(), 1)}; - const Bool dirty = mipmapObject.IsStorageDirty(uploadTarget, level); - const MG_State::GLState::MipmapDirtyRegion dirtyRegion = - dirty ? mipmapObject.GetStorageDirtyRegion(uploadTarget, level) - : MG_State::GLState::MipmapDirtyRegion{}; - if (dirty && dirtyRegion.CoversWholeLevel(extent)) return false; + const SizeT texelCount = + static_cast(extent.x()) * static_cast(extent.y()) * static_cast(extent.z()); + Vector pending; + mipmapObject.GetStorageDirtyFootprint(uploadTarget, level, pending); + // Disjoint boxes, so their texels add up; a union box spanning the level is not enough. + SizeT pendingTexels = 0; + for (const auto& box : pending) { + pendingTexels += box.TexelCount(); + } + if (!pending.empty() && pendingTexels >= texelCount) return false; if (mipmapObject.GetMipmapCompressedFormat(uploadTarget, level) != GL_NONE) return false; if (!g_GLESFuncs.glGetTexLevelParameteriv) return false; @@ -2833,28 +2839,18 @@ namespace MobileGL::MG_Backend::DirectGLES { return false; } const SizeT shadowBytes = mipmapObject.GetMipmapByteSize(uploadTarget, level); - const SizeT texelCount = - static_cast(extent.x()) * static_cast(extent.y()) * static_cast(extent.z()); auto* shadow = static_cast(mipmapObject.MapMipmapData(uploadTarget, level)); if (shadow == nullptr || snapshot.size() != shadowBytes || texelCount == 0 || shadowBytes % texelCount) { return false; } - if (dirty) { - // The rects index the SHADOW's texel grid (a 1D array's layers are its rows), which + if (!pending.empty()) { + // The boxes index the SHADOW's texel grid (a 1D array's layers are its rows), which // is also the snapshot's byte order. const SizeT bpp = shadowBytes / texelCount; const SizeT rowBytes = static_cast(extent.x()) * bpp; const SizeT sliceBytes = static_cast(extent.y()) * rowBytes; - constexpr SizeT kMaxPending = MG_State::GLState::MipmapStorage::kMaxDirtyRects; - MG_State::GLState::MipmapDirtyRegion pending[kMaxPending]; - SizeT pendingCount = mipmapObject.GetStorageDirtyRects(uploadTarget, level, pending, kMaxPending); - if (pendingCount == 0) { - pending[0] = dirtyRegion; - pendingCount = 1; - } - for (SizeT i = 0; i < pendingCount; ++i) { - const auto& box = pending[i]; + for (const auto& box : pending) { if (box.Empty()) continue; const SizeT spanBytes = static_cast(box.hi.x() - box.lo.x()) * bpp; for (Int z = box.lo.z(); z < box.hi.z(); ++z) { @@ -4340,6 +4336,9 @@ namespace MobileGL::MG_Backend::DirectGLES { ApplyImageBindableStorageWidening(imageWidening, &glInternalFormat, &glFormat, &glType); const auto& uploadTargets = textureMipmapObject->GetUploadTargets(); ScopedDefaultUnpackState unpackState; + // Per-level scratch of the exact-footprint path, hoisted to reuse capacity. + Vector dirtyFootprint; + Vector stagingBlockOverflow; for (auto& uploadTarget : uploadTargets) { for (SizeT level = 0; level < mipmapCount; ++level) { if (!textureMipmapObject->IsStorageDirty(uploadTarget, level)) { @@ -4413,13 +4412,16 @@ namespace MobileGL::MG_Backend::DirectGLES { const SizeT texelCount = static_cast(texelSize.x()) * static_cast(texelSize.y()) * static_cast(std::max(texelSize.z(), 1)); - const Bool subRectEligible = - !dirtyRegion.Empty() && !dirtyRegion.CoversWholeLevel(texelSize) && texelCount > 0 && - uploadBytes > 0 && uploadBytes % texelCount == 0 && + // Whether a box of the level can be addressed at all: whole texels in + // the upload bytes, and an ES image shaped like the shadow. + const Bool regionsAddressable = + !dirtyRegion.Empty() && texelCount > 0 && uploadBytes > 0 && + uploadBytes % texelCount == 0 && (layersAsRows ? std::max(texelSize.z(), 1) == 1 : uploadSize.x() == texelSize.x() && uploadSize.y() == texelSize.y() && std::max(uploadSize.z(), 1) == std::max(texelSize.z(), 1)); - const SizeT bpp = subRectEligible ? uploadBytes / texelCount : 0; + const Bool subRectEligible = regionsAddressable && !dirtyRegion.CoversWholeLevel(texelSize); + const SizeT bpp = regionsAddressable ? uploadBytes / texelCount : 0; const IntVec3 regionSize = {dirtyRegion.hi.x() - dirtyRegion.lo.x(), dirtyRegion.hi.y() - dirtyRegion.lo.y(), dirtyRegion.hi.z() - dirtyRegion.lo.z()}; @@ -4456,6 +4458,27 @@ namespace MobileGL::MG_Backend::DirectGLES { dirtyRectCount = 0; } } + // Everything above - the union box, the ring's one-box rule, the rect + // list with its merges, fold and 3/4 cut-off - may upload shadow texels + // the client did not write, which is only harmless while the driver + // still holds those texels as they were last uploaded. Once it may hold + // GPU writes, the level goes up as exactly the texels written, one box + // (one ring job) each, whatever that costs: more boxes, and a union box + // that spans the whole level no longer means a whole-level upload. + const MG_State::GLState::MipmapDirtyRegion* uploadRects = dirtyRects; + SizeT uploadRectCount = dirtyRectCount; + if (m_driverMayHoldGpuWrites && regionsAddressable) { + textureMipmapObject->GetStorageDirtyFootprint(uploadTarget, level, dirtyFootprint); + // One box is the union box itself, which the branches below already + // upload exactly (as the whole level when it is the whole level). + if (dirtyFootprint.size() >= 2) { + uploadRects = dirtyFootprint.data(); + uploadRectCount = dirtyFootprint.size(); + } else { + uploadRectCount = 0; + } + } + const Bool perRectUpload = regionsAddressable && uploadRectCount >= 2; const auto rectUploadPtr = [&](const MG_State::GLState::MipmapDirtyRegion& rect) { return static_cast(uploadData) + static_cast(rect.lo.z()) * levelSliceBytes + @@ -4491,15 +4514,21 @@ namespace MobileGL::MG_Backend::DirectGLES { // client-pointer path makes the driver walk, which strides over // the whole level width. UnpackStagingBlock - stagingBlocks[MG_State::GLState::MipmapStorage::kMaxDirtyRects]; + stagingBlockArray[MG_State::GLState::MipmapStorage::kMaxDirtyRects]; + UnpackStagingBlock* stagingBlocks = stagingBlockArray; SizeT stagingBlockCount = 0; const Bool ringUsable = BufferImpl::UnpackRingAvailable(); if (!ringUsable) { // Nothing to plan: every branch below keeps the client // pointer and its ROW_LENGTH striding, unchanged. - } else if (subRectEligible && dirtyRectCount >= 2) { - for (SizeT r = 0; r < dirtyRectCount; ++r) { - const auto& rect = dirtyRects[r]; + } else if (perRectUpload) { + // Only an exact footprint outgrows the array (see uploadRects). + if (uploadRectCount > MG_State::GLState::MipmapStorage::kMaxDirtyRects) { + stagingBlockOverflow.resize(uploadRectCount); + stagingBlocks = stagingBlockOverflow.data(); + } + for (SizeT r = 0; r < uploadRectCount; ++r) { + const auto& rect = uploadRects[r]; stagingBlocks[r] = { rectUploadPtr(rect), static_cast(rect.hi.x() - rect.lo.x()) * bpp, @@ -4509,7 +4538,7 @@ namespace MobileGL::MG_Backend::DirectGLES { levelSliceBytes, 0}; } - stagingBlockCount = dirtyRectCount; + stagingBlockCount = uploadRectCount; } else if (subRectEligible) { stagingBlocks[0] = {regionPtr, static_cast(regionSize.x()) * bpp, @@ -4550,10 +4579,10 @@ namespace MobileGL::MG_Backend::DirectGLES { switch (MapToBackendTextureTarget(stateTextureObject->GetTarget())) { case TextureTarget::Texture2D: case TextureTarget::TextureCubeMap: - if (subRectEligible && dirtyRectCount >= 2) { + if (perRectUpload) { if (!ringStaged) g_GLESFuncs.glPixelStorei(GL_UNPACK_ROW_LENGTH, texelSize.x()); - for (SizeT r = 0; r < dirtyRectCount; ++r) { - const auto& rect = dirtyRects[r]; + for (SizeT r = 0; r < uploadRectCount; ++r) { + const auto& rect = uploadRects[r]; g_GLESFuncs.glTexSubImage2D( glUploadTarget, static_cast(level), rect.lo.x(), rect.lo.y(), static_cast(rect.hi.x() - rect.lo.x()), @@ -4589,18 +4618,18 @@ namespace MobileGL::MG_Backend::DirectGLES { // ES 3.2 has GL_TEXTURE_CUBE_MAP_ARRAY natively and it stores exactly // like a 2D array whose depth is 6 * the cube count. case TextureTarget::TextureCubeMapArray: - if (subRectEligible && dirtyRectCount >= 2) { + if (perRectUpload) { if (!ringStaged) { g_GLESFuncs.glPixelStorei(GL_UNPACK_ROW_LENGTH, texelSize.x()); g_GLESFuncs.glPixelStorei(GL_UNPACK_IMAGE_HEIGHT, imageHeight); } - for (SizeT r = 0; r < dirtyRectCount; ++r) { - const auto box = ToBackendBox(dirtyRects[r]); + for (SizeT r = 0; r < uploadRectCount; ++r) { + const auto box = ToBackendBox(uploadRects[r]); g_GLESFuncs.glTexSubImage3D( glUploadTarget, static_cast(level), box.x, box.y, box.z, box.w, box.h, box.d, glFormat, glType, ringStaged ? UnpackRingPixelOffset(stagingBlocks[r].offset) - : static_cast(rectUploadPtr(dirtyRects[r]))); + : static_cast(rectUploadPtr(uploadRects[r]))); } if (!ringStaged) { g_GLESFuncs.glPixelStorei(GL_UNPACK_ROW_LENGTH, 0); @@ -5147,6 +5176,21 @@ namespace MobileGL::MG_Backend::DirectGLES { MG_State::GLState::TextureState::MAX_TEXTURE_IMAGE_UNITS> g_boundTexturesCache; StateBackendObjectRegistry g_backendTextureObjects; + + 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. + for (Uint hop = 0; storage && storage->IsTextureView() && hop < 4; ++hop) { + storage = storage->GetViewStorageOwner(); + } + if (!storage) return; + auto* slot = g_backendTextureObjects.Find(storage.get()); + auto& backendObj = slot ? *slot : g_backendTextureObjects.GetOrCreate(storage); + if (!backendObj) { + backendObj = MakeShared(); + } + backendObj->NoteDriverSideWrite(); + } } // namespace TextureImpl namespace FramebufferImpl { @@ -5307,6 +5351,9 @@ namespace MobileGL::MG_Backend::DirectGLES { return false; } backendTextureObject->SyncMipmapsToBackend(textureObject); + // From here on every draw, clear and blit into this framebuffer can write the + // texture behind the shadow's back. + TextureImpl::NoteDriverSideTextureWrite(textureObject); if (attachmentObject.IsLayered()) { g_GLESFuncs.glFramebufferTexture(glFBOTarget, glBackendAttachment, backendTextureObject->GetBackendTextureId(), diff --git a/MobileGL/MG_Backend/DirectGLES/Managers.h b/MobileGL/MG_Backend/DirectGLES/Managers.h index 6603f2cc..8aa931e7 100644 --- a/MobileGL/MG_Backend/DirectGLES/Managers.h +++ b/MobileGL/MG_Backend/DirectGLES/Managers.h @@ -976,6 +976,9 @@ namespace MobileGL::MG_Backend::DirectGLES { // carrier the storage is an integer texture holding codes and glGetTexImage still owes // the application floats. Bool RequiresImageBindableStorage() const { return m_imageBindableStorageRequired; } + // See m_driverMayHoldGpuWrites. Callers holding the STATE texture use + // NoteDriverSideTextureWrite, which also reaches the storage behind a view. + void NoteDriverSideWrite() { m_driverMayHoldGpuWrites = true; } void Bind(GLenum target, Uint unit = TempTextureUnit); Uint GetBackendTextureId() const; @@ -1059,6 +1062,15 @@ namespace MobileGL::MG_Backend::DirectGLES { Bool m_isInitialized = false; Bool m_imageBindableStorageRequired = false; Bool m_backendStorageImmutable = false; + // Whether the driver texture may hold texels the shadow never saw. Set the first time + // anything other than a texel upload can write it - an application framebuffer + // attachment, a writable image binding, a copy or blit into it, a driver-side mipmap + // generation - and never cleared. Until then the driver holds, outside a level's + // pending writes, exactly the shadow bytes it was last sent, so those writes may go up + // as their union box (one unpack-ring job, which is what Mali needs); from then on + // only the texels the client wrote may go up, since the shadow is stale everywhere + // else. See SyncMipmapsToBackend. + Bool m_driverMayHoldGpuWrites = false; // Latches the "this driver has no buffer textures" report to once per texture. The // report is emitted from the respecify path, which bails before recording the state // it was asked to apply - so without the latch the texture stays permanently dirty @@ -1126,6 +1138,11 @@ namespace MobileGL::MG_Backend::DirectGLES { SharedPtr& SyncTextureObjectToBackend( const SharedPtr& textureObject, Bool imageBindableStorageRequired = false); + // Every path that lets the DRIVER write a texture's texels calls this (see + // BackendTextureObject::m_driverMayHoldGpuWrites): an application framebuffer attachment, + // a writable image binding, a copy, blit or mipmap generation into it. A view writes its + // storage texture's image, so the storage texture is the one marked. + void NoteDriverSideTextureWrite(const SharedPtr& textureObject); // Brings every texture the next draw reads - the touched units' bindings and the draw // 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. diff --git a/MobileGL/MG_Backend/DirectVulkan/Renderer/VkTextureManager.cpp b/MobileGL/MG_Backend/DirectVulkan/Renderer/VkTextureManager.cpp index c00b0752..309a274e 100644 --- a/MobileGL/MG_Backend/DirectVulkan/Renderer/VkTextureManager.cpp +++ b/MobileGL/MG_Backend/DirectVulkan/Renderer/VkTextureManager.cpp @@ -2879,11 +2879,10 @@ namespace MobileGL::MG_Backend::DirectVulkan { IntVec3 regionLo = {0, 0, 0}; IntVec3 regionSize = {0, 0, 0}; SizeT texelBytes = 0; - // Scatter refinement of the single dirty box: when the storage's rect - // list reports the writes' true footprint (~100 sprites whose union box - // spans the whole atlas), each rect is staged tightly and copied with - // its own VkBufferImageCopy in ONE vkCmdCopyBufferToImage. Empty means - // "stage the one box above". Only set while subRegion. + // The writes' exact footprint when it is more than one box (~100 sprites + // whose union box spans the whole atlas): each rect is staged tightly and + // copied with its own VkBufferImageCopy in ONE vkCmdCopyBufferToImage. + // Empty means "stage the one box above". Only set while subRegion. Vector rects; }; @@ -2895,6 +2894,7 @@ namespace MobileGL::MG_Backend::DirectVulkan { targets.push_back(uploadTarget); } const TextureFormatInfo formatInfo = ResolveTextureFormatInfo(mipmapTexture.GetFormat()); + Vector dirtyFootprint; VkDeviceSize stagingSize = 0; for (const TextureUploadTarget target : targets) { @@ -2956,8 +2956,16 @@ namespace MobileGL::MG_Backend::DirectVulkan { const SizeT texelCount = static_cast(texelSize.x()) * static_cast(texelSize.y()) * static_cast(std::max(texelSize.z(), 1)); - if (!region.Empty() && !region.CoversWholeLevel(texelSize) && texelCount > 0 && - levelBytes % texelCount == 0) { + // Exactly the texels the client wrote, never their union box or the storage's + // rect list (which merges touching writes into their bounding box, folds when + // full and is withheld past 3/4 of the box): the image may hold what the GPU + // drew or cleared anywhere else, and the shadow is stale there. Each box is one + // more VkBufferImageCopy in the same vkCmdCopyBufferToImage, which costs next + // to nothing, so there is no cheaper shape worth being wrong for. A union box + // that spans the level with gaps in it is not a whole-level write either. + mipmapTexture.GetStorageDirtyFootprint(target, level, dirtyFootprint); + if (!region.Empty() && (dirtyFootprint.size() >= 2 || !region.CoversWholeLevel(texelSize)) && + texelCount > 0 && levelBytes % texelCount == 0) { uploadItem.subRegion = true; uploadItem.regionLo = region.lo; uploadItem.regionSize = {region.hi.x() - region.lo.x(), region.hi.y() - region.lo.y(), @@ -2967,17 +2975,10 @@ namespace MobileGL::MG_Backend::DirectVulkan { static_cast(uploadItem.regionSize.y()) * static_cast(uploadItem.regionSize.z()) * uploadItem.texelBytes; - // Scatter refinement: the storage only hands out its rect list - // when the rects' summed area is materially smaller than the - // union box (0 otherwise), so taking it always stages fewer - // bytes than the box - the very amplification this path exists - // to avoid paying twice. - MG_State::GLState::MipmapDirtyRegion - dirtyRects[MG_State::GLState::MipmapStorage::kMaxDirtyRects]; - const SizeT dirtyRectCount = mipmapTexture.GetStorageDirtyRects( - target, level, dirtyRects, MG_State::GLState::MipmapStorage::kMaxDirtyRects); - if (dirtyRectCount >= 2) { - uploadItem.rects.assign(dirtyRects, dirtyRects + dirtyRectCount); + // Several boxes: each is staged tightly and copied on its own. One box is + // the union box above. + if (dirtyFootprint.size() >= 2) { + uploadItem.rects.assign(dirtyFootprint.begin(), dirtyFootprint.end()); SizeT rectTexels = 0; for (const auto& rect : uploadItem.rects) { rectTexels += rect.TexelCount(); @@ -3211,6 +3212,7 @@ namespace MobileGL::MG_Backend::DirectVulkan { const Bool depthSelectsArrayLayer = outResource.viewType == VK_IMAGE_VIEW_TYPE_1D_ARRAY || outResource.viewType == VK_IMAGE_VIEW_TYPE_2D_ARRAY || outResource.viewType == VK_IMAGE_VIEW_TYPE_CUBE_ARRAY; + Vector rectCopies; for (const auto& item : uploadItems) { if (!item.rects.empty()) { // Multi-rect item: one VkBufferImageCopy per rect, all submitted in a @@ -3218,7 +3220,7 @@ namespace MobileGL::MG_Backend::DirectVulkan { // construction, so no two copies write the same texels. Multi-rect // implies subRegion, which implies a plain color aspect - the combined // depth-stencil split below can never see one of these. - VkBufferImageCopy rectCopies[MG_State::GLState::MipmapStorage::kMaxDirtyRects]; + rectCopies.resize(item.rects.size()); Uint32 rectCopyCount = 0; VkDeviceSize runningOffset = item.offset; for (const auto& rect : item.rects) { @@ -3249,7 +3251,7 @@ namespace MobileGL::MG_Backend::DirectVulkan { runningOffset += static_cast(rect.TexelCount() * item.texelBytes); } vkCmdCopyBufferToImage(commandBuffer, stagingBuffer, outResource.image, - VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL, rectCopyCount, rectCopies); + VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL, rectCopyCount, rectCopies.data()); continue; } const Uint32 depthOrLayers = item.texelSize.z() > 0 ? static_cast(item.texelSize.z()) : 1u; diff --git a/MobileGL/MG_State/GLState/TextureState/MipmapStorage.cpp b/MobileGL/MG_State/GLState/TextureState/MipmapStorage.cpp index 58affe99..3ba2ecdf 100644 --- a/MobileGL/MG_State/GLState/TextureState/MipmapStorage.cpp +++ b/MobileGL/MG_State/GLState/TextureState/MipmapStorage.cpp @@ -8,6 +8,9 @@ #include "MipmapStorage.h" +#include +#include + namespace MobileGL { namespace MG_State { namespace GLState { @@ -26,6 +29,135 @@ namespace MobileGL { IntVec3{std::max(a.hi.x(), b.hi.x()), std::max(a.hi.y(), b.hi.y()), std::max(a.hi.z(), b.hi.z())}}; } + + MipmapDirtyRegion RegionIntersection(const MipmapDirtyRegion& a, const MipmapDirtyRegion& b) { + return {IntVec3{std::max(a.lo.x(), b.lo.x()), std::max(a.lo.y(), b.lo.y()), + std::max(a.lo.z(), b.lo.z())}, + IntVec3{std::min(a.hi.x(), b.hi.x()), std::min(a.hi.y(), b.hi.y()), + std::min(a.hi.z(), b.hi.z())}}; + } + + // For two boxes of written texels: whether their bounding box holds no texel + // outside them, i.e. whether merging them keeps the rect list exact. + Bool UnionIsExact(const MipmapDirtyRegion& a, const MipmapDirtyRegion& b) { + return RegionUnion(a, b).TexelCount() == + a.TexelCount() + b.TexelCount() - RegionIntersection(a, b).TexelCount(); + } + + Bool SameFootprintXY(const MipmapDirtyRegion& a, const MipmapDirtyRegion& b) { + return a.lo.x() == b.lo.x() && a.hi.x() == b.hi.x() && a.lo.y() == b.lo.y() && + a.hi.y() == b.hi.y(); + } + + Bool FootprintXYLess(const MipmapDirtyRegion& a, const MipmapDirtyRegion& b) { + if (a.lo.y() != b.lo.y()) return a.lo.y() < b.lo.y(); + if (a.lo.x() != b.lo.x()) return a.lo.x() < b.lo.x(); + if (a.hi.y() != b.hi.y()) return a.hi.y() < b.hi.y(); + return a.hi.x() < b.hi.x(); + } + + // Cuts the union of a set of (possibly overlapping) write boxes into disjoint boxes + // holding exactly its texels, at a cost set by the rows written, not by the level. + // Every box is split into row runs, the runs of each row are sorted and merged, and + // a sweep down each slice lets a row's run continue the box above it when it spans + // the same columns on the very next row; a slice's box then continues the previous + // slice's when that slice is the one before and the box covers the same rows and + // columns. + void DecomposeFootprintWrites(const Vector& writes, Vector& out) { + struct Run { + Int z, y, x0, x1; + }; + Vector runs; + for (const auto& box : writes) { + for (Int z = box.lo.z(); z < box.hi.z(); ++z) + for (Int y = box.lo.y(); y < box.hi.y(); ++y) runs.push_back({z, y, box.lo.x(), box.hi.x()}); + } + std::sort(runs.begin(), runs.end(), [](const Run& a, const Run& b) { + if (a.z != b.z) return a.z < b.z; + if (a.y != b.y) return a.y < b.y; + return a.x0 < b.x0; + }); + SizeT merged = 0; + for (const Run& run : runs) { + Run* last = merged > 0 ? &runs[merged - 1] : nullptr; + if (last && last->z == run.z && last->y == run.y && run.x0 <= last->x1) { + last->x1 = std::max(last->x1, run.x1); + } else { + runs[merged++] = run; + } + } + runs.resize(merged); + + Vector open, next, slice; + Vector previousSlice, currentSlice; // indices into `out`, in FootprintXYLess order + Int sliceZ = 0, previousZ = 0, rowY = 0; + Bool haveSlice = false; + // Ends every open box below row `rowY` and folds the slice into `out`. + const auto closeSlice = [&]() { + for (auto& box : open) { + box.hi = {box.hi.x(), rowY + 1, sliceZ + 1}; + slice.push_back(box); + } + open.clear(); + std::sort(slice.begin(), slice.end(), FootprintXYLess); + if (previousZ + 1 != sliceZ) previousSlice.clear(); + currentSlice.clear(); + SizeT p = 0; + for (const auto& box : slice) { + while (p < previousSlice.size() && FootprintXYLess(out[previousSlice[p]], box)) ++p; + if (p < previousSlice.size() && SameFootprintXY(out[previousSlice[p]], box)) { + out[previousSlice[p]].hi = {box.hi.x(), box.hi.y(), sliceZ + 1}; + currentSlice.push_back(previousSlice[p++]); + } else { + currentSlice.push_back(out.size()); + out.push_back(box); + } + } + std::swap(previousSlice, currentSlice); + slice.clear(); + previousZ = sliceZ; + }; + for (SizeT i = 0; i < runs.size();) { + const Int z = runs[i].z; + const Int y = runs[i].y; + if (haveSlice && z != sliceZ) { + closeSlice(); + } else if (haveSlice && y != rowY + 1) { + // A row with no runs in between: nothing open continues across it. + for (auto& box : open) { + box.hi = {box.hi.x(), rowY + 1, sliceZ + 1}; + slice.push_back(box); + } + open.clear(); + } + haveSlice = true; + sliceZ = z; + next.clear(); + SizeT o = 0; + for (; i < runs.size() && runs[i].z == z && runs[i].y == y; ++i) { + const Int x0 = runs[i].x0; + const Int x1 = runs[i].x1; + // `open` is sorted by x and disjoint: every box left of this run that + // it cannot continue ends on the row above. + while (o < open.size() && open[o].lo.x() < x0) { + open[o].hi = {open[o].hi.x(), y, z + 1}; + slice.push_back(open[o++]); + } + if (o < open.size() && open[o].lo.x() == x0 && open[o].hi.x() == x1) { + next.push_back(open[o++]); + } else { + next.push_back({IntVec3{x0, y, z}, IntVec3{x1, y + 1, z + 1}}); + } + } + for (; o < open.size(); ++o) { + open[o].hi = {open[o].hi.x(), y, z + 1}; + slice.push_back(open[o]); + } + std::swap(open, next); + rowY = y; + } + if (haveSlice) closeSlice(); + } } // namespace SizeT MipmapStorage::GetLevelCount() const { @@ -46,6 +178,7 @@ namespace MobileGL { m_isDirty.resize(requiredLevelCount, false); m_dirtyRegions.resize(requiredLevelCount); m_dirtyRects.resize(requiredLevelCount); + m_footprintWrites.resize(requiredLevelCount); m_compressedData.resize(requiredLevelCount); m_compressedFormats.resize(requiredLevelCount, GL_NONE); m_requestedCompressedFormats.resize(requiredLevelCount, GL_NONE); @@ -68,6 +201,9 @@ namespace MobileGL { if (level < m_dirtyRects.size()) { m_dirtyRects[level].clear(); } + if (level < m_footprintWrites.size()) { + m_footprintWrites[level].clear(); + } auto& data = m_data[level]; data.resize(input.byteSize, 0); @@ -132,6 +268,7 @@ namespace MobileGL { m_isDirty.resize(levelCount); m_dirtyRegions.resize(levelCount); m_dirtyRects.resize(levelCount); + m_footprintWrites.resize(levelCount); m_compressedData.resize(levelCount); m_compressedFormats.resize(levelCount); m_requestedCompressedFormats.resize(levelCount); @@ -188,6 +325,9 @@ namespace MobileGL { if (level < m_dirtyRects.size()) { m_dirtyRects[level].clear(); } + if (level < m_footprintWrites.size()) { + m_footprintWrites[level].clear(); + } } bool MipmapStorage::IsDirty(Uint level) const { @@ -212,11 +352,17 @@ namespace MobileGL { auto& rects = m_dirtyRects[level]; if (!m_isDirty[level]) { rects.clear(); // stale-safety; MarkDirty(false) already cleared it + m_footprintWrites[level].clear(); } else if (rects.empty() && level < m_dirtyRegions.size() && !m_dirtyRegions[level].Empty()) { rects.push_back(m_dirtyRegions[level]); } + // Entering the log inside the insert already recorded this write. + const Bool logged = !m_footprintWrites[level].empty(); InsertDirtyRect(level, incoming); + if (logged) { + m_footprintWrites[level].push_back(incoming); + } } if (level < m_dirtyRegions.size()) { MipmapDirtyRegion& region = m_dirtyRegions[level]; @@ -248,6 +394,9 @@ namespace MobileGL { merged = false; for (SizeT i = 0; i < rects.size(); ++i) { if (RegionsTouch(rects[i], incoming)) { + if (!UnionIsExact(rects[i], incoming)) { + EnterFootprintLog(level, incoming); + } incoming = RegionUnion(rects[i], incoming); rects[i] = rects.back(); rects.pop_back(); @@ -260,6 +409,7 @@ namespace MobileGL { rects.push_back(incoming); return; } + EnterFootprintLog(level, incoming); // Full: fold the incoming rect into the neighbour whose box grows least // (least new area dragged into the upload), then re-insert the grown // box - it may now touch others. The removal above guarantees the @@ -310,6 +460,30 @@ namespace MobileGL { } return rects.size(); } + + void MipmapStorage::EnterFootprintLog(Uint level, const MipmapDirtyRegion& pending) { + auto& log = m_footprintWrites[level]; + if (!log.empty()) return; + log.assign(m_dirtyRects[level].begin(), m_dirtyRects[level].end()); + log.push_back(pending); + } + + void MipmapStorage::GetDirtyFootprint(Uint level, Vector& outRects) const { + outRects.clear(); + if (level >= m_isDirty.size() || !m_isDirty[level] || level >= m_dirtyRegions.size()) return; + if (level < m_footprintWrites.size() && !m_footprintWrites[level].empty()) { + DecomposeFootprintWrites(m_footprintWrites[level], outRects); + return; + } + // No log: every merge so far was exact and nothing folded, so the list is the + // footprint - and an empty list leaves the union box, which is then a whole-level + // write (MarkDirty, a respecify) or the single box one write left. + if (level < m_dirtyRects.size() && !m_dirtyRects[level].empty()) { + outRects.assign(m_dirtyRects[level].begin(), m_dirtyRects[level].end()); + } else if (!m_dirtyRegions[level].Empty()) { + outRects.push_back(m_dirtyRegions[level]); + } + } } // namespace GLState } // namespace MG_State } // namespace MobileGL diff --git a/MobileGL/MG_State/GLState/TextureState/MipmapStorage.h b/MobileGL/MG_State/GLState/TextureState/MipmapStorage.h index fe632db4..6774c07e 100644 --- a/MobileGL/MG_State/GLState/TextureState/MipmapStorage.h +++ b/MobileGL/MG_State/GLState/TextureState/MipmapStorage.h @@ -80,6 +80,15 @@ namespace MobileGL { // area so close to the union box's that one big upload beats many small // ones (fewer driver calls wins when the bytes are nearly equal). SizeT GetDirtyRects(Uint level, MipmapDirtyRegion* outRects, SizeT maxRects) const; + // The level's pending writes EXACTLY: pairwise-disjoint boxes holding every texel + // written since the level was last clean and no other, however many there are and + // however much of the union box they fill. Empty while the level is clean. The + // rect list above only COVERS the writes - a merge replaces two touching rects by + // their bounding box and a full list folds - which is harmless while the texels + // outside the writes are the same in the shadow and in the backend's copy, and + // wrong once the backend's copy may hold texels the GPU wrote: the shadow is stale + // there, and anything uploaded beyond the writes goes up over them. + void GetDirtyFootprint(Uint level, Vector& outRects) const; // The bytes an application handed to glCompressedTexImage*, kept verbatim beside the // (uncompressed) texel shadow rather than in place of it. GL 4.6 core 8.11 requires @@ -112,6 +121,10 @@ namespace MobileGL { // Insert one clamped, non-empty write box, keeping the list disjoint // and bounded (see kMaxDirtyRects). void InsertDirtyRect(Uint level, MipmapDirtyRegion incoming); + // Starts the level's footprint log (see m_footprintWrites) the moment the rect list + // is about to stop being exact. `pending` is the write being inserted, merged with + // the rects it has absorbed so far - still exact, and no longer in the list. + void EnterFootprintLog(Uint level, const MipmapDirtyRegion& pending); Vector m_texelSizes; Vector> m_data; @@ -125,6 +138,14 @@ namespace MobileGL { // already-dirty level seeds the list from the union box accumulated so // far and refines from there. Vector> m_dirtyRects; + // Per level, the boxes whose union is exactly the pending writes' footprint, as they + // came (overlaps and all; GetDirtyFootprint cuts them apart). EMPTY is the resting + // state and means the rect list (or, while that is empty, the union box) is itself + // exact; the log starts only at the first inexact merge or fold, from the list as it + // stood just before, and then takes every write verbatim until the level is clean or + // respecified - one box per write, so its cost follows the writes and never the + // level's size. clear() keeps the capacity, like the rect list's. + Vector> m_footprintWrites; Vector> m_compressedData; Vector m_compressedFormats; Vector m_requestedCompressedFormats; diff --git a/MobileGL/MG_State/GLState/TextureState/MipmapUploadTargetArray.h b/MobileGL/MG_State/GLState/TextureState/MipmapUploadTargetArray.h index 5142422c..d1ec8f5d 100644 --- a/MobileGL/MG_State/GLState/TextureState/MipmapUploadTargetArray.h +++ b/MobileGL/MG_State/GLState/TextureState/MipmapUploadTargetArray.h @@ -90,6 +90,11 @@ namespace MobileGL { return m_storage[targetIndex].GetDirtyRects(level, outRects, maxRects); } + void GetDirtyFootprint(Uint targetIndex, Uint level, Vector& outRects) const { + MOBILEGL_ASSERT(targetIndex < TargetCount, "GetDirtyFootprint: target invalid"); + m_storage[targetIndex].GetDirtyFootprint(level, outRects); + } + void SetCompressedImage(Uint targetIndex, Uint level, GLenum internalFormat, const void* data, SizeT size) { MOBILEGL_ASSERT(targetIndex < TargetCount, "SetCompressedImage: target invalid"); diff --git a/MobileGL/MG_State/GLState/TextureState/TextureObject.cpp b/MobileGL/MG_State/GLState/TextureState/TextureObject.cpp index 039e301e..4522cb79 100644 --- a/MobileGL/MG_State/GLState/TextureState/TextureObject.cpp +++ b/MobileGL/MG_State/GLState/TextureState/TextureObject.cpp @@ -379,6 +379,12 @@ namespace MobileGL { outRects, maxRects); } + void TextureObjectWithOneMipmap::GetStorageDirtyFootprint(TextureUploadTarget uploadTarget, + Uint mipmapLevel, + Vector& outRects) const { + m_textureStorage.GetDirtyFootprint(GetIndexOfTextureUploadTarget(uploadTarget), mipmapLevel, outRects); + } + void TextureObjectWithOneMipmap::SetMipmapCompressedImage(TextureUploadTarget uploadTarget, Uint mipmapLevel, GLenum internalFormat, const void* data, SizeT size) { m_textureStorage.SetCompressedImage(GetIndexOfTextureUploadTarget(uploadTarget), mipmapLevel, diff --git a/MobileGL/MG_State/GLState/TextureState/TextureObject.h b/MobileGL/MG_State/GLState/TextureState/TextureObject.h index 48ef2777..80cab6e2 100644 --- a/MobileGL/MG_State/GLState/TextureState/TextureObject.h +++ b/MobileGL/MG_State/GLState/TextureState/TextureObject.h @@ -269,6 +269,18 @@ namespace MobileGL::MG_State::GLState { (void)maxRects; return 0; } + // Exactly the texels written since the level was last clean, as disjoint boxes - not a + // cover of them like the rects above, and never withheld. See MipmapStorage:: + // GetDirtyFootprint for why a backend whose copy of the level may hold GPU-written texels + // uploads these and nothing else. The base fallback answers the whole-level box its + // MarkStorageDirtyRegion recorded. + virtual void GetStorageDirtyFootprint(TextureUploadTarget uploadTarget, Uint mipmapLevel, + Vector& outRects) const { + outRects.clear(); + if (IsStorageDirty(uploadTarget, mipmapLevel)) { + outRects.push_back(GetStorageDirtyRegion(uploadTarget, mipmapLevel)); + } + } // The compressed image a glCompressedTexImage* call shadowed for this level, kept verbatim // next to the texel data rather than instead of it - see MipmapStorage. The texel shadow @@ -351,6 +363,8 @@ namespace MobileGL::MG_State::GLState { MipmapDirtyRegion GetStorageDirtyRegion(TextureUploadTarget uploadTarget, Uint mipmapLevel) const override; SizeT GetStorageDirtyRects(TextureUploadTarget uploadTarget, Uint mipmapLevel, MipmapDirtyRegion* outRects, SizeT maxRects) const override; + void GetStorageDirtyFootprint(TextureUploadTarget uploadTarget, Uint mipmapLevel, + Vector& outRects) const override; void SetMipmapCompressedImage(TextureUploadTarget uploadTarget, Uint mipmapLevel, GLenum internalFormat, const void* data, SizeT size) override; GLenum GetMipmapCompressedFormat(TextureUploadTarget uploadTarget, Uint mipmapLevel) const override; diff --git a/MobileGL/MG_State/GLState/TextureState/TextureObject2DCube.cpp b/MobileGL/MG_State/GLState/TextureState/TextureObject2DCube.cpp index 765477c5..3f2c9338 100644 --- a/MobileGL/MG_State/GLState/TextureState/TextureObject2DCube.cpp +++ b/MobileGL/MG_State/GLState/TextureState/TextureObject2DCube.cpp @@ -75,6 +75,11 @@ namespace MobileGL { outRects, maxRects); } + void TextureObject2DCube::GetStorageDirtyFootprint(TextureUploadTarget uploadTarget, Uint mipmapLevel, + Vector& outRects) const { + m_textureStorage.GetDirtyFootprint(GetIndexOfTextureUploadTarget(uploadTarget), mipmapLevel, outRects); + } + void TextureObject2DCube::SetMipmapCompressedImage(TextureUploadTarget uploadTarget, Uint mipmapLevel, GLenum internalFormat, const void* data, SizeT size) { m_textureStorage.SetCompressedImage(GetIndexOfTextureUploadTarget(uploadTarget), mipmapLevel, diff --git a/MobileGL/MG_State/GLState/TextureState/TextureObject2DCube.h b/MobileGL/MG_State/GLState/TextureState/TextureObject2DCube.h index d705db02..546d6a04 100644 --- a/MobileGL/MG_State/GLState/TextureState/TextureObject2DCube.h +++ b/MobileGL/MG_State/GLState/TextureState/TextureObject2DCube.h @@ -33,6 +33,8 @@ namespace MobileGL { Uint mipmapLevel) const override; SizeT GetStorageDirtyRects(TextureUploadTarget uploadTarget, Uint mipmapLevel, MipmapDirtyRegion* outRects, SizeT maxRects) const override; + void GetStorageDirtyFootprint(TextureUploadTarget uploadTarget, Uint mipmapLevel, + Vector& outRects) const override; void SetMipmapCompressedImage(TextureUploadTarget uploadTarget, Uint mipmapLevel, GLenum internalFormat, const void* data, SizeT size) override; GLenum GetMipmapCompressedFormat(TextureUploadTarget uploadTarget, Uint mipmapLevel) const override; diff --git a/MobileGL/MG_State/GLState/TextureState/TextureObjectView.cpp b/MobileGL/MG_State/GLState/TextureState/TextureObjectView.cpp index c299c1c2..cf02f662 100644 --- a/MobileGL/MG_State/GLState/TextureState/TextureObjectView.cpp +++ b/MobileGL/MG_State/GLState/TextureState/TextureObjectView.cpp @@ -322,6 +322,15 @@ namespace MobileGL::MG_State::GLState { return m_ownerMipmap->GetStorageDirtyRegion(ToOwnerUploadTarget(uploadTarget), ToOwnerLevel(mipmapLevel)); } + // In the owner's texel space, like GetStorageDirtyRegion. + void TextureObjectView::GetStorageDirtyFootprint(TextureUploadTarget uploadTarget, Uint mipmapLevel, + Vector& outRects) const { + outRects.clear(); + if (m_ownerMipmap == nullptr) return; + m_ownerMipmap->GetStorageDirtyFootprint(ToOwnerUploadTarget(uploadTarget), ToOwnerLevel(mipmapLevel), + outRects); + } + void TextureObjectView::SetMipmapCompressedImage(TextureUploadTarget uploadTarget, Uint mipmapLevel, GLenum internalFormat, const void* data, SizeT size) { if (m_ownerMipmap == nullptr) return; diff --git a/MobileGL/MG_State/GLState/TextureState/TextureObjectView.h b/MobileGL/MG_State/GLState/TextureState/TextureObjectView.h index 9a4d9ecb..93b3b1c2 100644 --- a/MobileGL/MG_State/GLState/TextureState/TextureObjectView.h +++ b/MobileGL/MG_State/GLState/TextureState/TextureObjectView.h @@ -82,6 +82,8 @@ namespace MobileGL::MG_State::GLState { void MarkStorageDirtyRegion(TextureUploadTarget uploadTarget, Uint mipmapLevel, IntVec3 offset, IntVec3 size) override; MipmapDirtyRegion GetStorageDirtyRegion(TextureUploadTarget uploadTarget, Uint mipmapLevel) const override; + void GetStorageDirtyFootprint(TextureUploadTarget uploadTarget, Uint mipmapLevel, + Vector& outRects) const override; void SetMipmapCompressedImage(TextureUploadTarget uploadTarget, Uint mipmapLevel, GLenum internalFormat, const void* data, SizeT size) override; GLenum GetMipmapCompressedFormat(TextureUploadTarget uploadTarget, Uint mipmapLevel) const override; From 21c7e8cd635acb69058b017829993c74a72a4dc0 Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Tue, 29 Sep 2026 09:06:07 -0400 Subject: [PATCH 2/9] [Test] (MG_IntegrationTest, MG_Test): scattered writes over a GPU clear on both backends - two distant 2x2 boxes, a hundred boxes, boxes filling 3/4 of their union and three quadrants spanning the level, the 3/4 and level-spanning cases again at the image-bindable re-mint, and a MipmapStorage footprint fuzz against a reference bitmap --- .../Scenarios/ImageBindableRemintScenario.cpp | 67 ++++++ .../PartialUploadOverGpuContentScenario.cpp | 93 ++++++++ MobileGL/MG_Test/State/CMakeLists.txt | 25 ++ .../MG_Test/State/MipmapFootprintTest.cpp | 214 ++++++++++++++++++ 4 files changed, 399 insertions(+) create mode 100644 MobileGL/MG_Test/State/MipmapFootprintTest.cpp diff --git a/MobileGL/MG_IntegrationTest/Scenarios/ImageBindableRemintScenario.cpp b/MobileGL/MG_IntegrationTest/Scenarios/ImageBindableRemintScenario.cpp index e06587fa..72e486bd 100644 --- a/MobileGL/MG_IntegrationTest/Scenarios/ImageBindableRemintScenario.cpp +++ b/MobileGL/MG_IntegrationTest/Scenarios/ImageBindableRemintScenario.cpp @@ -29,6 +29,7 @@ // the last sync, and a pending client write that has to land on top of GPU content. Magma never // re-mints from a shadow (it copies image to image) and is the control backend. +#include #include #include #include @@ -240,6 +241,35 @@ void main() { o_color = texelFetch(u_tex, ivec2(gl_FragCoord.xy) % 4, 0); } EXPECT_EQ(bad, 0) << bad << " of " << edge * edge << " texels differ; first " << first.str(); } + // One glTexSubImage of opaque blue per {x, y, w, h} box, left pending. + static void WriteBlueBoxes(GLuint tex, const std::vector>& boxes) { + glBindTexture(GL_TEXTURE_2D, tex); + for (const auto& b : boxes) { + std::vector blue(static_cast(b[2] * b[3]) * 4u, 0); + for (std::size_t i = 0; i < blue.size(); i += 4) { + blue[i + 2] = 255; + blue[i + 3] = 255; + } + glTexSubImage2D(GL_TEXTURE_2D, 0, b[0], b[1], b[2], b[3], GL_RGBA, GL_UNSIGNED_BYTE, blue.data()); + } + glBindTexture(GL_TEXTURE_2D, 0); + } + + // kEdge x kEdge green, blue inside the boxes. + static std::vector GreenWithBlueBoxes(const std::vector>& boxes) { + auto want = Solid(kEdge, 0, 255, 0, 255); + for (const auto& b : boxes) + for (int y = b[1]; y < b[1] + b[3]; ++y) + for (int x = b[0]; x < b[0] + b[2]; ++x) { + const std::size_t at = static_cast((y * kEdge + x) * 4); + want[at] = 0; + want[at + 1] = 0; + want[at + 2] = 255; + want[at + 3] = 255; + } + return want; + } + GLuint m_sampleProgram = 0, m_vao = 0, m_fbo = 0; ColorFbo m_scratch{}; std::vector m_textures; @@ -326,6 +356,43 @@ void main() { o_color = texelFetch(u_tex, ivec2(gl_FragCoord.xy) % 4, 0); } ExpectImage(ImageCopy(t, 0, GL_RGBA8, "rgba8"), kEdge, want); } + // Several pending writes: only their texels land on the GPU's clear, never the gaps between + // them. The re-mint merged the union box of the shadow over the readback whenever the rect + // list was withheld, and kept the shadow outright when that box spanned the level. + + // Two 3x7 columns with a one-texel gap: 42 of their union's 49 texels, past the 3/4 at which + // the storage withholds its rect list. + TEST_F(ImageBindableRemintScenario, PendingSubImagesFillingThreeQuartersOfTheirUnionLandOnTheGpuClear) { + if (!Ready()) return; + const std::vector> boxes = {{0, 0, 3, 7}, {4, 0, 3, 7}}; + const GLuint t = NewTexture(); + const auto p = Pattern(kEdge, 0x40); + glBindTexture(GL_TEXTURE_2D, t); + glTexImage2D(GL_TEXTURE_2D, 0, GL_RGBA8, kEdge, kEdge, 0, GL_RGBA, GL_UNSIGNED_BYTE, p.data()); + Nearest(GL_TEXTURE_2D, t, 0); + SyncBySampling(t); + GpuClear(t, 0, kGreen); + SyncBySampling(t); + WriteBlueBoxes(t, boxes); + ExpectImage(ImageCopy(t, 0, GL_RGBA8, "rgba8"), kEdge, GreenWithBlueBoxes(boxes)); + } + + // Three quadrants: the union box is the whole level; the fourth quadrant is the GPU's. + TEST_F(ImageBindableRemintScenario, PendingSubImagesWhoseUnionSpansTheLevelLandOnTheGpuClear) { + if (!Ready()) return; + const std::vector> boxes = {{0, 0, 4, 4}, {4, 0, 4, 4}, {0, 4, 4, 4}}; + const GLuint t = NewTexture(); + const auto p = Pattern(kEdge, 0x40); + glBindTexture(GL_TEXTURE_2D, t); + glTexImage2D(GL_TEXTURE_2D, 0, GL_RGBA8, kEdge, kEdge, 0, GL_RGBA, GL_UNSIGNED_BYTE, p.data()); + Nearest(GL_TEXTURE_2D, t, 0); + SyncBySampling(t); + GpuClear(t, 0, kGreen); + SyncBySampling(t); + WriteBlueBoxes(t, boxes); + ExpectImage(ImageCopy(t, 0, GL_RGBA8, "rgba8"), kEdge, GreenWithBlueBoxes(boxes)); + } + TEST_F(ImageBindableRemintScenario, GeneratedMipLevelSurvives) { if (!Ready()) return; const GLuint t = NewTexture(); diff --git a/MobileGL/MG_IntegrationTest/Scenarios/PartialUploadOverGpuContentScenario.cpp b/MobileGL/MG_IntegrationTest/Scenarios/PartialUploadOverGpuContentScenario.cpp index 617ccf3a..bcd5960d 100644 --- a/MobileGL/MG_IntegrationTest/Scenarios/PartialUploadOverGpuContentScenario.cpp +++ b/MobileGL/MG_IntegrationTest/Scenarios/PartialUploadOverGpuContentScenario.cpp @@ -30,6 +30,15 @@ // earlier clear or draw landed over it: the box read back green. Each order is covered - the // clear already recorded, still queued, synced by a draw or by the readback, the reverse order // in which the clear must win, and a draw instead of a clear. +// +// The last part is about SCATTER: several boxes written between two syncs, with the GPU's clear in +// the gaps between them. Everything a backend has to go on is the union box and the storage's +// rect list, and both describe more than the writes - the box spans the gaps, the list merges +// touching rects into their bounding box, folds once it holds 96, and is withheld once its rects +// fill 3/4 of the box; a box spanning the level went up as the whole level. Espryt also uploaded +// the union box whenever its unpack ring was in use. Each case below reaches one of those: two +// distant boxes, a hundred of them, boxes filling more than 3/4 of their union, and boxes whose +// union spans the level. #include #include @@ -316,6 +325,60 @@ void main() { ExpectClearAroundBox(rgb, kEdge, kEdge, kBoxAt, kBoxAt, boxWins ? kBoxEdge : 0, boxWins ? kBoxEdge : 0); } + struct Box { + int x, y, w, h; + }; + + // An edge x edge RGBA8 level uploaded red, cleared green on the GPU, then written blue in + // every box by one glTexSubImage each, with no sync in between; read back after a sync. + // Blue inside the boxes, green everywhere else. + void RunScatteredWrites(int edge, const std::vector& boxes) { + const GLuint t = NewTexture(GL_TEXTURE_2D); + const auto red = Fill(Rgba8(), edge * edge, 1.0f, 0.0f, 0.0f); + glBindTexture(GL_TEXTURE_2D, t); + glPixelStorei(GL_UNPACK_ALIGNMENT, 1); + glTexImage2D(GL_TEXTURE_2D, 0, GL_RGBA8, edge, edge, 0, GL_RGBA, GL_UNSIGNED_BYTE, red.data()); + glBindTexture(GL_TEXTURE_2D, 0); + ASSERT_EQ(FirstGLError(), 0u) << "definition"; + SampleRgba8(t); + ClearRgba8Green(t); + SampleRgba8(t); + glBindTexture(GL_TEXTURE_2D, t); + for (const Box& box : boxes) { + const auto blue = Fill(Rgba8(), box.w * box.h, 0.0f, 0.0f, 1.0f); + glTexSubImage2D(GL_TEXTURE_2D, 0, box.x, box.y, box.w, box.h, GL_RGBA, GL_UNSIGNED_BYTE, + blue.data()); + } + glBindTexture(GL_TEXTURE_2D, 0); + glPixelStorei(GL_UNPACK_ALIGNMENT, 4); + ASSERT_EQ(FirstGLError(), 0u) << "writes"; + SampleRgba8(t); + ASSERT_TRUE(Attach(t, -1)); + const auto rgb = ReadRgb(Rgba8(), edge, edge); + Detach(); + int bad = 0, stale = 0; + std::ostringstream first; + for (int y = 0; y < edge; ++y) + for (int x = 0; x < edge; ++x) { + bool inBox = false; + for (const Box& box : boxes) + inBox |= x >= box.x && x < box.x + box.w && y >= box.y && y < box.y + box.h; + const float want[3] = {0.0f, inBox ? 0.0f : 1.0f, inBox ? 1.0f : 0.0f}; + const float* got = &rgb[static_cast(y * edge + x) * 3u]; + bool texelBad = false; + for (int c = 0; c < 3; ++c) texelBad |= std::fabs(got[c] - want[c]) > 0.02f; + if (!texelBad) continue; + stale += got[0] > 0.5f; + if (bad++ == 0) { + first << "(" << x << "," << y << ") got " << got[0] << "," << got[1] << "," << got[2] + << " want " << want[0] << "," << want[1] << "," << want[2]; + } + } + EXPECT_EQ(bad, 0) << bad << " of " << edge * edge << " texels wrong, " << stale + << " of them red - the stale shadow went up outside the writes; first " + << first.str(); + } + GLuint m_vao = 0, m_fbo = 0; ColorFbo m_scratch{}; std::vector m_programs; @@ -424,5 +487,35 @@ void main() { SampleRgba8(t); ExpectRgba8(t, true); } + + // Two boxes in opposite corners: their union box is 7x7, of which the writes are 8 texels. + TEST_F(PartialUploadOverGpuContentScenario, TwoDistantSubImagesKeepTheGpuClearBetweenThem) { + if (!Ready()) return; + RunScatteredWrites(kEdge, {{0, 0, 2, 2}, {5, 5, 2, 2}}); + } + + // A hundred 2x2 boxes two texels apart: more than the 96 rects the storage keeps, so its + // list folds neighbours together, gaps and all. + TEST_F(PartialUploadOverGpuContentScenario, AHundredSubImagesKeepTheGpuClearBetweenThem) { + if (!Ready()) return; + std::vector boxes; + for (int j = 0; j < 10; ++j) + for (int i = 0; i < 10; ++i) boxes.push_back({4 * i + 1, 4 * j + 1, 2, 2}); + RunScatteredWrites(40, boxes); + } + + // Four 4x4 boxes around a one-texel cross: 64 of their union's 81 texels, past the 3/4 at + // which the storage stops handing its rect list out. + TEST_F(PartialUploadOverGpuContentScenario, SubImagesFillingThreeQuartersOfTheirUnionKeepTheGpuClearBetweenThem) { + if (!Ready()) return; + RunScatteredWrites(16, {{2, 2, 4, 4}, {7, 2, 4, 4}, {2, 7, 4, 4}, {7, 7, 4, 4}}); + } + + // Three quadrants: the union box is the whole level, and the list merges the touching + // boxes into it; the fourth quadrant is still the GPU's. + TEST_F(PartialUploadOverGpuContentScenario, SubImagesWhoseUnionSpansTheLevelKeepTheGpuClearInTheGap) { + if (!Ready()) return; + RunScatteredWrites(kEdge, {{0, 0, 4, 4}, {4, 0, 4, 4}, {0, 4, 4, 4}}); + } } // namespace } // namespace MGITest diff --git a/MobileGL/MG_Test/State/CMakeLists.txt b/MobileGL/MG_Test/State/CMakeLists.txt index c04845c5..020f2482 100644 --- a/MobileGL/MG_Test/State/CMakeLists.txt +++ b/MobileGL/MG_Test/State/CMakeLists.txt @@ -94,3 +94,28 @@ if (MSVC) endif() gtest_discover_tests(NegativeApiErrorsTest DISCOVERY_TIMEOUT 30 PROPERTIES LABELS unit) + +add_executable( + MipmapFootprintTest + MipmapFootprintTest.cpp +) + +target_include_directories(MipmapFootprintTest PRIVATE + ${MGL_ROOT}/include + ${MGL_ROOT}/MobileGL + ${MGL_ROOT}/3rdparty/xxHash + ${MGL_ROOT}/3rdparty/Vulkan-Headers/include + ${MGL_ROOT}/3rdparty/SPIRV-Reflect +) + +target_link_libraries( + MipmapFootprintTest PRIVATE + GTest::gtest_main + ${LINK_LIBRARIES} +) + +if (MSVC) + target_compile_options(MipmapFootprintTest PRIVATE /Zc:preprocessor) +endif() + +gtest_discover_tests(MipmapFootprintTest DISCOVERY_TIMEOUT 30 PROPERTIES LABELS unit) diff --git a/MobileGL/MG_Test/State/MipmapFootprintTest.cpp b/MobileGL/MG_Test/State/MipmapFootprintTest.cpp new file mode 100644 index 00000000..46a5adc3 --- /dev/null +++ b/MobileGL/MG_Test/State/MipmapFootprintTest.cpp @@ -0,0 +1,214 @@ +// MobileGL - MobileGL/MG_Test/State/MipmapFootprintTest.cpp +// Copyright (c) 2025-2026 MobileGL-Dev +// Licensed under the GNU Lesser General Public License v3.0: +// https://www.gnu.org/licenses/gpl-3.0.txt +// https://www.gnu.org/licenses/lgpl-3.0.txt +// SPDX-License-Identifier: LGPL-3.0-only +// End of Source File Header +// +// MipmapStorage::GetDirtyFootprint against a reference bitmap of the writes. +// +// A backend whose copy of a level may hold GPU-written texels uploads exactly the footprint, so +// the footprint has to be the written texels and nothing else: every texel covered, no texel +// outside the writes, no texel twice. The rect list beside it only has to COVER the writes, and +// its merges, fold and 3/4 cut-off are what the footprint must not inherit - so the patterns +// below are the ones that trip them: touching boxes whose bounding box has gaps, more disjoint +// boxes than the list keeps, and random mixes of both, in 2D and 3D. The cover property of the +// list and the union box's exactness are checked alongside, since the footprint work sits inside +// the same insert. + +#include + +#include +#include +#include + +#include "Includes.h" + +#include + +using namespace MobileGL; +using MG_State::GLState::MipmapDirtyRegion; +using MG_State::GLState::MipmapStorage; + +namespace { + struct Level { + MipmapStorage storage; + IntVec3 size; + std::vector written; + + explicit Level(IntVec3 levelSize) : size(levelSize) { + storage.AllocateLevel(0, {levelSize, Texels()}); + written.assign(Texels(), false); + } + + SizeT Texels() const { + return static_cast(size.x()) * static_cast(size.y()) * static_cast(size.z()); + } + + SizeT Index(Int x, Int y, Int z) const { + return (static_cast(z) * static_cast(size.y()) + static_cast(y)) * + static_cast(size.x()) + + static_cast(x); + } + + void Write(IntVec3 offset, IntVec3 extent) { + storage.MarkDirtyRegion(0, offset, extent); + for (Int z = offset.z(); z < offset.z() + extent.z(); ++z) + for (Int y = offset.y(); y < offset.y() + extent.y(); ++y) + for (Int x = offset.x(); x < offset.x() + extent.x(); ++x) written[Index(x, y, z)] = true; + } + + void Expect(const char* what) const { + SCOPED_TRACE(what); + ASSERT_TRUE(storage.IsDirty(0)); + std::vector footprint; + storage.GetDirtyFootprint(0, footprint); + std::vector covered(Texels(), 0); + for (const auto& box : footprint) { + ASSERT_FALSE(box.Empty()); + ASSERT_GE(box.lo.x(), 0); + ASSERT_GE(box.lo.y(), 0); + ASSERT_GE(box.lo.z(), 0); + ASSERT_LE(box.hi.x(), size.x()); + ASSERT_LE(box.hi.y(), size.y()); + ASSERT_LE(box.hi.z(), size.z()); + for (Int z = box.lo.z(); z < box.hi.z(); ++z) + for (Int y = box.lo.y(); y < box.hi.y(); ++y) + for (Int x = box.lo.x(); x < box.hi.x(); ++x) ++covered[Index(x, y, z)]; + } + SizeT missing = 0, extra = 0, twice = 0; + MipmapDirtyRegion bounds{size, IntVec3{0, 0, 0}}; + for (Int z = 0; z < size.z(); ++z) + for (Int y = 0; y < size.y(); ++y) + for (Int x = 0; x < size.x(); ++x) { + const SizeT i = Index(x, y, z); + missing += written[i] && covered[i] == 0; + extra += !written[i] && covered[i] > 0; + twice += covered[i] > 1; + if (written[i]) { + bounds.lo = {std::min(bounds.lo.x(), x), std::min(bounds.lo.y(), y), + std::min(bounds.lo.z(), z)}; + bounds.hi = {std::max(bounds.hi.x(), x + 1), std::max(bounds.hi.y(), y + 1), + std::max(bounds.hi.z(), z + 1)}; + } + } + EXPECT_EQ(missing, 0u) << "written texels outside the footprint"; + EXPECT_EQ(extra, 0u) << "unwritten texels inside the footprint"; + EXPECT_EQ(twice, 0u) << "texels in more than one footprint box"; + + const MipmapDirtyRegion region = storage.GetDirtyRegion(0); + EXPECT_TRUE(region.lo == bounds.lo && region.hi == bounds.hi) + << "the union box is not the writes' bounding box"; + + MipmapDirtyRegion rects[MipmapStorage::kMaxDirtyRects]; + const SizeT rectCount = storage.GetDirtyRects(0, rects, MipmapStorage::kMaxDirtyRects); + SizeT uncovered = 0; + for (Int z = 0; z < size.z(); ++z) + for (Int y = 0; y < size.y(); ++y) + for (Int x = 0; x < size.x(); ++x) { + if (!written[Index(x, y, z)] || rectCount == 0) continue; + bool inRect = false; + for (SizeT r = 0; r < rectCount && !inRect; ++r) + inRect = x >= rects[r].lo.x() && x < rects[r].hi.x() && y >= rects[r].lo.y() && + y < rects[r].hi.y() && z >= rects[r].lo.z() && z < rects[r].hi.z(); + uncovered += !inRect; + } + EXPECT_EQ(uncovered, 0u) << "the rect list no longer covers the writes"; + } + }; + + TEST(MipmapFootprintTest, TwoDistantBoxesAreTwoBoxes) { + Level level({8, 8, 1}); + level.Write({0, 0, 0}, {2, 2, 1}); + level.Write({5, 5, 0}, {2, 2, 1}); + level.Expect("two distant 2x2"); + std::vector footprint; + level.storage.GetDirtyFootprint(0, footprint); + EXPECT_EQ(footprint.size(), 2u); + } + + // Touching boxes whose bounding box has a gap: the rect list merges them anyway. + TEST(MipmapFootprintTest, ThreeQuadrantsLeaveTheFourthOut) { + Level level({8, 8, 1}); + level.Write({0, 0, 0}, {4, 4, 1}); + level.Write({4, 0, 0}, {4, 4, 1}); + level.Write({0, 4, 0}, {4, 4, 1}); + level.Expect("three quadrants"); + } + + TEST(MipmapFootprintTest, OverlappingStaggeredBoxes) { + Level level({16, 16, 1}); + level.Write({0, 0, 0}, {6, 6, 1}); + level.Write({3, 3, 0}, {6, 6, 1}); + level.Write({6, 6, 0}, {6, 6, 1}); + level.Expect("staggered overlaps"); + } + + // More disjoint boxes than the list keeps: it folds, the footprint must not. + TEST(MipmapFootprintTest, MoreBoxesThanTheListKeeps) { + Level level({40, 40, 1}); + for (int j = 0; j < 10; ++j) + for (int i = 0; i < 10; ++i) level.Write({4 * i + 1, 4 * j + 1, 0}, {2, 2, 1}); + level.Expect("a hundred 2x2"); + std::vector footprint; + level.storage.GetDirtyFootprint(0, footprint); + EXPECT_EQ(footprint.size(), 100u); + } + + // Scanlines of one width merge exactly and stay one box without ever starting the log. + TEST(MipmapFootprintTest, AbuttingScanlinesStayOneBox) { + Level level({32, 32, 1}); + for (int y = 4; y < 20; ++y) level.Write({3, y, 0}, {10, 1, 1}); + level.Expect("scanlines"); + std::vector footprint; + level.storage.GetDirtyFootprint(0, footprint); + EXPECT_EQ(footprint.size(), 1u); + } + + // A whole-level dirty is a whole-level footprint, and a write on top of it changes nothing. + TEST(MipmapFootprintTest, WholeLevelDirtyIsTheWholeLevel) { + Level level({8, 8, 2}); + level.storage.MarkDirty(0, true); + level.written.assign(level.Texels(), true); + level.Write({1, 1, 0}, {2, 2, 1}); + level.Expect("whole level"); + } + + // Clean resets the footprint, log included. + TEST(MipmapFootprintTest, CleanStartsOver) { + Level level({8, 8, 1}); + level.Write({0, 0, 0}, {4, 4, 1}); + level.Write({4, 0, 0}, {4, 4, 1}); + level.Write({0, 4, 0}, {4, 4, 1}); + level.storage.MarkDirty(0, false); + std::vector footprint; + level.storage.GetDirtyFootprint(0, footprint); + EXPECT_TRUE(footprint.empty()); + level.written.assign(level.Texels(), false); + level.Write({5, 5, 0}, {2, 2, 1}); + level.Expect("after clean"); + } + + TEST(MipmapFootprintTest, RandomWrites) { + std::mt19937 rng(0x5eed1234u); + const IntVec3 sizes[] = {{8, 8, 1}, {33, 17, 1}, {64, 64, 1}, {12, 9, 5}, {7, 1, 1}}; + for (int round = 0; round < 400; ++round) { + const IntVec3 size = sizes[round % 5]; + Level level(size); + const int writes = 1 + static_cast(rng() % (round % 3 == 0 ? 160u : 12u)); + for (int w = 0; w < writes; ++w) { + const Int maxEdge = round % 2 == 0 ? 3 : std::max(size.x(), size.y()); + const Int ex = 1 + static_cast(rng() % static_cast(std::min(maxEdge, size.x()))); + const Int ey = 1 + static_cast(rng() % static_cast(std::min(maxEdge, size.y()))); + const Int ez = 1 + static_cast(rng() % static_cast(size.z())); + const Int ox = static_cast(rng() % static_cast(size.x() - ex + 1)); + const Int oy = static_cast(rng() % static_cast(size.y() - ey + 1)); + const Int oz = static_cast(rng() % static_cast(size.z() - ez + 1)); + level.Write({ox, oy, oz}, {ex, ey, ez}); + } + level.Expect(("round " + std::to_string(round)).c_str()); + if (HasFatalFailure() || HasNonfatalFailure()) return; + } + } +} // namespace From d20c2d01ae48f5feb0c7499ed76be9249163ad76 Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Tue, 29 Sep 2026 09:10:19 -0400 Subject: [PATCH 3/9] [Fix] (Magma): a frame's recording is submitted every 16384 draws/dispatches and continues on a fresh command buffer, with at most frames-in-flight + 1 submissions outstanding - rd12's 981k-draw loading frame no longer grows one command buffer until Adreno's 16 KiB driver mappings exhaust vm.max_map_count (MOBILEGL_MAGMA_MAX_DRAWS_PER_COMMAND_BUFFER, 0 = unbounded) --- MobileGL/Config.h | 6 +++ MobileGL/ConfigLoader.cpp | 2 + .../DirectVulkan/Renderer/VulkanRenderer.cpp | 53 +++++++++++++++++++ .../DirectVulkan/Renderer/VulkanRenderer.h | 13 +++++ README.md | 1 + 5 files changed, 75 insertions(+) diff --git a/MobileGL/Config.h b/MobileGL/Config.h index 8e9962f0..5e96abcf 100644 --- a/MobileGL/Config.h +++ b/MobileGL/Config.h @@ -132,6 +132,12 @@ namespace MobileGL::MG_Config { Bool MagmaR11G11B10FFallback = false; // MOBILEGL_MAGMA_FRAMESINFLIGHT: requested Magma frames in flight, defaulting to 3. Uint32 MagmaFramesInFlight = 3; + // MOBILEGL_MAGMA_MAX_DRAWS_PER_COMMAND_BUFFER: GL draws and dispatches Magma records into + // one command buffer before it submits it and continues on a fresh one. 0 = unbounded. + // Drivers back a command buffer with GPU memory that is only returned when the buffer is + // freed; Adreno maps it into the process in 16 KiB chunks, so one ~1M-draw loading frame + // exhausts vm.max_map_count. See VulkanRenderer::SplitOversizedRecording. + Uint32 MagmaMaxDrawsPerCommandBuffer = 16384; // MOBILEGL_ESPRYT_AVOID_SAMPLER_MIPMAP_MIN_FILTER: avoid mipmap min filters in samplers, // resolves certain rendering bugs on ANGLE + llvmpipe. Bool EsprytAvoidSamplerMipmapMinFilter = false; diff --git a/MobileGL/ConfigLoader.cpp b/MobileGL/ConfigLoader.cpp index 47809384..8e60e82a 100644 --- a/MobileGL/ConfigLoader.cpp +++ b/MobileGL/ConfigLoader.cpp @@ -177,6 +177,8 @@ namespace MobileGL::MG_ConfigLoader { features.AdvertiseFp64 = QueryEnvFlag("MOBILEGL_ADVERTISE_FP64"); features.MagmaR11G11B10FFallback = QueryEnvFlag("MOBILEGL_MAGMA_R11G11B10F_FALLBACK"); features.MagmaFramesInFlight = QueryEnvUint32("MOBILEGL_MAGMA_FRAMESINFLIGHT", 3, 1, 64); + features.MagmaMaxDrawsPerCommandBuffer = + QueryEnvUint32("MOBILEGL_MAGMA_MAX_DRAWS_PER_COMMAND_BUFFER", 16384, 0, 1u << 24); features.EsprytAvoidSamplerMipmapMinFilter = QueryEnvFlag("MOBILEGL_ESPRYT_AVOID_SAMPLER_MIPMAP_MIN_FILTER"); features.EsprytAvoidExplicitLodBias = QueryEnvFlag("MOBILEGL_ESPRYT_AVOID_EXPLICIT_LOD_BIAS"); diff --git a/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.cpp b/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.cpp index b8369408..aaa66be1 100644 --- a/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.cpp +++ b/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.cpp @@ -6381,6 +6381,8 @@ void main() { Bool VulkanRenderer::SetupDraw(FrameContext::FrameData& frame, GLenum mode, Flags aspects, const DrawCmdParam& drawParams, const IndexBufferView* pIndexBufferView) { + // First, before anything of this draw is recorded or resolved. + SplitOversizedRecording(); // Sync each sampled texture at most once across this whole draw: the layout // probe loop, the post-transition loop, and ResolveSamplerDescriptor would // otherwise each re-run the full SyncTexture path on the same textures. @@ -6981,6 +6983,7 @@ void main() { } void VulkanRenderer::DispatchCompute(GLuint numGroupsX, GLuint numGroupsY, GLuint numGroupsZ) { + SplitOversizedRecording(); m_textureManager->CollectGarbage(); auto& frame = m_frameContext.GetCurrent(); // The DISPATCH accessor: with a pipeline bound this is its compute stage program @@ -7034,6 +7037,7 @@ void main() { } void VulkanRenderer::DispatchComputeIndirect(GLintptr indirect) { + SplitOversizedRecording(); m_textureManager->CollectGarbage(); auto& frame = m_frameContext.GetCurrent(); // See DispatchCompute: the dispatch accessor, not the draw one. @@ -12669,6 +12673,54 @@ void main() { return FlushPendingCommands(); } + void VulkanRenderer::SplitOversizedRecording() { + const Uint32 maxDraws = MG_Config::Features.MagmaMaxDrawsPerCommandBuffer; + if (maxDraws == 0) { + return; + } + // Counted before the check, so a buffer carries at most maxDraws calls before the one + // that splits it. OnFrameCommandRecordingBegan resets the count on every new recording. + if (++m_drawsInRecording <= maxDraws || !HasPendingRecordedWork()) { + return; + } + // Nothing of the current call is recorded yet, so the split falls between two GL calls, + // exactly where a glFlush would put it: the render pass ends with STORE, the next draw + // re-begins it with LOAD, and queries and transform feedback are bracketed per draw. + if (!FlushPendingCommands()) { + return; + } + // Fresh command buffer: the same reset every other mid-frame flush site applies. + m_lastSampledSetValid = false; + ++m_oversizedRecordingSplits; + MGLOG_I_ONCE("DirectVulkan: a frame recorded more than %u draws into one command buffer; submitting it and " + "continuing on a fresh one (MOBILEGL_MAGMA_MAX_DRAWS_PER_COMMAND_BUFFER, 0 = unbounded)", + maxDraws); + MGLOG_D("DirectVulkan: split an oversized recording (%llu so far, submit index %llu)", + static_cast(m_oversizedRecordingSplits), + static_cast(m_submitCounter)); + + // Splitting only bounds the driver's command memory if the submitted buffers are freed + // as fast as they are produced. A GPU that falls behind would park them all on the + // retired list, which is the same growth spread over more buffers - so hold the queue + // to frames-in-flight + 1 outstanding submissions, the steady state of a pipelined + // frame plus this split. Wait on the fence directly rather than through + // WaitForSubmitIndex: its opportunistic TryDrainFrameTransients may rewind the transient + // arena, and this runs at the start of a draw whose caller may already hold slices of it. + const SizeT maxOutstanding = static_cast(m_frameContext.GetFrameCount()) + 1; + while (m_inFlightSubmits.size() > maxOutstanding) { + const SubmitRecord oldest = m_inFlightSubmits.front(); + if (oldest.fence == VK_NULL_HANDLE) { + break; + } + const VkResult waitResult = vkWaitForFences(m_device, 1, &oldest.fence, VK_TRUE, UINT64_MAX); + if (waitResult != VK_SUCCESS) { + MGLOG_E_ONCE("SplitOversizedRecording: vkWaitForFences returned %d", waitResult); + break; + } + OnSubmitsCompletedUpTo(oldest.submitIndex); + } + } + Bool VulkanRenderer::WaitForSubmitIndex(Uint64 submitIndex, Uint64 timeoutNs, Bool flushIfPending) { if (IsSubmitIndexComplete(submitIndex)) { return true; @@ -12707,6 +12759,7 @@ void main() { } void VulkanRenderer::OnFrameCommandRecordingBegan(VkCommandBuffer commandBuffer) { + m_drawsInRecording = 0; // Dynamic state does not survive a command-buffer boundary. ResetDynamicStateShadow(); InvalidateSetupDrawSnapshots(); diff --git a/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.h b/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.h index cbcde43a..90980415 100644 --- a/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.h +++ b/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.h @@ -490,6 +490,15 @@ namespace MobileGL::MG_Backend::DirectVulkan { // only), so the presenting path's frames-in-flight pipelining is // untouched. Returns true when the drain ran. Bool TryDrainFrameTransients(); + // Bounded recording: called at the top of every GL draw and dispatch, before anything + // of that call is recorded. Once the open frame command buffer holds + // MagmaMaxDrawsPerCommandBuffer draws/dispatches, submits it (FlushPendingCommands) so + // the call continues on a fresh one, then waits until at most frames-in-flight + 1 + // submissions are outstanding. A driver backs a command buffer with GPU memory it only + // returns when the buffer is freed (Adreno: one 16 KiB mapping per chunk), so a frame + // that is never split grows it without bound - rd12's loading frame records ~1M draws + // into one buffer and runs the process out of vm.max_map_count. + void SplitOversizedRecording(); Vector m_inFlightSubmits; Vector m_freeSubmitFences; @@ -501,6 +510,10 @@ namespace MobileGL::MG_Backend::DirectVulkan { // aging clocks, while present-less loops still cross a boundary every few // iterations. Reset in Present. Uint32 m_drainsSinceLastPresent = 0; + // GL draws/dispatches recorded into the open frame command buffer; reset whenever a + // recording begins (OnFrameCommandRecordingBegan). See SplitOversizedRecording. + Uint32 m_drawsInRecording = 0; + Uint64 m_oversizedRecordingSplits = 0; NativeWindowType m_window = 0; void* m_platformDisplay = nullptr; diff --git a/README.md b/README.md index 7c38f274..ac6c21e5 100644 --- a/README.md +++ b/README.md @@ -162,6 +162,7 @@ MobileGL supports runtime configuration via environment variables. | `MOBILEGL_ADVERTISE_FP64` | Advertise `GL_ARB_gpu_shader_fp64`. GLSL `double`/`dvec`/`dmat` compile and run either way - they are narrowed to 32 bits - so this only changes whether an application is told it has 64-bit precision, which it does not. | `0`, `1` | `0` | | `MOBILEGL_MAGMA_R11G11B10F_FALLBACK` | Use Magma's R11G11B10F format fallback. | `0`, `1` | `0` | | `MOBILEGL_MAGMA_FRAMESINFLIGHT` | Set Magma frames in flight. | Integer `1`–`64` | `3` | +| `MOBILEGL_MAGMA_MAX_DRAWS_PER_COMMAND_BUFFER` | Draws and dispatches Magma records into one command buffer before it submits the buffer mid-frame and continues on a fresh one. `0` never splits. | Integer `0`–`16777216` | `16384` | | `MOBILEGL_ESPRYT_AVOID_SAMPLER_MIPMAP_MIN_FILTER` | Avoid sampler mipmap minification filters. | `0`, `1` | `0` | | `MOBILEGL_COHERENT_AS_FLUSH` | Treat persistent `GL_MAP_FLUSH_EXPLICIT_BIT` maps as coherent (app-compat for engines like Flywheel that never flush them). | `0`, `1` | `0` | | `MOBILEGL_ESPRYT_FORCE_DS_READBACK_EMULATION` | Always emulate depth/stencil `glReadPixels`/`glGetTexImage` by shader sampling on Espryt, instead of using the driver's own depth/stencil readback where it has one. | `0`, `1` | `0` | From 4d3d677a7f7cf3554f835897ff8049d479efbee3 Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Tue, 29 Sep 2026 09:10:19 -0400 Subject: [PATCH 4/9] [Test] (Magma): OversizedRecordingSplitScenario - a 1024-draw frame, one pixel and one uniform value per draw, lands whole on both backends; the DirectVulkan.SplitRecording lane pins the draw budget to 64 and asserts the split fired --- MobileGL/MG_IntegrationTest/CMakeLists.txt | 23 ++ .../OversizedRecordingSplitScenario.cpp | 219 ++++++++++++++++++ 2 files changed, 242 insertions(+) create mode 100644 MobileGL/MG_IntegrationTest/Scenarios/OversizedRecordingSplitScenario.cpp diff --git a/MobileGL/MG_IntegrationTest/CMakeLists.txt b/MobileGL/MG_IntegrationTest/CMakeLists.txt index e960b6fd..f69a9993 100644 --- a/MobileGL/MG_IntegrationTest/CMakeLists.txt +++ b/MobileGL/MG_IntegrationTest/CMakeLists.txt @@ -128,6 +128,7 @@ add_executable(MobileGLIntegrationTest Scenarios/ClearTexImageUndefinedLevelZeroScenario.cpp Scenarios/RenderbufferBlendFormatScenario.cpp Scenarios/DualSourceBlendScenario.cpp + Scenarios/OversizedRecordingSplitScenario.cpp ) target_include_directories(MobileGLIntegrationTest PRIVATE @@ -376,6 +377,12 @@ mgl_itest_join_environment(MGL_ITEST_VULKAN_PRIMGEN_REROUTE_ENVIRONMENT "MOBILEGL_BACKEND_TYPE=DirectVulkan" "MOBILEGL_MAGMA_PRIMGEN_QUERY_REROUTE=1" "MOBILEGL_LOG_FILE_PATH=${CMAKE_CURRENT_BINARY_DIR}/primgen-query-reroute.log" ${MGL_ITEST_VULKAN_ENV}) +# The command-buffer draw budget pinned far below one frame's draws, with the log path for the +# split's latched MGLOG_I - the same arming reasoning as the two entries above. +mgl_itest_join_environment(MGL_ITEST_VULKAN_SPLIT_RECORDING_ENVIRONMENT + "MOBILEGL_BACKEND_TYPE=DirectVulkan" "MOBILEGL_MAGMA_MAX_DRAWS_PER_COMMAND_BUFFER=64" + "MOBILEGL_LOG_FILE_PATH=${CMAKE_CURRENT_BINARY_DIR}/oversized-recording-split.log" + ${MGL_ITEST_VULKAN_ENV}) # The point-size demotion pinned on, per backend, with a per-lane log file for the arming # assertion - the same MOBILEGL_LOG_FILE_PATH reasoning as the UnlocatedIoBlocks lane above. # Two lanes because the demotion runs in the SHARED phase-B chain and each backend then @@ -588,6 +595,22 @@ gtest_discover_tests(MobileGLIntegrationTest ENVIRONMENT "${MGL_ITEST_VULKAN_PRIMGEN_REROUTE_ENVIRONMENT}" ) +# OversizedRecordingSplitScenario with the command-buffer draw budget PINNED to 64. The ambient +# registrations run its 1024-draw frame under the default budget, which never splits it; this +# entry splits it 15 times, so the pixels prove a split loses nothing, and the arming assertion +# (the renderer's latched MGLOG_I) proves the split happened at all - on a renderer that records +# the whole frame into one command buffer, it is the assertion that goes red. DirectVulkan only: +# the budget steers nothing on DirectGLES, whose driver flushes on its own. +gtest_discover_tests(MobileGLIntegrationTest + TEST_PREFIX "DirectVulkan.SplitRecording." + TEST_FILTER "OversizedRecordingSplitScenario.*" + DISCOVERY_TIMEOUT 30 + PROPERTIES + LABELS integration-gpu + TIMEOUT ${MGL_ITEST_TIMEOUT} + ENVIRONMENT "${MGL_ITEST_VULKAN_SPLIT_RECORDING_ENVIRONMENT}" +) + # The packed16 copy scenarios again, with the 8-bit storage widening PINNED ON. The ambient # registrations above cover the narrow storage - on every CI driver the widening's POST # probe finds no field-order mirror, so Auto keeps the native 16-bit path - which means the diff --git a/MobileGL/MG_IntegrationTest/Scenarios/OversizedRecordingSplitScenario.cpp b/MobileGL/MG_IntegrationTest/Scenarios/OversizedRecordingSplitScenario.cpp new file mode 100644 index 00000000..41d13fbe --- /dev/null +++ b/MobileGL/MG_IntegrationTest/Scenarios/OversizedRecordingSplitScenario.cpp @@ -0,0 +1,219 @@ +// MobileGL - MobileGL/MG_IntegrationTest/Scenarios/OversizedRecordingSplitScenario.cpp +// Copyright (c) 2025-2026 MobileGL-Dev +// Licensed under the GNU Lesser General Public License v3.0: +// https://www.gnu.org/licenses/gpl-3.0.txt +// https://www.gnu.org/licenses/lgpl-3.0.txt +// SPDX-License-Identifier: LGPL-3.0-only +// End of Source File Header +// +// Scenario - A FRAME WITH MORE DRAWS THAN ONE COMMAND BUFFER MAY HOLD IS SPLIT, AND LOSES NOTHING. +// +// DirectVulkan records a frame into one command buffer, and used to submit it only at Present. +// A driver keeps a command buffer's GPU memory until the buffer is freed, and the Adreno 830 +// driver maps that memory into the process in 16 KiB chunks. Minecraft rd12's loading frame +// records 981,654 draws (3.1M commands) into one buffer. It grew to ~40k such mappings and +// aborted inside the driver's own calloc once vm.max_map_count (65530) was spent. The renderer +// now submits the open buffer after MOBILEGL_MAGMA_MAX_DRAWS_PER_COMMAND_BUFFER draws and goes +// on recording into a fresh one (VulkanRenderer::SplitOversizedRecording). +// +// Each draw here writes exactly one pixel of a 32x32 target, in its own colour, selected by a +// per-draw uniform, so every draw carries its own uniform state, like Minecraft's chunk draws. +// After 1024 draws every pixel must hold its own draw's colour. Each of these split bugs leaves +// a pixel wrong: a re-begun render pass that does not LOAD, a draw recorded into the retired +// buffer, or state that is not re-emitted on the fresh one. +// +// The ambient registrations run the same draws unsplit, because the default budget is far above +// 1024; that is the control. The DirectVulkan.SplitRecording. entry pins the budget to 64, so +// the frame is split 15 times. There the case also asserts the renderer's latched MGLOG_I, the +// one assertion that is red on a renderer that never splits. + +#include +#include +#include +#include +#include +#include + +#include "../Harness/HeadlessGL.h" +#include "../Harness/ScenarioFixture.h" + +#ifdef GLAPI +#undef GLAPI +#endif +#define GL_GLEXT_PROTOTYPES +#include +#include +#undef GL_GLEXT_PROTOTYPES + +namespace MGITest { + namespace { + + constexpr int kSide = 32; + constexpr int kDraws = kSide * kSide; + + // aPos spans the unit square; the uniform moves it onto cell (i % 32, i / 32) of a + // 32x32 grid, which is exactly one pixel of a 32x32 target. + constexpr const char* kVS = R"(#version 330 core +in vec2 aPos; +uniform int uIndex; +void main() { + vec2 cell = vec2(float(uIndex % 32), float(uIndex / 32)); + gl_Position = vec4((cell + aPos) / 16.0 - 1.0, 0.0, 1.0); +} +)"; + + // Multiples of 8/255 survive an RGBA8 round trip exactly, so the readback bytes are + // (8x, 8y, 255, 255) for the pixel at (x, y) and nothing else can produce them. + constexpr const char* kFS = R"(#version 330 core +uniform int uIndex; +out vec4 o_color; +void main() { + o_color = vec4(float(uIndex % 32) * 8.0 / 255.0, float(uIndex / 32) * 8.0 / 255.0, 1.0, 1.0); +} +)"; + + // The budget the process was launched with, or 0 when it is unset or not a number. + unsigned long PinnedDrawBudget() { + const char* value = std::getenv("MOBILEGL_MAGMA_MAX_DRAWS_PER_COMMAND_BUFFER"); + if (value == nullptr || *value == '\0') return 0; + char* end = nullptr; + const unsigned long parsed = std::strtoul(value, &end, 10); + return (end != nullptr && *end == '\0') ? parsed : 0; + } + + // The library log, for the arming assertion. Same machinery and reasoning as + // UnlocatedIoBlockScenario: MOBILEGL_LOG_FILE_PATH is read at log-init, every process in + // the lane appends to the file, and only bytes appended after the snapshot count. + std::filesystem::path LibraryLogPath() { + const char* path = std::getenv("MOBILEGL_LOG_FILE_PATH"); + return (path != nullptr && *path != '\0') ? std::filesystem::path(path) : std::filesystem::path(); + } + + std::uintmax_t LibraryLogSize() { + std::error_code ec; + const std::filesystem::path path = LibraryLogPath(); + if (path.empty()) return 0; + const std::uintmax_t size = std::filesystem::file_size(path, ec); + return ec ? 0 : size; + } + + std::string LibraryLogSince(std::uintmax_t offset) { + const std::filesystem::path path = LibraryLogPath(); + if (path.empty()) return {}; + std::ifstream file(path, std::ios::binary); + if (!file.good()) return {}; + file.seekg(static_cast(offset)); + return std::string((std::istreambuf_iterator(file)), std::istreambuf_iterator()); + } + + ::testing::AssertionResult EveryPixelHoldsItsOwnDraw(const Image& image) { + int wrong = 0; + int firstX = -1, firstY = -1; + Rgba8 firstSeen{}; + for (int y = 0; y < kSide; ++y) { + for (int x = 0; x < kSide; ++x) { + const Rgba8 expected{static_cast(8 * x), static_cast(8 * y), 255, 255}; + const Rgba8 seen = image.At(x, y); + if (seen != expected) { + if (wrong == 0) { + firstX = x; + firstY = y; + firstSeen = seen; + } + ++wrong; + } + } + } + if (wrong == 0) { + return ::testing::AssertionSuccess(); + } + return ::testing::AssertionFailure() + << wrong << " of " << kDraws << " pixels do not hold their own draw's colour. First: (" << firstX + << ", " << firstY << ") = " << firstSeen << ", drawn by draw " << (firstY * kSide + firstX) + << " (a pixel no draw reached still holds the clear colour 0,0,0,0)"; + } + + class OversizedRecordingSplitScenario : public ScenarioTest { + protected: + // kDraws single-pixel quads into a fresh target, one uniform value per draw, with no + // readback, flush or rebind in between, so the renderer sees one uninterrupted + // recording. Then the whole target is read back. + Image DrawEveryPixelWithItsOwnDraw(std::string* error) { + const unsigned int program = CompileProgram(kVS, kFS, error); + if (program == 0) return Image(); + ColorFbo target = MakeColorFbo(kSide, kSide); + if (target.fbo == 0) { + *error = "MakeColorFbo failed"; + glDeleteProgram(program); + return Image(); + } + + static const float kUnitQuad[] = {0.0f, 0.0f, 1.0f, 0.0f, 0.0f, 1.0f, 1.0f, 1.0f}; + GLuint vao = 0, vbo = 0; + glGenVertexArrays(1, &vao); + glBindVertexArray(vao); + glGenBuffers(1, &vbo); + glBindBuffer(GL_ARRAY_BUFFER, vbo); + glBufferData(GL_ARRAY_BUFFER, sizeof(kUnitQuad), kUnitQuad, GL_STATIC_DRAW); + glEnableVertexAttribArray(0); + glVertexAttribPointer(0, 2, GL_FLOAT, GL_FALSE, 2 * sizeof(float), nullptr); + + BindFbo(target); + glDisable(GL_SCISSOR_TEST); + glDisable(GL_DEPTH_TEST); + glDisable(GL_BLEND); + ClearTo(0.0f, 0.0f, 0.0f, 0.0f); + glUseProgram(program); + const GLint indexLocation = glGetUniformLocation(program, "uIndex"); + for (int i = 0; i < kDraws; ++i) { + glUniform1i(indexLocation, i); + glDrawArrays(GL_TRIANGLE_STRIP, 0, 4); + } + Image image = ReadPixels(kSide, kSide); + + glUseProgram(0); + glBindVertexArray(0); + glDeleteBuffers(1, &vbo); + glDeleteVertexArrays(1, &vao); + BindDefaultFramebuffer(); + DestroyColorFbo(target); + glDeleteProgram(program); + return image; + } + }; + + } // namespace + + TEST_F(OversizedRecordingSplitScenario, EveryDrawOfTheFrameLandsAcrossCommandBufferSplits) { + if (!Ready()) return; + + // Taken before the draws, so the line the arming check looks for can only have been + // written by them. The renderer latches it at its first split. + const std::uintmax_t logBefore = LibraryLogSize(); + + std::string error; + const Image image = DrawEveryPixelWithItsOwnDraw(&error); + ASSERT_FALSE(image.Empty()) << error; + EXPECT_TRUE(EveryPixelHoldsItsOwnDraw(image)); + EXPECT_EQ(FirstGLError(), 0u); + + // Arming: only where the budget is pinned below this frame's draw count, on the backend + // that has the budget, and with a log to read. Everywhere else this case is the unsplit + // control and stops here. + const unsigned long budget = PinnedDrawBudget(); + const bool splitExpected = budget > 0 && budget < static_cast(kDraws) && + Gl().BackendName() == std::string("DirectVulkan") && !LibraryLogPath().empty(); + RecordProperty("arming_checked", splitExpected ? "yes" : "no"); + if (!splitExpected) { + return; + } + const std::string appended = LibraryLogSince(logBefore); + EXPECT_NE(appended.find("draws into one command buffer; submitting it"), std::string::npos) + << "MOBILEGL_MAGMA_MAX_DRAWS_PER_COMMAND_BUFFER=" << budget << " and one frame recorded " << kDraws + << " draws, but DirectVulkan never reported splitting the recording. A renderer that does not " + "split grows a single command buffer without bound (rd12 on Adreno: ~40k driver mappings, " + "then SIGABRT). See VulkanRenderer::SplitOversizedRecording. Log appended by this test:\n" + << appended; + } + +} // namespace MGITest From 9c32d9fa68b85d454ee562900c88c1d6b22eabdf Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Tue, 29 Sep 2026 09:44:50 -0400 Subject: [PATCH 5/9] [Test] (Magma): OversizedRecordingSplitScenario carries state across the split - a depth occluder, a stencil mark, an additive blend destination, a 4x multisample target resolved after it, occlusion and primitives-generated queries and a transform-feedback capture left open across it, each checked on both backends and in the budget-64 lane --- MobileGL/MG_IntegrationTest/CMakeLists.txt | 10 +- .../OversizedRecordingSplitScenario.cpp | 695 ++++++++++++++++-- 2 files changed, 630 insertions(+), 75 deletions(-) diff --git a/MobileGL/MG_IntegrationTest/CMakeLists.txt b/MobileGL/MG_IntegrationTest/CMakeLists.txt index f69a9993..a3a71922 100644 --- a/MobileGL/MG_IntegrationTest/CMakeLists.txt +++ b/MobileGL/MG_IntegrationTest/CMakeLists.txt @@ -596,10 +596,12 @@ gtest_discover_tests(MobileGLIntegrationTest ) # OversizedRecordingSplitScenario with the command-buffer draw budget PINNED to 64. The ambient -# registrations run its 1024-draw frame under the default budget, which never splits it; this -# entry splits it 15 times, so the pixels prove a split loses nothing, and the arming assertion -# (the renderer's latched MGLOG_I) proves the split happened at all - on a renderer that records -# the whole frame into one command buffer, it is the assertion that goes red. DirectVulkan only: +# registrations run its cases under the default budget, which never splits them; this entry +# splits every case at least once between the draws that leave state (depth, stencil, a blend +# destination, multisample samples, an open query, an active capture) and the draws that +# consume it, so the results prove a split carries all of it, and the arming assertion (the +# renderer's latched MGLOG_I) proves the split happened at all - on a renderer that records the +# whole frame into one command buffer, it is the assertion that goes red. DirectVulkan only: # the budget steers nothing on DirectGLES, whose driver flushes on its own. gtest_discover_tests(MobileGLIntegrationTest TEST_PREFIX "DirectVulkan.SplitRecording." diff --git a/MobileGL/MG_IntegrationTest/Scenarios/OversizedRecordingSplitScenario.cpp b/MobileGL/MG_IntegrationTest/Scenarios/OversizedRecordingSplitScenario.cpp index 41d13fbe..b6e7deec 100644 --- a/MobileGL/MG_IntegrationTest/Scenarios/OversizedRecordingSplitScenario.cpp +++ b/MobileGL/MG_IntegrationTest/Scenarios/OversizedRecordingSplitScenario.cpp @@ -16,23 +16,29 @@ // now submits the open buffer after MOBILEGL_MAGMA_MAX_DRAWS_PER_COMMAND_BUFFER draws and goes // on recording into a fresh one (VulkanRenderer::SplitOversizedRecording). // -// Each draw here writes exactly one pixel of a 32x32 target, in its own colour, selected by a -// per-draw uniform, so every draw carries its own uniform state, like Minecraft's chunk draws. -// After 1024 draws every pixel must hold its own draw's colour. Each of these split bugs leaves -// a pixel wrong: a re-begun render pass that does not LOAD, a draw recorded into the retired -// buffer, or state that is not re-emitted on the fresh one. +// The first case writes each pixel of a 32x32 target with its own draw and its own uniform +// value, so it pins that no draw is lost and no per-draw state is left behind on either side of +// a split. The rest put GL state that one draw leaves for a later one ACROSS the split: a depth +// occluder, a stencil mark, a blend destination, the samples of a multisample target, an +// occlusion or primitives-generated query that is still open, and a transform-feedback capture +// that is still active. Each draws its "before" part, then kFillers draws, then its "after" +// part. kFillers is more than twice the SplitRecording lane's budget of 64, so at least one +// split lands between the two parts wherever the recording's count stood. // // The ambient registrations run the same draws unsplit, because the default budget is far above -// 1024; that is the control. The DirectVulkan.SplitRecording. entry pins the budget to 64, so -// the frame is split 15 times. There the case also asserts the renderer's latched MGLOG_I, the -// one assertion that is red on a renderer that never splits. +// any of these frames; that is the control. The DirectVulkan.SplitRecording. entry pins the +// budget to 64, and there every case also asserts the renderer's latched MGLOG_I - the one +// assertion that is red on a renderer that never splits. +#include #include #include #include #include #include +#include #include +#include #include "../Harness/HeadlessGL.h" #include "../Harness/ScenarioFixture.h" @@ -50,6 +56,11 @@ namespace MGITest { constexpr int kSide = 32; constexpr int kDraws = kSide * kSide; + // More than twice the SplitRecording lane's budget (64): wherever the count stands when + // they start, at least one split lands among them. + constexpr int kFillers = 150; + // Draws inside the query and capture spans: also more than twice the budget. + constexpr int kSpanDraws = 200; // aPos spans the unit square; the uniform moves it onto cell (i % 32, i / 32) of a // 32x32 grid, which is exactly one pixel of a 32x32 target. @@ -72,6 +83,37 @@ void main() { } )"; + // The state cases: the unit-square vertices (or a triangle of them) stretched over an + // NDC rectangle at a given depth, in a uniform colour. A zero rectangle makes a + // zero-area primitive - a draw that reaches the renderer and rasterizes nothing. + constexpr const char* kRectVS = R"(#version 330 core +in vec2 aPos; +uniform vec4 uRect; +uniform float uDepth; +void main() { gl_Position = vec4(mix(uRect.xy, uRect.zw, aPos), uDepth, 1.0); } +)"; + constexpr const char* kRectFS = R"(#version 330 core +uniform vec4 uColor; +out vec4 o_color; +void main() { o_color = uColor; } +)"; + + // The capture case: one point per draw, carrying 2 * uIndex + 1. + constexpr const char* kCaptureVS = R"(#version 330 core +uniform int uIndex; +out float vs_value; +void main() { + vs_value = float(uIndex) * 2.0 + 1.0; + gl_Position = vec4(0.0, 0.0, 0.0, 1.0); +} +)"; + constexpr const char* kCaptureFS = R"(#version 330 core +out vec4 o_color; +void main() { o_color = vec4(1.0); } +)"; + + constexpr float kCapturePoison = -12345.0f; + // The budget the process was launched with, or 0 when it is unset or not a number. unsigned long PinnedDrawBudget() { const char* value = std::getenv("MOBILEGL_MAGMA_MAX_DRAWS_PER_COMMAND_BUFFER"); @@ -106,6 +148,13 @@ void main() { return std::string((std::istreambuf_iterator(file)), std::istreambuf_iterator()); } + std::string Describe(const Rgba8& c) { + std::ostringstream os; + os << "rgba(" << static_cast(c.r) << ", " << static_cast(c.g) << ", " << static_cast(c.b) + << ", " << static_cast(c.a) << ")"; + return os.str(); + } + ::testing::AssertionResult EveryPixelHoldsItsOwnDraw(const Image& image) { int wrong = 0; int firstX = -1, firstY = -1; @@ -133,87 +182,591 @@ void main() { << " (a pixel no draw reached still holds the clear colour 0,0,0,0)"; } + // Every pixel of columns [x0, x1] (all rows) is `expected`, each channel within `tolerance`. + ::testing::AssertionResult ColumnsAre(const Image& image, int x0, int x1, const Rgba8& expected, + int tolerance, const char* what) { + int wrong = 0; + int firstX = -1, firstY = -1; + Rgba8 firstSeen{}; + for (int y = 0; y < kSide; ++y) { + for (int x = x0; x <= x1; ++x) { + const Rgba8 seen = image.At(x, y); + const bool close = std::abs(seen.r - expected.r) <= tolerance && + std::abs(seen.g - expected.g) <= tolerance && + std::abs(seen.b - expected.b) <= tolerance && + std::abs(seen.a - expected.a) <= tolerance; + if (!close) { + if (wrong == 0) { + firstX = x; + firstY = y; + firstSeen = seen; + } + ++wrong; + } + } + } + if (wrong == 0) return ::testing::AssertionSuccess(); + return ::testing::AssertionFailure() + << what << ": " << wrong << " pixels of columns " << x0 << ".." << x1 << " are not " + << Describe(expected) << "; first (" << firstX << ", " << firstY << ") = " << Describe(firstSeen); + } + class OversizedRecordingSplitScenario : public ScenarioTest { protected: - // kDraws single-pixel quads into a fresh target, one uniform value per draw, with no - // readback, flush or rebind in between, so the renderer sees one uninterrupted - // recording. Then the whole target is read back. - Image DrawEveryPixelWithItsOwnDraw(std::string* error) { - const unsigned int program = CompileProgram(kVS, kFS, error); - if (program == 0) return Image(); - ColorFbo target = MakeColorFbo(kSide, kSide); - if (target.fbo == 0) { - *error = "MakeColorFbo failed"; - glDeleteProgram(program); - return Image(); - } + void SetUp() override { + ScenarioTest::SetUp(); + if (!Ready()) return; + // Taken before the case draws anything, so the line the arming check looks for + // can only have been written by this case. The renderer latches it at its first + // split, and ctest gives every case a process of its own. + m_logBefore = LibraryLogSize(); - static const float kUnitQuad[] = {0.0f, 0.0f, 1.0f, 0.0f, 0.0f, 1.0f, 1.0f, 1.0f}; - GLuint vao = 0, vbo = 0; - glGenVertexArrays(1, &vao); - glBindVertexArray(vao); - glGenBuffers(1, &vbo); - glBindBuffer(GL_ARRAY_BUFFER, vbo); - glBufferData(GL_ARRAY_BUFFER, sizeof(kUnitQuad), kUnitQuad, GL_STATIC_DRAW); + // 0..3: the unit square as a strip. 4..6: its lower-left triangle. 7..9: its + // upper-right triangle - the two share the diagonal, so together they cover every + // sample of the square exactly once. + static const float kVertices[] = {0.0f, 0.0f, 1.0f, 0.0f, 0.0f, 1.0f, 1.0f, 1.0f, 0.0f, 0.0f, + 1.0f, 0.0f, 0.0f, 1.0f, 1.0f, 0.0f, 1.0f, 1.0f, 0.0f, 1.0f}; + glGenVertexArrays(1, &m_vao); + glBindVertexArray(m_vao); + glGenBuffers(1, &m_vbo); + glBindBuffer(GL_ARRAY_BUFFER, m_vbo); + glBufferData(GL_ARRAY_BUFFER, sizeof(kVertices), kVertices, GL_STATIC_DRAW); glEnableVertexAttribArray(0); glVertexAttribPointer(0, 2, GL_FLOAT, GL_FALSE, 2 * sizeof(float), nullptr); - - BindFbo(target); - glDisable(GL_SCISSOR_TEST); - glDisable(GL_DEPTH_TEST); - glDisable(GL_BLEND); - ClearTo(0.0f, 0.0f, 0.0f, 0.0f); - glUseProgram(program); - const GLint indexLocation = glGetUniformLocation(program, "uIndex"); - for (int i = 0; i < kDraws; ++i) { - glUniform1i(indexLocation, i); - glDrawArrays(GL_TRIANGLE_STRIP, 0, 4); - } - Image image = ReadPixels(kSide, kSide); - - glUseProgram(0); - glBindVertexArray(0); - glDeleteBuffers(1, &vbo); - glDeleteVertexArrays(1, &vao); - BindDefaultFramebuffer(); - DestroyColorFbo(target); - glDeleteProgram(program); - return image; } + + void TearDown() override { + if (Ready()) { + glDisable(GL_DEPTH_TEST); + glDisable(GL_STENCIL_TEST); + glDisable(GL_BLEND); + glDisable(GL_RASTERIZER_DISCARD); + glDepthMask(GL_TRUE); + glStencilMask(0xFF); + glColorMask(GL_TRUE, GL_TRUE, GL_TRUE, GL_TRUE); + glUseProgram(0); + glBindVertexArray(0); + BindDefaultFramebuffer(); + if (m_rectProgram != 0) glDeleteProgram(m_rectProgram); + if (m_vbo != 0) glDeleteBuffers(1, &m_vbo); + if (m_vao != 0) glDeleteVertexArrays(1, &m_vao); + for (GLuint rbo : m_renderbuffers) glDeleteRenderbuffers(1, &rbo); + for (GLuint fbo : m_framebuffers) glDeleteFramebuffers(1, &fbo); + for (ColorFbo& target : m_colorTargets) DestroyColorFbo(target); + } + ScenarioTest::TearDown(); + } + + // In the SplitRecording lane (budget pinned below what every case draws, DirectVulkan, + // a log to read), a split must have happened during this case. Elsewhere this case is + // the unsplit control and there is nothing to arm. + void ExpectTheRecordingWasSplitWhereTheBudgetIsPinned() { + const unsigned long budget = PinnedDrawBudget(); + const bool splitExpected = budget > 0 && budget * 2 < static_cast(kFillers) && + Gl().BackendName() == std::string("DirectVulkan") && + !LibraryLogPath().empty(); + RecordProperty("arming_checked", splitExpected ? "yes" : "no"); + if (!splitExpected) return; + const std::string appended = LibraryLogSince(m_logBefore); + EXPECT_NE(appended.find("draws into one command buffer; submitting it"), std::string::npos) + << "MOBILEGL_MAGMA_MAX_DRAWS_PER_COMMAND_BUFFER=" << budget << " and this case recorded more " + << "than " << kFillers << " draws in one frame, but DirectVulkan never reported splitting the " + "recording. A renderer that does not split grows a single command buffer without bound " + "(rd12 on Adreno: ~40k driver mappings, then SIGABRT). See " + "VulkanRenderer::SplitOversizedRecording. Log appended by this case:\n" + << appended; + } + + bool BuildRectProgram(std::string* error) { + m_rectProgram = CompileProgram(kRectVS, kRectFS, error); + if (m_rectProgram == 0) return false; + m_rectLoc = glGetUniformLocation(m_rectProgram, "uRect"); + m_depthLoc = glGetUniformLocation(m_rectProgram, "uDepth"); + m_colorLoc = glGetUniformLocation(m_rectProgram, "uColor"); + glUseProgram(m_rectProgram); + glBindVertexArray(m_vao); + return true; + } + + // The rectangle [x0, x1] x [y0, y1] in NDC; `first`/`count` pick the strip or a triangle. + void DrawRect(float x0, float y0, float x1, float y1, float depth, float r, float g, float b, float a, + GLenum mode = GL_TRIANGLE_STRIP, GLint first = 0, GLsizei count = 4) { + glUniform4f(m_rectLoc, x0, y0, x1, y1); + glUniform1f(m_depthLoc, depth); + glUniform4f(m_colorLoc, r, g, b, a); + glDrawArrays(mode, first, count); + } + + // One pixel of the 32x32 grid, by index. + void DrawPixel(int index, float r, float g, float b, float a) { + const float x = static_cast(index % kSide); + const float y = static_cast(index / kSide); + DrawRect(x / 16.0f - 1.0f, y / 16.0f - 1.0f, (x + 1.0f) / 16.0f - 1.0f, (y + 1.0f) / 16.0f - 1.0f, 0.0f, + r, g, b, a); + } + + // Draws that reach the renderer and rasterize nothing: zero-area strips. + void DrawFillers(int count) { + for (int i = 0; i < count; ++i) { + DrawRect(0.0f, 0.0f, 0.0f, 0.0f, 0.0f, 0.0f, 0.0f, 0.0f, 0.0f); + } + } + + // A kSide x kSide RGBA8 target, optionally with a DEPTH24_STENCIL8 renderbuffer. + // Returns fbo 0 when the driver will not complete it. + ColorFbo MakeTarget(bool withDepthStencil) { + ColorFbo target = MakeColorFbo(kSide, kSide); + if (target.fbo == 0) return target; + m_colorTargets.push_back(target); + if (withDepthStencil) { + GLuint rbo = 0; + glGenRenderbuffers(1, &rbo); + m_renderbuffers.push_back(rbo); + glBindRenderbuffer(GL_RENDERBUFFER, rbo); + glRenderbufferStorage(GL_RENDERBUFFER, GL_DEPTH24_STENCIL8, kSide, kSide); + glBindFramebuffer(GL_FRAMEBUFFER, target.fbo); + glFramebufferRenderbuffer(GL_FRAMEBUFFER, GL_DEPTH_STENCIL_ATTACHMENT, GL_RENDERBUFFER, rbo); + if (glCheckFramebufferStatus(GL_FRAMEBUFFER) != GL_FRAMEBUFFER_COMPLETE) { + target.fbo = 0; + } + } + return target; + } + + Image ReadTarget(const ColorFbo& target) { + glBindFramebuffer(GL_FRAMEBUFFER, target.fbo); + return ReadPixels(kSide, kSide); + } + + std::uintmax_t m_logBefore = 0; + GLuint m_vao = 0; + GLuint m_vbo = 0; + GLuint m_rectProgram = 0; + GLint m_rectLoc = -1; + GLint m_depthLoc = -1; + GLint m_colorLoc = -1; + std::vector m_colorTargets; + std::vector m_renderbuffers; + std::vector m_framebuffers; }; } // namespace + // kDraws single-pixel quads into a fresh target, one uniform value per draw, with no + // readback, flush or rebind in between. Every pixel must hold its own draw's colour. TEST_F(OversizedRecordingSplitScenario, EveryDrawOfTheFrameLandsAcrossCommandBufferSplits) { if (!Ready()) return; - // Taken before the draws, so the line the arming check looks for can only have been - // written by them. The renderer latches it at its first split. - const std::uintmax_t logBefore = LibraryLogSize(); - std::string error; - const Image image = DrawEveryPixelWithItsOwnDraw(&error); - ASSERT_FALSE(image.Empty()) << error; + const unsigned int program = CompileProgram(kVS, kFS, &error); + ASSERT_NE(program, 0u) << error; + ColorFbo target = MakeTarget(false); + ASSERT_NE(target.fbo, 0u) << "MakeColorFbo failed"; + + BindFbo(target); + glDisable(GL_SCISSOR_TEST); + glDisable(GL_DEPTH_TEST); + glDisable(GL_BLEND); + ClearTo(0.0f, 0.0f, 0.0f, 0.0f); + glUseProgram(program); + glBindVertexArray(m_vao); + const GLint indexLocation = glGetUniformLocation(program, "uIndex"); + for (int i = 0; i < kDraws; ++i) { + glUniform1i(indexLocation, i); + glDrawArrays(GL_TRIANGLE_STRIP, 0, 4); + } + const Image image = ReadPixels(kSide, kSide); + glUseProgram(0); + glDeleteProgram(program); + EXPECT_TRUE(EveryPixelHoldsItsOwnDraw(image)); EXPECT_EQ(FirstGLError(), 0u); + ExpectTheRecordingWasSplitWhereTheBudgetIsPinned(); + } - // Arming: only where the budget is pinned below this frame's draw count, on the backend - // that has the budget, and with a log to read. Everywhere else this case is the unsplit - // control and stops here. - const unsigned long budget = PinnedDrawBudget(); - const bool splitExpected = budget > 0 && budget < static_cast(kDraws) && - Gl().BackendName() == std::string("DirectVulkan") && !LibraryLogPath().empty(); - RecordProperty("arming_checked", splitExpected ? "yes" : "no"); - if (!splitExpected) { - return; + // (a) An occluder drawn before the split must still fail the depth test of a draw after it: + // the depth attachment's contents have to survive the render pass ending and re-beginning. + TEST_F(OversizedRecordingSplitScenario, ADepthOccluderDrawnBeforeTheSplitHidesADrawAfterIt) { + if (!Ready()) return; + std::string error; + ASSERT_TRUE(BuildRectProgram(&error)) << error; + ColorFbo target = MakeTarget(true); + if (target.fbo == 0) GTEST_SKIP() << "no complete RGBA8 + DEPTH24_STENCIL8 target on this driver"; + + BindFbo(target); + glClearDepth(1.0); + ClearTo(0.0f, 0.0f, 0.0f, 1.0f); + glEnable(GL_DEPTH_TEST); + glDepthFunc(GL_LESS); + glDepthMask(GL_TRUE); + + DrawRect(-1.0f, -1.0f, 0.0f, 1.0f, -0.5f, 1.0f, 0.0f, 0.0f, 1.0f); // left half, near, red + DrawFillers(kFillers); + DrawRect(-1.0f, -1.0f, 1.0f, 1.0f, 0.5f, 0.0f, 1.0f, 0.0f, 1.0f); // everything, far, green + + const Image image = ReadTarget(target); + EXPECT_TRUE(ColumnsAre(image, 0, kSide / 2 - 1, Rgba8{255, 0, 0, 255}, 0, + "the near occluder drawn before the split must hide the far draw after it")); + EXPECT_TRUE(ColumnsAre(image, kSide / 2, kSide - 1, Rgba8{0, 255, 0, 255}, 0, + "where nothing occludes, the far draw after the split must land")); + EXPECT_EQ(FirstGLError(), 0u); + ExpectTheRecordingWasSplitWhereTheBudgetIsPinned(); + } + + // (b) A stencil mark made before the split must gate a draw after it. + TEST_F(OversizedRecordingSplitScenario, AStencilMarkMadeBeforeTheSplitGatesADrawAfterIt) { + if (!Ready()) return; + std::string error; + ASSERT_TRUE(BuildRectProgram(&error)) << error; + ColorFbo target = MakeTarget(true); + if (target.fbo == 0) GTEST_SKIP() << "no complete RGBA8 + DEPTH24_STENCIL8 target on this driver"; + + BindFbo(target); + glDisable(GL_DEPTH_TEST); + glStencilMask(0xFF); + glClearStencil(0); + glClear(GL_STENCIL_BUFFER_BIT); + ClearTo(0.0f, 0.0f, 0.0f, 1.0f); + glEnable(GL_STENCIL_TEST); + + glStencilFunc(GL_ALWAYS, 1, 0xFF); + glStencilOp(GL_KEEP, GL_KEEP, GL_REPLACE); + DrawRect(-1.0f, -1.0f, 0.0f, 1.0f, 0.0f, 0.0f, 0.0f, 1.0f, 1.0f); // left half: stencil 1, blue + glStencilFunc(GL_EQUAL, 1, 0xFF); + glStencilOp(GL_KEEP, GL_KEEP, GL_KEEP); + DrawFillers(kFillers); + DrawRect(-1.0f, -1.0f, 1.0f, 1.0f, 0.0f, 0.0f, 1.0f, 0.0f, 1.0f); // everything, green, where stencil == 1 + + const Image image = ReadTarget(target); + EXPECT_TRUE(ColumnsAre(image, 0, kSide / 2 - 1, Rgba8{0, 255, 0, 255}, 0, + "where the mark was made before the split, the draw after it must pass")); + EXPECT_TRUE(ColumnsAre(image, kSide / 2, kSide - 1, Rgba8{0, 0, 0, 255}, 0, + "where no mark was made, the draw after the split must be stencilled out")); + EXPECT_EQ(FirstGLError(), 0u); + ExpectTheRecordingWasSplitWhereTheBudgetIsPinned(); + } + + // (c) Additive blending onto pixels written before the split: every one of kSpanDraws + // full-target draws adds exactly 1/255 of red onto a target whose green was written first, + // so the result counts every draw on both sides of every split. + TEST_F(OversizedRecordingSplitScenario, BlendingAccumulatesOntoPixelsWrittenBeforeTheSplit) { + if (!Ready()) return; + std::string error; + ASSERT_TRUE(BuildRectProgram(&error)) << error; + ColorFbo target = MakeTarget(false); + ASSERT_NE(target.fbo, 0u) << "MakeColorFbo failed"; + + BindFbo(target); + glDisable(GL_DEPTH_TEST); + ClearTo(0.0f, 0.0f, 0.0f, 0.0f); + glEnable(GL_BLEND); + glBlendEquation(GL_FUNC_ADD); + glBlendFunc(GL_ONE, GL_ONE); + + DrawRect(-1.0f, -1.0f, 1.0f, 1.0f, 0.0f, 0.0f, 55.0f / 255.0f, 0.0f, 0.0f); + for (int i = 0; i < kSpanDraws; ++i) { + DrawRect(-1.0f, -1.0f, 1.0f, 1.0f, 0.0f, 1.0f / 255.0f, 0.0f, 0.0f, 0.0f); } - const std::string appended = LibraryLogSince(logBefore); - EXPECT_NE(appended.find("draws into one command buffer; submitting it"), std::string::npos) - << "MOBILEGL_MAGMA_MAX_DRAWS_PER_COMMAND_BUFFER=" << budget << " and one frame recorded " << kDraws - << " draws, but DirectVulkan never reported splitting the recording. A renderer that does not " - "split grows a single command buffer without bound (rd12 on Adreno: ~40k driver mappings, " - "then SIGABRT). See VulkanRenderer::SplitOversizedRecording. Log appended by this test:\n" - << appended; + + const Image image = ReadTarget(target); + EXPECT_TRUE(ColumnsAre(image, 0, kSide - 1, Rgba8{static_cast(kSpanDraws), 55, 0, 0}, 1, + "every additive draw, and the green written before any split, must be in the sum")); + EXPECT_EQ(FirstGLError(), 0u); + ExpectTheRecordingWasSplitWhereTheBudgetIsPinned(); + } + + // (d) A multisample target drawn on both sides of the split, then resolved. Before the + // split a triangle covers the lower-left half of the target; after it, the complementary + // triangle covers the upper-right half. On the diagonal each pixel's samples are shared + // between the two, so only a target that kept every SAMPLE across the split resolves them to + // red + green = 255. One that was resolved or re-broadcast at the split gives about 190 + // there, and one whose samples were dropped gives black or garbage. + TEST_F(OversizedRecordingSplitScenario, AMultisampleTargetKeepsItsSamplesAcrossTheSplit) { + if (!Ready()) return; + std::string error; + ASSERT_TRUE(BuildRectProgram(&error)) << error; + + GLint maxSamples = 0; + glGetIntegerv(GL_MAX_SAMPLES, &maxSamples); + if (maxSamples < 2) { + GTEST_SKIP() << "GL_MAX_SAMPLES is " << maxSamples << "; this needs a multisample renderbuffer"; + } + const GLsizei samples = std::min(4, maxSamples); + GLuint msFbo = 0, msRbo = 0; + glGenFramebuffers(1, &msFbo); + m_framebuffers.push_back(msFbo); + glGenRenderbuffers(1, &msRbo); + m_renderbuffers.push_back(msRbo); + glBindRenderbuffer(GL_RENDERBUFFER, msRbo); + glRenderbufferStorageMultisample(GL_RENDERBUFFER, samples, GL_RGBA8, kSide, kSide); + glBindFramebuffer(GL_FRAMEBUFFER, msFbo); + glFramebufferRenderbuffer(GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0, GL_RENDERBUFFER, msRbo); + if (glCheckFramebufferStatus(GL_FRAMEBUFFER) != GL_FRAMEBUFFER_COMPLETE) { + GTEST_SKIP() << "no complete " << samples << "x multisample RGBA8 renderbuffer on this driver"; + } + ColorFbo resolved = MakeTarget(false); + ASSERT_NE(resolved.fbo, 0u) << "MakeColorFbo failed"; + + glBindFramebuffer(GL_FRAMEBUFFER, msFbo); + glViewport(0, 0, kSide, kSide); + glDisable(GL_DEPTH_TEST); + ClearTo(0.0f, 0.0f, 0.0f, 1.0f); + DrawRect(-1.0f, -1.0f, 1.0f, 1.0f, 0.0f, 1.0f, 0.0f, 0.0f, 1.0f, GL_TRIANGLES, 4, 3); // lower-left, red + DrawFillers(kFillers); + DrawRect(-1.0f, -1.0f, 1.0f, 1.0f, 0.0f, 0.0f, 1.0f, 0.0f, 1.0f, GL_TRIANGLES, 7, 3); // upper-right, green + + glBindFramebuffer(GL_READ_FRAMEBUFFER, msFbo); + glBindFramebuffer(GL_DRAW_FRAMEBUFFER, resolved.fbo); + glBlitFramebuffer(0, 0, kSide, kSide, 0, 0, kSide, kSide, GL_COLOR_BUFFER_BIT, GL_NEAREST); + const Image image = ReadTarget(resolved); + + int wrong = 0; + std::string first; + for (int y = 0; y < kSide && wrong < 1000; ++y) { + for (int x = 0; x < kSide; ++x) { + const Rgba8 seen = image.At(x, y); + bool ok = false; + // The diagonal x + y = kSide in pixel units passes through the pixels whose + // x + y == kSide - 1; every other pixel lies wholly on one side of it. + if (x + y < kSide - 1) { + ok = seen == Rgba8{255, 0, 0, 255}; + } else if (x + y > kSide - 1) { + ok = seen == Rgba8{0, 255, 0, 255}; + } else { + ok = seen.b == 0 && seen.a == 255 && seen.r > 0 && seen.g > 0 && + std::abs(static_cast(seen.r) + static_cast(seen.g) - 255) <= 3; + } + if (!ok) { + if (wrong == 0) { + std::ostringstream os; + os << "(" << x << ", " << y << ") = " << Describe(seen); + first = os.str(); + } + ++wrong; + } + } + } + EXPECT_EQ(wrong, 0) << wrong << " resolved pixels are wrong; first " << first + << ". Lower-left of the diagonal must be red, upper-right green, and a pixel on it " + "red + green = 255 with both present (its samples were split between a draw before " + "the command-buffer split and one after it)"; + EXPECT_EQ(FirstGLError(), 0u); + ExpectTheRecordingWasSplitWhereTheBudgetIsPinned(); + } + + // (e) Occlusion queries open across the split. GL_SAMPLES_PASSED counts kSpanDraws + // single-pixel draws on both sides of it; GL_ANY_SAMPLES_PASSED is answered by the one draw + // made before the split, the fillers after it pass nothing - with a fillers-only control + // that must answer false. + TEST_F(OversizedRecordingSplitScenario, OcclusionQueriesOpenAcrossTheSplitCountBothSides) { + if (!Ready()) return; + std::string error; + ASSERT_TRUE(BuildRectProgram(&error)) << error; + ColorFbo target = MakeTarget(false); + ASSERT_NE(target.fbo, 0u) << "MakeColorFbo failed"; + BindFbo(target); + glDisable(GL_DEPTH_TEST); + ClearTo(0.0f, 0.0f, 0.0f, 1.0f); + + GLuint queries[3] = {}; + glGenQueries(3, queries); + FirstGLError(); + + // Any samples: control first (fillers only), then one real pixel before the split. + glBeginQuery(GL_ANY_SAMPLES_PASSED, queries[0]); + const unsigned int anyBeginError = FirstGLError(); + if (anyBeginError != 0u) { + glDeleteQueries(3, queries); + GTEST_SKIP() << Gl().BackendName() << " does not accept GL_ANY_SAMPLES_PASSED (" + << GLErrorName(anyBeginError) << ")"; + } + DrawFillers(kFillers); + glEndQuery(GL_ANY_SAMPLES_PASSED); + glBeginQuery(GL_ANY_SAMPLES_PASSED, queries[1]); + DrawPixel(0, 1.0f, 1.0f, 1.0f, 1.0f); + DrawFillers(kFillers); + glEndQuery(GL_ANY_SAMPLES_PASSED); + GLuint anyControl = 7, anyAcross = 7; + glGetQueryObjectuiv(queries[0], GL_QUERY_RESULT, &anyControl); + glGetQueryObjectuiv(queries[1], GL_QUERY_RESULT, &anyAcross); + EXPECT_EQ(anyControl, static_cast(GL_FALSE)) << "fillers alone rasterize nothing"; + EXPECT_EQ(anyAcross, static_cast(GL_TRUE)) + << "the only passing draw was made before the split; the query that spans it lost that answer"; + + // Samples passed: kSpanDraws distinct single-sample pixels, spanning at least one split. + glBeginQuery(GL_SAMPLES_PASSED, queries[2]); + const unsigned int samplesBeginError = FirstGLError(); + if (samplesBeginError == 0u) { + for (int i = 0; i < kSpanDraws; ++i) { + DrawPixel(i, 1.0f, 1.0f, 1.0f, 1.0f); + } + glEndQuery(GL_SAMPLES_PASSED); + GLuint samplesPassed = 0; + glGetQueryObjectuiv(queries[2], GL_QUERY_RESULT, &samplesPassed); + if (Gl().BackendName() == std::string("DirectGLES")) { + // OpenGL ES has no sample-counting occlusion query, only ANY_SAMPLES_PASSED*, so + // DirectGLES answers GL_SAMPLES_PASSED with a 0/1 approximation (DirectGLES.cpp, + // the occlusion-query result path). The exact count is not this backend's to give. + RecordProperty("samples_passed", "0/1 approximation on OpenGL ES; exact count not checked"); + EXPECT_EQ(samplesPassed, 1u) << "samples passed, so the approximation must read 1"; + } else { + EXPECT_EQ(samplesPassed, static_cast(kSpanDraws)) + << "each of the " << kSpanDraws << " draws covers exactly one single-sample pixel"; + } + } else { + RecordProperty("samples_passed", std::string("not accepted: ") + GLErrorName(samplesBeginError)); + } + glDeleteQueries(3, queries); + EXPECT_EQ(FirstGLError(), 0u); + ExpectTheRecordingWasSplitWhereTheBudgetIsPinned(); + } + + // (e) GL_PRIMITIVES_GENERATED open across the split: kSpanDraws strips of two triangles. + // A one-draw query first establishes that this stack counts at all. + TEST_F(OversizedRecordingSplitScenario, APrimitivesGeneratedQueryOpenAcrossTheSplitCountsBothSides) { + if (!Ready()) return; + std::string error; + ASSERT_TRUE(BuildRectProgram(&error)) << error; + ColorFbo target = MakeTarget(false); + ASSERT_NE(target.fbo, 0u) << "MakeColorFbo failed"; + BindFbo(target); + glDisable(GL_DEPTH_TEST); + ClearTo(0.0f, 0.0f, 0.0f, 1.0f); + + GLuint queries[2] = {}; + glGenQueries(2, queries); + FirstGLError(); + glBeginQuery(GL_PRIMITIVES_GENERATED, queries[0]); + const unsigned int beginError = FirstGLError(); + if (beginError != 0u) { + glDeleteQueries(2, queries); + GTEST_SKIP() << Gl().BackendName() << " does not accept GL_PRIMITIVES_GENERATED (" + << GLErrorName(beginError) << ")"; + } + DrawPixel(0, 1.0f, 1.0f, 1.0f, 1.0f); + glEndQuery(GL_PRIMITIVES_GENERATED); + GLuint control = 0; + glGetQueryObjectuiv(queries[0], GL_QUERY_RESULT, &control); + if (control == 0u) { + glDeleteQueries(2, queries); + GTEST_SKIP() << "GL_PRIMITIVES_GENERATED reads 0 for a single unsplit strip on " << Gl().BackendName() + << " (" << Gl().RendererString() << "): nothing this stack counts, so nothing to carry"; + } + ASSERT_EQ(control, 2u) << "one strip of four vertices is two triangles"; + + glBeginQuery(GL_PRIMITIVES_GENERATED, queries[1]); + for (int i = 0; i < kSpanDraws; ++i) { + DrawPixel(i, 1.0f, 1.0f, 1.0f, 1.0f); + } + glEndQuery(GL_PRIMITIVES_GENERATED); + GLuint generated = 0; + glGetQueryObjectuiv(queries[1], GL_QUERY_RESULT, &generated); + EXPECT_EQ(generated, static_cast(2 * kSpanDraws)) + << kSpanDraws << " strips of two triangles each, drawn across at least one split"; + glDeleteQueries(2, queries); + EXPECT_EQ(FirstGLError(), 0u); + ExpectTheRecordingWasSplitWhereTheBudgetIsPinned(); + } + + // (f) Transform feedback active across the split: kSpanDraws one-point draws, draw i + // capturing 2i + 1, must append in order into one buffer with nothing past the end, and + // GL_TRANSFORM_FEEDBACK_PRIMITIVES_WRITTEN must count every one. + TEST_F(OversizedRecordingSplitScenario, ATransformFeedbackCaptureActiveAcrossTheSplitAppendsEveryDraw) { + if (!Ready()) return; + + auto compile = [](GLenum stage, const char* source, std::string* log) -> GLuint { + const GLuint shader = glCreateShader(stage); + glShaderSource(shader, 1, &source, nullptr); + glCompileShader(shader); + GLint ok = 0; + glGetShaderiv(shader, GL_COMPILE_STATUS, &ok); + if (!ok) { + char buffer[2048] = {}; + glGetShaderInfoLog(shader, sizeof(buffer) - 1, nullptr, buffer); + *log = buffer; + glDeleteShader(shader); + return 0; + } + return shader; + }; + std::string log; + const GLuint vs = compile(GL_VERTEX_SHADER, kCaptureVS, &log); + ASSERT_NE(vs, 0u) << log; + const GLuint fs = compile(GL_FRAGMENT_SHADER, kCaptureFS, &log); + ASSERT_NE(fs, 0u) << log; + const GLuint program = glCreateProgram(); + glAttachShader(program, vs); + glAttachShader(program, fs); + const char* varyings[] = {"vs_value"}; + glTransformFeedbackVaryings(program, 1, varyings, GL_INTERLEAVED_ATTRIBS); + glLinkProgram(program); + glDeleteShader(vs); + glDeleteShader(fs); + GLint linked = 0; + glGetProgramiv(program, GL_LINK_STATUS, &linked); + if (!linked) { + char buffer[2048] = {}; + glGetProgramInfoLog(program, sizeof(buffer) - 1, nullptr, buffer); + glDeleteProgram(program); + FAIL() << "capture program failed to link: " << buffer; + } + + constexpr int kSlack = 4; + std::vector poison(kSpanDraws + kSlack, kCapturePoison); + GLuint buffer = 0; + glGenBuffers(1, &buffer); + glBindBuffer(GL_TRANSFORM_FEEDBACK_BUFFER, buffer); + glBufferData(GL_TRANSFORM_FEEDBACK_BUFFER, static_cast(poison.size() * sizeof(float)), + poison.data(), GL_STATIC_COPY); + glBindBufferBase(GL_TRANSFORM_FEEDBACK_BUFFER, 0, buffer); + GLuint written = 0; + glGenQueries(1, &written); + + glUseProgram(program); + glBindVertexArray(m_vao); + const GLint indexLocation = glGetUniformLocation(program, "uIndex"); + glEnable(GL_RASTERIZER_DISCARD); + glBeginQuery(GL_TRANSFORM_FEEDBACK_PRIMITIVES_WRITTEN, written); + glBeginTransformFeedback(GL_POINTS); + for (int i = 0; i < kSpanDraws; ++i) { + glUniform1i(indexLocation, i); + glDrawArrays(GL_POINTS, 0, 1); + } + glEndTransformFeedback(); + glEndQuery(GL_TRANSFORM_FEEDBACK_PRIMITIVES_WRITTEN); + glDisable(GL_RASTERIZER_DISCARD); + glUseProgram(0); + + GLuint primitivesWritten = 0; + glGetQueryObjectuiv(written, GL_QUERY_RESULT, &primitivesWritten); + std::vector captured(poison.size(), 0.0f); + glBindBuffer(GL_TRANSFORM_FEEDBACK_BUFFER, buffer); + glGetBufferSubData(GL_TRANSFORM_FEEDBACK_BUFFER, 0, static_cast(captured.size() * sizeof(float)), + captured.data()); + glBindBufferBase(GL_TRANSFORM_FEEDBACK_BUFFER, 0, 0); + glBindBuffer(GL_TRANSFORM_FEEDBACK_BUFFER, 0); + glDeleteQueries(1, &written); + glDeleteBuffers(1, &buffer); + glDeleteProgram(program); + + EXPECT_EQ(primitivesWritten, static_cast(kSpanDraws)); + int wrong = 0; + std::string first; + for (int i = 0; i < static_cast(captured.size()); ++i) { + const float expected = i < kSpanDraws ? static_cast(2 * i + 1) : kCapturePoison; + if (captured[i] != expected) { + if (wrong == 0) { + std::ostringstream os; + os << "record " << i << " = " << captured[i] << ", expected " << expected; + first = os.str(); + } + ++wrong; + } + } + EXPECT_EQ(wrong, 0) << wrong << " of " << captured.size() << " records are wrong; first " << first + << " (records past " << kSpanDraws << " must keep the poison)"; + EXPECT_EQ(FirstGLError(), 0u); + ExpectTheRecordingWasSplitWhereTheBudgetIsPinned(); } } // namespace MGITest From c2de6576ad1cd95a554c28326eee1473f26c8305 Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Tue, 29 Sep 2026 10:34:15 -0400 Subject: [PATCH 6/9] [Fix] (Magma): a frame slot's next descriptor pool doubles its LARGEST pool of the same update-after-bind flavour, capped at 4096 sets - doubling the first (oldest, smallest) one made every grown pool the same size, so rd12's 15,016-set loading frame grew one slot to 60 pools (1x128 + 59x256); now 9 (64 ... 4096, 4096, 4096) --- .../DirectVulkan/Renderer/UniformManager.cpp | 46 ++++++++++++------- .../DirectVulkan/Renderer/UniformManager.h | 22 ++++++++- 2 files changed, 51 insertions(+), 17 deletions(-) diff --git a/MobileGL/MG_Backend/DirectVulkan/Renderer/UniformManager.cpp b/MobileGL/MG_Backend/DirectVulkan/Renderer/UniformManager.cpp index a2465a58..1496774d 100644 --- a/MobileGL/MG_Backend/DirectVulkan/Renderer/UniformManager.cpp +++ b/MobileGL/MG_Backend/DirectVulkan/Renderer/UniformManager.cpp @@ -2137,32 +2137,46 @@ namespace MobileGL::MG_Backend::DirectVulkan { return true; } + Uint32 UniformManager::NextDescriptorPoolMaxSets(const Vector& pools, Bool updateAfterBind, + Uint32 initialMaxSets) { + const Uint32 minSets = std::max(1, initialMaxSets); + const Uint32 maxSets = std::max(minSets, kMaxDescriptorPoolSets); + Uint32 largest = 0; + for (const auto& pool : pools) { + if (pool.updateAfterBind == updateAfterBind) { + largest = std::max(largest, pool.maxSets); + } + } + if (largest == 0) { + return minSets; + } + const Uint64 doubled = static_cast(largest) * 2; + return static_cast(std::clamp(doubled, minSets, maxSets)); + } + Bool UniformManager::GrowFrameDescriptorPool(FrameResources& frame, Uint32 frameIndex, Bool updateAfterBind) { if (frame.descriptorPools.empty()) { return false; } - const auto matchingBucket = std::find_if( - frame.descriptorPools.begin(), frame.descriptorPools.end(), - [updateAfterBind](const DescriptorPoolBucket& candidate) { return candidate.updateAfterBind == updateAfterBind; }); - const Uint32 currentMaxSets = matchingBucket != frame.descriptorPools.end() - ? std::max(1, matchingBucket->maxSets) - : m_setsPerFrame; - const Uint32 grownMaxSets = currentMaxSets <= (std::numeric_limits::max() / 2) ? (currentMaxSets * 2) - : currentMaxSets; - + Uint32 grownMaxSets = NextDescriptorPoolMaxSets(frame.descriptorPools, updateAfterBind, m_setsPerFrame); VkDescriptorPool grownPool = VK_NULL_HANDLE; - if (!CreateDescriptorPool(grownMaxSets, updateAfterBind, grownPool)) { - MGLOG_E_ONCE("UniformDescriptorBinder::GrowFrameDescriptorPool failed: cannot create grown pool (%u -> %u sets)", - currentMaxSets, grownMaxSets); - return false; + // A large pool is a large driver allocation; if the device refuses one, settle for less + // rather than failing the draw - halving down to the base size, where the old policy lived. + while (!CreateDescriptorPool(grownMaxSets, updateAfterBind, grownPool)) { + if (grownMaxSets <= m_setsPerFrame) { + MGLOG_E_ONCE("UniformDescriptorBinder::GrowFrameDescriptorPool failed: cannot create grown pool (%u sets)", + grownMaxSets); + return false; + } + grownMaxSets = std::max(m_setsPerFrame, grownMaxSets / 2); } frame.descriptorPools.push_back({grownPool, grownMaxSets, 0, updateAfterBind}); frame.activeDescriptorPoolIndex = static_cast(frame.descriptorPools.size() - 1); - MGLOG_D( - "UniformDescriptorBinder: frame %u descriptor pool exhausted, grew pool (%u -> %u sets), poolCount=%zu", - frameIndex, currentMaxSets, grownMaxSets, frame.descriptorPools.size()); + MGLOG_D("UniformDescriptorBinder: frame %u descriptor pool exhausted, grew a %u-set pool (updateAfterBind=%d), " + "poolCount=%zu", + frameIndex, grownMaxSets, updateAfterBind ? 1 : 0, frame.descriptorPools.size()); return true; } diff --git a/MobileGL/MG_Backend/DirectVulkan/Renderer/UniformManager.h b/MobileGL/MG_Backend/DirectVulkan/Renderer/UniformManager.h index d1b363ed..d6951c57 100644 --- a/MobileGL/MG_Backend/DirectVulkan/Renderer/UniformManager.h +++ b/MobileGL/MG_Backend/DirectVulkan/Renderer/UniformManager.h @@ -130,13 +130,33 @@ namespace MobileGL::MG_Backend::DirectVulkan { static Bool ProgramSamplesOnlySingleLevelTextures(const MG_State::GLState::ProgramObject& program, const ProgramFactory::VkProgramObject& programObj); - private: + // ---- per-frame descriptor pools ---------------------------------------------------- + // Each frame slot owns a list of pools. The first is the base pool (non-update-after-bind, + // setsPerFrame sets) and is never destroyed while the manager lives; every later one was + // added by GrowFrameDescriptorPool when an allocation found no room. A pool serves one + // updateAfterBind flavour, because a set can only come from a pool created with the flag + // its layout was. struct DescriptorPoolBucket { VkDescriptorPool handle = VK_NULL_HANDLE; Uint32 maxSets = 0; Uint32 allocatedSets = 0; Bool updateAfterBind = false; }; + // One pool's maxSets never exceeds this. It bounds one pool's driver memory: CreateDescriptorPool + // declares 8 descriptors of each of 6 types per set, so a 4096-set pool asks for 196,608 + // descriptors - about 12 MiB at 64 bytes each on a driver that backs every declared one. + static constexpr Uint32 kMaxDescriptorPoolSets = 4096; + // The growth policy, pure so the host tests can drive it: the size of the next pool a slot + // adds for `updateAfterBind` layouts, given the slot's existing pools. Twice the LARGEST pool + // of that flavour, capped at kMaxDescriptorPoolSets; `initialMaxSets` when the slot has no + // pool of that flavour yet. So a frame that needs N sets adds O(log N) pools while N stays + // under the cap, and one more pool per kMaxDescriptorPoolSets sets beyond it. Taking the + // largest (not the first) pool is the point: the first same-flavour pool is the oldest, + // smallest one, and doubling IT made every grown pool the same size. + static Uint32 NextDescriptorPoolMaxSets(const Vector& pools, Bool updateAfterBind, + Uint32 initialMaxSets); + + private: // A cached descriptor set together with the pool it was allocated from, so a // layout-destroyed purge can vkFreeDescriptorSets it back and credit the From 53fa16c2c27e3937eda81dcbae38d5a0e6c34b33 Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Tue, 29 Sep 2026 10:34:16 -0400 Subject: [PATCH 7/9] [Fix] (Magma): a frame slot whose descriptor pools stay under a quarter full for 120 frames is handed back whole - its set cache cleared, grown pools destroyed, base pool reset and every memo holding a set handle dropped - so rd12's loading burst no longer lives for the whole process (live sets 15,042 -> 39, pools 65 -> 6); MOBILEGL_MAGMA_DESCRIPTOR_TRIM_FRAMES (0 = never) and a test-visible pool census --- MobileGL/Config.h | 6 ++ MobileGL/ConfigLoader.cpp | 1 + .../DirectVulkan/DescriptorPoolCensus.h | 29 ++++++ .../MG_Backend/DirectVulkan/DirectVulkan.cpp | 4 + .../DirectVulkan/Renderer/UniformManager.cpp | 96 ++++++++++++++++++- .../DirectVulkan/Renderer/UniformManager.h | 58 ++++++++++- .../DirectVulkan/Renderer/VulkanRenderer.h | 4 + README.md | 1 + 8 files changed, 191 insertions(+), 8 deletions(-) create mode 100644 MobileGL/MG_Backend/DirectVulkan/DescriptorPoolCensus.h diff --git a/MobileGL/Config.h b/MobileGL/Config.h index 5e96abcf..642a9bdc 100644 --- a/MobileGL/Config.h +++ b/MobileGL/Config.h @@ -138,6 +138,12 @@ namespace MobileGL::MG_Config { // freed; Adreno maps it into the process in 16 KiB chunks, so one ~1M-draw loading frame // exhausts vm.max_map_count. See VulkanRenderer::SplitOversizedRecording. Uint32 MagmaMaxDrawsPerCommandBuffer = 16384; + // MOBILEGL_MAGMA_DESCRIPTOR_TRIM_FRAMES: how many consecutive frames (strictly, BeginFrame + // epochs) a frame slot's descriptor pools must stay under a quarter full before Magma + // hands the slot's whole set cache back and lets it re-grow. 0 = never trim. A loading + // frame otherwise keeps its burst of sets (rd12: ~15k, where steady frames use a few dozen) + // for the life of the process. See UniformManager::TrimFrameDescriptorPools. + Uint32 MagmaDescriptorTrimFrames = 120; // MOBILEGL_ESPRYT_AVOID_SAMPLER_MIPMAP_MIN_FILTER: avoid mipmap min filters in samplers, // resolves certain rendering bugs on ANGLE + llvmpipe. Bool EsprytAvoidSamplerMipmapMinFilter = false; diff --git a/MobileGL/ConfigLoader.cpp b/MobileGL/ConfigLoader.cpp index 8e60e82a..3b8ba099 100644 --- a/MobileGL/ConfigLoader.cpp +++ b/MobileGL/ConfigLoader.cpp @@ -179,6 +179,7 @@ namespace MobileGL::MG_ConfigLoader { features.MagmaFramesInFlight = QueryEnvUint32("MOBILEGL_MAGMA_FRAMESINFLIGHT", 3, 1, 64); features.MagmaMaxDrawsPerCommandBuffer = QueryEnvUint32("MOBILEGL_MAGMA_MAX_DRAWS_PER_COMMAND_BUFFER", 16384, 0, 1u << 24); + features.MagmaDescriptorTrimFrames = QueryEnvUint32("MOBILEGL_MAGMA_DESCRIPTOR_TRIM_FRAMES", 120, 0, 1u << 20); features.EsprytAvoidSamplerMipmapMinFilter = QueryEnvFlag("MOBILEGL_ESPRYT_AVOID_SAMPLER_MIPMAP_MIN_FILTER"); features.EsprytAvoidExplicitLodBias = QueryEnvFlag("MOBILEGL_ESPRYT_AVOID_EXPLICIT_LOD_BIAS"); diff --git a/MobileGL/MG_Backend/DirectVulkan/DescriptorPoolCensus.h b/MobileGL/MG_Backend/DirectVulkan/DescriptorPoolCensus.h new file mode 100644 index 00000000..f3ca3020 --- /dev/null +++ b/MobileGL/MG_Backend/DirectVulkan/DescriptorPoolCensus.h @@ -0,0 +1,29 @@ +// MobileGL - MobileGL/MG_Backend/DirectVulkan/DescriptorPoolCensus.h +// Copyright (c) 2025-2026 MobileGL-Dev +// Licensed under the GNU Lesser General Public License v3.0: +// https://www.gnu.org/licenses/gpl-3.0.txt +// https://www.gnu.org/licenses/lgpl-3.0.txt +// SPDX-License-Identifier: LGPL-3.0-only +// End of Source File Header + +#pragma once + +#include + +namespace MobileGL::MG_Backend::DirectVulkan { + // A count of Magma's per-frame descriptor pools and the sets they hold (UniformManager), for + // tests. Dependency-free on purpose: the integration scenarios include it next to the system GL + // headers. They link the static library on desktop; the shipping shared library is built with + // hidden visibility, so on Android nothing outside the library can call GetDescriptorPoolCensus. + struct DescriptorPoolCensus { + bool available = false; // false when no Magma renderer is live (e.g. the DirectGLES backend) + uint32_t frameSlots = 0; + uint32_t pools = 0; // descriptor pools across every frame slot + uint32_t maxPoolsInOneSlot = 0; + uint64_t capacitySets = 0; // sum of maxSets over those pools + uint64_t cachedSets = 0; // sets allocated from them and held in the per-layout caches + uint64_t slotTrims = 0; // whole-slot trims since the renderer came up + }; + + DescriptorPoolCensus GetDescriptorPoolCensus(); +} // namespace MobileGL::MG_Backend::DirectVulkan diff --git a/MobileGL/MG_Backend/DirectVulkan/DirectVulkan.cpp b/MobileGL/MG_Backend/DirectVulkan/DirectVulkan.cpp index cae6e388..0e439b31 100644 --- a/MobileGL/MG_Backend/DirectVulkan/DirectVulkan.cpp +++ b/MobileGL/MG_Backend/DirectVulkan/DirectVulkan.cpp @@ -1446,4 +1446,8 @@ namespace MobileGL::MG_Backend::DirectVulkan { Optional GetRequestedSwapInterval() { return g_requestedSwapInterval; } + + DescriptorPoolCensus GetDescriptorPoolCensus() { + return pVulkanRenderer ? pVulkanRenderer->GetDescriptorPoolCensus() : DescriptorPoolCensus{}; + } } // namespace MobileGL::MG_Backend::DirectVulkan diff --git a/MobileGL/MG_Backend/DirectVulkan/Renderer/UniformManager.cpp b/MobileGL/MG_Backend/DirectVulkan/Renderer/UniformManager.cpp index 1496774d..8a417540 100644 --- a/MobileGL/MG_Backend/DirectVulkan/Renderer/UniformManager.cpp +++ b/MobileGL/MG_Backend/DirectVulkan/Renderer/UniformManager.cpp @@ -303,6 +303,11 @@ namespace MobileGL::MG_Backend::DirectVulkan { m_samplerResolveMemoHighWater = 0; m_setsPerFrame = setsPerFrame; m_peakDescriptorSetsObserved = 0; + m_trimQuietEpochs = MG_Config::Features.MagmaDescriptorTrimFrames; + // The renderer records its first frame into slot 0 before any BeginFrame runs. + m_epochSlot = 0; + m_epochOpen = true; + m_descriptorSlotTrims = 0; m_textureManager = textureManager; m_samplerManager = samplerManager; @@ -312,6 +317,8 @@ namespace MobileGL::MG_Backend::DirectVulkan { frame.activeDescriptorPoolIndex = 0; frame.allocatedSetsThisFrame = 0; frame.peakAllocatedSetsThisFrame = 0; + frame.capacitySets = m_setsPerFrame; + frame.quietEpochs = 0; frame.descriptorPools.clear(); VkDescriptorPool initialPool = VK_NULL_HANDLE; @@ -361,8 +368,12 @@ namespace MobileGL::MG_Backend::DirectVulkan { frame.activeDescriptorPoolIndex = 0; frame.allocatedSetsThisFrame = 0; frame.peakAllocatedSetsThisFrame = 0; + frame.capacitySets = 0; + frame.quietEpochs = 0; } m_frames.clear(); + m_epochSlot = 0; + m_epochOpen = false; m_bufferManager = nullptr; m_programFactory = nullptr; @@ -396,6 +407,20 @@ namespace MobileGL::MG_Backend::DirectVulkan { "UniformDescriptorBinder: new descriptor set peak observed=%u (base setsPerFrame=%u, frame=%u, pools=%zu)", m_peakDescriptorSetsObserved, m_setsPerFrame, frameIndex, frame.descriptorPools.size()); } + // The epoch that just ended is the one of the slot begun last - this slot again on the + // drain path, the previous frame's slot on the Present path. Its peak is still unreset. + if (m_epochOpen && m_epochSlot < m_frames.size()) { + NoteEndedEpochForTrim(m_frames[m_epochSlot].peakAllocatedSetsThisFrame); + } + m_epochSlot = frameIndex; + m_epochOpen = true; + // No pending command buffer references this slot's sets here (see TrimFrameDescriptorPools + // for why that holds at both of BeginFrame's call sites), so a quiet grown slot can be + // handed back whole. + if (m_trimQuietEpochs != 0 && frame.descriptorPools.size() > 1 && + frame.capacitySets >= kDescriptorTrimMinCapacity && frame.quietEpochs >= m_trimQuietEpochs) { + TrimFrameDescriptorPools(frame, frameIndex); + } frame.activeDescriptorPoolIndex = 0; frame.allocatedSetsThisFrame = 0; frame.peakAllocatedSetsThisFrame = 0; @@ -404,11 +429,7 @@ namespace MobileGL::MG_Backend::DirectVulkan { } // The frame's descriptor sets are recycled above, so last frame's reuse targets // are gone: start the per-draw descriptor-reuse cache fresh this frame. - for (auto& entry : m_descriptorReuseMemo) { - entry.valid = false; - } - m_fastRebindMemo.valid = false; - m_lastBindValid = false; + InvalidateDescriptorSetMemos(); // Re-fingerprint the bound sampler set fresh this frame so any GL object address // reuse cannot outlive a single frame (see SamplerResolveMemo). Only the entries a // resolve has actually written can be valid, so the high-water mark bounds the @@ -422,6 +443,68 @@ namespace MobileGL::MG_Backend::DirectVulkan { } } + void UniformManager::InvalidateDescriptorSetMemos() { + // Every VkDescriptorSet handle held outside the per-layout caches - in this backend they all + // live in this class. A trim can hand a destroyed set's handle value to a fresh allocation, + // so the bind-dedup shadow must drop too, not only the two reuse memos. + for (auto& entry : m_descriptorReuseMemo) { + entry.valid = false; + } + m_fastRebindMemo.valid = false; + m_lastBindValid = false; + } + + void UniformManager::NoteEndedEpochForTrim(Uint32 epochPeakSets) { + for (auto& slot : m_frames) { + if (static_cast(epochPeakSets) * kDescriptorTrimHeadroom > slot.capacitySets) { + slot.quietEpochs = 0; + } else if (slot.quietEpochs < std::numeric_limits::max()) { + ++slot.quietEpochs; + } + } + } + + void UniformManager::TrimFrameDescriptorPools(FrameResources& frame, Uint32 frameIndex) { + const SizeT poolsBefore = frame.descriptorPools.size(); + const Uint64 capacityBefore = frame.capacitySets; + // Holders first: the per-layout cache and every memo that may name one of these sets. + frame.descriptorSetCacheByLayout.clear(); + InvalidateDescriptorSetMemos(); + for (SizeT i = 1; i < frame.descriptorPools.size(); ++i) { + if (frame.descriptorPools[i].handle != VK_NULL_HANDLE) { + vkDestroyDescriptorPool(m_device, frame.descriptorPools[i].handle, nullptr); + } + } + frame.descriptorPools.resize(1); + auto& base = frame.descriptorPools[0]; + // Frees every set allocated from it; the spec defines no failure for vkResetDescriptorPool. + vkResetDescriptorPool(m_device, base.handle, 0); + base.allocatedSets = 0; + frame.activeDescriptorPoolIndex = 0; + frame.capacitySets = base.maxSets; + frame.quietEpochs = 0; + ++m_descriptorSlotTrims; + MGLOG_D("UniformDescriptorBinder: frame %u trimmed its descriptor pools (%zu pools / %llu sets -> 1 / %u)", + frameIndex, poolsBefore, static_cast(capacityBefore), base.maxSets); + } + + DescriptorPoolCensus UniformManager::GetDescriptorPoolCensus() const { + DescriptorPoolCensus census; + census.available = true; + census.frameSlots = static_cast(m_frames.size()); + for (const auto& frame : m_frames) { + const auto poolCount = static_cast(frame.descriptorPools.size()); + census.pools += poolCount; + census.maxPoolsInOneSlot = std::max(census.maxPoolsInOneSlot, poolCount); + census.capacitySets += frame.capacitySets; + for (const auto& cacheEntryPair : frame.descriptorSetCacheByLayout) { + census.cachedSets += cacheEntryPair.second.sets.size(); + } + } + census.slotTrims = m_descriptorSlotTrims; + return census; + } + void UniformManager::OnDescriptorSetLayoutDestroyed(VkDescriptorSetLayout descriptorSetLayout) { SizeT purgedSets = 0; for (auto& frame : m_frames) { @@ -2174,6 +2257,9 @@ namespace MobileGL::MG_Backend::DirectVulkan { frame.descriptorPools.push_back({grownPool, grownMaxSets, 0, updateAfterBind}); frame.activeDescriptorPoolIndex = static_cast(frame.descriptorPools.size() - 1); + frame.capacitySets += grownMaxSets; + // Growing is the opposite of quiet: a trim must wait a full quiet run from here. + frame.quietEpochs = 0; MGLOG_D("UniformDescriptorBinder: frame %u descriptor pool exhausted, grew a %u-set pool (updateAfterBind=%d), " "poolCount=%zu", frameIndex, grownMaxSets, updateAfterBind ? 1 : 0, frame.descriptorPools.size()); diff --git a/MobileGL/MG_Backend/DirectVulkan/Renderer/UniformManager.h b/MobileGL/MG_Backend/DirectVulkan/Renderer/UniformManager.h index d6951c57..99dc160a 100644 --- a/MobileGL/MG_Backend/DirectVulkan/Renderer/UniformManager.h +++ b/MobileGL/MG_Backend/DirectVulkan/Renderer/UniformManager.h @@ -12,6 +12,7 @@ #include "VkBufferManager.h" #include "VkSamplerManager.h" #include "VkTextureManager.h" +#include "../DescriptorPoolCensus.h" #include "../VkIncludes.h" #include @@ -63,9 +64,11 @@ namespace MobileGL::MG_Backend::DirectVulkan { // FREE_DESCRIPTOR_SET_BIT) and the pool accounting is credited, so program // churn recycles pool capacity instead of abandoning it. GPU-safe: the layout // only dies after >1024 idle frame boundaries, so no in-flight command buffer - // references its sets. This is the only eviction path for the per-layout - // caches - a live layout's entry must never be purged (its sets would be - // unreachable pool slots), so there is deliberately no age-based sweep here. + // references its sets. This is the only PER-LAYOUT eviction path for the + // caches - a live layout's entry must never be purged on its own (its sets would be + // unreachable pool slots), so there is deliberately no age-based sweep here. The + // other path, TrimFrameDescriptorPools, drops a slot's whole cache together with + // the pools behind it. void OnDescriptorSetLayoutDestroyed(VkDescriptorSetLayout descriptorSetLayout); // One record per visited CombinedImageSampler DESCRIPTOR (post fallback substitution, // in binding order, and within a binding in array-element order): the resolved texture @@ -155,6 +158,18 @@ namespace MobileGL::MG_Backend::DirectVulkan { // smallest one, and doubling IT made every grown pool the same size. static Uint32 NextDescriptorPoolMaxSets(const Vector& pools, Bool updateAfterBind, Uint32 initialMaxSets); + // Trim policy (see TrimFrameDescriptorPools). A slot is "quiet" in an epoch whose peak set + // count times kDescriptorTrimHeadroom still fits its capacity; a slot that has been quiet for + // MOBILEGL_MAGMA_DESCRIPTOR_TRIM_FRAMES consecutive epochs, and holds at least + // kDescriptorTrimMinCapacity sets, is trimmed. The headroom is what keeps the trim from + // thrashing: geometric growth leaves a re-grown slot at least a third full (its pools sum to + // under twice the largest, and the largest is under twice the need that added it), so the + // epoch that grew a slot is never quiet for it. The floor keeps a slot sized for ordinary + // frames from being trimmed at all - the trim exists to give back a burst. + static constexpr Uint32 kDescriptorTrimHeadroom = 4; + static constexpr Uint32 kDescriptorTrimMinCapacity = 1024; + // For tests (DescriptorPoolCensus.h): pools, capacity, cached sets and trims over every slot. + DescriptorPoolCensus GetDescriptorPoolCensus() const; private: @@ -171,6 +186,10 @@ namespace MobileGL::MG_Backend::DirectVulkan { Uint32 cursor = 0; }; + // descriptorSetCacheByLayout is the per-frame set cache: BeginFrame rewinds every cursor, so + // a slot re-writes the sets it allocated in earlier frames instead of allocating new ones, and + // the cache only ever grows - except through TrimFrameDescriptorPools, which gives a slot's + // whole burst back at once (and OnDescriptorSetLayoutDestroyed, one dead layout at a time). struct FrameResources { Vector descriptorPools; UnorderedMap descriptorSetCacheByLayout; @@ -178,6 +197,10 @@ namespace MobileGL::MG_Backend::DirectVulkan { Uint32 activeDescriptorPoolIndex = 0; Uint32 allocatedSetsThisFrame = 0; Uint32 peakAllocatedSetsThisFrame = 0; + // Sum of maxSets over descriptorPools, kept in step by Initialize, Grow and Trim. + Uint64 capacitySets = 0; + // Consecutive epochs (see NoteEndedEpochForTrim) that were quiet for THIS slot's capacity. + Uint32 quietEpochs = 0; }; static Bool ResolveSamplerTexture(const MG_State::GLState::ProgramObject& program, @@ -303,6 +326,28 @@ namespace MobileGL::MG_Backend::DirectVulkan { const Vector& dynamicOffsets); Bool CreateDescriptorPool(Uint32 maxSets, Bool updateAfterBind, VkDescriptorPool& outPool) const; Bool GrowFrameDescriptorPool(FrameResources& frame, Uint32 frameIndex, Bool updateAfterBind); + // An epoch is the stretch between two BeginFrame calls: one frame, or the part of one between + // two drains (VulkanRenderer::TryDrainFrameTransients). Called by BeginFrame with the epoch + // that just ended, it updates every slot's quiet count - a burst in any slot counts against + // all of them, so bursts that land in the other slots still hold a slot's trim off. + void NoteEndedEpochForTrim(Uint32 epochPeakSets); + // Gives a slot's whole descriptor burst back: clears its per-layout cache, destroys every + // grown pool and resets the base pool, so the slot re-grows geometrically from setsPerFrame. + // Whole-slot rather than per-set: vkFreeDescriptorSets fragments a pool and can never empty + // a grown one. Called only from BeginFrame, which runs only when no pending command buffer + // references the slot's sets: + // - Present path (VulkanRenderer::Present, after WaitAndAcquireNextImage): the slot's + // imageInFlightFence has been waited. That fence rides the slot's LAST submission - the + // Present submit - and every mid-frame submission of the slot (FlushPendingCommands, + // SplitOversizedRecording, readbacks) went to the same queue earlier, so the fence's + // signal covers them (vkQueueSubmit's fence scope includes all earlier submissions). + // - Drain path (VulkanRenderer::TryDrainFrameTransients): every submission has been + // observed complete and no recording is open. + // Sets are per slot: a slot's sets are only ever bound into recordings made while it is the + // current slot, so trimming one slot never touches another slot's in-flight work. + void TrimFrameDescriptorPools(FrameResources& frame, Uint32 frameIndex); + // Drops every handle this class holds to a descriptor set outside the per-layout caches. + void InvalidateDescriptorSetMemos(); VkResult AllocateDescriptorSetsFromActivePool( Uint32 frameIndex, const ProgramFactory::VkProgramObject& programObj, VkDescriptorSet& outDescriptorSet); VkResult AcquireDescriptorSet(Uint32 frameIndex, @@ -320,6 +365,13 @@ namespace MobileGL::MG_Backend::DirectVulkan { Uint32 m_maxBindings = 0; Uint32 m_setsPerFrame = 0; Uint32 m_peakDescriptorSetsObserved = 0; + // MOBILEGL_MAGMA_DESCRIPTOR_TRIM_FRAMES, latched at Initialize (0 = never trim). + Uint32 m_trimQuietEpochs = 0; + // The slot whose epoch is open (the last one BeginFrame was called for); valid once + // m_epochOpen is set. Its peak is the ended epoch's peak at the next BeginFrame. + Uint32 m_epochSlot = 0; + Bool m_epochOpen = false; + Uint64 m_descriptorSlotTrims = 0; VkTextureManager* m_textureManager = nullptr; VkSamplerManager* m_samplerManager = nullptr; mutable SharedPtr m_fallbackTexture2D; diff --git a/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.h b/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.h index 90980415..5db812fc 100644 --- a/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.h +++ b/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.h @@ -285,6 +285,10 @@ namespace MobileGL::MG_Backend::DirectVulkan { // signal as soon as the GPU actually finishes, which MC 1.21.5's // fence-paced ring buffers rely on to recycle their space. Uint64 GetSyncPointSubmitIndex() const; + // For tests: the per-frame descriptor pools (see DescriptorPoolCensus.h). + DescriptorPoolCensus GetDescriptorPoolCensus() const { + return m_uniformManager ? m_uniformManager->GetDescriptorPoolCensus() : DescriptorPoolCensus{}; + } // Non-blocking: polls outstanding submission fences and reports // whether every submission up to `submitIndex` has completed. Bool IsSubmitIndexComplete(Uint64 submitIndex); diff --git a/README.md b/README.md index ac6c21e5..1e865575 100644 --- a/README.md +++ b/README.md @@ -163,6 +163,7 @@ MobileGL supports runtime configuration via environment variables. | `MOBILEGL_MAGMA_R11G11B10F_FALLBACK` | Use Magma's R11G11B10F format fallback. | `0`, `1` | `0` | | `MOBILEGL_MAGMA_FRAMESINFLIGHT` | Set Magma frames in flight. | Integer `1`–`64` | `3` | | `MOBILEGL_MAGMA_MAX_DRAWS_PER_COMMAND_BUFFER` | Draws and dispatches Magma records into one command buffer before it submits the buffer mid-frame and continues on a fresh one. `0` never splits. | Integer `0`–`16777216` | `16384` | +| `MOBILEGL_MAGMA_DESCRIPTOR_TRIM_FRAMES` | Frames a frame slot's descriptor pools must stay under a quarter full before Magma frees the slot's cached descriptor sets and grown pools. `0` never trims. | Integer `0`–`1048576` | `120` | | `MOBILEGL_ESPRYT_AVOID_SAMPLER_MIPMAP_MIN_FILTER` | Avoid sampler mipmap minification filters. | `0`, `1` | `0` | | `MOBILEGL_COHERENT_AS_FLUSH` | Treat persistent `GL_MAP_FLUSH_EXPLICIT_BIT` maps as coherent (app-compat for engines like Flywheel that never flush them). | `0`, `1` | `0` | | `MOBILEGL_ESPRYT_FORCE_DS_READBACK_EMULATION` | Always emulate depth/stencil `glReadPixels`/`glGetTexImage` by shader sampling on Espryt, instead of using the driver's own depth/stencil readback where it has one. | `0`, `1` | `0` | From 9dccabf03cbe9ee96e8bf5bfd9c1aa66f618e01a Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Tue, 29 Sep 2026 10:34:16 -0400 Subject: [PATCH 8/9] [Test] (Magma): descriptor pool growth and trim - the pure sizing policy adds O(log N) pools for an N-set frame, and DescriptorPoolGrowthScenario's 4096-distinct-set frame lands whole on both backends, grows at most 7 pools, and is given back after quiet frames and re-grown while they are still in flight --- MobileGL/MG_IntegrationTest/CMakeLists.txt | 1 + .../DescriptorPoolGrowthScenario.cpp | 346 ++++++++++++++++++ MobileGL/MG_Test/SanityTest.cpp | 94 +++++ 3 files changed, 441 insertions(+) create mode 100644 MobileGL/MG_IntegrationTest/Scenarios/DescriptorPoolGrowthScenario.cpp diff --git a/MobileGL/MG_IntegrationTest/CMakeLists.txt b/MobileGL/MG_IntegrationTest/CMakeLists.txt index a3a71922..79501418 100644 --- a/MobileGL/MG_IntegrationTest/CMakeLists.txt +++ b/MobileGL/MG_IntegrationTest/CMakeLists.txt @@ -129,6 +129,7 @@ add_executable(MobileGLIntegrationTest Scenarios/RenderbufferBlendFormatScenario.cpp Scenarios/DualSourceBlendScenario.cpp Scenarios/OversizedRecordingSplitScenario.cpp + Scenarios/DescriptorPoolGrowthScenario.cpp ) target_include_directories(MobileGLIntegrationTest PRIVATE diff --git a/MobileGL/MG_IntegrationTest/Scenarios/DescriptorPoolGrowthScenario.cpp b/MobileGL/MG_IntegrationTest/Scenarios/DescriptorPoolGrowthScenario.cpp new file mode 100644 index 00000000..8cc3c342 --- /dev/null +++ b/MobileGL/MG_IntegrationTest/Scenarios/DescriptorPoolGrowthScenario.cpp @@ -0,0 +1,346 @@ +// MobileGL - MobileGL/MG_IntegrationTest/Scenarios/DescriptorPoolGrowthScenario.cpp +// Copyright (c) 2025-2026 MobileGL-Dev +// Licensed under the GNU Lesser General Public License v3.0: +// https://www.gnu.org/licenses/gpl-3.0.txt +// https://www.gnu.org/licenses/lgpl-3.0.txt +// SPDX-License-Identifier: LGPL-3.0-only +// End of Source File Header +// +// Scenario - ONE FRAME THAT NEEDS THOUSANDS OF DISTINCT DESCRIPTOR SETS. +// +// DirectVulkan takes a draw's descriptor set from a per-frame-slot cache and adds a pool to the +// slot when the cache runs dry (UniformManager). Two things went wrong with a burst like +// Minecraft rd12's loading frame (15,016 sets in one slot): +// - Growth was linear. The next pool was sized at twice the slot's FIRST pool of the flavour, +// which is always its oldest and smallest, so every grown pool came out the same size: the +// loading slot ended with 59 pools of 256 sets. +// - The burst was kept forever. The cache only rewinds, so the slot held all 15k sets (and the +// pools behind them) for the life of the process, while steady frames used a few dozen. +// Now each pool doubles the slot's largest one (capped), and a slot whose pools stay under a +// quarter full for MOBILEGL_MAGMA_DESCRIPTOR_TRIM_FRAMES frames is handed back whole and re-grows. +// +// Each of kDraws draws paints its own pixel of a 64x64 target and samples one of kTextures +// textures, a different one from each of the previous draws' so neither the renderer's per-draw +// descriptor reuse memo (four entries) nor its dynamic-offset-only rebind can let two draws share +// a set: one frame needs kDraws sets. Every pixel must hold its own draw's texture colour, on both +// backends. On DirectVulkan the cases also read the pools through GetDescriptorPoolCensus - which +// only the desktop build can reach, since the integration harness links the static library there +// and the hidden-visibility shipping library on Android. + +#include +#include +#include +#include +#include +#include + +#include "../Harness/HeadlessGL.h" +#include "../Harness/ScenarioFixture.h" + +#if !defined(__ANDROID__) +#include +#endif + +#ifdef GLAPI +#undef GLAPI +#endif +#define GL_GLEXT_PROTOTYPES +#include +#include +#undef GL_GLEXT_PROTOTYPES + +namespace MGITest { + namespace { + + constexpr int kSide = 64; + constexpr int kDraws = kSide * kSide; // well under the 16384-draw command-buffer split budget + // More than the descriptor reuse memo's four entries, so the texture a draw samples is never + // one of the last four draws'. + constexpr int kTextures = 8; + // The base pool size (VulkanRenderer's kDescriptorSetsPerFrame) and the growth cap + // (UniformManager::kMaxDescriptorPoolSets). + constexpr std::uint64_t kBaseSets = 64; + constexpr std::uint64_t kMaxPoolSets = 4096; + + constexpr const char* kVS = R"(#version 330 core +in vec2 aPos; +uniform int uIndex; +void main() { + vec2 cell = vec2(float(uIndex % 64), float(uIndex / 64)); + gl_Position = vec4((cell + aPos) / 32.0 - 1.0, 0.0, 1.0); +} +)"; + constexpr const char* kFS = R"(#version 330 core +uniform sampler2D uTex; +out vec4 o_color; +void main() { o_color = texture(uTex, vec2(0.5)); } +)"; + + // Texture t's single texel; every channel differs between any two textures. + Rgba8 TextureColor(int t) { + return Rgba8{static_cast(16 + 32 * t), static_cast(240 - 32 * t), + static_cast(8 + 29 * t), 255}; + } + + std::string Describe(const Rgba8& c) { + std::ostringstream os; + os << "rgba(" << static_cast(c.r) << ", " << static_cast(c.g) << ", " << static_cast(c.b) + << ", " << static_cast(c.a) << ")"; + return os.str(); + } + + unsigned long EnvUnsigned(const char* name, unsigned long fallback) { + const char* value = std::getenv(name); + if (value == nullptr || *value == '\0') return fallback; + char* end = nullptr; + const unsigned long parsed = std::strtoul(value, &end, 10); + return (end != nullptr && *end == '\0') ? parsed : fallback; + } + + // Pools a geometric policy adds for n sets from an empty flavour: 64, 128, ... up to the cap, + // then one per cap. The old first-pool rule needed n/128 or n/256 of them. + std::uint64_t GeometricPoolBound(std::uint64_t n) { + std::uint64_t pools = 0, capacity = 0, size = kBaseSets; + while (capacity < n) { + capacity += size; + ++pools; + size = std::min(size * 2, kMaxPoolSets); + } + return pools; + } + + struct Census { + bool available = false; + std::uint64_t pools = 0; + std::uint64_t capacitySets = 0; + std::uint64_t cachedSets = 0; + std::uint64_t slotTrims = 0; + }; + + Census ReadCensus() { + Census out; +#if !defined(__ANDROID__) + const auto census = MobileGL::MG_Backend::DirectVulkan::GetDescriptorPoolCensus(); + out.available = census.available; + out.pools = census.pools; + out.capacitySets = census.capacitySets; + out.cachedSets = census.cachedSets; + out.slotTrims = census.slotTrims; +#endif + return out; + } + + std::string Describe(const Census& c) { + std::ostringstream os; + os << "pools=" << c.pools << " capacity=" << c.capacitySets << " cachedSets=" << c.cachedSets + << " slotTrims=" << c.slotTrims; + return os.str(); + } + + ::testing::AssertionResult EveryPixelHoldsItsDrawsTexture(const Image& image, const char* when) { + int wrong = 0; + int firstX = -1, firstY = -1; + Rgba8 firstSeen{}; + for (int y = 0; y < kSide; ++y) { + for (int x = 0; x < kSide; ++x) { + const Rgba8 expected = TextureColor((y * kSide + x) % kTextures); + const Rgba8 seen = image.At(x, y); + if (seen != expected) { + if (wrong == 0) { + firstX = x; + firstY = y; + firstSeen = seen; + } + ++wrong; + } + } + } + if (wrong == 0) return ::testing::AssertionSuccess(); + const int draw = firstY * kSide + firstX; + return ::testing::AssertionFailure() + << when << ": " << wrong << " of " << kDraws << " pixels do not hold their own draw's texture. " + << "First: (" << firstX << ", " << firstY << ") = " << Describe(firstSeen) << ", drawn by draw " + << draw << " with texture " << (draw % kTextures) << " = " + << Describe(TextureColor(draw % kTextures)) << " (a pixel no draw reached holds 0,0,0,0)"; + } + + class DescriptorPoolGrowthScenario : public ScenarioTest { + protected: + void SetUp() override { + ScenarioTest::SetUp(); + if (!Ready()) return; + static const float kVertices[] = {0.0f, 0.0f, 1.0f, 0.0f, 0.0f, 1.0f, 1.0f, 1.0f}; + glGenVertexArrays(1, &m_vao); + glBindVertexArray(m_vao); + glGenBuffers(1, &m_vbo); + glBindBuffer(GL_ARRAY_BUFFER, m_vbo); + glBufferData(GL_ARRAY_BUFFER, sizeof(kVertices), kVertices, GL_STATIC_DRAW); + glEnableVertexAttribArray(0); + glVertexAttribPointer(0, 2, GL_FLOAT, GL_FALSE, 2 * sizeof(float), nullptr); + + m_textures.resize(kTextures); + glGenTextures(kTextures, m_textures.data()); + for (int t = 0; t < kTextures; ++t) { + const Rgba8 c = TextureColor(t); + const std::uint8_t texel[4] = {c.r, c.g, c.b, c.a}; + glBindTexture(GL_TEXTURE_2D, m_textures[t]); + glTexImage2D(GL_TEXTURE_2D, 0, GL_RGBA8, 1, 1, 0, GL_RGBA, GL_UNSIGNED_BYTE, texel); + glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_NEAREST); + glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, GL_NEAREST); + glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_WRAP_S, GL_CLAMP_TO_EDGE); + glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_WRAP_T, GL_CLAMP_TO_EDGE); + } + glBindTexture(GL_TEXTURE_2D, 0); + } + + void TearDown() override { + if (Ready()) { + glUseProgram(0); + glBindVertexArray(0); + glBindTexture(GL_TEXTURE_2D, 0); + BindDefaultFramebuffer(); + if (m_program != 0) glDeleteProgram(m_program); + if (!m_textures.empty()) glDeleteTextures(static_cast(m_textures.size()), m_textures.data()); + if (m_vbo != 0) glDeleteBuffers(1, &m_vbo); + if (m_vao != 0) glDeleteVertexArrays(1, &m_vao); + DestroyColorFbo(m_target); + } + ScenarioTest::TearDown(); + } + + ::testing::AssertionResult Prepare() { + std::string error; + m_program = CompileProgram(kVS, kFS, &error); + if (m_program == 0) return ::testing::AssertionFailure() << error; + m_indexLocation = glGetUniformLocation(m_program, "uIndex"); + m_target = MakeColorFbo(kSide, kSide); + if (m_target.fbo == 0) return ::testing::AssertionFailure() << "MakeColorFbo failed"; + glUseProgram(m_program); + glUniform1i(glGetUniformLocation(m_program, "uTex"), 0); + glActiveTexture(GL_TEXTURE0); + glBindVertexArray(m_vao); + BindFbo(m_target); + glDisable(GL_SCISSOR_TEST); + glDisable(GL_DEPTH_TEST); + glDisable(GL_BLEND); + return ::testing::AssertionSuccess(); + } + + // Draw i paints pixel (i % 64, i / 64) with texture i % kTextures. + void Draw(int i) { + glBindTexture(GL_TEXTURE_2D, m_textures[i % kTextures]); + glUniform1i(m_indexLocation, i); + glDrawArrays(GL_TRIANGLE_STRIP, 0, 4); + } + + // The burst: every pixel, one draw and one descriptor set each. + void DrawBurst() { + ClearTo(0.0f, 0.0f, 0.0f, 0.0f); + for (int i = 0; i < kDraws; ++i) Draw(i); + } + + static bool IsVulkan() { return Gl().BackendName() == std::string("DirectVulkan"); } + + GLuint m_vao = 0; + GLuint m_vbo = 0; + GLuint m_program = 0; + GLint m_indexLocation = -1; + std::vector m_textures; + ColorFbo m_target; + }; + + } // namespace + + // One frame, kDraws distinct sets. Every pixel lands, and the frame slot grows O(log N) pools + // for it, not N / 128. + TEST_F(DescriptorPoolGrowthScenario, AFrameOfDistinctSetsLandsWholeAndGrowsItsPoolsGeometrically) { + if (!Ready()) return; + ASSERT_TRUE(Prepare()); + + const Census before = ReadCensus(); + DrawBurst(); + const Image image = ReadPixels(kSide, kSide); + const Census after = ReadCensus(); + + EXPECT_TRUE(EveryPixelHoldsItsDrawsTexture(image, "one frame of distinct descriptor sets")); + EXPECT_EQ(FirstGLError(), 0u); + + RecordProperty("census_checked", (IsVulkan() && after.available) ? "yes" : "no"); + if (!IsVulkan() || !after.available) return; + // Falsifiability first: had the renderer's memos collapsed the draws onto shared sets, the + // slot would never have needed to grow and the bound below would pass on any policy. + ASSERT_GE(after.cachedSets, before.cachedSets + kDraws) + << "the frame did not allocate one descriptor set per draw, so it does not exercise growth. Before: " + << Describe(before) << "; after: " << Describe(after); + const std::uint64_t added = after.pools - before.pools; + RecordProperty("census_before", Describe(before)); + RecordProperty("census_after", Describe(after)); + EXPECT_LE(added, GeometricPoolBound(kDraws + 1)) + << "a frame needing " << kDraws << " descriptor sets added " << added << " pools; doubling the slot's " + << "largest pool needs at most " << GeometricPoolBound(kDraws + 1) << ". Before: " << Describe(before) + << "; after: " << Describe(after); + } + + // Burst, then quiet frames until the renderer hands the burst back, then the burst again while + // the quiet frames are still in flight. Nothing waits between frames except where noted, so the + // trim runs with other frame slots' work outstanding. + TEST_F(DescriptorPoolGrowthScenario, AGivenBackBurstLeavesNoStaleSetBehind) { + if (!Ready()) return; + ASSERT_TRUE(Prepare()); + const unsigned long trimFrames = EnvUnsigned("MOBILEGL_MAGMA_DESCRIPTOR_TRIM_FRAMES", 120); + const unsigned long framesInFlight = EnvUnsigned("MOBILEGL_MAGMA_FRAMESINFLIGHT", 3); + // The trim fires once trimFrames quiet frames have followed the burst, at the burst slot's next + // turn; past this many frames it is not coming. + const unsigned long quietFrameLimit = trimFrames + 2 * framesInFlight + 16; + + DrawBurst(); + Gl().EndFrame(); + const Census afterBurst = ReadCensus(); + + // Quiet frames: one draw each, the same pixel and texture the burst gave draw 0. + const bool observe = IsVulkan() && afterBurst.available && trimFrames != 0; + unsigned long quietFrames = 0; + Census afterQuiet = afterBurst; + for (; quietFrames < quietFrameLimit; ++quietFrames) { + Draw(0); + Gl().EndFrame(); + if (observe) { + afterQuiet = ReadCensus(); + if (afterQuiet.slotTrims > afterBurst.slotTrims) { + ++quietFrames; + break; + } + } + } + + // The burst again, straight away. The slot it lands in re-grows from its base pool. + DrawBurst(); + const Image image = ReadPixels(kSide, kSide); + const Census afterSecondBurst = ReadCensus(); + + EXPECT_TRUE(EveryPixelHoldsItsDrawsTexture(image, "the burst drawn after the first one was given back")); + EXPECT_EQ(FirstGLError(), 0u); + + RecordProperty("census_checked", observe ? "yes" : "no"); + if (!observe) return; + RecordProperty("census_after_burst", Describe(afterBurst)); + RecordProperty("quiet_frames", std::to_string(quietFrames)); + RecordProperty("census_after_quiet", Describe(afterQuiet)); + RecordProperty("census_after_second_burst", Describe(afterSecondBurst)); + ASSERT_GE(afterBurst.cachedSets, static_cast(kDraws)) + << "the burst did not allocate one descriptor set per draw: " << Describe(afterBurst); + EXPECT_GT(afterQuiet.slotTrims, afterBurst.slotTrims) + << quietFrames << " frames of one draw each followed a " << kDraws << "-set frame, but no frame slot was " + << "trimmed (MOBILEGL_MAGMA_DESCRIPTOR_TRIM_FRAMES=" << trimFrames << "). After the burst: " + << Describe(afterBurst) << "; after the quiet frames: " << Describe(afterQuiet); + EXPECT_LE(afterQuiet.cachedSets + kDraws / 2, afterBurst.cachedSets) + << "the trim did not give the burst's sets back. After the burst: " << Describe(afterBurst) + << "; after the quiet frames: " << Describe(afterQuiet); + EXPECT_LT(afterQuiet.pools, afterBurst.pools) + << "the trim did not give the burst's pools back. After the burst: " << Describe(afterBurst) + << "; after the quiet frames: " << Describe(afterQuiet); + EXPECT_GE(afterSecondBurst.cachedSets, afterQuiet.cachedSets + kDraws) + << "the second burst was not served by freshly allocated sets: " << Describe(afterSecondBurst); + } + +} // namespace MGITest diff --git a/MobileGL/MG_Test/SanityTest.cpp b/MobileGL/MG_Test/SanityTest.cpp index 7af54ec0..97a27320 100644 --- a/MobileGL/MG_Test/SanityTest.cpp +++ b/MobileGL/MG_Test/SanityTest.cpp @@ -3061,6 +3061,100 @@ TEST(DirectVulkanSanity, GraphicsSamplerFeedbackOnlyAliasesWritableOverlappingMi EXPECT_FALSE(UniformManager::SamplerOverlapsWritableImageSubresource(1, 3, 4, GL_WRITE_ONLY)); } +// Magma's per-frame descriptor pools (UniformManager::NextDescriptorPoolMaxSets). The old rule +// doubled the FIRST pool of the flavour, which is always the slot's oldest and smallest one, so +// every grown pool came out the same size: rd12's loading frame (15,016 sets) grew one frame slot +// to 59 update-after-bind pools of 256 sets. The policy must double the LARGEST pool of the +// flavour, up to the cap. +namespace { + using MobileGL::MG_Backend::DirectVulkan::UniformManager; + constexpr MobileGL::Uint32 kBaseSets = 64; // VulkanRenderer's kDescriptorSetsPerFrame + + // A frame slot as GrowFrameDescriptorPool leaves it after a frame needing `needed` sets of one + // flavour: the base pool (non-update-after-bind, never usable by the other flavour) plus the + // policy's next pool, added until the flavour's pools hold `needed` sets. + std::vector GrowSlotUntil(MobileGL::Uint64 needed, bool updateAfterBind) { + std::vector pools{{VK_NULL_HANDLE, kBaseSets, 0, false}}; + const auto flavourCapacity = [&]() { + MobileGL::Uint64 sum = 0; + for (const auto& pool : pools) { + if (pool.updateAfterBind == updateAfterBind) sum += pool.maxSets; + } + return sum; + }; + while (flavourCapacity() < needed && pools.size() < 100000) { + pools.push_back({VK_NULL_HANDLE, UniformManager::NextDescriptorPoolMaxSets(pools, updateAfterBind, kBaseSets), + 0, updateAfterBind}); + } + return pools; + } + + // Pools a geometric policy needs for n sets: 64, 128, ... up to the cap, then one per cap. + MobileGL::Uint64 GeometricPoolBound(MobileGL::Uint64 n) { + MobileGL::Uint64 pools = 0, capacity = 0, size = kBaseSets; + while (capacity < n) { + capacity += size; + ++pools; + size = std::min(size * 2, UniformManager::kMaxDescriptorPoolSets); + } + return pools; + } +} // namespace + +TEST(DirectVulkanSanity, DescriptorPoolGrowthIsGeometricInTheSetsAFrameNeeds) { + for (const bool updateAfterBind : {false, true}) { + for (const MobileGL::Uint64 needed : {65ull, 1000ull, 4096ull, 8128ull, 15016ull, 40000ull}) { + const auto pools = GrowSlotUntil(needed, updateAfterBind); + MobileGL::Uint64 flavourPools = 0, capacity = 0; + MobileGL::Uint32 previous = 0; + for (const auto& pool : pools) { + if (pool.updateAfterBind != updateAfterBind) continue; + ++flavourPools; + capacity += pool.maxSets; + EXPECT_LE(pool.maxSets, UniformManager::kMaxDescriptorPoolSets) << "one pool past the cap"; + EXPECT_GE(pool.maxSets, previous) << "a later pool is smaller than an earlier one"; + previous = pool.maxSets; + } + // O(log N) under the cap (8128 = 64 + ... + 4096 is where the cap is reached), then one + // pool per 4096 sets. The first-match rule needed ~N/128 here: 118 pools for 15,016. + EXPECT_LE(flavourPools, GeometricPoolBound(needed)) + << needed << " sets (updateAfterBind=" << updateAfterBind << ") took " << flavourPools << " pools"; + EXPECT_GE(capacity, needed); + // The slot does not over-reserve by more than the last pool it had to add. + EXPECT_LT(capacity - needed, static_cast(previous)); + } + } + // rd12's loading frame, the case that motivated this: 9 pools in the slot, not 59. + const auto rd12 = GrowSlotUntil(15016, true); + EXPECT_EQ(rd12.size(), 1u + 9u); +} + +TEST(DirectVulkanSanity, DescriptorPoolGrowthDoublesTheLargestPoolOfTheSameFlavour) { + using Bucket = UniformManager::DescriptorPoolBucket; + // The largest pool decides, wherever it sits in the list - a trim or a flavour switch can + // leave a small pool after a large one. + EXPECT_EQ(UniformManager::NextDescriptorPoolMaxSets( + {{VK_NULL_HANDLE, 64, 0, false}, {VK_NULL_HANDLE, 512, 0, false}, {VK_NULL_HANDLE, 128, 0, false}}, + false, kBaseSets), + 1024u); + // Flavours never size each other: the base pool is non-update-after-bind, so the first + // update-after-bind pool starts at the base size however large the other flavour grew. + const std::vector mixed{{VK_NULL_HANDLE, 64, 0, false}, + {VK_NULL_HANDLE, 2048, 0, false}, + {VK_NULL_HANDLE, 64, 0, true}, + {VK_NULL_HANDLE, 128, 0, true}}; + EXPECT_EQ(UniformManager::NextDescriptorPoolMaxSets(mixed, true, kBaseSets), 256u); + EXPECT_EQ(UniformManager::NextDescriptorPoolMaxSets(mixed, false, kBaseSets), 4096u); + EXPECT_EQ(UniformManager::NextDescriptorPoolMaxSets({{VK_NULL_HANDLE, 64, 0, false}}, true, kBaseSets), 64u); + // The cap holds, and the base size is a floor. + EXPECT_EQ(UniformManager::NextDescriptorPoolMaxSets({{VK_NULL_HANDLE, 4096, 0, false}}, false, kBaseSets), + UniformManager::kMaxDescriptorPoolSets); + EXPECT_EQ(UniformManager::NextDescriptorPoolMaxSets({{VK_NULL_HANDLE, 3000, 0, false}}, false, kBaseSets), + UniformManager::kMaxDescriptorPoolSets); + EXPECT_EQ(UniformManager::NextDescriptorPoolMaxSets({{VK_NULL_HANDLE, 16, 0, false}}, false, kBaseSets), 64u); + EXPECT_EQ(UniformManager::NextDescriptorPoolMaxSets({}, false, kBaseSets), 64u); +} + // GL_MAX_COMBINED_*_UNIFORM_COMPONENTS is components + blocks * (blockSize / 4). The product was // formed in signed 32-bit, and a Vulkan host that reports a large VkPhysicalDeviceLimits:: // maxUniformBufferRange (a Mali driver answers 0xFFFFFFFF, which the loader saturates to From c14283ee2dacb8d462f1c89ddd51ee5e39f26bbf Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Tue, 29 Sep 2026 10:58:33 -0400 Subject: [PATCH 9/9] [Docs] (Magma): the trim threshold's comment states the bound its argument proves - a growth adds at most twice what the slot held, so the slot stays over a third full --- MobileGL/MG_Backend/DirectVulkan/Renderer/UniformManager.h | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/MobileGL/MG_Backend/DirectVulkan/Renderer/UniformManager.h b/MobileGL/MG_Backend/DirectVulkan/Renderer/UniformManager.h index 99dc160a..0b8f5469 100644 --- a/MobileGL/MG_Backend/DirectVulkan/Renderer/UniformManager.h +++ b/MobileGL/MG_Backend/DirectVulkan/Renderer/UniformManager.h @@ -162,9 +162,10 @@ namespace MobileGL::MG_Backend::DirectVulkan { // count times kDescriptorTrimHeadroom still fits its capacity; a slot that has been quiet for // MOBILEGL_MAGMA_DESCRIPTOR_TRIM_FRAMES consecutive epochs, and holds at least // kDescriptorTrimMinCapacity sets, is trimmed. The headroom is what keeps the trim from - // thrashing: geometric growth leaves a re-grown slot at least a third full (its pools sum to - // under twice the largest, and the largest is under twice the need that added it), so the - // epoch that grew a slot is never quiet for it. The floor keeps a slot sized for ordinary + // thrashing: geometric growth leaves a re-grown slot at least a third full (the pool a growth + // adds is at most twice the largest before it, so at most twice everything the slot held, and + // the epoch that grew it needed more than that), so the epoch that grew a slot is never quiet + // for it. The floor keeps a slot sized for ordinary // frames from being trimmed at all - the trim exists to give back a burst. static constexpr Uint32 kDescriptorTrimHeadroom = 4; static constexpr Uint32 kDescriptorTrimMinCapacity = 1024;