diff --git a/MobileGL/MG_Impl/GLImpl/Sampler/GL_Sampler.cpp b/MobileGL/MG_Impl/GLImpl/Sampler/GL_Sampler.cpp index effcddbe..b2c2ad2f 100644 --- a/MobileGL/MG_Impl/GLImpl/Sampler/GL_Sampler.cpp +++ b/MobileGL/MG_Impl/GLImpl/Sampler/GL_Sampler.cpp @@ -23,6 +23,17 @@ namespace MobileGL::MG_Impl::GLImpl { return static_cast(*(const GLint*)param); } + // An enum-valued pname through whichever scalar spelling the caller used. GL 4.6 core 8.10 + // lets every scalar sampler pname be set through either type and converts the value; reading + // a glSamplerParameterf argument as a GLint took the IEEE bits of, say, 9728.0f for the enum, + // and the parameter silently became garbage. Truncation, as the validator + // (ValidateSamplerFloatParam) and the texture-side float setter convert. + GLint ReadSamplerEnum(const void* param, Bool isFloat, Bool isUnsignedInteger) { + if (isFloat) return static_cast(*(const GLfloat*)param); + if (isUnsignedInteger) return static_cast(*(const GLuint*)param); + return *(const GLint*)param; + } + // GL_TEXTURE_BORDER_COLOR is the only sampler parameter with more than one component, and it // is also the only one whose meaning depends on WHICH entry point wrote it. Everything else // reads exactly one component and does not care. @@ -122,41 +133,47 @@ namespace MobileGL::MG_Impl::GLImpl { if (!SamplerImpl::ValidateSamplerObject(sampler)) return; if (!ValidateSamplerParameterValue(pname, param, isFloat, isUnsignedInteger)) return; + // Every scalar pname reads the caller's own type and converts it (GL 4.6 core 8.10); + // GL_TEXTURE_BORDER_COLOR alone reads its array, below. + const Bool isScalar = !IsVectorOnlySamplerPname(pname); + const GLint enumParam = isScalar ? ReadSamplerEnum(param, isFloat, isUnsignedInteger) : 0; + const Float scalarParam = isScalar ? ReadSamplerScalar(param, isFloat, isUnsignedInteger) : 0.0f; + using namespace MG_Util; switch (pname) { case GL_TEXTURE_WRAP_S: - samplerObj->SetWrapS(MG_Util::ConvertGLEnumToSamplerWrapMode(*(const GLint*)param)); + samplerObj->SetWrapS(MG_Util::ConvertGLEnumToSamplerWrapMode(enumParam)); break; case GL_TEXTURE_WRAP_T: - samplerObj->SetWrapT(MG_Util::ConvertGLEnumToSamplerWrapMode(*(const GLint*)param)); + samplerObj->SetWrapT(MG_Util::ConvertGLEnumToSamplerWrapMode(enumParam)); break; case GL_TEXTURE_WRAP_R: - samplerObj->SetWrapR(MG_Util::ConvertGLEnumToSamplerWrapMode(*(const GLint*)param)); + samplerObj->SetWrapR(MG_Util::ConvertGLEnumToSamplerWrapMode(enumParam)); break; case GL_TEXTURE_MIN_FILTER: - samplerObj->SetMinFilter(MG_Util::ConvertGLEnumToSamplerFilterMode(*(const GLint*)param)); - samplerObj->SetMipmapMode(MG_Util::ConvertGLEnumToSamplerMipmapMode(*(const GLint*)param)); + samplerObj->SetMinFilter(MG_Util::ConvertGLEnumToSamplerFilterMode(enumParam)); + samplerObj->SetMipmapMode(MG_Util::ConvertGLEnumToSamplerMipmapMode(enumParam)); break; case GL_TEXTURE_MAG_FILTER: - samplerObj->SetMagFilter(MG_Util::ConvertGLEnumToSamplerFilterMode(*(const GLint*)param)); + samplerObj->SetMagFilter(MG_Util::ConvertGLEnumToSamplerFilterMode(enumParam)); break; case GL_TEXTURE_MIN_LOD: - samplerObj->SetLodRange(*(const GLfloat*)param, samplerObj->GetMaxLod()); + samplerObj->SetLodRange(scalarParam, samplerObj->GetMaxLod()); break; case GL_TEXTURE_MAX_LOD: - samplerObj->SetLodRange(samplerObj->GetMinLod(), *(const GLfloat*)param); + samplerObj->SetLodRange(samplerObj->GetMinLod(), scalarParam); break; case GL_TEXTURE_LOD_BIAS: - samplerObj->SetLodBias(*(const GLfloat*)param); + samplerObj->SetLodBias(scalarParam); break; case GL_TEXTURE_MAX_ANISOTROPY_EXT: - samplerObj->SetMaxAnisotropy(ReadSamplerScalar(param, isFloat, isUnsignedInteger)); + samplerObj->SetMaxAnisotropy(scalarParam); break; case GL_TEXTURE_COMPARE_MODE: - samplerObj->SetCompareMode(MG_Util::ConvertGLEnumToSamplerCompareMode(*(const GLint*)param)); + samplerObj->SetCompareMode(MG_Util::ConvertGLEnumToSamplerCompareMode(enumParam)); break; case GL_TEXTURE_COMPARE_FUNC: - samplerObj->SetSamplerCompareFunc(MG_Util::ConvertGLEnumToSamplerCompareFunc(*(const GLint*)param)); + samplerObj->SetSamplerCompareFunc(MG_Util::ConvertGLEnumToSamplerCompareFunc(enumParam)); break; case GL_TEXTURE_BORDER_COLOR: // The only four-component sampler parameter: the caller's form decides which diff --git a/MobileGL/MG_Test/Texture/TextureTest.cpp b/MobileGL/MG_Test/Texture/TextureTest.cpp index 92879be7..204481c7 100644 --- a/MobileGL/MG_Test/Texture/TextureTest.cpp +++ b/MobileGL/MG_Test/Texture/TextureTest.cpp @@ -731,6 +731,56 @@ TEST_F(TextureTest, BorderColorFormChangeBumpsTheVersionEvenWhenTheNumbersDoNotM DrainPendingGlErrors(); } +// GL 4.6 core 8.10: every sampler pname may be set through EITHER scalar type, and the value is +// converted - an enum through glSamplerParameterf, a LOD through glSamplerParameteri. The setter used +// to read the caller's bytes as the pname's own type: glSamplerParameterf(GL_TEXTURE_MIN_FILTER, +// GL_NEAREST) handed the converter the IEEE bits of 9728.0f, the filter became garbage and the sampler +// kept a mipmap filter, so a single-level mutable texture sampled through it was incomplete and read +// as opaque black. That is how a translation layer that funnels every sampler parameter through the +// float entry point (enums cast to float) drew solid black icons. +TEST_F(TextureTest, SamplerParametersSetThroughTheOtherScalarTypeAreConverted) { + GLuint sampler = 0; + MG_Impl::GLImpl::GenSamplers(1, &sampler); + ASSERT_NE(sampler, 0u); + const auto& samplerObject = MG_State::pGLContext->GetSamplerObject(sampler); + ASSERT_NE(samplerObject, nullptr); + + MG_Impl::GLImpl::SamplerParameterf(sampler, GL_TEXTURE_MIN_FILTER, static_cast(GL_NEAREST)); + MG_Impl::GLImpl::SamplerParameterf(sampler, GL_TEXTURE_MAG_FILTER, static_cast(GL_NEAREST)); + MG_Impl::GLImpl::SamplerParameterf(sampler, GL_TEXTURE_WRAP_S, static_cast(GL_CLAMP_TO_EDGE)); + MG_Impl::GLImpl::SamplerParameterf(sampler, GL_TEXTURE_WRAP_T, static_cast(GL_MIRRORED_REPEAT)); + MG_Impl::GLImpl::SamplerParameterf(sampler, GL_TEXTURE_WRAP_R, static_cast(GL_CLAMP_TO_EDGE)); + MG_Impl::GLImpl::SamplerParameterf(sampler, GL_TEXTURE_COMPARE_MODE, + static_cast(GL_COMPARE_REF_TO_TEXTURE)); + MG_Impl::GLImpl::SamplerParameterf(sampler, GL_TEXTURE_COMPARE_FUNC, static_cast(GL_GREATER)); + ASSERT_EQ(MG_Impl::GLImpl::GetError(), GL_NO_ERROR); + EXPECT_EQ(samplerObject->GetMinFilter(), SamplerFilterMode::Nearest); + EXPECT_EQ(samplerObject->GetMipmapMode(), SamplerMipmapMode::None) + << "a NEAREST min filter set through the float spelling kept a mipmap mode"; + EXPECT_EQ(samplerObject->GetMagFilter(), SamplerFilterMode::Nearest); + EXPECT_EQ(samplerObject->GetWrapS(), SamplerWrapMode::ClampToEdge); + EXPECT_EQ(samplerObject->GetWrapT(), SamplerWrapMode::MirroredRepeat); + EXPECT_EQ(samplerObject->GetWrapR(), SamplerWrapMode::ClampToEdge); + EXPECT_EQ(samplerObject->GetCompareMode(), SamplerCompareMode::CompareToTexture); + EXPECT_EQ(samplerObject->GetSamplerCompareFunc(), SamplerCompareFunc::Greater); + GLint minFilter = 0; + MG_Impl::GLImpl::GetSamplerParameteriv(sampler, GL_TEXTURE_MIN_FILTER, &minFilter); + EXPECT_EQ(minFilter, GL_NEAREST); + + // And the other direction: a LOD written through the integer spellings is a number, not bits. + MG_Impl::GLImpl::SamplerParameteri(sampler, GL_TEXTURE_MIN_LOD, -2); + MG_Impl::GLImpl::SamplerParameteri(sampler, GL_TEXTURE_MAX_LOD, 5); + const GLuint bias = 3; + MG_Impl::GLImpl::SamplerParameterIuiv(sampler, GL_TEXTURE_LOD_BIAS, &bias); + ASSERT_EQ(MG_Impl::GLImpl::GetError(), GL_NO_ERROR); + EXPECT_FLOAT_EQ(samplerObject->GetMinLod(), -2.0f); + EXPECT_FLOAT_EQ(samplerObject->GetMaxLod(), 5.0f); + EXPECT_FLOAT_EQ(samplerObject->GetLodBias(), 3.0f); + + MG_Impl::GLImpl::DeleteSamplers(1, &sampler); + DrainPendingGlErrors(); +} + // GL 4.6 core 8.10: the scalar sampler setters take "the value of pname", so a four-component pname is // INVALID_ENUM there. Taking the address of the by-value argument and handing it to the vector path - // which is what these used to do - both lost the error and read twelve bytes past a stack scalar.