From 8cf2e2aea998fc0ad6bfd9e521fb9396182d9967 Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Thu, 27 Aug 2026 03:18:15 -0400 Subject: [PATCH] [Fix] (Getter): answer GL_PATCH_DEFAULT_*_LEVEL from the float state in glGetBooleanv and write every component in glGetInteger64v --- MobileGL/MG_Impl/GLImpl/Getter/GL_Getter.cpp | 25 ++++++-- MobileGL/MG_Test/State/RenderStateTest.cpp | 67 ++++++++++++++++++++ 2 files changed, 88 insertions(+), 4 deletions(-) diff --git a/MobileGL/MG_Impl/GLImpl/Getter/GL_Getter.cpp b/MobileGL/MG_Impl/GLImpl/Getter/GL_Getter.cpp index e2549c0e..57e4b70b 100644 --- a/MobileGL/MG_Impl/GLImpl/Getter/GL_Getter.cpp +++ b/MobileGL/MG_Impl/GLImpl/Getter/GL_Getter.cpp @@ -686,6 +686,21 @@ namespace MobileGL::MG_Impl::GLImpl { *params = value != 0.0f ? GL_TRUE : GL_FALSE; return; } + // Float-native state, so GL 4.6 core 2.2.2's "zero becomes FALSE, every other value + // becomes TRUE" has to be applied to the VALUE. Answering these through the integer getter + // below instead - which rounds - reported GL_FALSE for a perfectly non-zero level of 0.25, + // and every other float state in this function already reads through GetFloatv for exactly + // that reason. + case GL_PATCH_DEFAULT_OUTER_LEVEL: + case GL_PATCH_DEFAULT_INNER_LEVEL: { + const GLsizei componentCount = pname == GL_PATCH_DEFAULT_OUTER_LEVEL ? 4 : 2; + GLfloat levels[4] = {}; + GetFloatv(pname, levels); + for (GLsizei i = 0; i < componentCount; ++i) { + params[i] = levels[i] != 0.0f ? GL_TRUE : GL_FALSE; + } + return; + } default: break; } @@ -695,12 +710,8 @@ namespace MobileGL::MG_Impl::GLImpl { switch (pname) { case GL_COLOR_WRITEMASK: case GL_SCISSOR_BOX: - case GL_PATCH_DEFAULT_OUTER_LEVEL: CopyIntsToBooleans(ints, 4, params); return; - case GL_PATCH_DEFAULT_INNER_LEVEL: - CopyIntsToBooleans(ints, 2, params); - return; default: *params = ints[0] ? GL_TRUE : GL_FALSE; return; @@ -1242,12 +1253,17 @@ namespace MobileGL::MG_Impl::GLImpl { GLint ints[4] = {}; GetIntegerv(pname, ints); + // GL 4.6 core 22.1 gives glGetInteger64v the same accepted-pname set as glGetIntegerv, so + // every pname the integer getter answers with several components owes them all here too. + // A pname that reaches the `default:` arm writes params[0] and leaves the caller's other + // components holding whatever they held, with no error to say so. switch (pname) { case GL_BLEND_COLOR: case GL_COLOR_CLEAR_VALUE: case GL_COLOR_WRITEMASK: case GL_SCISSOR_BOX: case GL_VIEWPORT: + case GL_PATCH_DEFAULT_OUTER_LEVEL: for (int i = 0; i < 4; ++i) { params[i] = static_cast(ints[i]); } @@ -1257,6 +1273,7 @@ namespace MobileGL::MG_Impl::GLImpl { case GL_MAX_VIEWPORT_DIMS: case GL_POINT_SIZE_RANGE: case GL_VIEWPORT_BOUNDS_RANGE: + case GL_PATCH_DEFAULT_INNER_LEVEL: params[0] = static_cast(ints[0]); params[1] = static_cast(ints[1]); return; diff --git a/MobileGL/MG_Test/State/RenderStateTest.cpp b/MobileGL/MG_Test/State/RenderStateTest.cpp index 5403a008..9523b515 100644 --- a/MobileGL/MG_Test/State/RenderStateTest.cpp +++ b/MobileGL/MG_Test/State/RenderStateTest.cpp @@ -26,6 +26,8 @@ #include "Includes.h" #include "Init.h" +#include + #include #include #include @@ -695,6 +697,18 @@ TEST_F(RenderStateTest, PatchDefaultLevelsRoundTripThroughEveryGetter) { EXPECT_DOUBLE_EQ(outerDoubles[3], 5.25) << "glGetDoublev must widen all four, not just the first"; ExpectSingleGlError(GL_NO_ERROR); + // glGetInteger64v shares glGetIntegerv's accepted-pname set (GL 4.6 core 22.1), so it owes the + // same component count. Its own table listed neither pname, so three of the four words were + // left holding whatever the caller's buffer held - and no error said so. + GLint64 outerLongs[4] = {9, 9, 9, 9}; + MG_Impl::GLImpl::GetInteger64v(GL_PATCH_DEFAULT_OUTER_LEVEL, outerLongs); + EXPECT_EQ(outerLongs[0], 2); + EXPECT_EQ(outerLongs[3], 5) << "glGetInteger64v must write all four, not just the first"; + GLint64 innerLongs[2] = {9, 9}; + MG_Impl::GLImpl::GetInteger64v(GL_PATCH_DEFAULT_INNER_LEVEL, innerLongs); + EXPECT_EQ(innerLongs[1], 7); + ExpectSingleGlError(GL_NO_ERROR); + // Put the context back where the rest of the binary expects it. const GLfloat defaults4[4] = {1.0f, 1.0f, 1.0f, 1.0f}; const GLfloat defaults2[2] = {1.0f, 1.0f}; @@ -703,6 +717,34 @@ TEST_F(RenderStateTest, PatchDefaultLevelsRoundTripThroughEveryGetter) { DrainPendingGlErrors(); } +// GL 4.6 core 2.2.2: a float state comes back through glGetBooleanv as GL_FALSE only when it is +// zero. Deriving the answer from glGetIntegerv - which rounds - reported GL_FALSE for a level of +// 0.25, which is neither zero nor anything the application asked to be rounded. +TEST_F(RenderStateTest, PatchDefaultLevelsBelowHalfAreStillTrueAsBooleans) { + const GLfloat fractional[4] = {0.25f, 0.0f, 0.4f, 0.25f}; + MG_Impl::GLImpl::PatchParameterfv(GL_PATCH_DEFAULT_OUTER_LEVEL, fractional); + const GLfloat fractionalInner[2] = {0.25f, 0.0f}; + MG_Impl::GLImpl::PatchParameterfv(GL_PATCH_DEFAULT_INNER_LEVEL, fractionalInner); + ExpectSingleGlError(GL_NO_ERROR); + + GLboolean outer[4] = {}; + MG_Impl::GLImpl::GetBooleanv(GL_PATCH_DEFAULT_OUTER_LEVEL, outer); + EXPECT_EQ(outer[0], GL_TRUE) << "0.25 is not zero"; + EXPECT_EQ(outer[1], GL_FALSE) << "0.0 is the one value that is false"; + EXPECT_EQ(outer[2], GL_TRUE); + GLboolean inner[2] = {}; + MG_Impl::GLImpl::GetBooleanv(GL_PATCH_DEFAULT_INNER_LEVEL, inner); + EXPECT_EQ(inner[0], GL_TRUE); + EXPECT_EQ(inner[1], GL_FALSE); + ExpectSingleGlError(GL_NO_ERROR); + + const GLfloat defaults4[4] = {1.0f, 1.0f, 1.0f, 1.0f}; + const GLfloat defaults2[2] = {1.0f, 1.0f}; + MG_Impl::GLImpl::PatchParameterfv(GL_PATCH_DEFAULT_OUTER_LEVEL, defaults4); + MG_Impl::GLImpl::PatchParameterfv(GL_PATCH_DEFAULT_INNER_LEVEL, defaults2); + DrainPendingGlErrors(); +} + TEST_F(RenderStateTest, PatchParameterfvRejectsEveryOtherPname) { const GLfloat levels[4] = {1.0f, 1.0f, 1.0f, 1.0f}; MG_Impl::GLImpl::PatchParameterfv(GL_PATCH_VERTICES, levels); @@ -733,6 +775,31 @@ TEST_F(RenderStateTest, PatchDefaultLevelsAreTreatedAsPipelineState) { EXPECT_GT(state.GetPipelineStateVersion(), settled); } +// glPatchParameterfv accepts NaN by design, and NaN is never equal to itself under IEEE `==`. A +// value-compared redundant-write guard therefore never settles: every re-set of the identical +// tuple bumps the pipeline-state version, and - one level down - DirectGLES's staleness clause +// re-transpiles, re-compiles and re-links the synthesized pass-through stage on every draw. Both +// compare BIT PATTERNS instead, which is what DirectVulkan's module key already hashes. +TEST_F(RenderStateTest, ARedundantNaNPatchLevelWriteSettlesInsteadOfBumpingForever) { + const Float notANumber = std::numeric_limits::quiet_NaN(); + MG_State::GLState::RenderState state; + state.SetPatchDefaultOuterLevel(FloatVec4(notANumber, 1.0f, 1.0f, 1.0f)); + const Uint afterFirst = state.GetPipelineStateVersion(); + + state.SetPatchDefaultOuterLevel(FloatVec4(notANumber, 1.0f, 1.0f, 1.0f)); + EXPECT_EQ(state.GetPipelineStateVersion(), afterFirst) + << "the identical NaN tuple is not a state change"; + + state.SetPatchDefaultInnerLevel(FloatVec2(notANumber, 1.0f)); + const Uint afterInner = state.GetPipelineStateVersion(); + state.SetPatchDefaultInnerLevel(FloatVec2(notANumber, 1.0f)); + EXPECT_EQ(state.GetPipelineStateVersion(), afterInner); + + // A genuinely different tuple still moves, so the guard has not simply gone blind. + state.SetPatchDefaultOuterLevel(FloatVec4(notANumber, 2.0f, 1.0f, 1.0f)); + EXPECT_GT(state.GetPipelineStateVersion(), afterInner); +} + // --- desktop GL_PRIMITIVE_RESTART state -------------------------------------------------------- // // The cap and its index are what a desktop application enables instead of ES's