diff --git a/MobileGL/MG_Impl/GLImpl/Texture/GL_Texture.cpp b/MobileGL/MG_Impl/GLImpl/Texture/GL_Texture.cpp index 88201752..e54a95f3 100644 --- a/MobileGL/MG_Impl/GLImpl/Texture/GL_Texture.cpp +++ b/MobileGL/MG_Impl/GLImpl/Texture/GL_Texture.cpp @@ -865,11 +865,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)) @@ -2300,7 +2300,7 @@ namespace MobileGL::MG_Impl::GLImpl { for (SizeT i = 0; i < static_cast(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); } } @@ -2523,14 +2523,7 @@ namespace MobileGL::MG_Impl::GLImpl { return; } - if (!MG_State::pGLContext->ValidateTextureName(texture)) { - MG_State::pGLContext->RecordError( - ErrorCode::InvalidOperation, - MakeUnique("MG_Impl/GLImpl", "BindTexture_State", "Invalid texture name")); - return; - } - - if (!TextureImpl::ValidateTextureName(texture, true)) return; + if (!TextureImpl::ValidateTextureName(texture)) return; // ======================= Processing ================================ Bool doesTextureExist = MG_State::pGLContext->ValidateTextureObject(texture); diff --git a/MobileGL/MG_State/GLState/TextureState/TextureState.cpp b/MobileGL/MG_State/GLState/TextureState/TextureState.cpp index 31617504..84f6d7fd 100644 --- a/MobileGL/MG_State/GLState/TextureState/TextureState.cpp +++ b/MobileGL/MG_State/GLState/TextureState/TextureState.cpp @@ -109,8 +109,8 @@ namespace MobileGL::MG_State::GLState { // re-resolved instead of dangling. BumpTextureBindGeneration(); m_textureObjects.erase(index); + m_indexGenerator.Delete(index); } - m_indexGenerator.Delete(index); } } diff --git a/MobileGL/MG_Test/Texture/TextureTest.cpp b/MobileGL/MG_Test/Texture/TextureTest.cpp index d22a09f2..e159e04a 100644 --- a/MobileGL/MG_Test/Texture/TextureTest.cpp +++ b/MobileGL/MG_Test/Texture/TextureTest.cpp @@ -8,6 +8,8 @@ #include +#include + #include "Includes.h" #include "Init.h" #include @@ -131,6 +133,123 @@ TEST_F(TextureTest, CreateTexturesCreatesObjectsWithoutBinding) { EXPECT_EQ(MG_Impl::GLImpl::GetError(), GL_NO_ERROR); } +TEST_F(TextureTest, DeleteGeneratedReservationThenBindCreatesObjectForSubImageUpload) { + 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_TRUE(MG_State::pGLContext->ValidateTextureName(texture)); + EXPECT_FALSE(MG_State::pGLContext->ValidateTextureObject(texture)); + EXPECT_EQ(MG_Impl::GLImpl::GetError(), GL_NO_ERROR); + + 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->ValidateTextureName(texture)); + 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); +} + +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_VALUE); + 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::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_VALUE); + 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;