From b6a2bf08d4caa875b20e2e3f544984e72185eeee Mon Sep 17 00:00:00 2001 From: BZLZHH Date: Sat, 1 Aug 2026 16:21:28 -0400 Subject: [PATCH] [Fix] (DirectGLES): resolve an aliased texture unit by the sampler's type Desktop GL_TEXTURE_1D/1D_ARRAY are emulated on ES GL_TEXTURE_2D/2D_ARRAY, so one native binding serves two of a unit's frontend slots. An earlier fix settled the real-versus-default case; two REAL textures can collide just as easily, and there the slot iteration order decided it. KHR-GL3x.texture_size_promotion keeps its 1D source texture and its 2D destination texture bound to the same unit, so the shader sampled the render target it was drawing into instead of the source. GL resolves this from the shader's sampler type, so ask the program: the frontend's uniform reflection still carries the original GLSL type, which maps straight back to the target the lookup means. Only consulted when a collision actually happens, so an ordinary unit costs nothing, and the first binding placed stands when the program gives no answer rather than being overwritten by whichever slot happens to come last. Also adds the read-colour clamp that goes with it: GL clamps a glReadPixels from a fixed-point colour buffer to [0,1] (GL_CLAMP_READ_COLOR defaults to GL_FIXED_ONLY), which ES has no equivalent for at all - a GL_R16_SNORM target holding -0.125 read back unclamped. Applied to the wide rows before they are repacked, for float, half, short and byte reads alike, and deliberately NOT for glGetTexImage, which reaches the same helper through a scratch framebuffer but is not subject to read-colour clamping. texture_size_promotion now clears every 1D case (it stops at the first failure and has moved on to GL_TEXTURE_RECTANGLE, which DirectGLES does not emulate at all yet), and KHR-GL33.texture_swizzle's GL_DEPTH_COMPONENT32 1D cases pass. DirectVulkan re-verified unchanged. --- MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp | 99 ++++++++++++++++++- MobileGL/MG_Backend/DirectGLES/Managers.cpp | 25 +++++ MobileGL/MG_Backend/DirectGLES/Managers.h | 7 ++ 3 files changed, 127 insertions(+), 4 deletions(-) diff --git a/MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp b/MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp index fbf83738..01783abd 100644 --- a/MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp +++ b/MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp @@ -163,6 +163,23 @@ namespace MobileGL::MG_Backend::DirectGLES { case GL_UNSIGNED_INT_SAMPLER_CUBE_MAP_ARRAY: case GL_SAMPLER_CUBE_MAP_ARRAY_SHADOW: return TextureTarget::TextureCubeMapArray; + case GL_SAMPLER_2D_RECT: + case GL_INT_SAMPLER_2D_RECT: + case GL_UNSIGNED_INT_SAMPLER_2D_RECT: + case GL_SAMPLER_2D_RECT_SHADOW: + return TextureTarget::TextureRectangle; + case GL_SAMPLER_2D_MULTISAMPLE: + case GL_INT_SAMPLER_2D_MULTISAMPLE: + case GL_UNSIGNED_INT_SAMPLER_2D_MULTISAMPLE: + return TextureTarget::Texture2DMultisample; + case GL_SAMPLER_2D_MULTISAMPLE_ARRAY: + case GL_INT_SAMPLER_2D_MULTISAMPLE_ARRAY: + case GL_UNSIGNED_INT_SAMPLER_2D_MULTISAMPLE_ARRAY: + return TextureTarget::Texture2DMultisampleArray; + case GL_SAMPLER_BUFFER: + case GL_INT_SAMPLER_BUFFER: + case GL_UNSIGNED_INT_SAMPLER_BUFFER: + return TextureTarget::TextureBuffer; default: return TextureTarget::Unknown; } @@ -1272,11 +1289,32 @@ namespace MobileGL::MG_Backend::DirectGLES { #ifdef TRACY_ENABLE ZoneScopedNC("BindCurrentTextures", TRACY_ZONECOLOR_BACKEND); #endif + // Frontend target the current program samples at a given unit; resolves an + // aliased native binding when two real textures compete for it (see below). + // Only consulted on a conflict, so the ordinary unit costs nothing. + const auto& currentProgram = MG_State::pGLContext->GetCurrentProgram(); + const auto sampledTargetForUnit = [¤tProgram](Int unit) { + if (!currentProgram || !currentProgram->GetLinkStatus()) { + return TextureTarget::Unknown; + } + const Uint maxUniformLocation = currentProgram->GetMaxUniformLocation(); + for (Uint location = 0; location <= maxUniformLocation; ++location) { + if (currentProgram->GetUniformSamplerOrImageUnitIndex(location) != unit) continue; + const auto target = SamplerUniformTextureTarget(currentProgram->GetUniformType(location)); + if (target != TextureTarget::Unknown) { + return target; + } + } + return TextureTarget::Unknown; + }; + // Units past the frontend's high-water mark have provably-empty slots. const Int maxTouchedUnit = MG_State::pGLContext->GetMaxTouchedTextureUnit(); for (Int unit = 0; unit <= maxTouchedUnit; ++unit) { auto& textureUnit = MG_State::pGLContext->GetTextureUnitObject(unit); Array boundBackendTargets{}; + Array claimedByFrontendTarget{}; + claimedByFrontendTarget.fill(TextureTarget::Unknown); // Two passes over the slots, because desktop 1D/1D-array targets alias ES // 2D/2D-array targets: a unit can hold a real texture on one of an aliased @@ -1307,7 +1345,18 @@ namespace MobileGL::MG_Backend::DirectGLES { continue; } const auto backendTarget = TextureImpl::MapToBackendTextureTarget(target); - if (isDefaultObject && boundBackendTargets[static_cast(backendTarget)]) continue; + const SizeT backendTargetIndex = static_cast(backendTarget); + if (isDefaultObject && boundBackendTargets[backendTargetIndex]) continue; + + // Two REAL textures can want the same native target as well - an app is + // free to keep a 1D texture and a 2D texture bound to one unit, and GL + // resolves which one is sampled from the shader's sampler type. Ask the + // program; without an answer the first binding placed stands rather than + // being silently overwritten by whichever slot comes last. + if (!isDefaultObject && boundBackendTargets[backendTargetIndex] && + claimedByFrontendTarget[backendTargetIndex] != target) { + if (sampledTargetForUnit(unit) != target) continue; + } const GLenum targetGL = TextureImpl::ConvertTextureTargetToBackendGLEnum(target); // Bind texture object @@ -1315,7 +1364,8 @@ namespace MobileGL::MG_Backend::DirectGLES { if (backendTextureIt == TextureImpl::g_backendTextureObjects.end()) continue; backendTextureIt->second->Bind(targetGL, unit); - boundBackendTargets[static_cast(backendTarget)] = true; + boundBackendTargets[backendTargetIndex] = true; + claimedByFrontendTarget[backendTargetIndex] = target; } } @@ -3887,7 +3937,8 @@ namespace MobileGL::MG_Backend::DirectGLES { // (format, type) layout. Returns false when the combination is not convertible (the caller keeps its // "not implemented" skip); returns true when the request was handled, even if it degraded to a logged no-op. static Bool ReadPixelsViaFormatConversion(GLint x, GLint y, GLsizei width, GLsizei height, GLenum format, - GLenum type, void* pixels, Bool honorPackImageParams = false) { + GLenum type, void* pixels, Bool honorPackImageParams = false, + Bool applyFixedPointReadClamp = true) { ReadbackChannelMapping mapping{}; if (!GetReadbackChannelMapping(format, mapping)) { return false; @@ -4014,6 +4065,45 @@ namespace MobileGL::MG_Backend::DirectGLES { ExpandNarrowWideRead(wide, static_cast(width) * static_cast(height), readChannels, wideType); } + // GL clamps a read from a fixed-point colour buffer to [0,1] (GL_CLAMP_READ_COLOR + // defaults to GL_FIXED_ONLY). Formats the backend substitutes with a floating-point + // one keep the out-of-range value the app stored, so apply the clamp here - a + // GL_R16_SNORM target holding -0.125 must still read back as 0. + // glReadPixels only: GL_CLAMP_READ_COLOR does not apply to glGetTexImage, which + // reaches this helper through the same scratch-framebuffer path. + if (applyFixedPointReadClamp && FramebufferImpl::IsFixedPointFallbackReadAttachment()) { + const SizeT componentSize = GetReadbackComponentSize(wideType); + const SizeT valueCount = componentSize != 0 ? wide.size() / componentSize : 0; + switch (wideType) { + case GL_FLOAT: { + auto* values = reinterpret_cast(wide.data()); + for (SizeT i = 0; i < valueCount; ++i) values[i] = std::clamp(values[i], 0.0f, 1.0f); + break; + } + case GL_HALF_FLOAT: { + auto* values = reinterpret_cast(wide.data()); + for (SizeT i = 0; i < valueCount; ++i) { + values[i] = MG_Util::EncodeFloatToHalfBits( + std::clamp(MG_Util::DecodeHalfBitsToFloat(values[i]), 0.0f, 1.0f)); + } + break; + } + case GL_SHORT: { + // Signed normalized: the negative half is exactly what the clamp removes. + auto* values = reinterpret_cast(wide.data()); + for (SizeT i = 0; i < valueCount; ++i) values[i] = std::max(values[i], 0); + break; + } + case GL_BYTE: { + auto* values = reinterpret_cast(wide.data()); + for (SizeT i = 0; i < valueCount; ++i) values[i] = std::max(values[i], 0); + break; + } + default: + break; + } + } + if (!ReadbackImpl::StoreWideRowsToClient(wide.data(), wideType, width, height, /*sliceCount=*/1, mapping, type, pixels, honorPackImageParams)) { return false; @@ -4393,7 +4483,8 @@ namespace MobileGL::MG_Backend::DirectGLES { return; } if (tempFBOComplete && ReadPixelsViaFormatConversion(0, 0, size.x(), size.y(), format, type, pixels, - applyPackImageParams)) { + applyPackImageParams, + /*applyFixedPointReadClamp=*/false)) { MGLOG_D("GetTexImage: finished via client-format conversion"); return; } diff --git a/MobileGL/MG_Backend/DirectGLES/Managers.cpp b/MobileGL/MG_Backend/DirectGLES/Managers.cpp index c80bfe00..c1d2cc92 100644 --- a/MobileGL/MG_Backend/DirectGLES/Managers.cpp +++ b/MobileGL/MG_Backend/DirectGLES/Managers.cpp @@ -2679,6 +2679,31 @@ namespace MobileGL::MG_Backend::DirectGLES { return false; } + Bool IsFixedPointFallbackReadAttachment() { + const auto& readFBO = + MG_State::pGLContext->GetFramebufferBindingSlot(FramebufferTarget::Read).GetBoundObject(); + if (!readFBO) { + return false; + } + const auto readBuffer = readFBO->GetReadBuffer(); + if (readBuffer < FramebufferAttachmentType::Color0 || readBuffer > FramebufferAttachmentType::Color31) { + return false; + } + // Any signed-normalized attachment, not just the ones currently substituted: + // ES has no GL_CLAMP_READ_COLOR at all, so even a natively stored SNORM buffer + // hands back the negative half that desktop GL clamps away. + const auto& attachmentObject = readFBO->GetAttachment(readBuffer); + if (attachmentObject.IsTexture()) { + const auto& textureObject = attachmentObject.GetTexture(); + return textureObject && IsSnormFormat(textureObject->GetFormat()); + } + if (attachmentObject.IsRenderbuffer()) { + const auto& renderbufferObject = attachmentObject.GetRenderbuffer(); + return renderbufferObject && IsSnormFormat(renderbufferObject->GetInternalFormat()); + } + return false; + } + void BackendFramebufferObject::SyncReadBufferToBackend( const SharedPtr& stateFBOObject) { if (!stateFBOObject) { diff --git a/MobileGL/MG_Backend/DirectGLES/Managers.h b/MobileGL/MG_Backend/DirectGLES/Managers.h index ff68d78c..ccc53916 100644 --- a/MobileGL/MG_Backend/DirectGLES/Managers.h +++ b/MobileGL/MG_Backend/DirectGLES/Managers.h @@ -438,6 +438,13 @@ namespace MobileGL::MG_Backend::DirectGLES { extern StateBackendObjectRegistry g_backendFramebufferObjects; + // True when the read buffer names a fixed-point (norm/snorm) attachment that the + // backend actually stores in a floating-point format. GL clamps a read from a + // fixed-point colour buffer to [0,1] (GL_CLAMP_READ_COLOR defaults to + // GL_FIXED_ONLY); the substituted float storage would not, so the readback path + // has to apply the clamp itself. + Bool IsFixedPointFallbackReadAttachment(); + extern Array g_fboBindVersions; // Tracks the bound FBO's object version (bumped on any attachment/drawbuffer change) // per target: re-attaching textures or changing draw buffers on an already-bound FBO