From ac3a83b2070532304fa54bbd90a69107a2de6904 Mon Sep 17 00:00:00 2001 From: BZLZHH Date: Fri, 7 Aug 2026 06:00:54 -0400 Subject: [PATCH] [Fix] (MG_Util, MG_Test): never take a non-null eglGetProcAddress result as support On GLVND Linux eglGetProcAddress returns a non-NULL trampoline for ANY name - including a fabricated one - so pointer-nullness can never signal driver support. The three EXT multi-draw entry points were registered as required (spurious error logs on drivers without them) and their pointers were trusted; the NVIDIA ES driver hands back a stub for glMultiDrawElementsBaseVertexEXT that SILENTLY DROPS draws, which once made a "77% faster" multi-draw batch that rendered nothing. The three entries are optional now, and two extension-derived capability flags follow the established Supports* pattern - each is an extension-string check AND a resolved pointer, so a flag alone is sufficient at a call site: SupportsMultiDrawIndirect: GL_EXT_multi_draw_indirect + both entry points resolved. SupportsMultiDrawElementsBaseVertex: (GL_EXT or GL_OES_draw_elements_base_vertex) + GL_EXT_multi_draw_arrays + the entry point resolved. The multi_draw_arrays conjunct is the registry fact the stub exploited: glMultiDrawElementsBaseVertexEXT exists only in interaction with GL_EXT_multi_draw_arrays, and this NVIDIA driver advertises everything else EXCEPT that one - so the entry point is genuinely unsupported while eglGetProcAddress still "resolves" it. Two DriverPost rows report both capabilities (INFO when absent - a fallback always exists). Unit tests pin the stub shape, the exact NVIDIA shape, the supported shape and extension-without-pointer. Proven load-bearing: forcing the old pointer-only condition on the NVIDIA ES driver reproduces the silent drop exactly (sodium retrace SSIM 1.000000 -> 0.329522, no crash, no GL error); with the gate the same run is a literal 1.000000. Unit suite 423/423 (two new tests), retrace subset 10/10, integration suite 52/52. --- .../BackendLoader/BackendLoaderTest.cpp | 58 +++++++++++++++++++ .../MG_Util/BackendLoaders/OpenGL/Loader.cpp | 40 ++++++++++++- .../MG_Util/BackendLoaders/OpenGL/Loader.h | 13 +++++ MobileGL/MG_Util/SelfTest/DriverPost.cpp | 25 ++++++++ 4 files changed, 133 insertions(+), 3 deletions(-) diff --git a/MobileGL/MG_Test/BackendLoader/BackendLoaderTest.cpp b/MobileGL/MG_Test/BackendLoader/BackendLoaderTest.cpp index 9ba15f6a..ae1b6e59 100644 --- a/MobileGL/MG_Test/BackendLoader/BackendLoaderTest.cpp +++ b/MobileGL/MG_Test/BackendLoader/BackendLoaderTest.cpp @@ -598,3 +598,61 @@ TEST(TextureAnisotropyCapabilities, ExtensionPresenceIsDetectedExactly) { ASSERT_TRUE(MobileGL::MG_Util::BackendLoader::FillInGLESCapabilities(presentCaps, funcs)); EXPECT_TRUE(presentCaps.SupportsTextureFilterAnisotropy); } + +// eglGetProcAddress may return a non-NULL stub for an entry point the context does not +// implement (NVIDIA's ES driver does exactly that for glMultiDrawElementsBaseVertexEXT and +// the stub silently drops draws), so a resolved pointer must NEVER flip these flags on its +// own: the extension string is the authority, and the pointer only confirms callability. +TEST(MultiDrawCapabilities, PointerAloneNeverCountsAsSupport) { + ResetFakeDriver(); + g_fake.maxVertexSsboBlocks = 0; + auto funcs = MakeFakeGLESFunctions(); + // Simulate the stub hazard: every pointer resolved, no extension advertised. + funcs.glMultiDrawArraysIndirectEXT = [](GLenum, const void*, GLsizei, GLsizei) {}; + funcs.glMultiDrawElementsIndirectEXT = [](GLenum, GLenum, const void*, GLsizei, GLsizei) {}; + funcs.glMultiDrawElementsBaseVertexEXT = [](GLenum, const GLsizei*, GLenum, const void* const*, + GLsizei, const GLint*) {}; + + MobileGL::MG_External::GLESCapabilities stubCaps; + ASSERT_TRUE(MobileGL::MG_Util::BackendLoader::FillInGLESCapabilities(stubCaps, funcs)); + EXPECT_FALSE(stubCaps.SupportsMultiDrawIndirect); + EXPECT_FALSE(stubCaps.SupportsMultiDrawElementsBaseVertex); + + // The real NVIDIA shape: both draw_elements_base_vertex extensions advertised but + // GL_EXT_multi_draw_arrays missing, so glMultiDrawElementsBaseVertexEXT (added only by + // their interaction with GL_EXT_multi_draw_arrays) is still a stub. + ResetFakeDriver(); + g_fake.maxVertexSsboBlocks = 0; + g_fake.extensions.emplace_back("GL_EXT_draw_elements_base_vertex"); + g_fake.extensions.emplace_back("GL_OES_draw_elements_base_vertex"); + MobileGL::MG_External::GLESCapabilities nvidiaShapedCaps; + ASSERT_TRUE(MobileGL::MG_Util::BackendLoader::FillInGLESCapabilities(nvidiaShapedCaps, funcs)); + EXPECT_FALSE(nvidiaShapedCaps.SupportsMultiDrawElementsBaseVertex); + + // Fully supported: extensions advertised and pointers resolved. + ResetFakeDriver(); + g_fake.maxVertexSsboBlocks = 0; + g_fake.extensions.emplace_back("GL_EXT_multi_draw_indirect"); + g_fake.extensions.emplace_back("GL_OES_draw_elements_base_vertex"); + g_fake.extensions.emplace_back("GL_EXT_multi_draw_arrays"); + MobileGL::MG_External::GLESCapabilities supportedCaps; + ASSERT_TRUE(MobileGL::MG_Util::BackendLoader::FillInGLESCapabilities(supportedCaps, funcs)); + EXPECT_TRUE(supportedCaps.SupportsMultiDrawIndirect); + EXPECT_TRUE(supportedCaps.SupportsMultiDrawElementsBaseVertex); +} + +TEST(MultiDrawCapabilities, ExtensionWithoutResolvedPointerIsNotSupport) { + // Extensions advertised but the loader could not resolve the entry points (default fake + // table leaves them null): the flags must stay false so no caller dereferences null. + ResetFakeDriver(); + g_fake.maxVertexSsboBlocks = 0; + g_fake.extensions.emplace_back("GL_EXT_multi_draw_indirect"); + g_fake.extensions.emplace_back("GL_EXT_draw_elements_base_vertex"); + g_fake.extensions.emplace_back("GL_EXT_multi_draw_arrays"); + const auto funcs = MakeFakeGLESFunctions(); + + MobileGL::MG_External::GLESCapabilities caps; + ASSERT_TRUE(MobileGL::MG_Util::BackendLoader::FillInGLESCapabilities(caps, funcs)); + EXPECT_FALSE(caps.SupportsMultiDrawIndirect); + EXPECT_FALSE(caps.SupportsMultiDrawElementsBaseVertex); +} diff --git a/MobileGL/MG_Util/BackendLoaders/OpenGL/Loader.cpp b/MobileGL/MG_Util/BackendLoaders/OpenGL/Loader.cpp index 4251fbe5..68be1e5f 100644 --- a/MobileGL/MG_Util/BackendLoaders/OpenGL/Loader.cpp +++ b/MobileGL/MG_Util/BackendLoaders/OpenGL/Loader.cpp @@ -524,9 +524,15 @@ namespace MobileGL::MG_Util::BackendLoader { INIT_GLES_FUNC_OPTIONAL(glPolygonModeANGLE) INIT_GLES_FUNC_OPTIONAL(glColorMaskiEXT) INIT_GLES_FUNC_OPTIONAL(glColorMaskiOES) - INIT_GLES_FUNC(glMultiDrawArraysIndirectEXT) - INIT_GLES_FUNC(glMultiDrawElementsIndirectEXT) - INIT_GLES_FUNC(glMultiDrawElementsBaseVertexEXT) + // 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), + // so pointer presence proves nothing: callers must gate on the extension-derived + // SupportsMultiDrawIndirect / SupportsMultiDrawElementsBaseVertex capability flags, + // never on these pointers alone. + INIT_GLES_FUNC_OPTIONAL(glMultiDrawArraysIndirectEXT) + INIT_GLES_FUNC_OPTIONAL(glMultiDrawElementsIndirectEXT) + INIT_GLES_FUNC_OPTIONAL(glMultiDrawElementsBaseVertexEXT) } } @@ -814,6 +820,11 @@ namespace MobileGL::MG_Util::BackendLoader { GLint extCount = 0; glesFuncs.glGetIntegerv(GL_NUM_EXTENSIONS, &extCount); MGLOG_I("Detected %d OpenGL ES extensions:", extCount); + // Combined below: glMultiDrawElementsBaseVertexEXT exists only where EXT/OES + // draw_elements_base_vertex interacts with GL_EXT_multi_draw_arrays. + Bool hasMultiDrawIndirectExtension = false; + Bool hasDrawElementsBaseVertexExtension = false; + Bool hasMultiDrawArraysExtension = false; for (GLint i = 0; i < extCount; ++i) { const char* extension = (const char*)glesFuncs.glGetStringi(GL_EXTENSIONS, i); if (extension) { @@ -856,8 +867,27 @@ namespace MobileGL::MG_Util::BackendLoader { if (std::strcmp(extension, "GL_OES_shader_multisample_interpolation") == 0) { caps.SupportsShaderMultisampleInterpolation = true; } + if (std::strcmp(extension, "GL_EXT_multi_draw_indirect") == 0) { + hasMultiDrawIndirectExtension = true; + } + if (std::strcmp(extension, "GL_EXT_draw_elements_base_vertex") == 0 || + std::strcmp(extension, "GL_OES_draw_elements_base_vertex") == 0) { + hasDrawElementsBaseVertexExtension = true; + } + if (std::strcmp(extension, "GL_EXT_multi_draw_arrays") == 0) { + hasMultiDrawArraysExtension = true; + } } } + // The pointer check on top of the extension check makes each flag sufficient on its own + // at a call site; the extension check on top of the pointer keeps a stub returned by + // eglGetProcAddress (see AcquireGLESFunctions) from ever counting as support. + caps.SupportsMultiDrawIndirect = hasMultiDrawIndirectExtension && + glesFuncs.glMultiDrawArraysIndirectEXT != nullptr && + glesFuncs.glMultiDrawElementsIndirectEXT != nullptr; + caps.SupportsMultiDrawElementsBaseVertex = hasDrawElementsBaseVertexExtension && + hasMultiDrawArraysExtension && + glesFuncs.glMultiDrawElementsBaseVertexEXT != nullptr; caps.SupportsShaderMultisampleInterpolation = caps.SupportsShaderMultisampleInterpolation || caps.GLESVersion.Major > 3 || (caps.GLESVersion.Major == 3 && caps.GLESVersion.Minor >= 2); @@ -873,6 +903,10 @@ namespace MobileGL::MG_Util::BackendLoader { MGLOG_I(" indexed glColorMaski: %s", caps.SupportsIndexedColorMask ? "yes" : "no"); MGLOG_I(" dual-source blend (EXT_blend_func_extended): %s", caps.SupportsDualSourceBlend ? "yes" : "no"); + MGLOG_I(" multi-draw indirect (EXT_multi_draw_indirect): %s", + caps.SupportsMultiDrawIndirect ? "yes" : "no"); + MGLOG_I(" multi-draw base vertex (EXT/OES_draw_elements_base_vertex + EXT_multi_draw_arrays): %s", + caps.SupportsMultiDrawElementsBaseVertex ? "yes" : "no"); MGLOG_I("OpenGL ES capabilities:"); glesFuncs.glGetIntegerv(GL_UNIFORM_BUFFER_OFFSET_ALIGNMENT, &caps.UniformBufferOffsetAlignment); diff --git a/MobileGL/MG_Util/BackendLoaders/OpenGL/Loader.h b/MobileGL/MG_Util/BackendLoaders/OpenGL/Loader.h index abbf9757..37c5d647 100644 --- a/MobileGL/MG_Util/BackendLoaders/OpenGL/Loader.h +++ b/MobileGL/MG_Util/BackendLoaders/OpenGL/Loader.h @@ -1071,6 +1071,19 @@ namespace MobileGL { // GLES 3.2 core or GL_OES_shader_multisample_interpolation exposes // interpolateAtOffset and the three fragment-offset limit queries. Bool SupportsShaderMultisampleInterpolation = false; + // GL_EXT_multi_draw_indirect is present AND glMultiDrawArraysIndirectEXT / + // glMultiDrawElementsIndirectEXT both resolved. Multi-draw is not core in any ES + // version, and eglGetProcAddress may return a live-looking stub on drivers without + // the extension, so the pointers alone must never be used as the support signal. + Bool SupportsMultiDrawIndirect = false; + // glMultiDrawElementsBaseVertexEXT is callable. Per the Khronos registry the entry + // point is added by GL_EXT/OES_draw_elements_base_vertex ONLY in interaction with + // GL_EXT_multi_draw_arrays; NVIDIA's ES driver advertises both base_vertex + // extensions but not GL_EXT_multi_draw_arrays, and its eglGetProcAddress still + // hands back a non-NULL stub that silently drops every draw. Hence this flag + // requires (EXT or OES draw_elements_base_vertex) AND GL_EXT_multi_draw_arrays + // AND a resolved pointer; callers must gate on it, never on the pointer. + Bool SupportsMultiDrawElementsBaseVertex = false; // GL_RENDERER contains "ANGLE". Bool IsAngleRenderer = false; // GL_RENDERER contains both "ANGLE" and "llvmpipe". diff --git a/MobileGL/MG_Util/SelfTest/DriverPost.cpp b/MobileGL/MG_Util/SelfTest/DriverPost.cpp index b6241196..a99ad1df 100644 --- a/MobileGL/MG_Util/SelfTest/DriverPost.cpp +++ b/MobileGL/MG_Util/SelfTest/DriverPost.cpp @@ -298,6 +298,31 @@ namespace MobileGL::MG_Util::SelfTest { "not supported; no impact: the native indirect path deliberately does not " "rely on it (shader-side emulation handles baseInstance semantics)"); } + // Both multi-draw rows gate on the capability flags, not the entry-point pointers: + // eglGetProcAddress may hand back a non-NULL stub for these on drivers without the + // extension (NVIDIA ES does, and its glMultiDrawElementsBaseVertexEXT stub silently + // drops every draw), so the pointers prove nothing. Absence is INFO in both cases + // because MobileGL falls back to an equivalent per-draw loop. + if (caps.SupportsMultiDrawIndirect) { + builder.Pass("Multi-draw indirect", + "glMultiDrawArrays/ElementsIndirectEXT available via GL_EXT_multi_draw_indirect"); + } else { + builder.Info("Multi-draw indirect", + "GL_EXT_multi_draw_indirect not supported; no impact today: multi-draw " + "indirect is decomposed into per-command indirect draws regardless"); + } + if (caps.SupportsMultiDrawElementsBaseVertex) { + builder.Pass("Multi-draw base vertex", + "glMultiDrawElementsBaseVertexEXT available (EXT/OES_draw_elements_base_vertex " + "with GL_EXT_multi_draw_arrays); glMultiDrawElementsBaseVertex batches into one " + "driver call"); + } else { + builder.Info("Multi-draw base vertex", + "glMultiDrawElementsBaseVertexEXT not supported (needs EXT/OES_" + "draw_elements_base_vertex plus GL_EXT_multi_draw_arrays); " + "glMultiDrawElementsBaseVertex falls back to a per-draw loop with " + "identical output"); + } if (caps.SupportsTextureBorderClamp) { builder.Pass("Texture border clamp", "supported (GL_TEXTURE_BORDER_COLOR reaches the driver, so "