From 95876d9d8c8fc60f61ea00325faea95eb98ca4d8 Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Fri, 10 Jul 2026 21:21:40 -0400 Subject: [PATCH] [Feat] (MG_Impl/GLImpl, MG_State): implement glClampColor and glPolygonMode Fill the two empty // TODO state handlers with GL 3.3 Core-conformant behavior, backed by new RenderState fields and glGet* read-back. glClampColor: - Accept only GL_CLAMP_READ_COLOR (compat GL_CLAMP_VERTEX/FRAGMENT_COLOR rejected); clamp is one of GL_TRUE / GL_FALSE / GL_FIXED_ONLY. Note the Khronos man page wrongly omits GL_FIXED_ONLY from the accepted set, but it is legal AND the default, so it is accepted here. - Default GL_FIXED_ONLY; both error paths are GL_INVALID_ENUM with no state change. glGetIntegerv returns the raw tri-state enum; GetFloatv/ GetDoublev widen it and GetBooleanv converts nonzero to GL_TRUE via the existing fall-through, so one GetIntegerv case serves every getter. glPolygonMode: - Core accepts only face == GL_FRONT_AND_BACK (GL_FRONT/GL_BACK were removed in 3.1 core); mode is GL_POINT / GL_LINE / GL_FILL. Both errors are GL_INVALID_ENUM with no state change. - Keep separate front/back slots so GL_POLYGON_MODE round-trips its two values (identical under a core context). The raster effect (VkPolygonMode + fillModeNonSolid) remains a backend follow-up; this is the state layer. Tests: two RenderStateSanity round-trips; the glClampColor GL_FIXED_ONLY acceptance assertion is mutation-verified (rejecting it fails the test). Full SanityTest sweep green (29/29). --- MobileGL/MG_Impl/GLImpl/Getter/GL_Getter.cpp | 10 ++- .../GLImpl/RenderState/GL_RenderState.cpp | 48 +++++++++- MobileGL/MG_State/GLState/Core.cpp | 20 +++++ MobileGL/MG_State/GLState/Core.h | 5 ++ .../GLState/RenderState/RenderState.cpp | 25 ++++++ .../GLState/RenderState/RenderState.h | 15 ++++ MobileGL/MG_Test/SanityTest.cpp | 90 +++++++++++++++++++ 7 files changed, 209 insertions(+), 4 deletions(-) diff --git a/MobileGL/MG_Impl/GLImpl/Getter/GL_Getter.cpp b/MobileGL/MG_Impl/GLImpl/Getter/GL_Getter.cpp index 379d9916..1a5acb17 100644 --- a/MobileGL/MG_Impl/GLImpl/Getter/GL_Getter.cpp +++ b/MobileGL/MG_Impl/GLImpl/Getter/GL_Getter.cpp @@ -952,6 +952,12 @@ namespace MobileGL::MG_Impl::GLImpl { *params = static_cast(MG_Util::ConvertBlendFactorToGLEnum(srcRGB)); return; } + case GL_CLAMP_READ_COLOR: + // Tri-state enum (GL_TRUE / GL_FALSE / GL_FIXED_ONLY). glGetIntegerv returns the raw + // enum; GetFloatv/GetDoublev widen it and GetBooleanv converts nonzero to GL_TRUE, so + // this single case serves every getter flavor. + *params = static_cast(MG_State::pGLContext->GetClampReadColor()); + return; case GL_COLOR_CLEAR_VALUE: { const FloatVec4& clearColor = MG_State::pGLContext->GetClearColor(); params[0] = static_cast(clearColor.x()); @@ -1364,8 +1370,8 @@ namespace MobileGL::MG_Impl::GLImpl { *params = static_cast(MG_State::pGLContext->GetPointSize()); return; case GL_POLYGON_MODE: - params[0] = GL_FILL; - params[1] = GL_FILL; + params[0] = static_cast(MG_State::pGLContext->GetPolygonModeFront()); + params[1] = static_cast(MG_State::pGLContext->GetPolygonModeBack()); return; case GL_POLYGON_OFFSET_FACTOR: *params = static_cast(MG_State::pGLContext->GetPolygonOffsetFactor()); diff --git a/MobileGL/MG_Impl/GLImpl/RenderState/GL_RenderState.cpp b/MobileGL/MG_Impl/GLImpl/RenderState/GL_RenderState.cpp index db2e1692..333298e7 100644 --- a/MobileGL/MG_Impl/GLImpl/RenderState/GL_RenderState.cpp +++ b/MobileGL/MG_Impl/GLImpl/RenderState/GL_RenderState.cpp @@ -193,7 +193,28 @@ namespace MobileGL::MG_Impl::GLImpl { } void PolygonMode_State(GLenum face, GLenum mode) { - // TODO: implement + // GL 3.3 core: separate front/back polygon modes were removed in 3.1, so the only legal + // face is GL_FRONT_AND_BACK. GL_FRONT / GL_BACK must be rejected (some desktop drivers + // leniently accept them, but that is non-conformant). Both errors are GL_INVALID_ENUM and + // leave state untouched. + if (face != GL_FRONT_AND_BACK) { + MG_State::pGLContext->RecordError( + ErrorCode::InvalidEnum, + MakeUnique("MG_Impl/GLImpl", __func__, + "glPolygonMode face must be GL_FRONT_AND_BACK in the core profile; got " + + MG_Util::ConvertGLEnumToString(face) + ".")); + return; + } + if (mode != GL_POINT && mode != GL_LINE && mode != GL_FILL) { + MG_State::pGLContext->RecordError( + ErrorCode::InvalidEnum, + MakeUnique("MG_Impl/GLImpl", __func__, + "glPolygonMode mode must be GL_POINT, GL_LINE, or GL_FILL; got " + + MG_Util::ConvertGLEnumToString(mode) + ".")); + return; + } + // Core sets both faces together; keep two slots so GL_POLYGON_MODE round-trips its two values. + MG_State::pGLContext->SetPolygonMode(mode, mode); } void PointSize_State(GLfloat size) { @@ -503,7 +524,30 @@ namespace MobileGL::MG_Impl::GLImpl { } void ClampColor_State(GLenum target, GLenum clamp) { - // TODO: implement + // GL 3.3 core: the only legal target is GL_CLAMP_READ_COLOR. The compatibility-only + // GL_CLAMP_VERTEX_COLOR / GL_CLAMP_FRAGMENT_COLOR were removed from the core profile and + // must be rejected. + if (target != GL_CLAMP_READ_COLOR) { + MG_State::pGLContext->RecordError( + ErrorCode::InvalidEnum, + MakeUnique("MG_Impl/GLImpl", __func__, + "glClampColor target must be GL_CLAMP_READ_COLOR in the core profile; " + "got " + + MG_Util::ConvertGLEnumToString(target) + ".")); + return; + } + // clamp must be one of GL_TRUE, GL_FALSE, or GL_FIXED_ONLY. NOTE: the Khronos man page's + // Errors section wrongly omits GL_FIXED_ONLY, but the spec lists it as legal AND it is the + // default value, so it must be accepted here. + if (clamp != GL_TRUE && clamp != GL_FALSE && clamp != GL_FIXED_ONLY) { + MG_State::pGLContext->RecordError( + ErrorCode::InvalidEnum, + MakeUnique("MG_Impl/GLImpl", __func__, + "glClampColor clamp must be GL_TRUE, GL_FALSE, or GL_FIXED_ONLY; got " + + MG_Util::ConvertGLEnumToString(clamp) + ".")); + return; + } + MG_State::pGLContext->SetClampReadColor(clamp); } void BlendFuncSeparate_State(GLenum sfactorRGB, GLenum dfactorRGB, GLenum sfactorAlpha, GLenum dfactorAlpha) { diff --git a/MobileGL/MG_State/GLState/Core.cpp b/MobileGL/MG_State/GLState/Core.cpp index 1b7ffee2..b335b58c 100644 --- a/MobileGL/MG_State/GLState/Core.cpp +++ b/MobileGL/MG_State/GLState/Core.cpp @@ -352,6 +352,26 @@ namespace MobileGL::MG_State { return m_renderState.GetPointSpriteCoordOrigin(); } + void GLContext::SetClampReadColor(GLenum clamp) { + m_renderState.SetClampReadColor(clamp); + } + + GLenum GLContext::GetClampReadColor() const { + return m_renderState.GetClampReadColor(); + } + + void GLContext::SetPolygonMode(GLenum front, GLenum back) { + m_renderState.SetPolygonMode(front, back); + } + + GLenum GLContext::GetPolygonModeFront() const { + return m_renderState.GetPolygonModeFront(); + } + + GLenum GLContext::GetPolygonModeBack() const { + return m_renderState.GetPolygonModeBack(); + } + void GLContext::SetPointSize(Float size) { m_renderState.SetPointSize(size); } diff --git a/MobileGL/MG_State/GLState/Core.h b/MobileGL/MG_State/GLState/Core.h index c6559214..5c6429a3 100644 --- a/MobileGL/MG_State/GLState/Core.h +++ b/MobileGL/MG_State/GLState/Core.h @@ -131,6 +131,11 @@ namespace MobileGL { Float GetPointFadeThresholdSize() const; void SetPointSpriteCoordOrigin(GLenum origin); GLenum GetPointSpriteCoordOrigin() const; + void SetClampReadColor(GLenum clamp); + GLenum GetClampReadColor() const; + void SetPolygonMode(GLenum front, GLenum back); + GLenum GetPolygonModeFront() const; + GLenum GetPolygonModeBack() const; void SetCapability(CapabilityInput cap, Bool enabled); Bool IsCapabilityEnabled(CapabilityInput cap) const; void SetCapabilityIndexed(CapabilityInput cap, Uint index, Bool enabled); diff --git a/MobileGL/MG_State/GLState/RenderState/RenderState.cpp b/MobileGL/MG_State/GLState/RenderState/RenderState.cpp index 6488b50f..b9a6632e 100644 --- a/MobileGL/MG_State/GLState/RenderState/RenderState.cpp +++ b/MobileGL/MG_State/GLState/RenderState/RenderState.cpp @@ -103,6 +103,31 @@ namespace MobileGL { return m_parameters.PointSpriteCoordOrigin; } + void RenderState::SetClampReadColor(GLenum clamp) { + if (m_parameters.ClampReadColor == clamp) return; + m_parameters.ClampReadColor = clamp; + ++m_version; + } + + GLenum RenderState::GetClampReadColor() const { + return m_parameters.ClampReadColor; + } + + void RenderState::SetPolygonMode(GLenum front, GLenum back) { + if (m_parameters.PolygonModeFront == front && m_parameters.PolygonModeBack == back) return; + m_parameters.PolygonModeFront = front; + m_parameters.PolygonModeBack = back; + ++m_version; + } + + GLenum RenderState::GetPolygonModeFront() const { + return m_parameters.PolygonModeFront; + } + + GLenum RenderState::GetPolygonModeBack() const { + return m_parameters.PolygonModeBack; + } + void RenderState::SetPointSize(Float size) { if (m_parameters.PointSize == size) return; diff --git a/MobileGL/MG_State/GLState/RenderState/RenderState.h b/MobileGL/MG_State/GLState/RenderState/RenderState.h index 0bfc922b..3e8b476c 100644 --- a/MobileGL/MG_State/GLState/RenderState/RenderState.h +++ b/MobileGL/MG_State/GLState/RenderState/RenderState.h @@ -260,6 +260,14 @@ namespace MobileGL { Float PointFadeThresholdSize = 1.0f; GLenum PointSpriteCoordOrigin = GL_UPPER_LEFT; + // Color clamping (glClampColor). Core profile exposes only GL_CLAMP_READ_COLOR. + GLenum ClampReadColor = GL_FIXED_ONLY; + + // Polygon rasterization mode (glPolygonMode). Core profile sets front and back together, + // but GL_POLYGON_MODE still reports both slots, so keep them separate for a faithful query. + GLenum PolygonModeFront = GL_FILL; + GLenum PolygonModeBack = GL_FILL; + // Scissor Bool ColorLogicOpEnabled = false; Bool DebugOutputEnabled = false; @@ -310,6 +318,13 @@ namespace MobileGL { Float GetPointFadeThresholdSize() const; void SetPointSpriteCoordOrigin(GLenum origin); GLenum GetPointSpriteCoordOrigin() const; + // Color clamping (glClampColor). Core profile has only GL_CLAMP_READ_COLOR. + void SetClampReadColor(GLenum clamp); + GLenum GetClampReadColor() const; + // Polygon mode (glPolygonMode). Core sets both faces together; the query reports both. + void SetPolygonMode(GLenum front, GLenum back); + GLenum GetPolygonModeFront() const; + GLenum GetPolygonModeBack() const; // Capabilities void SetCapability(CapabilityInput cap, Bool enabled); diff --git a/MobileGL/MG_Test/SanityTest.cpp b/MobileGL/MG_Test/SanityTest.cpp index 30549280..4f403d19 100644 --- a/MobileGL/MG_Test/SanityTest.cpp +++ b/MobileGL/MG_Test/SanityTest.cpp @@ -761,3 +761,93 @@ TEST(RenderStateSanity, GetDoublevMatchesGetFloatvWidened) { MG_State::pGLContext.reset(); } + +TEST(RenderStateSanity, ClampColorStoresAndReadsBack) { + using namespace MobileGL; + using namespace MobileGL::MG_Impl::GLImpl; + MG_State::pGLContext = MakeUnique(); + + // Default GL_CLAMP_READ_COLOR is GL_FIXED_ONLY (NOT GL_TRUE/GL_FALSE). glGetIntegerv is the only + // getter that faithfully round-trips the tri-state. + GLint value = -1; + GetIntegerv(GL_CLAMP_READ_COLOR, &value); + EXPECT_EQ(value, GL_FIXED_ONLY); + + // All three legal clamp values round-trip. + ClampColor(GL_CLAMP_READ_COLOR, GL_TRUE); + EXPECT_EQ(GetError(), GL_NO_ERROR); + GetIntegerv(GL_CLAMP_READ_COLOR, &value); + EXPECT_EQ(value, GL_TRUE); + + ClampColor(GL_CLAMP_READ_COLOR, GL_FALSE); + GetIntegerv(GL_CLAMP_READ_COLOR, &value); + EXPECT_EQ(value, GL_FALSE); + + // GL_FIXED_ONLY MUST be accepted: the Khronos man page's Errors section wrongly omits it, but the + // spec lists it as legal and it is the default. A man-page-faithful implementation would reject + // this call -- this assertion is the guard against that regression. + ClampColor(GL_CLAMP_READ_COLOR, GL_FIXED_ONLY); + EXPECT_EQ(GetError(), GL_NO_ERROR); + GetIntegerv(GL_CLAMP_READ_COLOR, &value); + EXPECT_EQ(value, GL_FIXED_ONLY); + + // glGetBooleanv converts nonzero to GL_TRUE, so GL_FIXED_ONLY reads back as GL_TRUE (and cannot be + // distinguished from GL_TRUE); only GL_FALSE reads GL_FALSE. + GLboolean b = GL_FALSE; + GetBooleanv(GL_CLAMP_READ_COLOR, &b); + EXPECT_EQ(b, GL_TRUE); + ClampColor(GL_CLAMP_READ_COLOR, GL_FALSE); + GetBooleanv(GL_CLAMP_READ_COLOR, &b); + EXPECT_EQ(b, GL_FALSE); + + // Errors leave state unchanged. A non-GL_CLAMP_READ_COLOR target (here GL_FRONT, standing in for + // any illegal/compat target) and a bad clamp value both raise GL_INVALID_ENUM. + ClampColor(GL_FRONT, GL_TRUE); + EXPECT_EQ(GetError(), GL_INVALID_ENUM); + ClampColor(GL_CLAMP_READ_COLOR, GL_NICEST); + EXPECT_EQ(GetError(), GL_INVALID_ENUM); + GetIntegerv(GL_CLAMP_READ_COLOR, &value); + EXPECT_EQ(value, GL_FALSE); // unchanged by the failed calls + + MG_State::pGLContext.reset(); +} + +TEST(RenderStateSanity, PolygonModeStoresAndReadsBack) { + using namespace MobileGL; + using namespace MobileGL::MG_Impl::GLImpl; + MG_State::pGLContext = MakeUnique(); + + // GL_POLYGON_MODE reports TWO values (front, back); default GL_FILL for both. + GLint mode[2] = {-1, -1}; + GetIntegerv(GL_POLYGON_MODE, mode); + EXPECT_EQ(mode[0], GL_FILL); + EXPECT_EQ(mode[1], GL_FILL); + + // Core sets both faces together; each legal mode round-trips into both slots. + PolygonMode(GL_FRONT_AND_BACK, GL_LINE); + EXPECT_EQ(GetError(), GL_NO_ERROR); + GetIntegerv(GL_POLYGON_MODE, mode); + EXPECT_EQ(mode[0], GL_LINE); + EXPECT_EQ(mode[1], GL_LINE); + + PolygonMode(GL_FRONT_AND_BACK, GL_POINT); + GetIntegerv(GL_POLYGON_MODE, mode); + EXPECT_EQ(mode[0], GL_POINT); + EXPECT_EQ(mode[1], GL_POINT); + + // Core rejects separate faces: GL_FRONT/GL_BACK were removed in 3.1 core -> GL_INVALID_ENUM, no + // state change. (Some desktop drivers leniently accept them; this guards against copying that.) + PolygonMode(GL_FRONT, GL_FILL); + EXPECT_EQ(GetError(), GL_INVALID_ENUM); + PolygonMode(GL_BACK, GL_FILL); + EXPECT_EQ(GetError(), GL_INVALID_ENUM); + // A bad mode also raises GL_INVALID_ENUM. + PolygonMode(GL_FRONT_AND_BACK, GL_LINEAR); + EXPECT_EQ(GetError(), GL_INVALID_ENUM); + + GetIntegerv(GL_POLYGON_MODE, mode); + EXPECT_EQ(mode[0], GL_POINT); // unchanged by the failed calls + EXPECT_EQ(mode[1], GL_POINT); + + MG_State::pGLContext.reset(); +}