diff --git a/MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp b/MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp index 7f17d18d..1bf839b1 100644 --- a/MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp +++ b/MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp @@ -1034,7 +1034,6 @@ namespace MobileGL::MG_Backend::DirectGLES { } static void BindCurrentProgramWithResources(); - static void BindCurrentTextures(); void PrepareForDraw(DrawSyncBit syncBit) { #ifdef TRACY_ENABLE @@ -1077,7 +1076,7 @@ namespace MobileGL::MG_Backend::DirectGLES { // sample whatever texture the last sync left behind (e.g. Flywheel's depth // pyramid downsample reading a stale unit-0 binding instead of the depth // attachment). - static void BindCurrentTextures() { + void BindCurrentTextures() { #ifdef TRACY_ENABLE ZoneScopedNC("BindCurrentTextures", TRACY_ZONECOLOR_BACKEND); #endif @@ -1085,27 +1084,52 @@ namespace MobileGL::MG_Backend::DirectGLES { const Int maxTouchedUnit = MG_State::pGLContext->GetMaxTouchedTextureUnit(); for (Int unit = 0; unit <= maxTouchedUnit; ++unit) { auto& textureUnit = MG_State::pGLContext->GetTextureUnitObject(unit); + Array boundBackendTargets{}; for (const auto& bindingSlot : textureUnit.GetAllBindingSlots()) { const auto& textureObject = bindingSlot.GetBoundObject(); if (!textureObject) continue; - // A default texture (name 0) that was never given an image is the initial / - // "unbound" state of the slot; skip it exactly like the old null slot so bind-0 - // heavy apps pay nothing per draw for the always-populated slots. + + // An image-less default texture is the frontend's bind-0 state. Defer native + // unbinding until all slots have been considered: desktop 1D/1D-array targets + // alias ES 2D/2D-array targets, so a default alias must not clear a real binding. if (MG_State::GLState::IsUndefinedDefaultTexture(textureObject.get())) continue; - // Bind texture object auto target = textureObject->GetTarget(); if (!TextureImpl::IsSupportedTextureTarget(target)) { MGLOG_D(" Texture target %s is not supported, skipping.", MG_Util::ConvertTextureTargetToString(target).c_str()); continue; } + const GLenum targetGL = TextureImpl::ConvertTextureTargetToBackendGLEnum(target); + + // Bind texture object const auto& backendTextureIt = TextureImpl::g_backendTextureObjects.find(textureObject.get()); if (backendTextureIt == TextureImpl::g_backendTextureObjects.end()) continue; - GLenum targetGL = TextureImpl::ConvertTextureTargetToBackendGLEnum(target); backendTextureIt->second->Bind(targetGL, unit); + const auto backendTarget = TextureImpl::MapToBackendTextureTarget(target); + boundBackendTargets[static_cast(backendTarget)] = true; + } + + // Clear each native target that has no resolved frontend binding. This is the backend + // half of glBindTexture(..., 0); skipping image-less default objects would otherwise + // leave the previously sampled ES texture resident. Deduplicate mapped desktop targets + // so 1D/2D and 1D-array/2D-array aliases do not cause redundant binds. + Array visitedBackendTargets{}; + for (const auto& bindingSlot : textureUnit.GetAllBindingSlots()) { + const auto target = bindingSlot.GetTarget(); + if (!TextureImpl::IsSupportedTextureTarget(target)) continue; + + const auto backendTarget = TextureImpl::MapToBackendTextureTarget(target); + const auto backendTargetIndex = static_cast(backendTarget); + if (visitedBackendTargets[backendTargetIndex]) continue; + visitedBackendTargets[backendTargetIndex] = true; + + if (!boundBackendTargets[backendTargetIndex]) { + const GLenum targetGL = TextureImpl::ConvertTextureTargetToBackendGLEnum(target); + TextureImpl::UnbindTexture(unit, targetGL); + } } // Bind sampler object if necessary diff --git a/MobileGL/MG_Backend/DirectGLES/DirectGLES.h b/MobileGL/MG_Backend/DirectGLES/DirectGLES.h index 052eceb9..31edfc09 100644 --- a/MobileGL/MG_Backend/DirectGLES/DirectGLES.h +++ b/MobileGL/MG_Backend/DirectGLES/DirectGLES.h @@ -19,6 +19,10 @@ operation Utils::CheckGLESError(); namespace MobileGL::MG_Backend::DirectGLES { + // Re-establishes the frontend texture-unit bindings on the native ES context. + // Content uploads use scratch bindings, so draws and dispatches call this after + // texture synchronization. + void BindCurrentTextures(); void ClearBufferfi(GLenum buffer, GLint drawbuffer, GLfloat depth, GLint stencil); void ClearBufferfv(GLenum buffer, GLint drawbuffer, const GLfloat* value); void ClearBufferuiv(GLenum buffer, GLint drawbuffer, const GLuint* value); diff --git a/MobileGL/MG_State/GLState/TextureState/TextureState.cpp b/MobileGL/MG_State/GLState/TextureState/TextureState.cpp index 76a5b290..c4847985 100644 --- a/MobileGL/MG_State/GLState/TextureState/TextureState.cpp +++ b/MobileGL/MG_State/GLState/TextureState/TextureState.cpp @@ -122,11 +122,11 @@ namespace MobileGL::MG_State::GLState { // re-resolved instead of dangling. BumpTextureBindGeneration(); m_textureObjects.erase(index); + m_indexGenerator.Delete(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); + // Compatibility: legacy Minecraft may delete a generated name before its first bind, + // then bind and populate that same name. Keep such a reservation alive; only a real + // texture object reaching deletion releases its index above. } } diff --git a/MobileGL/MG_Test/SanityTest.cpp b/MobileGL/MG_Test/SanityTest.cpp index df2d8d20..4293ba9a 100644 --- a/MobileGL/MG_Test/SanityTest.cpp +++ b/MobileGL/MG_Test/SanityTest.cpp @@ -19,6 +19,7 @@ #include #include #include +#include #include #include #include @@ -103,6 +104,79 @@ namespace { private: MobileGL::MG_External::GLESCapabilities m_snapshot; }; + + struct TextureBindCall { + GLenum target; + GLuint texture; + }; + + MobileGL::Vector* g_textureBindCalls = nullptr; + GLuint g_nextBackendTextureId = 73; + + void RecordTextureBind(GLenum target, GLuint texture) { + if (g_textureBindCalls) { + g_textureBindCalls->push_back({target, texture}); + } + } + + void GenerateBackendTextures(GLsizei count, GLuint* textures) { + for (GLsizei i = 0; i < count; ++i) { + textures[i] = g_nextBackendTextureId++; + } + } + + void DeleteBackendTextures(GLsizei, const GLuint*) {} + + GLenum NoBackendError() { + return GL_NO_ERROR; + } + + // Isolates the DirectGLES globals touched by the binding-cache regression test. The test + // installs only the native ES entry points needed to construct/bind a backend texture and + // restores the process-wide state even when a gtest assertion unwinds the test body. + struct ScopedDirectGLESTextureBindings { + ScopedDirectGLESTextureBindings(): + previousContext(MobileGL::Move(MobileGL::MG_State::pGLContext)), + previousFunctions(MobileGL::MG_Backend::DirectGLES::g_GLESFuncs), + previousActiveUnit(MobileGL::MG_Backend::DirectGLES::TextureImpl::g_activeTextureUnit), + previousCache(MobileGL::MG_Backend::DirectGLES::TextureImpl::g_boundTexturesCache), + previousRegistry(MobileGL::MG_Backend::DirectGLES::TextureImpl::g_backendTextureObjects) { + MobileGL::MG_State::pGLContext = MobileGL::MakeUnique(); + MobileGL::MG_Backend::DirectGLES::TextureImpl::g_activeTextureUnit = 0; + MobileGL::MG_Backend::DirectGLES::TextureImpl::g_boundTexturesCache = {}; + MobileGL::MG_Backend::DirectGLES::TextureImpl::g_backendTextureObjects = {}; + + MobileGL::MG_External::GLESFunctionsTable functions{}; + functions.glBindTexture = RecordTextureBind; + functions.glDeleteTextures = DeleteBackendTextures; + functions.glGenTextures = GenerateBackendTextures; + functions.glGetError = NoBackendError; + MobileGL::MG_Backend::DirectGLES::SetGLESFuncsTable(functions); + g_textureBindCalls = &bindCalls; + } + + ~ScopedDirectGLESTextureBindings() { + g_textureBindCalls = nullptr; + MobileGL::MG_Backend::DirectGLES::TextureImpl::g_boundTexturesCache = {}; + MobileGL::MG_Backend::DirectGLES::TextureImpl::g_backendTextureObjects = previousRegistry; + MobileGL::MG_Backend::DirectGLES::SetGLESFuncsTable(previousFunctions); + MobileGL::MG_Backend::DirectGLES::TextureImpl::g_activeTextureUnit = previousActiveUnit; + MobileGL::MG_Backend::DirectGLES::TextureImpl::g_boundTexturesCache = previousCache; + MobileGL::MG_State::pGLContext = MobileGL::Move(previousContext); + } + + ScopedDirectGLESTextureBindings(const ScopedDirectGLESTextureBindings&) = delete; + ScopedDirectGLESTextureBindings& operator=(const ScopedDirectGLESTextureBindings&) = delete; + + MobileGL::Vector bindCalls; + + private: + MobileGL::UniquePtr previousContext; + MobileGL::MG_External::GLESFunctionsTable previousFunctions; + MobileGL::Uint previousActiveUnit; + decltype(MobileGL::MG_Backend::DirectGLES::TextureImpl::g_boundTexturesCache) previousCache; + decltype(MobileGL::MG_Backend::DirectGLES::TextureImpl::g_backendTextureObjects) previousRegistry; + }; } // namespace TEST(Sanity, BasicAssertions) { @@ -148,6 +222,52 @@ TEST(DirectGLESSanity, AdvertisesVoxyRequiredRenderingExtensionsWithoutRaisingGL extensions.end()); } +TEST(DirectGLESSanity, BindingZeroClearsPreviousNativeTextureBinding) { + using namespace MobileGL; + namespace DirectGLES = MG_Backend::DirectGLES; + + ScopedDirectGLESTextureBindings state; + + GLuint frontendTexture = 0; + MG_Impl::GLImpl::GenTextures(1, &frontendTexture); + ASSERT_NE(frontendTexture, 0u); + MG_Impl::GLImpl::BindTexture(GL_TEXTURE_2D, frontendTexture); + const auto& frontendTextureObject = MG_State::pGLContext->GetTextureUnitObject(0) + .GetBindingSlot(TextureTarget::Texture2D) + .GetBoundObject(); + ASSERT_NE(frontendTextureObject, nullptr); + ASSERT_EQ(frontendTextureObject->GetExternalIndex(), frontendTexture); + + auto& backendTexture = DirectGLES::TextureImpl::g_backendTextureObjects.GetOrCreate(frontendTextureObject); + backendTexture = MakeShared(); + const GLuint backendTextureId = backendTexture->GetBackendTextureId(); + + DirectGLES::BindCurrentTextures(); + ASSERT_EQ(state.bindCalls.size(), 1u); + EXPECT_EQ(state.bindCalls[0].target, GL_TEXTURE_2D); + EXPECT_EQ(state.bindCalls[0].texture, backendTextureId); + + // The default 1D slot maps to the same native ES target as 2D. It must not clear and force a + // redundant rebind while the real 2D frontend object remains current. + DirectGLES::BindCurrentTextures(); + EXPECT_EQ(state.bindCalls.size(), 1u); + + MG_Impl::GLImpl::BindTexture(GL_TEXTURE_2D, 0); + ASSERT_TRUE(MG_State::GLState::IsUndefinedDefaultTexture( + MG_State::pGLContext->GetTextureUnitObject(0) + .GetBindingSlot(TextureTarget::Texture2D) + .GetBoundObject() + .get())); + + DirectGLES::BindCurrentTextures(); + + ASSERT_EQ(state.bindCalls.size(), 2u); + EXPECT_EQ(state.bindCalls[1].target, GL_TEXTURE_2D); + EXPECT_EQ(state.bindCalls[1].texture, 0u); + EXPECT_EQ(DirectGLES::TextureImpl::g_boundTexturesCache[0][static_cast(TextureTarget::Texture2D)], + nullptr); +} + TEST(DirectGLESSanity, ProvidesNamedFramebufferBlitForDirectStateAccess) { MobileGL::MG_Backend::DirectGLES::BackendObject_DirectGLES backend; const auto& funcs = backend.GetBackendFunctions().GL; diff --git a/MobileGL/MG_Test/Texture/TextureTest.cpp b/MobileGL/MG_Test/Texture/TextureTest.cpp index 5c5d7205..476e749d 100644 --- a/MobileGL/MG_Test/Texture/TextureTest.cpp +++ b/MobileGL/MG_Test/Texture/TextureTest.cpp @@ -380,10 +380,10 @@ TEST_F(TextureTest, GenThenBindCreatesObjectForUnsizedPackedBgraSubImageUpload) 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) { +// Legacy Minecraft reserves a texture name, deletes it before first bind, then reuses the same +// name for the atlas upload. Preserve that generated reservation so the later bind can instantiate +// the object and subsequent sub-image uploads target it instead of the default texture. +TEST_F(TextureTest, DeleteGeneratedReservationThenBindCreatesObjectForSubImageUpload) { GLuint texture = 0; MG_Impl::GLImpl::GenTextures(1, &texture); ASSERT_NE(texture, 0u); @@ -392,22 +392,36 @@ TEST_F(TextureTest, DeleteGeneratedButUnboundNameReleasesReservationAndBindFails MG_Impl::GLImpl::DeleteTextures(1, &texture); EXPECT_EQ(MG_Impl::GLImpl::GetError(), GL_NO_ERROR); - EXPECT_FALSE(MG_State::pGLContext->ValidateTextureName(texture)); + EXPECT_TRUE(MG_State::pGLContext->ValidateTextureName(texture)); EXPECT_FALSE(MG_State::pGLContext->ValidateTextureObject(texture)); - // IsTexture answers about a dead name without raising anything (GL 3.3 core 6.1.4). EXPECT_EQ(MG_Impl::GLImpl::IsTexture(texture), GL_FALSE); EXPECT_EQ(MG_Impl::GLImpl::GetError(), GL_NO_ERROR); MG_Impl::GLImpl::BindTexture(GL_TEXTURE_2D, texture); - ExpectSingleGlError(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)); + 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) {