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

[Fix] (MG_Impl/Texture): validate the bound texture before dereferencing it in TexSubImage2D, and stop recording an error when glDeleteTextures is handed unknown names

Browse Source
This commit is contained in:
swung0x48
2026-07-16 21:34:40 -04:00
parent 8026838563
commit f61675e9ce
3 changed files with 151 additions and 4 deletions
Download Patch File Download Diff File Expand all files Collapse all files
MobileGL/MG_Impl/GLImpl/Texture/GL_Texture.cpp
+5 -4
View File
@@ -887,11 +887,11 @@ namespace MobileGL::MG_Impl::GLImpl {
auto& activeUnit = MG_State::pGLContext->GetTextureUnitObject(MG_State::pGLContext->GetActiveTextureUnit());
auto& bindingSlot = activeUnit.GetBindingSlot(textureTarget);
auto& textureObject = bindingSlot.GetBoundObject();
if (!TextureImpl::ValidateTextureObject(textureObject)) return;
TextureInternalFormat textureInternalFormat = textureObject->GetFormat();
MGLOG_D("%s: working on texture %d", __func__, textureObject->GetExternalIndex());
// ===================== Error Checking ==============================
if (!TextureImpl::ValidateTextureObject(textureObject)) return;
if (!TextureImpl::ValidateTextureSubImageOffsets(textureObject, xoffset, width, yoffset, height)) return;
if (!TextureImpl::ValidateTextureInternalFormatCompatibleWithInput(textureInputFormat, textureInternalFormat,
texturePixelDataType))
@@ -2329,7 +2329,7 @@ namespace MobileGL::MG_Impl::GLImpl {
for (SizeT i = 0; i < static_cast<SizeT>(n); ++i) {
Uint textureName = textures[i];
if (textureName == 0) continue;
if (!TextureImpl::ValidateTextureName(textureName, true)) continue;
if (!MG_State::pGLContext->ValidateTextureName(textureName)) continue;
MG_State::pGLContext->MarkTextureObjectForDeletion(textureName);
}
}
@@ -2552,6 +2552,9 @@ namespace MobileGL::MG_Impl::GLImpl {
return;
}
// GL 3.3 core 3.8.1: a name that GenTextures never returned - or that has since been deleted -
// is not a legal bind target in the core profile (no application-generated names), and the error
// is INVALID_OPERATION, not INVALID_VALUE.
if (!MG_State::pGLContext->ValidateTextureName(texture)) {
MG_State::pGLContext->RecordError(
ErrorCode::InvalidOperation,
@@ -2559,8 +2562,6 @@ namespace MobileGL::MG_Impl::GLImpl {
return;
}
if (!TextureImpl::ValidateTextureName(texture, true)) return;
// ======================= Processing ================================
Bool doesTextureExist = MG_State::pGLContext->ValidateTextureObject(texture);
if (!doesTextureExist) {
MobileGL/MG_State/GLState/TextureState/TextureState.cpp
+3
View File
@@ -110,6 +110,9 @@ namespace MobileGL::MG_State::GLState {
BumpTextureBindGeneration();
m_textureObjects.erase(index);
}
// Release the name itself even when GenTextures only reserved it and no bind ever
// instantiated an object: GL 3.3 core 3.8.1 makes a deleted name unused again (so a
// later bind of it must fail), and the reservation has to return to the free list.
m_indexGenerator.Delete(index);
}
}
MobileGL/MG_Test/Texture/TextureTest.cpp
+143
View File
@@ -8,6 +8,8 @@
#include <gtest/gtest.h>
#include <limits>
#include "Includes.h"
#include "Init.h"
#include <MG_Backend/BackendObjects.h>
@@ -134,6 +136,147 @@ TEST_F(TextureTest, CreateTexturesCreatesObjectsWithoutBinding) {
EXPECT_EQ(MG_Impl::GLImpl::GetError(), GL_NO_ERROR);
}
TEST_F(TextureTest, GenThenBindCreatesObjectForUnsizedPackedBgraSubImageUpload) {
GLuint texture = 0;
MG_Impl::GLImpl::GenTextures(1, &texture);
ASSERT_NE(texture, 0u);
ASSERT_TRUE(MG_State::pGLContext->ValidateTextureName(texture));
// GenTextures only reserves the name; the object appears on first bind.
ASSERT_FALSE(MG_State::pGLContext->ValidateTextureObject(texture));
EXPECT_EQ(MG_Impl::GLImpl::IsTexture(texture), GL_FALSE);
MG_Impl::GLImpl::BindTexture(GL_TEXTURE_2D, texture);
const auto textureObject = MG_State::pGLContext->GetTextureObject(texture);
ASSERT_NE(textureObject, nullptr);
EXPECT_TRUE(MG_State::pGLContext->ValidateTextureObject(texture));
EXPECT_EQ(MG_Impl::GLImpl::IsTexture(texture), GL_TRUE);
MG_Impl::GLImpl::TexImage2D(GL_TEXTURE_2D, 0, GL_RGBA, 2, 1, 0, GL_BGRA,
GL_UNSIGNED_INT_8_8_8_8_REV, nullptr);
const Uint8 pixels[] = {
10, 20, 30, 40,
50, 60, 70, 80,
};
MG_Impl::GLImpl::TexSubImage2D(GL_TEXTURE_2D, 0, 0, 0, 2, 1, GL_BGRA,
GL_UNSIGNED_INT_8_8_8_8_REV, pixels);
const auto* stored = GetBoundTexture2DLevelBytes(texture);
ASSERT_NE(stored, nullptr);
const Uint8 expected[] = {
30, 20, 10, 40,
70, 60, 50, 80,
};
EXPECT_EQ(std::memcmp(stored, expected, sizeof(expected)), 0);
EXPECT_EQ(MG_Impl::GLImpl::GetError(), GL_NO_ERROR);
}
// GL 3.3 core 3.8.1: DeleteTextures makes the name unused again whether or not a bind ever
// instantiated an object, so the reservation must go back to the generator's free list rather
// than leaking, and binding the dead name afterwards must fail.
TEST_F(TextureTest, DeleteGeneratedButUnboundNameReleasesReservationAndBindFails) {
GLuint texture = 0;
MG_Impl::GLImpl::GenTextures(1, &texture);
ASSERT_NE(texture, 0u);
ASSERT_TRUE(MG_State::pGLContext->ValidateTextureName(texture));
ASSERT_FALSE(MG_State::pGLContext->ValidateTextureObject(texture));
MG_Impl::GLImpl::DeleteTextures(1, &texture);
EXPECT_EQ(MG_Impl::GLImpl::GetError(), GL_NO_ERROR);
EXPECT_FALSE(MG_State::pGLContext->ValidateTextureName(texture));
EXPECT_FALSE(MG_State::pGLContext->ValidateTextureObject(texture));
EXPECT_EQ(MG_Impl::GLImpl::IsTexture(texture), GL_FALSE);
MG_Impl::GLImpl::BindTexture(GL_TEXTURE_2D, texture);
EXPECT_EQ(MG_Impl::GLImpl::GetError(), GL_INVALID_OPERATION);
EXPECT_FALSE(MG_State::pGLContext->ValidateTextureObject(texture));
// The freed reservation is recycled (the generator's free list is LIFO, so the very same
// name comes back) - a delete that skipped the release would hand out a fresh name here.
GLuint recycled = 0;
MG_Impl::GLImpl::GenTextures(1, &recycled);
EXPECT_EQ(recycled, texture);
EXPECT_TRUE(MG_State::pGLContext->ValidateTextureName(recycled));
}
TEST_F(TextureTest, DeleteInstantiatedTextureInvalidatesNameUntilRegenerated) {
GLuint textures[2] = {};
MG_Impl::GLImpl::GenTextures(2, textures);
ASSERT_NE(textures[0], 0u);
ASSERT_NE(textures[1], 0u);
MG_Impl::GLImpl::BindTexture(GL_TEXTURE_2D, textures[0]);
ASSERT_TRUE(MG_State::pGLContext->ValidateTextureObject(textures[0]));
MG_Impl::GLImpl::DeleteTextures(1, &textures[0]);
EXPECT_FALSE(MG_State::pGLContext->ValidateTextureName(textures[0]));
EXPECT_FALSE(MG_State::pGLContext->ValidateTextureObject(textures[0]));
MG_Impl::GLImpl::BindTexture(GL_TEXTURE_2D, textures[1]);
const auto fallbackObject = MG_State::pGLContext->GetTextureObject(textures[1]);
ASSERT_NE(fallbackObject, nullptr);
ASSERT_EQ(MG_Impl::GLImpl::GetError(), GL_NO_ERROR);
MG_Impl::GLImpl::BindTexture(GL_TEXTURE_2D, textures[0]);
EXPECT_EQ(MG_Impl::GLImpl::GetError(), GL_INVALID_OPERATION);
EXPECT_EQ(MG_State::pGLContext->GetTextureUnitObject(0)
.GetBindingSlot(TextureTarget::Texture2D)
.GetBoundObject(),
fallbackObject);
}
TEST_F(TextureTest, DeleteUnknownNamesIsSilentButBindUnknownNameIsInvalid) {
GLuint validTexture = 0;
MG_Impl::GLImpl::GenTextures(1, &validTexture);
MG_Impl::GLImpl::BindTexture(GL_TEXTURE_2D, validTexture);
const auto boundObject = MG_State::pGLContext->GetTextureObject(validTexture);
ASSERT_NE(boundObject, nullptr);
constexpr GLuint unknownNames[] = {0, std::numeric_limits<GLuint>::max()};
MG_Impl::GLImpl::DeleteTextures(2, unknownNames);
EXPECT_EQ(MG_Impl::GLImpl::GetError(), GL_NO_ERROR);
MG_Impl::GLImpl::BindTexture(GL_TEXTURE_2D, unknownNames[1]);
EXPECT_EQ(MG_Impl::GLImpl::GetError(), GL_INVALID_OPERATION);
EXPECT_EQ(MG_State::pGLContext->GetTextureUnitObject(0)
.GetBindingSlot(TextureTarget::Texture2D)
.GetBoundObject(),
boundObject);
EXPECT_FALSE(MG_State::pGLContext->ValidateTextureName(unknownNames[1]));
EXPECT_FALSE(MG_State::pGLContext->ValidateTextureObject(unknownNames[1]));
}
TEST_F(TextureTest, BindTextureUnitEnumAsNameIsSilentNoOp) {
GLuint validTexture = 0;
MG_Impl::GLImpl::GenTextures(1, &validTexture);
MG_Impl::GLImpl::BindTexture(GL_TEXTURE_2D, validTexture);
const auto boundObject = MG_State::pGLContext->GetTextureObject(validTexture);
ASSERT_NE(boundObject, nullptr);
ASSERT_EQ(MG_Impl::GLImpl::GetError(), GL_NO_ERROR);
constexpr GLuint textureUnitEnum = GL_TEXTURE7;
ASSERT_FALSE(MG_State::pGLContext->ValidateTextureName(textureUnitEnum));
MG_Impl::GLImpl::BindTexture(GL_TEXTURE_2D, textureUnitEnum);
EXPECT_EQ(MG_Impl::GLImpl::GetError(), GL_NO_ERROR);
EXPECT_EQ(MG_State::pGLContext->GetTextureUnitObject(0)
.GetBindingSlot(TextureTarget::Texture2D)
.GetBoundObject(),
boundObject);
EXPECT_FALSE(MG_State::pGLContext->ValidateTextureName(textureUnitEnum));
EXPECT_FALSE(MG_State::pGLContext->ValidateTextureObject(textureUnitEnum));
}
TEST_F(TextureTest, TexSubImage2DWithoutBoundTextureReportsErrorInsteadOfDereferencingNull) {
MG_Impl::GLImpl::BindTexture(GL_TEXTURE_2D, 0);
ASSERT_EQ(MG_Impl::GLImpl::GetError(), GL_NO_ERROR);
const Uint8 pixel[] = {1, 2, 3, 4};
MG_Impl::GLImpl::TexSubImage2D(GL_TEXTURE_2D, 0, 0, 0, 1, 1, GL_RGBA, GL_UNSIGNED_BYTE, pixel);
EXPECT_EQ(MG_Impl::GLImpl::GetError(), GL_INVALID_OPERATION);
}
TEST_F(TextureTest, TextureStorageAndSubImageModifyNamedObjectOnly) {
GLuint namedTexture = 0;
GLuint boundTexture = 0;