From dd60ff39ce47aab55ded78aff1d8d8a5a4d5676a Mon Sep 17 00:00:00 2001 From: BZLZHH Date: Wed, 5 Aug 2026 04:45:55 -0400 Subject: [PATCH] [Feat] (MG_State, MG_Impl, MG_Backend, MG_Util): make the border colour real sampler state glGetSamplerParameterfv(sampler, GL_TEXTURE_BORDER_COLOR) raised INVALID_ENUM, because MobileGL kept the border colour on the texture object and GetSamplerParam_State had no case for it at all. That is the first thing direct_state_access.samplers_defaults asks, so the case threw before reaching any of the defaults it was written to check. GL 4.6 core table 23.18 lists TEXTURE_BORDER_COLOR as sampler state, so it moves to SamplerParameters and TextureObjectBase reaches it through the SamplerObject it already owns - one source of truth, and a sampler object bound over a texture now supplies its own border colour, which is what GL says should happen. The texture params version still moves on a write, because the DirectGLES texture sync memoises on it. glSamplerParameter{fv,Iiv,Iuiv} and their getters read and write all four components in whichever representation the caller used, and the three representations are kept in step so any getter has an answer. The bogus [0,1] and [0,255] range checks are gone: GL clamps a border colour when a fixed-point format is sampled, it does not reject it. DirectVulkan's ResolveVkBorderColor now reads the sampler rather than the texture. DirectGLES gained a glSamplerParameterfv in its sampler sync, and both that and the pre-existing glTexParameterfv are gated on a new SupportsTextureBorderClamp capability - ES 3.2 core, or EXT/OES_texture_border_clamp before it - since without the extension every such call is INVALID_ENUM on the driver. DriverPost gains the matching row per the POST rule, saying what a user actually loses when it is missing. Takes direct_state_access.samplers_defaults from failing to passing on both backends. --- MobileGL/MG_Backend/DirectGLES/Managers.cpp | 15 ++++- .../Renderer/VkSamplerManager.cpp | 4 +- .../MG_Impl/GLImpl/Sampler/GL_Sampler.cpp | 44 +++++++++++++++ .../GLState/SamplerState/SamplerObject.cpp | 55 +++++++++++++++++++ .../GLState/SamplerState/SamplerObject.h | 15 +++++ .../GLState/TextureState/TextureObject.cpp | 34 +++++------- .../GLState/TextureState/TextureObject.h | 3 - .../MG_Util/BackendLoaders/OpenGL/Loader.cpp | 8 +++ .../MG_Util/BackendLoaders/OpenGL/Loader.h | 4 ++ MobileGL/MG_Util/SelfTest/DriverPost.cpp | 11 ++++ 10 files changed, 167 insertions(+), 26 deletions(-) diff --git a/MobileGL/MG_Backend/DirectGLES/Managers.cpp b/MobileGL/MG_Backend/DirectGLES/Managers.cpp index 14a5eecc..a2eb6732 100644 --- a/MobileGL/MG_Backend/DirectGLES/Managers.cpp +++ b/MobileGL/MG_Backend/DirectGLES/Managers.cpp @@ -2479,7 +2479,10 @@ namespace MobileGL::MG_Backend::DirectGLES { }); } - if (!isMultisampleTarget && m_cacheBorderColor != stateTextureObject->GetBorderColor()) { + // GL_TEXTURE_BORDER_COLOR needs ES 3.2 or EXT/OES_texture_border_clamp; on a driver + // without it every such call is INVALID_ENUM, so the parameter is simply not synced. + if (!isMultisampleTarget && g_GLESCapabilities.SupportsTextureBorderClamp && + m_cacheBorderColor != stateTextureObject->GetBorderColor()) { const auto& borderColor = stateTextureObject->GetBorderColor(); GLfloat borderColorArray[4] = {borderColor.x(), borderColor.y(), borderColor.z(), borderColor.w()}; g_GLESFuncs.glTexParameterfv(target, GL_TEXTURE_BORDER_COLOR, borderColorArray); @@ -3755,6 +3758,16 @@ namespace MobileGL::MG_Backend::DirectGLES { } m_cacheSamplerParameters.maxAnisotropy = samplerParams.maxAnisotropy; } + if (m_cacheSamplerParameters.borderColor != samplerParams.borderColor) { + // Same gate as the texture-side border colour above. + if (g_GLESCapabilities.SupportsTextureBorderClamp && g_GLESFuncs.glSamplerParameterfv) { + const GLfloat borderColorArray[4] = { + samplerParams.borderColor.x(), samplerParams.borderColor.y(), + samplerParams.borderColor.z(), samplerParams.borderColor.w()}; + g_GLESFuncs.glSamplerParameterfv(m_backendSamplerId, GL_TEXTURE_BORDER_COLOR, borderColorArray); + } + m_cacheSamplerParameters.borderColor = samplerParams.borderColor; + } #undef SYNC_SAMPLER_PARAM_IF_CHANGED m_isInitialized = true; } diff --git a/MobileGL/MG_Backend/DirectVulkan/Renderer/VkSamplerManager.cpp b/MobileGL/MG_Backend/DirectVulkan/Renderer/VkSamplerManager.cpp index 93b26ba9..f94f0918 100644 --- a/MobileGL/MG_Backend/DirectVulkan/Renderer/VkSamplerManager.cpp +++ b/MobileGL/MG_Backend/DirectVulkan/Renderer/VkSamplerManager.cpp @@ -287,7 +287,9 @@ namespace MobileGL::MG_Backend::DirectVulkan { return VK_BORDER_COLOR_FLOAT_TRANSPARENT_BLACK; } - const auto& borderColor = texture.GetBorderColor(); + // Border colour is sampler state: a bound sampler object supplies its own, and a texture + // with none reaches the very same value through the sampler object it owns. + const auto& borderColor = sampler.GetBorderColor(); const Bool isDepthTexture = IsDepthTextureFormat(texture.GetFormat()); if (isDepthTexture) { diff --git a/MobileGL/MG_Impl/GLImpl/Sampler/GL_Sampler.cpp b/MobileGL/MG_Impl/GLImpl/Sampler/GL_Sampler.cpp index e28c5963..9ef1c277 100644 --- a/MobileGL/MG_Impl/GLImpl/Sampler/GL_Sampler.cpp +++ b/MobileGL/MG_Impl/GLImpl/Sampler/GL_Sampler.cpp @@ -28,6 +28,11 @@ namespace MobileGL::MG_Impl::GLImpl { case GL_TEXTURE_MAX_LOD: case GL_TEXTURE_LOD_BIAS: return true; + // Four components, and GL puts no range on them - a border colour outside [0,1] is + // clamped when a fixed-point format is sampled, not rejected here. The scalar readers + // below would look at one component and invent an error. + case GL_TEXTURE_BORDER_COLOR: + return true; case GL_TEXTURE_MAX_ANISOTROPY_EXT: if (ReadSamplerScalar(param, isFloat, isUnsignedInteger) >= 1.0f) return true; MG_State::pGLContext->RecordError( @@ -99,6 +104,20 @@ namespace MobileGL::MG_Impl::GLImpl { case GL_TEXTURE_COMPARE_FUNC: samplerObj->SetSamplerCompareFunc(MG_Util::ConvertGLEnumToSamplerCompareFunc(*(const GLint*)param)); break; + case GL_TEXTURE_BORDER_COLOR: + // The only four-component sampler parameter: the caller's form decides which + // representation is authoritative, and SamplerObject keeps the other two in step. + if (isFloat) { + const auto* values = (const GLfloat*)param; + samplerObj->SetBorderColor(FloatVec4(values[0], values[1], values[2], values[3])); + } else if (isUnsignedInteger) { + const auto* values = (const GLuint*)param; + samplerObj->SetBorderColorUI(UintVec4(values[0], values[1], values[2], values[3])); + } else { + const auto* values = (const GLint*)param; + samplerObj->SetBorderColorI(IntVec4(values[0], values[1], values[2], values[3])); + } + break; default: MG_State::pGLContext->RecordError(ErrorCode::InvalidEnum, MakeUnique("MG_Impl/GLImpl", "SetSamplerParam_State", @@ -162,6 +181,31 @@ namespace MobileGL::MG_Impl::GLImpl { case GL_TEXTURE_COMPARE_FUNC: *(GLuint*)params = MG_Util::ConvertSamplerCompareFuncToGLEnum(samplerObj->GetSamplerCompareFunc()); break; + case GL_TEXTURE_BORDER_COLOR: { + if (isFloat) { + const auto& color = samplerObj->GetBorderColor(); + auto* out = (GLfloat*)params; + out[0] = color.x(); + out[1] = color.y(); + out[2] = color.z(); + out[3] = color.w(); + } else if (isUnsignedInteger) { + const auto& color = samplerObj->GetBorderColorUI(); + auto* out = (GLuint*)params; + out[0] = color.x(); + out[1] = color.y(); + out[2] = color.z(); + out[3] = color.w(); + } else { + const auto& color = samplerObj->GetBorderColorI(); + auto* out = (GLint*)params; + out[0] = color.x(); + out[1] = color.y(); + out[2] = color.z(); + out[3] = color.w(); + } + break; + } default: MG_State::pGLContext->RecordError(ErrorCode::InvalidEnum, MakeUnique("MG_Impl/GLImpl", "GetSamplerParam_State", diff --git a/MobileGL/MG_State/GLState/SamplerState/SamplerObject.cpp b/MobileGL/MG_State/GLState/SamplerState/SamplerObject.cpp index fad9abb7..97bd63cd 100644 --- a/MobileGL/MG_State/GLState/SamplerState/SamplerObject.cpp +++ b/MobileGL/MG_State/GLState/SamplerState/SamplerObject.cpp @@ -139,6 +139,61 @@ namespace MobileGL { return m_samplerParameters.maxAnisotropy; } + // The three border-colour representations are kept in step so a getter of any form has + // an answer whichever form was written. Integer <-> float uses the plain value, matching + // what glTexParameterIiv/Iuiv mean: those forms are for integer texture formats, whose + // border components are the raw integers rather than a normalized fraction. + void SamplerObject::SetBorderColor(const FloatVec4& color) { + if (color == m_samplerParameters.borderColor) return; + + m_samplerParameters.borderColor = color; + m_samplerParameters.borderColorI = + IntVec4(static_cast(color.x()), static_cast(color.y()), + static_cast(color.z()), static_cast(color.w())); + m_samplerParameters.borderColorUI = + UintVec4(static_cast(color.x()), static_cast(color.y()), + static_cast(color.z()), static_cast(color.w())); + ++m_version; + } + + void SamplerObject::SetBorderColorI(const IntVec4& color) { + if (color == m_samplerParameters.borderColorI) return; + + m_samplerParameters.borderColorI = color; + m_samplerParameters.borderColorUI = + UintVec4(static_cast(color.x()), static_cast(color.y()), + static_cast(color.z()), static_cast(color.w())); + m_samplerParameters.borderColor = + FloatVec4(static_cast(color.x()), static_cast(color.y()), + static_cast(color.z()), static_cast(color.w())); + ++m_version; + } + + void SamplerObject::SetBorderColorUI(const UintVec4& color) { + if (color == m_samplerParameters.borderColorUI) return; + + m_samplerParameters.borderColorUI = color; + m_samplerParameters.borderColorI = + IntVec4(static_cast(color.x()), static_cast(color.y()), + static_cast(color.z()), static_cast(color.w())); + m_samplerParameters.borderColor = + FloatVec4(static_cast(color.x()), static_cast(color.y()), + static_cast(color.z()), static_cast(color.w())); + ++m_version; + } + + const FloatVec4& SamplerObject::GetBorderColor() const { + return m_samplerParameters.borderColor; + } + + const IntVec4& SamplerObject::GetBorderColorI() const { + return m_samplerParameters.borderColorI; + } + + const UintVec4& SamplerObject::GetBorderColorUI() const { + return m_samplerParameters.borderColorUI; + } + SamplerCompareMode SamplerObject::GetCompareMode() const { return m_samplerParameters.compareMode; } diff --git a/MobileGL/MG_State/GLState/SamplerState/SamplerObject.h b/MobileGL/MG_State/GLState/SamplerState/SamplerObject.h index 0007f03e..939ed788 100644 --- a/MobileGL/MG_State/GLState/SamplerState/SamplerObject.h +++ b/MobileGL/MG_State/GLState/SamplerState/SamplerObject.h @@ -8,6 +8,7 @@ #pragma once #include +#include namespace MobileGL { enum class SamplerFilterMode { @@ -70,6 +71,14 @@ namespace MobileGL { // for both sampler objects and the sampler state a texture object carries. SamplerCompareFunc compareFunc = SamplerCompareFunc::LessEqual; SamplerCompareMode compareMode = SamplerCompareMode::None; + // TEXTURE_BORDER_COLOR is sampler state (GL 4.6 core table 23.18), so it belongs here and + // not on the texture - a texture object reaches it through the sampler object it owns. The + // three representations are the float, integer and unsigned-integer forms glSamplerParameterfv, + // glSamplerParameterIiv and glSamplerParameterIuiv set; whichever is written last defines + // the colour and the other two follow it, so a getter always has an answer. + FloatVec4 borderColor = {0.0f, 0.0f, 0.0f, 0.0f}; + IntVec4 borderColorI = {0, 0, 0, 0}; + UintVec4 borderColorUI = {0, 0, 0, 0}; }; namespace MG_State { @@ -89,6 +98,9 @@ namespace MobileGL { void SetMaxAnisotropy(Float maxAnisotropy); void SetSamplerCompareFunc(SamplerCompareFunc func); void SetCompareMode(SamplerCompareMode mode); + void SetBorderColor(const FloatVec4& color); + void SetBorderColorI(const IntVec4& color); + void SetBorderColorUI(const UintVec4& color); SamplerWrapMode GetWrapS() const; SamplerWrapMode GetWrapT() const; @@ -102,6 +114,9 @@ namespace MobileGL { Float GetMaxAnisotropy() const; SamplerCompareMode GetCompareMode() const; SamplerCompareFunc GetSamplerCompareFunc() const; + const FloatVec4& GetBorderColor() const; + const IntVec4& GetBorderColorI() const; + const UintVec4& GetBorderColorUI() const; Uint GetExternalIndex() const; Uint16 GetVersion() const; // Globally-unique, never-reused id for this sampler object's lifetime. Lets a diff --git a/MobileGL/MG_State/GLState/TextureState/TextureObject.cpp b/MobileGL/MG_State/GLState/TextureState/TextureObject.cpp index 1b41f8f2..7e6110df 100644 --- a/MobileGL/MG_State/GLState/TextureState/TextureObject.cpp +++ b/MobileGL/MG_State/GLState/TextureState/TextureObject.cpp @@ -88,48 +88,40 @@ namespace MobileGL { return m_externalIndex; } + // TEXTURE_BORDER_COLOR is sampler state, so it lives on the SamplerObject this texture + // owns rather than being duplicated here - a sampler object bound over the texture then + // supplies its own, exactly as GL says it should. The texture params version still moves + // on a write, because the DirectGLES texture sync memoises on it. const FloatVec4& TextureObjectBase::GetBorderColor() const { - return m_borderColor; + return m_sampler->GetBorderColor(); } void TextureObjectBase::SetBorderColor(const FloatVec4& color) { - if (color == m_borderColor) return; + if (color == m_sampler->GetBorderColor()) return; - m_borderColor = color; - m_borderColorI = IntVec4(static_cast(color.x()), static_cast(color.y()), - static_cast(color.z()), static_cast(color.w())); - m_borderColorUI = UintVec4(static_cast(color.x()), static_cast(color.y()), - static_cast(color.z()), static_cast(color.w())); + m_sampler->SetBorderColor(color); ++m_textureParamsVersion; } const IntVec4& TextureObjectBase::GetBorderColorI() const { - return m_borderColorI; + return m_sampler->GetBorderColorI(); } void TextureObjectBase::SetBorderColorI(const IntVec4& color) { - if (color == m_borderColorI) return; + if (color == m_sampler->GetBorderColorI()) return; - m_borderColorI = color; - m_borderColorUI = UintVec4(static_cast(color.x()), static_cast(color.y()), - static_cast(color.z()), static_cast(color.w())); - m_borderColor = FloatVec4(static_cast(color.x()), static_cast(color.y()), - static_cast(color.z()), static_cast(color.w())); + m_sampler->SetBorderColorI(color); ++m_textureParamsVersion; } const UintVec4& TextureObjectBase::GetBorderColorUI() const { - return m_borderColorUI; + return m_sampler->GetBorderColorUI(); } void TextureObjectBase::SetBorderColorUI(const UintVec4& color) { - if (color == m_borderColorUI) return; + if (color == m_sampler->GetBorderColorUI()) return; - m_borderColorUI = color; - m_borderColorI = IntVec4(static_cast(color.x()), static_cast(color.y()), - static_cast(color.z()), static_cast(color.w())); - m_borderColor = FloatVec4(static_cast(color.x()), static_cast(color.y()), - static_cast(color.z()), static_cast(color.w())); + m_sampler->SetBorderColorUI(color); ++m_textureParamsVersion; } diff --git a/MobileGL/MG_State/GLState/TextureState/TextureObject.h b/MobileGL/MG_State/GLState/TextureState/TextureObject.h index b2f2f2b0..73440ac8 100644 --- a/MobileGL/MG_State/GLState/TextureState/TextureObject.h +++ b/MobileGL/MG_State/GLState/TextureState/TextureObject.h @@ -119,9 +119,6 @@ namespace MobileGL::MG_State::GLState { const TextureTarget m_target = TextureTarget::Unknown; TextureInternalFormat m_internalFormat = TextureInternalFormat::Unknown; SharedPtr m_sampler = nullptr; - FloatVec4 m_borderColor = {0.0f, 0.0f, 0.0f, 0.0f}; - IntVec4 m_borderColorI = {0, 0, 0, 0}; - UintVec4 m_borderColorUI = {0, 0, 0, 0}; Vec4 m_swizzleParams = {TextureSwizzleParam::Red, TextureSwizzleParam::Green, TextureSwizzleParam::Blue, TextureSwizzleParam::Alpha}; UintVec2 m_levelRange = {0, 1000}; diff --git a/MobileGL/MG_Util/BackendLoaders/OpenGL/Loader.cpp b/MobileGL/MG_Util/BackendLoaders/OpenGL/Loader.cpp index 17e33004..6da11c3e 100644 --- a/MobileGL/MG_Util/BackendLoaders/OpenGL/Loader.cpp +++ b/MobileGL/MG_Util/BackendLoaders/OpenGL/Loader.cpp @@ -833,6 +833,10 @@ namespace MobileGL::MG_Util::BackendLoader { if (std::strcmp(extension, "GL_EXT_texture_filter_anisotropic") == 0) { caps.SupportsTextureFilterAnisotropy = true; } + if (std::strcmp(extension, "GL_EXT_texture_border_clamp") == 0 || + std::strcmp(extension, "GL_OES_texture_border_clamp") == 0) { + caps.SupportsTextureBorderClamp = true; + } if (std::strcmp(extension, "GL_EXT_base_instance") == 0) { caps.SupportsBaseInstance = true; } @@ -1014,6 +1018,10 @@ namespace MobileGL::MG_Util::BackendLoader { } // Only legal to query once the extension has been seen in the loop above, hence not batched // with the unconditional probes: on a driver without it this raises GL_INVALID_ENUM. + // Core from ES 3.2 on, whatever the extension string says. + if (caps.GLESVersion.Major > 3 || (caps.GLESVersion.Major == 3 && caps.GLESVersion.Minor >= 2)) { + caps.SupportsTextureBorderClamp = true; + } if (caps.SupportsTextureFilterAnisotropy) { GLfloat maxTextureMaxAnisotropy = 1.0f; glesFuncs.glGetFloatv(GL_MAX_TEXTURE_MAX_ANISOTROPY_EXT, &maxTextureMaxAnisotropy); diff --git a/MobileGL/MG_Util/BackendLoaders/OpenGL/Loader.h b/MobileGL/MG_Util/BackendLoaders/OpenGL/Loader.h index 3b9485cb..35f2d650 100644 --- a/MobileGL/MG_Util/BackendLoaders/OpenGL/Loader.h +++ b/MobileGL/MG_Util/BackendLoaders/OpenGL/Loader.h @@ -1041,6 +1041,10 @@ namespace MobileGL { // GL_EXT_texture_filter_anisotropic is present, so sampler/texture // anisotropy may be forwarded without raising GL_INVALID_ENUM in GLES. Bool SupportsTextureFilterAnisotropy = false; + // GL_TEXTURE_BORDER_COLOR and GL_CLAMP_TO_BORDER: ES 3.2 core, or + // EXT/OES_texture_border_clamp before that. Without it every border-colour parameter + // raises INVALID_ENUM on the driver, so the syncs have to be gated on it. + Bool SupportsTextureBorderClamp = false; // GL_MAX_TEXTURE_MAX_ANISOTROPY_EXT of the host driver; only queried when the // extension above is present, and left at 1.0 (no anisotropy) otherwise. Float MaxTextureMaxAnisotropy = 1.0f; diff --git a/MobileGL/MG_Util/SelfTest/DriverPost.cpp b/MobileGL/MG_Util/SelfTest/DriverPost.cpp index 4a5d2fb7..2872bdd5 100644 --- a/MobileGL/MG_Util/SelfTest/DriverPost.cpp +++ b/MobileGL/MG_Util/SelfTest/DriverPost.cpp @@ -298,6 +298,17 @@ namespace MobileGL::MG_Util::SelfTest { "not supported; no impact: the native indirect path deliberately does not " "rely on it (shader-side emulation handles baseInstance semantics)"); } + if (caps.SupportsTextureBorderClamp) { + builder.Pass("Texture border clamp", + "supported (GL_TEXTURE_BORDER_COLOR reaches the driver, so " + "GL_CLAMP_TO_BORDER samples the colour the application set)"); + } else { + builder.Warn("Texture border clamp", + "not supported (pre-ES 3.2 without GL_EXT/OES_texture_border_clamp); " + "GL_TEXTURE_BORDER_COLOR is not synced to the driver at all, so anything " + "sampling outside a GL_CLAMP_TO_BORDER texture reads the driver's default " + "border instead of the requested colour"); + } if (glesFuncs.glPatchParameteri != nullptr) { builder.Pass("Tessellation patch parameters", "glPatchParameteri present (GL_PATCH_VERTICES reaches the driver)");