swung0x48/MobileGL
1
0
Fork 0
mirror of https://github.com/MobileGL-Dev/MobileGL synced 2026-09-09 04:38:30 +09:00
Code Issues Packages Projects Releases Wiki Activity

[Fix] (MG_State, MG_Impl/GLImpl): allocating a mipmap level no longer truncates the chain above it - AllocateLevel now only grows and the callers that genuinely redefine the whole level set (glTexStorage*, mip regeneration, multisample storage, level-0 respecification) drop the tail explicitly

Browse Source
This commit is contained in:
swung0x48
2026-07-20 21:02:54 -04:00
parent 3b175fb88a
commit e724e88eec
9 changed files with 188 additions and 6 deletions
Download Patch File Download Diff File Expand all files Collapse all files
MobileGL/MG_Impl/GLImpl/Texture/GL_Texture.cpp
+51
View File
@@ -350,6 +350,10 @@ namespace MobileGL::MG_Impl::GLImpl {
texture.AllocateStorage(uploadTarget, level, {levelTexelSize, levelByteSize});
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);
// 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
@@ -456,9 +460,46 @@ namespace MobileGL::MG_Impl::GLImpl {
textureObject->SetSamples(samples);
textureObject->SetFixedSampleLocations(fixedsamplelocations == GL_TRUE);
textureMipmapObject->AllocateStorage(textureUploadTarget, 0, {{width, height, depth}, 0});
// Multisample textures are single-level by definition, so a name that previously held a
// mip chain must not keep its tail now that AllocateStorage only grows.
textureMipmapObject->TruncateMipmapLevels(textureUploadTarget, 1);
textureMipmapObject->MarkStorageDirty(textureUploadTarget, 0, false);
}
// Redefining level 0 of a texture that already had a base image drops the rest of the chain,
// which is exactly what AllocateLevel used to do implicitly for every level. Keeping that
// behaviour for level 0 - and only for level 0 - is what makes the grow-only change safe:
// any level-0 respecification leaves the chain in precisely the state it would have had
// before, while an upload to level N no longer destroys the levels beneath it.
//
// Why it has to be *every* level-0 respecification and not just a size change: Minecraft's
// Mipmap Levels setting rebuilds the block atlas at the SAME dimensions with a different
// level count. A size-only test would leave the old tail in place, and because Mojang
// terminates its chains with a 0x0 level the result is the zero-then-nonzero pattern that
// IsComplete() rejects (TextureObject.cpp) - whereupon DirectGLES skips syncing the texture
// entirely (Managers.cpp) and the atlas samples black.
//
// The "already has a base image" test is what lets the fix work at all: a level that was
// never written reads back as {0,0,0}, so building a chain top-down - upload level N first,
// then level 0 - must not discard the levels just uploaded. That ordering is what
// KHR-GL33.texture_repeat_mode does.
// Scoped to the respecified upload target only, which is what AllocateLevel already did.
// Cube maps keep six independent chains while reporting a single level count (face +X), so
// respecifying a face other than +X can leave the count longer than that face - but that
// asymmetry predates this change and widening the truncation to all six faces would destroy
// mip data for faces the application never touched. Left alone deliberately.
void DiscardMipmapChainOnBaseRespecification(MG_State::GLState::TextureObjectMipmap* texture,
TextureUploadTarget uploadTarget, Uint level) {
if (level != 0) return;
const IntVec3 existingBaseSize = texture->GetMipmapTexelSize(uploadTarget, 0);
const Bool hasExistingBaseImage =
existingBaseSize.x() > 0 && existingBaseSize.y() > 0 && existingBaseSize.z() > 0;
if (!hasExistingBaseImage) return;
texture->TruncateMipmapLevels(uploadTarget, 1);
}
// Compressed texture upload is not implemented yet. GL_NUM_COMPRESSED_TEXTURE_FORMATS
// reports 0, so every compressed internalformat is by definition unsupported and
// GL_INVALID_ENUM is the specified error - unlike THROW_UNIMPL_EXCEPTION, which unwinds
@@ -1740,6 +1781,7 @@ namespace MobileGL::MG_Impl::GLImpl {
if (isProxy) {
MGLOG_D("%s: isProxy = true, not allocating", __func__);
} else {
DiscardMipmapChainOnBaseRespecification(textureMipmapObject, textureUploadTarget, level);
textureMipmapObject->AllocateStorage(textureUploadTarget, level, {{width, height, depth}, internalBytes});
}
@@ -1867,6 +1909,7 @@ namespace MobileGL::MG_Impl::GLImpl {
MGLOG_D("%s: isProxy = true, not allocating", __func__);
} else {
MGLOG_D("%s: Allocating %d bytes at mip %d", __func__, internalBytes, level);
DiscardMipmapChainOnBaseRespecification(textureMipmapObject, textureUploadTarget, level);
textureMipmapObject->AllocateStorage(textureUploadTarget, level,
{{width, height, 1}, internalBytes});
}
@@ -1955,6 +1998,7 @@ namespace MobileGL::MG_Impl::GLImpl {
"Texture object here should always be an object with mipmap");
auto textureMipmapObject = static_cast<MG_State::GLState::TextureObjectMipmap*>(textureObject.get());
if (!isProxy) {
DiscardMipmapChainOnBaseRespecification(textureMipmapObject, textureUploadTarget, level);
textureMipmapObject->AllocateStorage(textureUploadTarget, level, {{width, 1, 1}, internalBytes});
}
@@ -3195,6 +3239,9 @@ namespace MobileGL::MG_Impl::GLImpl {
textureMipmapObject->AllocateStorage(textureUploadTarget, level, {{levelWidth, 1, 1}, byteSize});
textureMipmapObject->MarkStorageDirty(textureUploadTarget, level, false);
}
// Immutable storage defines exactly `levels` levels; AllocateStorage only grows, so a
// longer pre-existing chain has to be dropped explicitly.
textureMipmapObject->TruncateMipmapLevels(textureUploadTarget, static_cast<Uint>(levels));
textureObject->SetImmutableLevels(static_cast<Uint>(levels));
}
@@ -3247,6 +3294,8 @@ namespace MobileGL::MG_Impl::GLImpl {
textureMipmapObject->AllocateStorage(textureUploadTarget, level, {{levelWidth, levelHeight, 1}, byteSize});
textureMipmapObject->MarkStorageDirty(textureUploadTarget, level, false);
}
// See TextureStorage1D.
textureMipmapObject->TruncateMipmapLevels(textureUploadTarget, static_cast<Uint>(levels));
textureObject->SetImmutableLevels(static_cast<Uint>(levels));
}
@@ -3299,6 +3348,8 @@ namespace MobileGL::MG_Impl::GLImpl {
{{levelWidth, levelHeight, levelDepth}, byteSize});
textureMipmapObject->MarkStorageDirty(textureUploadTarget, level, false);
}
// See TextureStorage1D.
textureMipmapObject->TruncateMipmapLevels(textureUploadTarget, static_cast<Uint>(levels));
textureObject->SetImmutableLevels(static_cast<Uint>(levels));
}
MobileGL/MG_State/GLState/TextureState/MipmapStorage.cpp
+22 -6
View File
@@ -16,17 +16,32 @@ namespace MobileGL {
}
void MipmapStorage::AllocateLevel(Uint level, MipmapInput input) {
m_data.reserve(std::bit_ceil(level + 1));
m_data.resize(level + 1);
m_texelSizes.reserve(std::bit_ceil(level + 1));
m_texelSizes.resize(level + 1);
m_texelSizes[level] = input.texelSize;
m_isDirty.resize(level + 1, false);
// Grow only. GL respecifies exactly the level it is handed, so allocating level 0
// must not disturb the levels above it - but resize() shrinks as readily as it
// grows, so this used to truncate the whole chain to a single level. Callers that
// genuinely redefine the complete level set say so with TruncateToLevelCount.
const SizeT requiredLevelCount = static_cast<SizeT>(level) + 1;
if (m_data.size() < requiredLevelCount) {
m_data.reserve(std::bit_ceil(requiredLevelCount));
m_data.resize(requiredLevelCount);
m_texelSizes.reserve(std::bit_ceil(requiredLevelCount));
m_texelSizes.resize(requiredLevelCount);
m_isDirty.resize(requiredLevelCount, false);
}
m_texelSizes[level] = input.texelSize;
auto& data = m_data[level];
data.resize(input.byteSize, 0);
}
void MipmapStorage::TruncateToLevelCount(SizeT levelCount) {
if (levelCount >= m_data.size()) return;
m_data.resize(levelCount);
m_texelSizes.resize(levelCount);
m_isDirty.resize(levelCount);
}
void MipmapStorage::UpdateSubData(Uint level, DataPtr input) {
auto& targetData = m_data;
MOBILEGL_ASSERT(level < targetData.size(), "UpdateSubData: level out of range");
@@ -55,6 +70,7 @@ namespace MobileGL {
}
SizeT MipmapStorage::GetByteSize(Uint level) const {
if (level >= m_data.size()) return 0;
return m_data[level].size();
}
MobileGL/MG_State/GLState/TextureState/MipmapStorage.h
+4
View File
@@ -19,6 +19,10 @@ namespace MobileGL {
public:
SizeT GetLevelCount() const;
void AllocateLevel(Uint level, MipmapInput input);
// Discard every level at or above levelCount. AllocateLevel never shrinks, so this
// is the only way a chain gets shorter - use it where the caller defines the whole
// level set (glTexStorage*, mip regeneration, atlas respecification).
void TruncateToLevelCount(SizeT levelCount);
void UpdateSubData(Uint level, DataPtr input);
void* MapData(Uint level);
IntVec3 GetTexelSize(Uint level) const;
MobileGL/MG_State/GLState/TextureState/MipmapUploadTargetArray.h
+8
View File
@@ -29,6 +29,14 @@ namespace MobileGL {
m_storage[targetIndex].AllocateLevel(level, input);
}
// Per-target, like AllocateLevel: cube-map faces are respecified independently, so
// truncating one face must not disturb the others.
void TruncateToLevelCount(Uint targetIndex, SizeT levelCount) {
MOBILEGL_ASSERT(targetIndex < TargetCount, "TruncateToLevelCount: target invalid");
m_storage[targetIndex].TruncateToLevelCount(levelCount);
}
void UpdateSubData(Uint targetIndex, Uint level, DataPtr input) {
MOBILEGL_ASSERT(targetIndex < TargetCount, "UpdateSubData: target invalid");
m_storage[targetIndex].UpdateSubData(level, input);
MobileGL/MG_State/GLState/TextureState/TextureObject.cpp
+4
View File
@@ -271,6 +271,10 @@ namespace MobileGL {
m_textureStorage.AllocateLevel(GetIndexOfTextureUploadTarget(uploadTarget), mipmapLevel, input);
}
void TextureObjectWithOneMipmap::TruncateMipmapLevels(TextureUploadTarget uploadTarget, Uint levelCount) {
m_textureStorage.TruncateToLevelCount(GetIndexOfTextureUploadTarget(uploadTarget), levelCount);
}
void TextureObjectWithOneMipmap::UpdateMipmapSubData(TextureUploadTarget uploadTarget, Uint mipmapLevel,
DataPtr input) {
m_textureStorage.UpdateSubData(GetIndexOfTextureUploadTarget(uploadTarget), mipmapLevel, input);
MobileGL/MG_State/GLState/TextureState/TextureObject.h
+5
View File
@@ -134,6 +134,10 @@ namespace MobileGL::MG_State::GLState {
virtual const IntVec3 GetMipmapTexelSize(TextureUploadTarget target, Uint mipmapLevel) const = 0;
virtual const SizeT GetMipmapByteSize(TextureUploadTarget target, Uint mipmapLevel) const = 0;
virtual void AllocateStorage(TextureUploadTarget uploadTarget, Uint mipmapLevel, MipmapInput input) = 0;
// AllocateStorage only ever grows the chain. Callers that define the complete level set -
// glTexStorage*, mip regeneration, or a level-0 respecification at a new size - drop the
// leftovers explicitly, so a stale tail can never make the texture silently incomplete.
virtual void TruncateMipmapLevels(TextureUploadTarget uploadTarget, Uint levelCount) = 0;
virtual void UpdateMipmapSubData(TextureUploadTarget uploadTarget, Uint mipmapLevel, DataPtr input) = 0;
virtual void* MapMipmapData(TextureUploadTarget uploadTarget, Uint mipmapLevel) = 0;
virtual void MarkStorageDirty(TextureUploadTarget uploadTarget, Uint mipmapLevel, Bool dirty = true) = 0;
@@ -175,6 +179,7 @@ namespace MobileGL::MG_State::GLState {
const IntVec3 GetMipmapTexelSize(TextureUploadTarget target, Uint mipmapLevel) const override;
const SizeT GetMipmapByteSize(TextureUploadTarget target, Uint mipmapLevel) const override;
void AllocateStorage(TextureUploadTarget uploadTarget, Uint mipmapLevel, MipmapInput input) override;
void TruncateMipmapLevels(TextureUploadTarget uploadTarget, Uint levelCount) override;
void UpdateMipmapSubData(TextureUploadTarget uploadTarget, Uint mipmapLevel, DataPtr input) override;
void* MapMipmapData(TextureUploadTarget uploadTarget, Uint mipmapLevel) override;
void MarkStorageDirty(TextureUploadTarget uploadTarget, Uint mipmapLevel, Bool dirty) override;
MobileGL/MG_State/GLState/TextureState/TextureObject2DCube.cpp
+4
View File
@@ -31,6 +31,10 @@ namespace MobileGL {
m_textureStorage.AllocateLevel(GetIndexOfTextureUploadTarget(uploadTarget), mipmapLevel, input);
}
void TextureObject2DCube::TruncateMipmapLevels(TextureUploadTarget uploadTarget, Uint levelCount) {
m_textureStorage.TruncateToLevelCount(GetIndexOfTextureUploadTarget(uploadTarget), levelCount);
}
void TextureObject2DCube::UpdateMipmapSubData(TextureUploadTarget uploadTarget, Uint mipmapLevel,
DataPtr input) {
m_textureStorage.UpdateSubData(GetIndexOfTextureUploadTarget(uploadTarget), mipmapLevel, input);
MobileGL/MG_State/GLState/TextureState/TextureObject2DCube.h
+1
View File
@@ -22,6 +22,7 @@ namespace MobileGL {
const IntVec3 GetMipmapTexelSize(TextureUploadTarget target, Uint mipmapLevel) const override;
const SizeT GetMipmapByteSize(TextureUploadTarget target, Uint mipmapLevel) const override;
void AllocateStorage(TextureUploadTarget uploadTarget, Uint mipmapLevel, MipmapInput input) override;
void TruncateMipmapLevels(TextureUploadTarget uploadTarget, Uint levelCount) override;
void UpdateMipmapSubData(TextureUploadTarget uploadTarget, Uint mipmapLevel, DataPtr input) override;
void* MapMipmapData(TextureUploadTarget uploadTarget, Uint mipmapLevel) override;
void MarkStorageDirty(TextureUploadTarget uploadTarget, Uint mipmapLevel, bool dirty) override;
MobileGL/MG_Test/Texture/TextureTest.cpp
+89
View File
@@ -1427,6 +1427,95 @@ TEST_F(TextureTest, TextureStorage1DAndSubImageModifyNamedObjectOnly) {
EXPECT_EQ(MG_Impl::GLImpl::GetError(), GL_NO_ERROR);
}
// Building a mip chain top-down - upload level N, then level 0 - must not destroy the levels
// already uploaded. AllocateLevel used to resize() the storage down to level+1 on every call, so
// the level-0 upload truncated the chain to a single level; the higher level then read back as
// {0,0,0}, IsComplete() rejected the zero-then-nonzero pattern, and DirectGLES answered that by
// skipping the texture's sync entirely. This is the shape KHR-GL33.texture_repeat_mode uses, and
// it accounted for 108 CTS failures in every GL version.
TEST_F(TextureTest, TexImage2DOnLevelZeroKeepsAnAlreadyUploadedHigherLevel) {
GLuint texture = 0;
MG_Impl::GLImpl::GenTextures(1, &texture);
MG_Impl::GLImpl::BindTexture(GL_TEXTURE_2D, texture);
MG_Impl::GLImpl::TexImage2D(GL_TEXTURE_2D, 1, GL_RGBA8, 49, 23, 0, GL_RGBA, GL_UNSIGNED_BYTE, nullptr);
MG_Impl::GLImpl::TexImage2D(GL_TEXTURE_2D, 0, GL_RGBA8, 98, 46, 0, GL_RGBA, GL_UNSIGNED_BYTE, nullptr);
const auto textureObject = MG_State::pGLContext->GetTextureObject(texture);
auto* mipmapObject = static_cast<MG_State::GLState::TextureObjectMipmap*>(textureObject.get());
ASSERT_NE(mipmapObject, nullptr);
EXPECT_EQ(mipmapObject->GetMipmapLevelCount(), 2u);
EXPECT_EQ(mipmapObject->GetMipmapTexelSize(TextureUploadTarget::Texture2D, 0), IntVec3(98, 46, 1));
EXPECT_EQ(mipmapObject->GetMipmapTexelSize(TextureUploadTarget::Texture2D, 1), IntVec3(49, 23, 1));
EXPECT_EQ(MG_Impl::GLImpl::GetError(), GL_NO_ERROR);
}
// The other half of the contract: respecifying a level 0 that already held an image still drops
// the chain, exactly as before. Minecraft rebinds the block-atlas name and calls glTexImage2D on
// level 0 before uploading the new levels; leaving the previous chain in place would strand a tail
// at the wrong sizes and - because Mojang terminates its chains with a 0x0 level - reproduce the
// same incomplete-texture black atlas the fix above exists to prevent.
TEST_F(TextureTest, TexImage2DRespecifyingAnExistingLevelZeroDropsTheStaleChain) {
GLuint texture = 0;
MG_Impl::GLImpl::GenTextures(1, &texture);
MG_Impl::GLImpl::BindTexture(GL_TEXTURE_2D, texture);
MG_Impl::GLImpl::TexImage2D(GL_TEXTURE_2D, 0, GL_RGBA8, 8, 8, 0, GL_RGBA, GL_UNSIGNED_BYTE, nullptr);
MG_Impl::GLImpl::TexImage2D(GL_TEXTURE_2D, 1, GL_RGBA8, 4, 4, 0, GL_RGBA, GL_UNSIGNED_BYTE, nullptr);
MG_Impl::GLImpl::TexImage2D(GL_TEXTURE_2D, 2, GL_RGBA8, 2, 2, 0, GL_RGBA, GL_UNSIGNED_BYTE, nullptr);
const auto textureObject = MG_State::pGLContext->GetTextureObject(texture);
auto* mipmapObject = static_cast<MG_State::GLState::TextureObjectMipmap*>(textureObject.get());
ASSERT_NE(mipmapObject, nullptr);
ASSERT_EQ(mipmapObject->GetMipmapLevelCount(), 3u);
MG_Impl::GLImpl::TexImage2D(GL_TEXTURE_2D, 0, GL_RGBA8, 16, 16, 0, GL_RGBA, GL_UNSIGNED_BYTE, nullptr);
EXPECT_EQ(mipmapObject->GetMipmapLevelCount(), 1u);
EXPECT_EQ(mipmapObject->GetMipmapTexelSize(TextureUploadTarget::Texture2D, 0), IntVec3(16, 16, 1));
EXPECT_EQ(MG_Impl::GLImpl::GetError(), GL_NO_ERROR);
}
// Same-size respecification has to drop the chain too. The Mipmap Levels video setting rebuilds
// the atlas at identical dimensions with a different level count, so a size-change-only test would
// let the old tail survive.
TEST_F(TextureTest, TexImage2DRespecifyingLevelZeroAtTheSameSizeStillDropsTheChain) {
GLuint texture = 0;
MG_Impl::GLImpl::GenTextures(1, &texture);
MG_Impl::GLImpl::BindTexture(GL_TEXTURE_2D, texture);
MG_Impl::GLImpl::TexImage2D(GL_TEXTURE_2D, 0, GL_RGBA8, 8, 8, 0, GL_RGBA, GL_UNSIGNED_BYTE, nullptr);
MG_Impl::GLImpl::TexImage2D(GL_TEXTURE_2D, 1, GL_RGBA8, 4, 4, 0, GL_RGBA, GL_UNSIGNED_BYTE, nullptr);
MG_Impl::GLImpl::TexImage2D(GL_TEXTURE_2D, 0, GL_RGBA8, 8, 8, 0, GL_RGBA, GL_UNSIGNED_BYTE, nullptr);
const auto textureObject = MG_State::pGLContext->GetTextureObject(texture);
auto* mipmapObject = static_cast<MG_State::GLState::TextureObjectMipmap*>(textureObject.get());
ASSERT_NE(mipmapObject, nullptr);
EXPECT_EQ(mipmapObject->GetMipmapLevelCount(), 1u);
EXPECT_EQ(MG_Impl::GLImpl::GetError(), GL_NO_ERROR);
}
// glTexStorage2D defines exactly `levels` levels. AllocateStorage only grows now, so the immutable
// path has to drop a longer pre-existing chain explicitly.
TEST_F(TextureTest, TexStorage2DTrimsALongerPreExistingMipChain) {
GLuint texture = 0;
MG_Impl::GLImpl::GenTextures(1, &texture);
MG_Impl::GLImpl::BindTexture(GL_TEXTURE_2D, texture);
MG_Impl::GLImpl::TexImage2D(GL_TEXTURE_2D, 0, GL_RGBA8, 8, 8, 0, GL_RGBA, GL_UNSIGNED_BYTE, nullptr);
MG_Impl::GLImpl::TexImage2D(GL_TEXTURE_2D, 1, GL_RGBA8, 4, 4, 0, GL_RGBA, GL_UNSIGNED_BYTE, nullptr);
MG_Impl::GLImpl::TexImage2D(GL_TEXTURE_2D, 2, GL_RGBA8, 2, 2, 0, GL_RGBA, GL_UNSIGNED_BYTE, nullptr);
MG_Impl::GLImpl::TexImage2D(GL_TEXTURE_2D, 3, GL_RGBA8, 1, 1, 0, GL_RGBA, GL_UNSIGNED_BYTE, nullptr);
MG_Impl::GLImpl::TexStorage2D(GL_TEXTURE_2D, 2, GL_RGBA8, 8, 8);
const auto textureObject = MG_State::pGLContext->GetTextureObject(texture);
auto* mipmapObject = static_cast<MG_State::GLState::TextureObjectMipmap*>(textureObject.get());
ASSERT_NE(mipmapObject, nullptr);
EXPECT_EQ(mipmapObject->GetMipmapLevelCount(), 2u);
EXPECT_EQ(MG_Impl::GLImpl::GetError(), GL_NO_ERROR);
}
TEST_F(TextureTest, TextureStorage3DAndSubImageModifyNamedObjectOnly) {
GLuint texture = 0;
MG_Impl::GLImpl::CreateTextures(GL_TEXTURE_3D, 1, &texture);