From 3b0e6270b7ac991170d007ccdee2c868b60454fd Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Mon, 12 Oct 2026 08:04:22 -0400 Subject: [PATCH] [Fix] (Backend): apply the polygon offset clamp on both backends --- MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp | 14 +++- .../DirectVulkan/Renderer/VulkanRenderer.cpp | 30 +++++++-- .../DirectVulkan/Renderer/VulkanRenderer.h | 3 + .../MG_Test/Framebuffer/FramebufferTest.cpp | 66 ++++++++++++++++++- .../MG_Util/BackendLoaders/OpenGL/Loader.cpp | 7 ++ .../MG_Util/BackendLoaders/OpenGL/Loader.h | 7 ++ 6 files changed, 117 insertions(+), 10 deletions(-) diff --git a/MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp b/MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp index 400973eb..a0ed4ab6 100644 --- a/MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp +++ b/MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp @@ -4120,8 +4120,18 @@ namespace MobileGL::MG_Backend::DirectGLES { if (headSpanDirty) { // Polygon offset (head-span scalars, like line width / point size below) if (forceFullPush || parameters.PolygonOffsetFactor != g_syncedRenderStateParameters.PolygonOffsetFactor || - parameters.PolygonOffsetUnits != g_syncedRenderStateParameters.PolygonOffsetUnits) { - g_GLESFuncs.glPolygonOffset(parameters.PolygonOffsetFactor, parameters.PolygonOffsetUnits); + parameters.PolygonOffsetUnits != g_syncedRenderStateParameters.PolygonOffsetUnits || + parameters.PolygonOffsetClamp != g_syncedRenderStateParameters.PolygonOffsetClamp) { + // glPolygonOffset IS glPolygonOffsetClampEXT(factor, units, 0), so where the + // driver has the clamp entry point it carries all three values; without it the + // clamp cannot be expressed and the offset goes through unclamped. + if (g_GLESCapabilities.SupportsPolygonOffsetClamp) { + g_GLESFuncs.glPolygonOffsetClampEXT(parameters.PolygonOffsetFactor, + parameters.PolygonOffsetUnits, + parameters.PolygonOffsetClamp); + } else { + g_GLESFuncs.glPolygonOffset(parameters.PolygonOffsetFactor, parameters.PolygonOffsetUnits); + } } } diff --git a/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.cpp b/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.cpp index eb680ede..0b345fdc 100644 --- a/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.cpp +++ b/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.cpp @@ -306,6 +306,7 @@ namespace MobileGL::MG_Backend::DirectVulkan { Bool depthBiasValid = false; Float depthBiasConstantFactor = 0.0f; Float depthBiasSlopeFactor = 0.0f; + Float depthBiasClamp = 0.0f; Bool lineWidthValid = false; Float lineWidth = 0.0f; Bool stencilValid = false; @@ -349,7 +350,7 @@ namespace MobileGL::MG_Backend::DirectVulkan { // RenderStateParameters field - RenderState.cpp): // ApplyGLViewportState : Viewports[0], DepthRanges[0], + extent/isDefaultFbo/preTransform // ApplyBlendConstants : BlendColor - // ApplyPolygonOffsetState : PolygonOffsetUnits, PolygonOffsetFactor + // ApplyPolygonOffsetState : PolygonOffsetUnits, PolygonOffsetFactor, PolygonOffsetClamp // ApplyLineWidthState : LineWidth (see the caveat below) // ApplyStencilState : StencilStates[0..1].{ValueMask, WriteMask, Ref} // scissor rect : ScissorTestEnabledMask bit 0, ScissorBoxes[0], @@ -364,6 +365,7 @@ namespace MobileGL::MG_Backend::DirectVulkan { Float blendColor[4] = {0.0f, 0.0f, 0.0f, 0.0f}; Float polygonOffsetFactor = 0.0f; Float polygonOffsetUnits = 0.0f; + Float polygonOffsetClamp = 0.0f; Float lineWidth = 0.0f; Uint32 stencilValueMask[2] = {0, 0}; Uint32 stencilWriteMask[2] = {0, 0}; @@ -393,7 +395,8 @@ namespace MobileGL::MG_Backend::DirectVulkan { } } return polygonOffsetFactor == other.polygonOffsetFactor && - polygonOffsetUnits == other.polygonOffsetUnits && lineWidth == other.lineWidth && + polygonOffsetUnits == other.polygonOffsetUnits && + polygonOffsetClamp == other.polygonOffsetClamp && lineWidth == other.lineWidth && extentX == other.extentX && extentY == other.extentY && preTransform == other.preTransform && scissorEnabled == other.scissorEnabled && isDefaultFbo == other.isDefaultFbo; @@ -455,6 +458,7 @@ namespace MobileGL::MG_Backend::DirectVulkan { MAGMA_TAIL_INPUT_IS_DYNAMIC(BlendColor); // ApplyBlendConstants MAGMA_TAIL_INPUT_IS_DYNAMIC(PolygonOffsetFactor); // ApplyPolygonOffsetState MAGMA_TAIL_INPUT_IS_DYNAMIC(PolygonOffsetUnits); // ApplyPolygonOffsetState + MAGMA_TAIL_INPUT_IS_DYNAMIC(PolygonOffsetClamp); // ApplyPolygonOffsetState MAGMA_TAIL_INPUT_IS_DYNAMIC(LineWidth); // ApplyLineWidthState MAGMA_TAIL_INPUT_IS_DYNAMIC(ScissorBoxes); // the scissor rect: ScissorBoxes[0] #undef MAGMA_TAIL_INPUT_IS_DYNAMIC @@ -652,18 +656,25 @@ namespace MobileGL::MG_Backend::DirectVulkan { } } - static void ApplyPolygonOffsetState(VkCommandBuffer commandBuffer) { + static void ApplyPolygonOffsetState(VkCommandBuffer commandBuffer, Bool depthBiasClampSupported) { const Float constantFactor = MG_Pipe::gPipeInputs->GetPolygonOffsetUnits(); const Float slopeFactor = MG_Pipe::gPipeInputs->GetPolygonOffsetFactor(); + // GL_POLYGON_OFFSET_CLAMP and depthBiasClamp share their semantics exactly (positive caps + // the offset from above, negative from below, zero disables). Without the device feature + // the value must be 0 (VUID-vkCmdSetDepthBias-depthBiasClamp-00790), so the offset then + // goes through unclamped. + const Float clamp = + depthBiasClampSupported ? MG_Pipe::gPipeInputs->GetRenderStateParameters().PolygonOffsetClamp : 0.0f; auto& shadow = *g_dynamicStateShadow; if (shadow.depthBiasValid && shadow.depthBiasConstantFactor == constantFactor && - shadow.depthBiasSlopeFactor == slopeFactor) { + shadow.depthBiasSlopeFactor == slopeFactor && shadow.depthBiasClamp == clamp) { return; } shadow.depthBiasValid = true; shadow.depthBiasConstantFactor = constantFactor; shadow.depthBiasSlopeFactor = slopeFactor; - vkCmdSetDepthBias(commandBuffer, constantFactor, 0.0f, slopeFactor); + shadow.depthBiasClamp = clamp; + vkCmdSetDepthBias(commandBuffer, constantFactor, clamp, slopeFactor); } static void ApplyLineWidthState(VkCommandBuffer commandBuffer) { @@ -3500,7 +3511,7 @@ namespace MobileGL::MG_Backend::DirectVulkan { // stencil masks are not per-viewport and a multi-viewport draw needs them just as // much. Only the viewport/scissor pair takes the array shape. ApplyBlendConstants(frame.commandBuffer); - ApplyPolygonOffsetState(frame.commandBuffer); + ApplyPolygonOffsetState(frame.commandBuffer, m_depthBiasClampFeatureEnabled); ApplyLineWidthState(frame.commandBuffer); ApplyStencilState(frame.commandBuffer); ApplyMultiViewportDynamicState(frame.commandBuffer, viewportCount, extent, @@ -3564,6 +3575,7 @@ namespace MobileGL::MG_Backend::DirectVulkan { key.blendColor[3] = p.BlendColor.w(); key.polygonOffsetFactor = p.PolygonOffsetFactor; key.polygonOffsetUnits = p.PolygonOffsetUnits; + key.polygonOffsetClamp = p.PolygonOffsetClamp; key.lineWidth = p.LineWidth; // StencilStates[0] is Front, [1] is Back (RenderState::GetStencilFaceIndex), // the same order ApplyStencilState reads them in. @@ -3590,7 +3602,7 @@ namespace MobileGL::MG_Backend::DirectVulkan { } ApplyGLViewportState(frame.commandBuffer, extent, preTransform, isDefaultFbo); ApplyBlendConstants(frame.commandBuffer); - ApplyPolygonOffsetState(frame.commandBuffer); + ApplyPolygonOffsetState(frame.commandBuffer, m_depthBiasClampFeatureEnabled); ApplyLineWidthState(frame.commandBuffer); ApplyStencilState(frame.commandBuffer); VkRect2D scissor{}; @@ -7359,6 +7371,10 @@ namespace MobileGL::MG_Backend::DirectVulkan { deviceFeatures.fillModeNonSolid = supportedDeviceFeatures.fillModeNonSolid; m_fillModeNonSolidFeatureEnabled = deviceFeatures.fillModeNonSolid == VK_TRUE; deviceFeatures.dualSrcBlend = supportedDeviceFeatures.dualSrcBlend; + // GL_POLYGON_OFFSET_CLAMP (glPolygonOffsetClamp) maps onto vkCmdSetDepthBias's clamp + // argument, which must stay 0 unless this feature is enabled. + deviceFeatures.depthBiasClamp = supportedDeviceFeatures.depthBiasClamp; + m_depthBiasClampFeatureEnabled = deviceFeatures.depthBiasClamp == VK_TRUE; m_dualSrcBlendFeatureEnabled = deviceFeatures.dualSrcBlend == VK_TRUE; // ARB_sample_shading. Without this feature a pipeline may not set sampleShadingEnable // (VUID-VkPipelineMultisampleStateCreateInfo-sampleShadingEnable-00784), so the GL enable diff --git a/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.h b/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.h index 679a3092..a98dc602 100644 --- a/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.h +++ b/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.h @@ -1155,6 +1155,9 @@ namespace MobileGL::MG_Backend::DirectVulkan { // drive a runtime fallback when the device lacks them. Bool m_fillModeNonSolidFeatureEnabled = false; Bool m_independentBlendFeatureEnabled = false; + // depthBiasClamp gates the clamp argument of vkCmdSetDepthBias (glPolygonOffsetClamp); + // without it the polygon offset is applied unclamped. + Bool m_depthBiasClampFeatureEnabled = false; // dualSrcBlend gates GL_SRC1_* blend factors (glBindFragDataLocationIndexed dual-source blend); // primitiveTopologyListRestart gates primitive restart on *list* topologies (strip/fan restart // needs no feature). Both cached at device creation and drive a hard-fail-at-draw when absent. diff --git a/MobileGL/MG_Test/Framebuffer/FramebufferTest.cpp b/MobileGL/MG_Test/Framebuffer/FramebufferTest.cpp index 2379ed6a..2d08b6f5 100644 --- a/MobileGL/MG_Test/Framebuffer/FramebufferTest.cpp +++ b/MobileGL/MG_Test/Framebuffer/FramebufferTest.cpp @@ -1112,7 +1112,21 @@ namespace { void StubClearStencil(GLint) {} void StubCullFace(GLenum) {} void StubFrontFace(GLenum) {} - void StubPolygonOffset(GLfloat, GLfloat) {} + // The polygon offset the driver was last handed, and through which entry point. + struct RecordedPolygonOffset { + Bool viaClamp = false; + Bool seen = false; + GLfloat factor = 0.0f; + GLfloat units = 0.0f; + GLfloat clamp = 0.0f; + }; + RecordedPolygonOffset g_driverPolygonOffset; + void StubPolygonOffset(GLfloat factor, GLfloat units) { + g_driverPolygonOffset = {false, true, factor, units, 0.0f}; + } + void StubPolygonOffsetClampEXT(GLfloat factor, GLfloat units, GLfloat clamp) { + g_driverPolygonOffset = {true, true, factor, units, clamp}; + } void StubLineWidth(GLfloat) {} void StubSampleCoverage(GLfloat, GLboolean) {} @@ -1277,6 +1291,56 @@ TEST_F(FramebufferTest, ClearIntoAWidenedDrawBufferKeepsAlphaWritableAndSubstitu EXPECT_EQ(MG_Impl::GLImpl::GetError(), GL_NO_ERROR); } +TEST_F(FramebufferTest, PolygonOffsetClampReachesTheDriverWhenItHasTheClampEntryPoint) { + ScopedRenderStateDriverStubs driver; + MG_Backend::DirectGLES::g_GLESFuncs.glPolygonOffsetClampEXT = StubPolygonOffsetClampEXT; + MG_Backend::DirectGLES::g_GLESCapabilities.SupportsPolygonOffsetClamp = true; + g_driverPolygonOffset = {}; + + MG_Impl::GLImpl::PolygonOffsetClamp(0.0f, 5000.0f, 0.0001f); + { + MG_Test::ScopedPipeVerb draw(MG_Pipe::MGPipeVerb::DrawArrays); + MG_Backend::DirectGLES::RenderStateImpl::SyncRenderState(/*forColorClear=*/false); + } + ASSERT_TRUE(g_driverPolygonOffset.seen); + EXPECT_TRUE(g_driverPolygonOffset.viaClamp) << "the clamp must not be dropped on the way to the driver"; + EXPECT_EQ(g_driverPolygonOffset.factor, 0.0f); + EXPECT_EQ(g_driverPolygonOffset.units, 5000.0f); + EXPECT_EQ(g_driverPolygonOffset.clamp, 0.0001f); + + // A clamp-only change (same factor and units) is still a change the driver has to see. + g_driverPolygonOffset = {}; + MG_Impl::GLImpl::PolygonOffset(0.0f, 5000.0f); + { + MG_Test::ScopedPipeVerb draw(MG_Pipe::MGPipeVerb::DrawArrays); + MG_Backend::DirectGLES::RenderStateImpl::SyncRenderState(/*forColorClear=*/false); + } + ASSERT_TRUE(g_driverPolygonOffset.seen) << "glPolygonOffset resets the clamp to 0"; + EXPECT_EQ(g_driverPolygonOffset.clamp, 0.0f); + + MG_Impl::GLImpl::PolygonOffset(0.0f, 0.0f); + EXPECT_EQ(MG_Impl::GLImpl::GetError(), GL_NO_ERROR); +} + +TEST_F(FramebufferTest, PolygonOffsetFallsBackToTheUnclampedEntryPointWithoutTheExtension) { + ScopedRenderStateDriverStubs driver; + MG_Backend::DirectGLES::g_GLESCapabilities.SupportsPolygonOffsetClamp = false; + g_driverPolygonOffset = {}; + + MG_Impl::GLImpl::PolygonOffsetClamp(1.0f, 2.0f, 0.5f); + { + MG_Test::ScopedPipeVerb draw(MG_Pipe::MGPipeVerb::DrawArrays); + MG_Backend::DirectGLES::RenderStateImpl::SyncRenderState(/*forColorClear=*/false); + } + ASSERT_TRUE(g_driverPolygonOffset.seen); + EXPECT_FALSE(g_driverPolygonOffset.viaClamp); + EXPECT_EQ(g_driverPolygonOffset.factor, 1.0f); + EXPECT_EQ(g_driverPolygonOffset.units, 2.0f); + + MG_Impl::GLImpl::PolygonOffset(0.0f, 0.0f); + EXPECT_EQ(MG_Impl::GLImpl::GetError(), GL_NO_ERROR); +} + TEST_F(FramebufferTest, ApplicationAlphaMaskOffIsStillHonouredOnANativeDrawBuffer) { // The doctoring only ever REMOVES alpha writes on a widened buffer; it must never add them // back on a buffer the application masked itself, and must never touch a native one. diff --git a/MobileGL/MG_Util/BackendLoaders/OpenGL/Loader.cpp b/MobileGL/MG_Util/BackendLoaders/OpenGL/Loader.cpp index 8fb61bd4..38390b34 100644 --- a/MobileGL/MG_Util/BackendLoaders/OpenGL/Loader.cpp +++ b/MobileGL/MG_Util/BackendLoaders/OpenGL/Loader.cpp @@ -664,6 +664,7 @@ namespace MobileGL::MG_Util::BackendLoader { INIT_GLES_FUNC_OPTIONAL(glClipControlEXT) INIT_GLES_FUNC_OPTIONAL(glColorMaskiEXT) INIT_GLES_FUNC_OPTIONAL(glColorMaskiOES) + INIT_GLES_FUNC_OPTIONAL(glPolygonOffsetClampEXT) // Extension-only multi-draw entry points. eglGetProcAddress may legally return a // non-NULL stub for these on drivers that do not implement them (NVIDIA's ES driver // returns one for glMultiDrawElementsBaseVertexEXT that silently drops every draw), @@ -1126,6 +1127,7 @@ namespace MobileGL::MG_Util::BackendLoader { Bool hasBaseInstanceExtension = false; Bool hasTessellationShaderExtension = false; Bool hasClipControlExtension = false; + Bool hasPolygonOffsetClampExtension = false; for (GLint i = 0; i < extCount; ++i) { const char* extension = (const char*)glesFuncs.glGetStringi(GL_EXTENSIONS, i); if (extension) { @@ -1192,6 +1194,9 @@ namespace MobileGL::MG_Util::BackendLoader { if (std::strcmp(extension, "GL_KHR_parallel_shader_compile") == 0) { caps.SupportsParallelShaderCompile = true; } + if (std::strcmp(extension, "GL_EXT_polygon_offset_clamp") == 0) { + hasPolygonOffsetClampExtension = true; + } if (std::strcmp(extension, "GL_EXT_blend_func_extended") == 0) { caps.SupportsDualSourceBlend = true; } @@ -1254,6 +1259,8 @@ namespace MobileGL::MG_Util::BackendLoader { glesFuncs.glDrawArraysInstancedBaseInstanceEXT != nullptr && glesFuncs.glDrawElementsInstancedBaseInstanceEXT != nullptr && glesFuncs.glDrawElementsInstancedBaseVertexBaseInstanceEXT != nullptr; + caps.SupportsPolygonOffsetClamp = + hasPolygonOffsetClampExtension && glesFuncs.glPolygonOffsetClampEXT != nullptr; // ES has no core texture views at any version, so this is extension-only by nature. // Each spelling must bring its OWN entry point: a driver that advertises the OES string // is not required to export glTextureViewEXT. diff --git a/MobileGL/MG_Util/BackendLoaders/OpenGL/Loader.h b/MobileGL/MG_Util/BackendLoaders/OpenGL/Loader.h index b3813eb6..2a955941 100644 --- a/MobileGL/MG_Util/BackendLoaders/OpenGL/Loader.h +++ b/MobileGL/MG_Util/BackendLoaders/OpenGL/Loader.h @@ -654,6 +654,8 @@ namespace MobileGL { GL_FUNC_TYPEDEF(void, glClipControlEXT, GLenum origin, GLenum depth) GL_FUNC_TYPEDEF(void, glColorMaskiEXT, GLuint index, GLboolean r, GLboolean g, GLboolean b, GLboolean a) GL_FUNC_TYPEDEF(void, glColorMaskiOES, GLuint index, GLboolean r, GLboolean g, GLboolean b, GLboolean a) + // GL_EXT_polygon_offset_clamp. ES has no core spelling at any version. + GL_FUNC_TYPEDEF(void, glPolygonOffsetClampEXT, GLfloat factor, GLfloat units, GLfloat clamp) GL_FUNC_TYPEDEF(void, glMultiDrawArraysIndirectEXT, GLenum mode, const void* indirect, GLsizei drawcount, GLsizei stride) @@ -1071,6 +1073,7 @@ namespace MobileGL { GL_FUNC_DECL(glClipControlEXT) GL_FUNC_DECL(glColorMaskiEXT) GL_FUNC_DECL(glColorMaskiOES) + GL_FUNC_DECL(glPolygonOffsetClampEXT) GL_FUNC_DECL(glMultiDrawArraysIndirectEXT) GL_FUNC_DECL(glMultiDrawElementsIndirectEXT) @@ -1216,6 +1219,10 @@ namespace MobileGL { // factors and layout(index = 1) fragment outputs. GLES core has no dual-source blending, // so without this a draw using a SRC1 factor cannot proceed. Bool SupportsDualSourceBlend = false; + // GL_EXT_polygon_offset_clamp is present and glPolygonOffsetClampEXT resolved. Without + // it the GL_POLYGON_OFFSET_CLAMP value cannot be applied and only glPolygonOffset's + // unclamped offset reaches the driver. + Bool SupportsPolygonOffsetClamp = false; // GL_NV_shader_noperspective_interpolation is present: the driver accepts the // `noperspective` interpolation qualifier in ESSL. GLES core has none, so without this // SPIRV-Cross's `#extension ... : require` would fail to compile and MobileGL falls back