From 42ad62b54c02d27bc70b775693b2983cb4cea766 Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Thu, 13 Aug 2026 04:35:52 -0400 Subject: [PATCH] [Fix, Test] (MG_Util, MG_IntegrationTest): advertise the GL 4.3 VIEWPORT_BOUNDS_RANGE floor on a GLES driver that has no such query, instead of a range admitting no origin --- MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp | 3 +- .../DirectVulkan/Renderer/VulkanRenderer.cpp | 12 +++- .../Scenarios/AdvertisedLimitsScenario.cpp | 56 +++++++++++++++++++ .../GLState/RenderState/RenderState.h | 5 +- .../MG_Util/BackendLoaders/OpenGL/Loader.cpp | 14 ++++- 5 files changed, 81 insertions(+), 9 deletions(-) diff --git a/MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp b/MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp index 51686a12..8c247a38 100644 --- a/MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp +++ b/MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp @@ -1614,7 +1614,8 @@ namespace MobileGL::MG_Backend::DirectGLES { g_syncedBackendViewport = backendViewport; } - // All 12 capability bools live after LogicOp in the struct, i.e. in the tail span. + // Every capability bool (and the scissor-test mask below) lives after LogicOp in the + // struct, i.e. in the tail span. if (tailSpanDirty) { #define SYNC_CAPABILITY(cap_mg, cap_gl) \ if (forceFullPush || parameters.cap_mg##Enabled != g_syncedRenderStateParameters.cap_mg##Enabled) { \ diff --git a/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.cpp b/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.cpp index d786fe22..432a81b0 100644 --- a/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.cpp +++ b/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.cpp @@ -446,9 +446,15 @@ namespace MobileGL::MG_Backend::DirectVulkan { const IntVec2& framebufferExtent, VkSurfaceTransformFlagBitsKHR preTransform, Bool isDefaultFramebuffer) { - // Rounded to integers on purpose: MobileGL advertises GL_VIEWPORT_SUBPIXEL_BITS = 0, so - // the fractional rectangle glViewportIndexedf can store is exact as STATE and snapped - // when it rasterizes. + // Snapped to integers. The viewport is float STATE (glViewportIndexedf may set a + // fractional origin, and GetFloati_v hands it back verbatim), but what rasterizes here is + // the rounded rectangle - a deliberate, documented infidelity rather than a spec claim: + // MobileGL passes the driver's VIEWPORT_SUBPIXEL_BITS through, so it does advertise + // subpixel viewport precision it does not deliver. Nothing in KHR-GL43.viewport_array or + // in Minecraft sets a fractional viewport (the conformance checks are all on the state + // round trip), which is why the honest-but-lossy path was kept over widening every + // default-framebuffer Y-flip/pre-transform helper to floats. See the KNOWN INFIDELITY + // note in MG_IntegrationTest/Scenarios/AdvertisedLimitsScenario.cpp. const FloatVec4& stored = MG_State::pGLContext->GetViewportIndexed(index); const IntVec4 viewportState(static_cast(std::lround(stored.x())), static_cast(std::lround(stored.y())), diff --git a/MobileGL/MG_IntegrationTest/Scenarios/AdvertisedLimitsScenario.cpp b/MobileGL/MG_IntegrationTest/Scenarios/AdvertisedLimitsScenario.cpp index 7cbead56..c04b9d17 100644 --- a/MobileGL/MG_IntegrationTest/Scenarios/AdvertisedLimitsScenario.cpp +++ b/MobileGL/MG_IntegrationTest/Scenarios/AdvertisedLimitsScenario.cpp @@ -199,5 +199,61 @@ namespace MGITest { "derived component limits are computed in"; } + // ARB_viewport_array's own limits. They are advertised from three different places - + // GL_MAX_VIEWPORTS from the frontend's indexed state width, the bounds range and the + // subpixel bits from the backend caps table - and each backend fills that table from a + // different source, so all three are checked on both lanes. + // + // GL_VIEWPORT_BOUNDS_RANGE is the one that shipped wrong: GLES has no such query, the + // DirectGLES loader's glGetFloatv(GL_VIEWPORT_BOUNDS_RANGE) therefore raised + // GL_INVALID_ENUM and left the probe's zero-initialized array in place, and MobileGL + // advertised [0, 0] - a range that admits no viewport origin at all, and the check that + // kept KHR-GL43.viewport_array.queries red on Espryt after the indexed-state work. + TEST_F(AdvertisedLimitsScenario, ViewportArrayLimitsMeetTheirGL43Floors) { + GLint maxViewports = -1; + glGetIntegerv(GL_MAX_VIEWPORTS, &maxViewports); + ASSERT_EQ(FirstGLError(), GLenum(GL_NO_ERROR)); + EXPECT_GE(maxViewports, 16) << "GL 4.3 core table 23.53 sets the MAX_VIEWPORTS minimum at 16"; + EXPECT_LE(maxViewports, 256) << "one viewport rectangle of indexed state is allocated per advertised " + "viewport, and the CTS sizes its arrays off this number"; + + GLfloat boundsRange[2] = {1.0f, -1.0f}; + glGetFloatv(GL_VIEWPORT_BOUNDS_RANGE, boundsRange); + ASSERT_EQ(FirstGLError(), GLenum(GL_NO_ERROR)); + EXPECT_LE(boundsRange[0], -32768.0f) + << "GL 4.6 core table 23.60 sets the VIEWPORT_BOUNDS_RANGE minimum at [-32768, 32767]; got [" + << boundsRange[0] << ", " << boundsRange[1] << "]"; + EXPECT_GE(boundsRange[1], 32767.0f) + << "GL 4.6 core table 23.60 sets the VIEWPORT_BOUNDS_RANGE minimum at [-32768, 32767]; got [" + << boundsRange[0] << ", " << boundsRange[1] << "]"; + + // KNOWN INFIDELITY, pinned here rather than hidden. MobileGL reports the driver's own + // VIEWPORT_SUBPIXEL_BITS (4 on llvmpipe, i.e. 1/16-pixel viewport precision), but the + // float viewport rectangle glViewportIndexedf stores is snapped to integers on its + // way to both backends (ComputeGLViewport, DirectGLES SyncRenderState). The STATE + // round trip is exact - which is all KHR-GL43.viewport_array.viewport_api checks, and + // all this cluster set out to fix - so the gap is in rasterization only: a fractional + // viewport origin rasterizes as if it had been rounded. Nothing in the suite or in + // Minecraft sets one. Only the spec floor is asserted; tightening this to EQ(0) would + // mean advertising no subpixel precision at all, which is a separate decision about a + // limit MobileGL currently passes through from the driver. + GLint subpixelBits = -1; + glGetIntegerv(GL_VIEWPORT_SUBPIXEL_BITS, &subpixelBits); + ASSERT_EQ(FirstGLError(), GLenum(GL_NO_ERROR)); + EXPECT_GE(subpixelBits, 0) << "GL 4.6 core table 23.60: VIEWPORT_SUBPIXEL_BITS has a minimum of 0, and " + "a negative value is what a sign-flipped uint32 looks like"; + + GLint viewportDims[2] = {-1, -1}; + glGetIntegerv(GL_MAX_VIEWPORT_DIMS, viewportDims); + ASSERT_EQ(FirstGLError(), GLenum(GL_NO_ERROR)); + GLint maxRenderbufferSize = -1; + glGetIntegerv(GL_MAX_RENDERBUFFER_SIZE, &maxRenderbufferSize); + ASSERT_EQ(FirstGLError(), GLenum(GL_NO_ERROR)); + // GL 4.6 core 13.6.1: MAX_VIEWPORT_DIMS must be at least as large as the largest + // renderable surface, or a full-size framebuffer could not be fully viewported. + EXPECT_GE(viewportDims[0], maxRenderbufferSize); + EXPECT_GE(viewportDims[1], maxRenderbufferSize); + } + } // namespace } // namespace MGITest diff --git a/MobileGL/MG_State/GLState/RenderState/RenderState.h b/MobileGL/MG_State/GLState/RenderState/RenderState.h index a79cd148..73b51fbd 100644 --- a/MobileGL/MG_State/GLState/RenderState/RenderState.h +++ b/MobileGL/MG_State/GLState/RenderState/RenderState.h @@ -232,8 +232,9 @@ namespace MobileGL { // (KHR-GL43.viewport_array.viewport_api compares with ==, no tolerance). glViewport's // integers are simply one way to write it. Index 0 is what a program that never assigns // gl_ViewportIndex rasterizes against, and what the classic glViewport / - // glGetIntegerv(GL_VIEWPORT) pair addresses; both backends consume index 0 only, rounded - // back to integers (GL_VIEWPORT_SUBPIXEL_BITS is 0, so rounding is the honest answer). + // glGetIntegerv(GL_VIEWPORT) pair addresses. Both backends rasterize the rectangle + // rounded back to integers; the STATE stays exact, which is the half the conformance + // suite checks (see the KNOWN INFIDELITY note in AdvertisedLimitsScenario.cpp). Array Viewports{}; // x, y, width, height Float LineWidth = 1.0f; Float PointSize = 1.0f; diff --git a/MobileGL/MG_Util/BackendLoaders/OpenGL/Loader.cpp b/MobileGL/MG_Util/BackendLoaders/OpenGL/Loader.cpp index b615e76a..9e28dca1 100644 --- a/MobileGL/MG_Util/BackendLoaders/OpenGL/Loader.cpp +++ b/MobileGL/MG_Util/BackendLoaders/OpenGL/Loader.cpp @@ -1000,7 +1000,11 @@ namespace MobileGL::MG_Util::BackendLoader { GLfloat smoothLineWidthRange[2] = {1.0f, 1.0f}; GLfloat smoothLineWidthGranularity = 1.0f; GLfloat aliasedPointSizeRange[2] = {1.0f, 1.0f}; - GLfloat viewportBoundsRange[2] = {0.0f, 0.0f}; + // GL 4.6 core table 23.60 sets the MINIMUM VIEWPORT_BOUNDS_RANGE at [-32768, 32767], and + // KHR-GL43.viewport_array.queries asserts exactly that floor. GLES has no such query, so + // the glGetFloatv below raises GL_INVALID_ENUM and leaves this untouched - starting it at + // {0, 0} advertised a range that admits no viewport origin at all. + GLfloat viewportBoundsRange[2] = {-32768.0f, 32767.0f}; GLint maxViewportDims[2] = {16384, 16384}; GLint viewportSubpixelBits = 0; GLint max3DTextureSize = 16384; @@ -1289,8 +1293,12 @@ namespace MobileGL::MG_Util::BackendLoader { caps.MaxViewports = maxViewports; caps.MaxViewportWidth = maxViewportDims[0]; caps.MaxViewportHeight = maxViewportDims[1]; - caps.ViewportBoundsRangeMin = viewportBoundsRange[0]; - caps.ViewportBoundsRangeMax = viewportBoundsRange[1]; + // Only ever WIDER than the core minimum: a driver that answered the query is allowed to + // exceed the floor but never to sit inside it, and a driver that rejected the query left + // the floor in place. Written as a clamp rather than a plain assignment so a partial + // write (one component answered, the other not) cannot narrow the range either. + caps.ViewportBoundsRangeMin = std::min(viewportBoundsRange[0], -32768.0f); + caps.ViewportBoundsRangeMax = std::max(viewportBoundsRange[1], 32767.0f); caps.ViewportSubpixelBits = viewportSubpixelBits; caps.MinFragmentInterpolationOffset = std::isfinite(minFragmentInterpolationOffset) && minFragmentInterpolationOffset <= -0.5f