swung0x48/MobileGL
1
0
Fork 0
mirror of https://github.com/MobileGL-Dev/MobileGL synced 2026-11-07 01:53:24 +09:00
Code Issues Packages Projects Releases Wiki Activity

[MG_Impl] guard the monolith mip generate arm against immutable storage and views

Browse Source 
EnsureGeneratedMipmapStorageAllocated branched on the transport. The split arm
planned the generation window from BASE_LEVEL/MAX_LEVEL and refused to touch
immutable storage (70c8230b); the monolith arm rebuilt the chain from the bound
object's level-0 extent and then truncated it. Through a view that is
destructive in a way the split arm had already been fixed for: a view's level-0
extent is one slice of the owner's layers, so a two-layer view of a four-layer
array resized every owner level the generation touched to two layers - the
storage behind the owner's other layers was gone - and redefined the owner's
levels out of the view's window as well.

Both arms now take one path. An immutable texture or a view returns before any
allocation: it already owns every level generation can write. A mutable texture
gets levels from the plan's Base..End window and nothing else moves - the chain
is not truncated, because neither MAX_LEVEL nor a generate is a truncation (a
level-0 respecification drops the stale tail itself, in
DiscardMipmapChainOnBaseRespecification). MipmapGenerationPlan.h is no longer
behind MOBILEGL_BUILD_DISAGGREGATED for the same reason: the window is a
property of the GL call, not of how the backend is reached, and the plan the
wire carrier publishes (MGPMipPlan::LevelCount, end-exclusive) is this one.

The window rule was asserted only by the split-only F1WireScenario, whose
fixture skips under every other transport - so the monolith arm, which is what
a non-disaggregated library runs for every application, had no coverage at all.
TextureViewScenario.GenerateMipmapThroughAViewLeavesTheOwnerShapeAlone runs in
an integration-gpu lane under both transports and pins the owner's layer counts,
its own level extents, and the pixels of the levels and layers outside the
view's window. It is DirectGLES-only, and the skip names why: Magma reallocates
the chain through the bound object in its own generate path
(DirectVulkan/Renderer/VulkanRenderer.cpp's EnsureGenerateMipmapStorageAllocated),
and a CPU sub-image into the layers of mip 2 or 3 does not read back through
Magma at all - a run of the same scenario with the generate removed fails
identically - so the per-level colours this case is built on are unobservable on
that backend rather than merely wrong.
This commit is contained in:
swung0x48
2026-09-20 19:17:17 -04:00
parent f1d3fa8f1a
commit ed919f0d64
3 changed files with 164 additions and 56 deletions
Download Patch File Download Diff File Expand all files Collapse all files
MobileGL/MG_Impl/GLImpl/Texture/GL_Texture.cpp
+25 -54
View File
@@ -508,65 +508,36 @@ namespace MobileGL::MG_Impl::GLImpl {
return size;
}
// glGenerateMipmap defines levels BASE_LEVEL+1 up to the level the base image's extent and
// MAX_LEVEL admit, and leaves every other level exactly as it was (GL 4.6 core 8.17). Both
// transports take this one path: the window, and the refusal to redefine the levels around
// it, are properties of the GL call rather than of how the backend is reached.
Bool EnsureGeneratedMipmapStorageAllocated(
MG_State::GLState::TextureObjectMipmap& texture,
TextureUploadTarget uploadTarget) {
#if MOBILEGL_BUILD_DISAGGREGATED
if (MG_Config::Transport != MG_Config::TransportMode::Monolith) {
const auto plan = ComputeMipmapGenerationRange(texture, uploadTarget);
if (plan.End <= plan.Base) return false;
// Immutable textures and views already own every generated level.
// Allocating through a view would redefine its owner's complete
// level, destroying layers and levels outside the view window.
if (texture.IsImmutable()) return true;
const IntVec3 baseSize = texture.GetMipmapTexelSize(uploadTarget, plan.Base);
const SizeT baseBytes = texture.GetMipmapByteSize(uploadTarget, plan.Base);
const SizeT texels = static_cast<SizeT>(baseSize.x()) * baseSize.y() * baseSize.z();
if (!baseBytes || !texels || baseBytes % texels) return false;
const SizeT pixelBytes = baseBytes / texels;
const Int axes = MipShrinkingAxisCount(texture.GetTarget());
for (Uint level = plan.Base + 1; level < plan.End; ++level) {
const auto size = ComputeMipmapTexelSize(baseSize, level - plan.Base, axes);
const SizeT bytes = pixelBytes * static_cast<SizeT>(size.x()) * size.y() * size.z();
texture.AllocateStorage(uploadTarget, level, {size, bytes});
texture.MarkStorageDirty(uploadTarget, level, false);
}
// Levels before BASE_LEVEL and after the generated end remain
// valid images. In particular, MAX_LEVEL is not a truncate call.
texture.BumpContentVersion();
return true;
}
#endif
const Uint existingLevelCount = texture.GetMipmapLevelCount();
if (existingLevelCount == 0) {
return false;
}
const IntVec3 baseTexelSize = texture.GetMipmapTexelSize(uploadTarget, 0);
const SizeT baseByteSize = texture.GetMipmapByteSize(uploadTarget, 0);
const SizeT baseTexelCount = static_cast<SizeT>(baseTexelSize.x()) *
static_cast<SizeT>(baseTexelSize.y()) *
static_cast<SizeT>(baseTexelSize.z());
if (baseTexelSize.x() <= 0 || baseTexelSize.y() <= 0 || baseTexelSize.z() <= 0 ||
baseByteSize == 0 || baseTexelCount == 0 || (baseByteSize % baseTexelCount) != 0) {
return false;
}
const SizeT bytesPerTexel = baseByteSize / baseTexelCount;
const Int shrinkingAxes = MipShrinkingAxisCount(texture.GetTarget());
const Uint requiredLevelCount = ComputeFullMipmapLevelCount(baseTexelSize, shrinkingAxes);
for (Uint level = 1; level < requiredLevelCount; ++level) {
const IntVec3 levelTexelSize = ComputeMipmapTexelSize(baseTexelSize, level, shrinkingAxes);
const SizeT levelByteSize = bytesPerTexel * static_cast<SizeT>(levelTexelSize.x()) *
static_cast<SizeT>(levelTexelSize.y()) *
static_cast<SizeT>(levelTexelSize.z());
texture.AllocateStorage(uploadTarget, level, {levelTexelSize, levelByteSize});
const auto plan = ComputeMipmapGenerationRange(texture, uploadTarget);
if (plan.End <= plan.Base) return false;
// Immutable textures and views already own every level generation can write.
// Allocating through a view would redefine its owner's levels at the VIEW's base
// extent - one slice of the owner's layers - destroying the storage of every layer and
// level outside the view window.
if (texture.IsImmutable()) return true;
const IntVec3 baseSize = texture.GetMipmapTexelSize(uploadTarget, plan.Base);
const SizeT baseBytes = texture.GetMipmapByteSize(uploadTarget, plan.Base);
const SizeT texels = static_cast<SizeT>(baseSize.x()) * baseSize.y() * baseSize.z();
if (!baseBytes || !texels || baseBytes % texels) return false;
const SizeT pixelBytes = baseBytes / texels;
const Int axes = MipShrinkingAxisCount(texture.GetTarget());
for (Uint level = plan.Base + 1; level < plan.End; ++level) {
const auto size = ComputeMipmapTexelSize(baseSize, level - plan.Base, axes);
const SizeT bytes = pixelBytes * static_cast<SizeT>(size.x()) * size.y() * size.z();
texture.AllocateStorage(uploadTarget, level, {size, bytes});
texture.MarkStorageDirty(uploadTarget, level, false);
}
// glGenerateMipmap defines exactly levels 0..requiredLevelCount-1. AllocateStorage only
// grows, so a previously longer chain (a bigger base image before respecification) would
// otherwise keep a tail of stale levels here and read as incomplete.
texture.TruncateMipmapLevels(uploadTarget, requiredLevelCount);
// Levels before BASE_LEVEL and after the generated end remain valid images and are left
// alone: neither MAX_LEVEL nor a generate truncates the chain. AllocateStorage only
// grows, so a longer chain a level-0 respecification left behind is dropped there
// (DiscardMipmapChainOnBaseRespecification) rather than here.
// Mip generation grows/regenerates the level set on the GPU without marking any CPU
// level dirty (MarkStorageDirty(...,false) above). Bump the content version so the
// backend re-syncs: a cached sampled VkImageView built for the pre-generate level
MobileGL/MG_Impl/GLImpl/Texture/MipmapGenerationPlan.h
+3 -2
View File
@@ -8,7 +8,9 @@
#pragma once
#include <Includes.h>
#if MOBILEGL_BUILD_DISAGGREGATED
// The plan is the GL call's own window, not a transport's, so both arms of
// EnsureGeneratedMipmapStorageAllocated compute it here and the wire carrier
// (MGPMipPlan::LevelCount) publishes the same end-exclusive number.
#include <MG_State/GLState/TextureState/TextureObject.h>
#include <algorithm>
@@ -42,4 +44,3 @@ namespace MobileGL::MG_Impl::GLImpl {
return result;
}
}
#endif
MobileGL/MG_IntegrationTest/Scenarios/TextureViewScenario.cpp
+136
View File
@@ -904,5 +904,141 @@ void main() {
"the view must still reach its storage after the original's name was deleted");
DestroyColorFbo(destination);
}
// ------------------------------------------------------------------------------------
// glGenerateMipmap THROUGH a view: the view's window is a write target, never a shape.
//
// THE DEFECT this pins. Generating through a view used to (re)allocate the storage
// OWNER from the VIEW's own level-zero extent. A view's level-zero extent is one slice
// of the owner's layers, so a two-layer view of a four-layer array resized every owner
// level it touched to two layers - the storage behind the owner's other layers was gone -
// and redefined the owner's other levels out of the view's window. GL 4.6 core 8.18 makes
// a view a window INTO storage it does not own: level sizes and layer counts belong to
// the owner, and generation through the view is a GPU-side write into that window.
//
// IT IS ASSERTED HERE, in the transport-agnostic view scenario, because the arm that
// reallocated is the MONOLITH one - what every application of a non-disaggregated library
// runs, and what the split-only F1WireScenario never reaches (its fixture skips under any
// other transport, which is why its copy of this rule runs nowhere).
//
// DirectGLES ONLY, for two facts about the other backend and neither of them this arm's:
// Magma reallocates the chain through the bound object in its OWN generate path
// (DirectVulkan/Renderer/VulkanRenderer.cpp's EnsureGenerateMipmapStorageAllocated), which
// through a view means its owner's levels - the same defect on the far side of the door this
// fix closes; and a CPU sub-image into layer 1 or 2 of mip 2 or 3 does not read back through
// Magma at all - a run with the generate removed fails identically - so the per-level colours
// this case is built on are unobservable there rather than wrong.
// ------------------------------------------------------------------------------------
TEST_F(TextureViewScenario, GenerateMipmapThroughAViewLeavesTheOwnerShapeAlone) {
if (!Ready() || IsSkipped()) return;
if (Gl().BackendName() != "DirectGLES") {
GTEST_SKIP() << "the DirectGLES monolith arm is the one that reallocated through the "
"view; Magma does its own reallocation there and reads no such sub-image";
}
constexpr int kLevels = 5;
constexpr int kBase = 16;
constexpr int kLayers = 4;
constexpr int kViewMinLevel = 1;
constexpr int kViewNumLevels = 3;
constexpr int kViewMinLayer = 1;
constexpr int kViewNumLayers = 2;
// One colour per level, so a level that was redefined from the view's base extent reads
// as another level's colour rather than as a plausible one.
const Rgba8 levelColour[kLevels] = {{255, 0, 0, 255}, {0, 0, 255, 255}, {255, 255, 0, 255},
{255, 0, 255, 255}, {0, 255, 255, 255}};
const GLuint storage = MakeTexture();
glBindTexture(GL_TEXTURE_2D_ARRAY, storage);
glTexStorage3D(GL_TEXTURE_2D_ARRAY, kLevels, GL_RGBA8, kBase, kBase, kLayers);
glPixelStorei(GL_UNPACK_ALIGNMENT, 1);
for (int level = 0; level < kLevels; ++level) {
const int side = kBase >> level;
std::vector<std::uint8_t> texels(static_cast<std::size_t>(side) * side * kLayers * 4);
for (std::size_t at = 0; at < texels.size(); at += 4) {
texels[at + 0] = levelColour[level].r;
texels[at + 1] = levelColour[level].g;
texels[at + 2] = levelColour[level].b;
texels[at + 3] = levelColour[level].a;
}
glTexSubImage3D(GL_TEXTURE_2D_ARRAY, level, 0, 0, 0, side, side, kLayers, GL_RGBA,
GL_UNSIGNED_BYTE, texels.data());
}
ASSERT_EQ(glGetError(), static_cast<GLenum>(GL_NO_ERROR)) << "seeding the owner raised an error";
// Levels 0..2 of the view alias the owner's 1..3. Its nonzero BASE_LEVEL makes the
// owner's level 2 the source of the generation, and MAX_LEVEL 2 ends it at the owner's
// level 3 - both expressed in the VIEW's coordinates.
const GLuint view = MakeTexture();
glTextureView(view, GL_TEXTURE_2D_ARRAY, storage, GL_RGBA8, kViewMinLevel, kViewNumLevels,
kViewMinLayer, kViewNumLayers);
ASSERT_EQ(glGetError(), static_cast<GLenum>(GL_NO_ERROR));
glTextureParameteri(view, GL_TEXTURE_BASE_LEVEL, 1);
glTextureParameteri(view, GL_TEXTURE_MAX_LEVEL, 2);
// The generation's source is the view's base level, the owner's level 2. Writing it in
// GREEN on the GPU gives the source and the level generated from it the same colour, so
// losing either one is visible below.
const GLuint fbo = MakeFbo();
glBindFramebuffer(GL_FRAMEBUFFER, fbo);
glDisable(GL_DEPTH_TEST);
glDisable(GL_STENCIL_TEST);
glDisable(GL_SCISSOR_TEST);
glViewport(0, 0, kBase >> 2, kBase >> 2);
for (int layer = kViewMinLayer; layer < kViewMinLayer + kViewNumLayers; ++layer) {
glFramebufferTextureLayer(GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0, storage, 2, layer);
ASSERT_EQ(glCheckFramebufferStatus(GL_FRAMEBUFFER), static_cast<GLenum>(GL_FRAMEBUFFER_COMPLETE))
<< "the source level's layer " << layer << " is not attachable";
glClearColor(0.0f, 1.0f, 0.0f, 1.0f);
glClear(GL_COLOR_BUFFER_BIT);
}
ASSERT_EQ(glGetError(), static_cast<GLenum>(GL_NO_ERROR)) << "clearing the source level raised an error";
glGenerateTextureMipmap(view);
ASSERT_EQ(glGetError(), static_cast<GLenum>(GL_NO_ERROR))
<< "glGenerateTextureMipmap on a view raised an error";
// The view addresses two layers of the owner; the owner keeps its own four.
GLint viewDepth = 0;
glBindTexture(GL_TEXTURE_2D_ARRAY, view);
glGetTexLevelParameteriv(GL_TEXTURE_2D_ARRAY, 0, GL_TEXTURE_DEPTH, &viewDepth);
glBindTexture(GL_TEXTURE_2D_ARRAY, storage);
EXPECT_EQ(viewDepth, kViewNumLayers);
for (int level = 0; level < kLevels; ++level) {
const int side = kBase >> level;
GLint width = 0, height = 0, depth = 0;
glGetTexLevelParameteriv(GL_TEXTURE_2D_ARRAY, level, GL_TEXTURE_WIDTH, &width);
glGetTexLevelParameteriv(GL_TEXTURE_2D_ARRAY, level, GL_TEXTURE_HEIGHT, &height);
glGetTexLevelParameteriv(GL_TEXTURE_2D_ARRAY, level, GL_TEXTURE_DEPTH, &depth);
EXPECT_EQ(width, side) << "the owner's mip " << level << " must keep its own width";
EXPECT_EQ(height, side) << "the owner's mip " << level << " must keep its own height";
ASSERT_EQ(depth, kLayers)
<< "generation through a " << kViewNumLayers << "-layer view must not resize its owner's mip "
<< level;
for (int layer = 0; layer < kLayers; ++layer) {
// Per-layer FBO readback rather than glGetTexImage: the question is what the
// GPU holds, and a layer outside the view's window proves it by still holding
// its own pixels.
glBindFramebuffer(GL_FRAMEBUFFER, fbo);
glFramebufferTextureLayer(GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0, storage, level, layer);
glReadBuffer(GL_COLOR_ATTACHMENT0);
ASSERT_EQ(glCheckFramebufferStatus(GL_FRAMEBUFFER), static_cast<GLenum>(GL_FRAMEBUFFER_COMPLETE))
<< "the owner's mip " << level << ", layer " << layer << " lost its storage";
const Image image = ReadPixels(side, side);
// Generation covers levels base+1..max of the view, i.e. the owner's levels 2
// and 3, over the two layers the view names. Everything else keeps its own
// image: the owner's level 4 is past the view's max level, and its layers 0 and
// 3 are outside the view's layer window.
const bool inWindow =
level >= 2 && level <= 3 && layer >= kViewMinLayer && layer < kViewMinLayer + kViewNumLayers;
const Rgba8 expected = inWindow ? Rgba8{0, 255, 0, 255} : levelColour[level];
const std::string what =
"the owner's mip " + std::to_string(level) + ", layer " + std::to_string(layer) +
(inWindow ? ", inside the view's window" : ", outside the view's window");
ExpectRegion(image, 0, side - 1, 0, side - 1, expected, 2, what.c_str());
}
}
ASSERT_EQ(glGetError(), static_cast<GLenum>(GL_NO_ERROR));
}
} // namespace
} // namespace MGITest