diff --git a/MobileGL/MG_Backend/DirectGLES/Managers.cpp b/MobileGL/MG_Backend/DirectGLES/Managers.cpp index d53426a1..728454f9 100644 --- a/MobileGL/MG_Backend/DirectGLES/Managers.cpp +++ b/MobileGL/MG_Backend/DirectGLES/Managers.cpp @@ -6846,10 +6846,21 @@ namespace MobileGL::MG_Backend::DirectGLES { const String outMembers = ExtractPerVertexBlockMembers(tessEvalStageEssl, /*input=*/true).value_or(String()); - const String source = BuildPassthroughTessControlEssl(ResolveBackendEsslVersion(), patchVertices, - inMembers, outMembers, - m_passthroughTessControlOuterLevel, - m_passthroughTessControlInnerLevel); + String source = BuildPassthroughTessControlEssl(ResolveBackendEsslVersion(), patchVertices, + inMembers, outMembers, + m_passthroughTessControlOuterLevel, + m_passthroughTessControlInnerLevel); + // The mirrored member lists can carry gl_PointSize - the neighbour stage declared it, + // so matching it is the whole point - and a redeclaration is exactly as illegal as a + // reference in ESSL without the extension. Same directive, same never-speculative + // rule as the per-stage loop; a driver with neither spelling gets nothing added and + // fails below with its own message, which is the honest outcome for a shape it + // cannot express. + const char* passthroughPointSizeExtension = + source.find("gl_PointSize") != String::npos + ? PointSizeExtensionName(g_GLESCapabilities.TessellationPointSizeSupport, /*tessellation=*/true) + : nullptr; + source = RequestPointSizeExtension(Move(source), passthroughPointSizeExtension); const GLuint backendShaderId = g_GLESFuncs.glCreateShader(GL_TESS_CONTROL_SHADER); if (backendShaderId == 0) { @@ -7373,6 +7384,40 @@ namespace MobileGL::MG_Backend::DirectGLES { source.find("gl_ViewportIndex") != String::npos; source = RequestViewportArrayExtension(std::move(source), needsViewportArrayExtension); + // The fourth header-level rewrite, and the same shape as the third: ESSL has no + // gl_PointSize in a tessellation or geometry stage at ANY version - 320 makes the + // stages core and still leaves the built-in behind EXT/OES_..._point_size - while + // SPIRV-Cross prints it bare. Without the directive the stage fails to compile + // with "`gl_PointSize' undeclared", which takes the whole program to program 0: + // the draw renders nothing AND glBeginTransformFeedback is rejected, so a capture + // of anything at all off that program silently comes back empty. The token probe + // keeps the line off every other program and PointSizeExtensionName returns + // nullptr - i.e. nothing is emitted - on a driver advertising neither spelling. + if (source.find("gl_PointSize") != String::npos) { + const Bool tessellationStage = glShaderType == GL_TESS_CONTROL_SHADER || + glShaderType == GL_TESS_EVALUATION_SHADER; + if (tessellationStage || glShaderType == GL_GEOMETRY_SHADER) { + const auto tier = tessellationStage ? g_GLESCapabilities.TessellationPointSizeSupport + : g_GLESCapabilities.GeometryPointSizeSupport; + const char* pointSizeExtension = PointSizeExtensionName(tier, tessellationStage); + if (pointSizeExtension == nullptr) { + // Latched, and an ERROR rather than a warning: what follows is a + // driver compile failure whose text names a built-in the application + // never mis-spelled, and the reason is a missing driver capability + // rather than anything in the shader. Saying so here is the whole + // difference between a legible skip and an unexplained black draw. + MGLOG_E_ONCE("This driver advertises neither the EXT nor the OES %s_point_size " + "extension, so its ESSL has no gl_PointSize in a %s stage; program %u " + "will fail to compile. Point size from a non-vertex stage is not " + "available on this device.", + tessellationStage ? "tessellation" : "geometry", + tessellationStage ? "tessellation" : "geometry", + stateProgramObject->GetExternalIndex()); + } + source = RequestPointSizeExtension(std::move(source), pointSizeExtension); + } + } + source = RebindImageUniformsToFrontendUnits(std::move(source), stateProgramObject); // The completion half of the format bake, for the formats SPIRV-Cross throws on // rather than prints (r8ui and the rest of its desktop-only set). Empty for every @@ -7495,11 +7540,17 @@ namespace MobileGL::MG_Backend::DirectGLES { // were being rejected outright, and the lane could not say why it was // rendering an empty translucent layer. A shader the driver refuses is // never noise, and one line per refused shader is bounded by program count. + // + // The SOURCE goes with it, the way the synthesized pass-through control + // stage's failure already prints its own: the driver log names a line and a + // column in text that exists nowhere but here, and without it the only way + // to read "`gl_PointSize' undeclared" is to rebuild the whole library at + // DEBUG. Still one line per refused shader. MGLOG_E("Shader compilation failed. State program ID: %u, stage: %s, backend shader ID: " - "%u, driver log: %s", + "%u, driver log: %s\nSource:\n%s", stateProgramObject->GetExternalIndex(), MG_Util::ConvertGLEnumToString(glShaderType).c_str(), backendShaderId, - log.data()); + log.data(), source.c_str()); m_backendProgramUsable = false; // Nothing will ever attach this one, so nothing else can free it. g_GLESFuncs.glDeleteShader(backendShaderId); diff --git a/MobileGL/MG_Backend/DirectGLES/Utils.cpp b/MobileGL/MG_Backend/DirectGLES/Utils.cpp index 8914be6b..f23bf969 100644 --- a/MobileGL/MG_Backend/DirectGLES/Utils.cpp +++ b/MobileGL/MG_Backend/DirectGLES/Utils.cpp @@ -713,6 +713,47 @@ namespace MobileGL::MG_Backend::DirectGLES { return glslCode; } + const char* PointSizeExtensionName(MG_External::GLESCapabilities::PointSizeTier tier, Bool tessellation) { + using Tier = MG_External::GLESCapabilities::PointSizeTier; + switch (tier) { + case Tier::ExtensionEXT: + return tessellation ? "GL_EXT_tessellation_point_size" : "GL_EXT_geometry_point_size"; + case Tier::ExtensionOES: + return tessellation ? "GL_OES_tessellation_point_size" : "GL_OES_geometry_point_size"; + default: + return nullptr; + } + } + + String RequestPointSizeExtension(String glslCode, const char* extensionName) { +#ifdef TRACY_ENABLE + ZoneScopedC(TRACY_ZONECOLOR_BACKEND); +#endif + // The gl_ViewportIndex story, one built-in over: ESSL 320 makes the tessellation and + // geometry STAGES core but leaves gl_PointSize out of their gl_PerVertex entirely, + // and SPIRV-Cross - which only ever sees a SPIR-V BuiltIn PointSize decoration - + // prints the identifier with no directive behind it. Same hard rule as the two + // neighbours: never emitted speculatively, because `#extension` on a name the driver + // does not advertise is a compile error of its own. + if (extensionName == nullptr || glslCode.find(extensionName) != String::npos) { + return glslCode; + } + const String directive = String("#extension ") + extensionName + " : require\n"; + // Right after the #version line, the one position that must stay first; + // ForceSupporterOutput's scan for the LAST #extension directive still finds + // whichever one that ends up being. + const SizeT versionPos = glslCode.find("#version"); + if (versionPos == String::npos) { + return directive + glslCode; + } + const SizeT lineEnd = glslCode.find('\n', versionPos); + if (lineEnd == String::npos) { + return glslCode + "\n" + directive; + } + glslCode.insert(lineEnd + 1, directive); + return glslCode; + } + String BakeImageFormatQualifiers(String glslCode, const UnorderedMap& esslFormatByUniformName) { #ifdef TRACY_ENABLE diff --git a/MobileGL/MG_Backend/DirectGLES/Utils.h b/MobileGL/MG_Backend/DirectGLES/Utils.h index c7239006..3ad92a45 100644 --- a/MobileGL/MG_Backend/DirectGLES/Utils.h +++ b/MobileGL/MG_Backend/DirectGLES/Utils.h @@ -273,6 +273,22 @@ namespace MobileGL::MG_Backend::DirectGLES { // error, so this is never emitted speculatively. A no-op when not needed or already // present. String RequestViewportArrayExtension(String glslCode, Bool needed); + // Adds `#extension : require` when a TESSELLATION or GEOMETRY stage's + // emitted ESSL names gl_PointSize. Desktop GL has that built-in in gl_PerVertex for every + // vertex-processing stage; ESSL does NOT have it in those two at any version - not even + // 320, where the stages themselves are core - until EXT/OES_tessellation_point_size resp. + // EXT/OES_geometry_point_size is requested. SPIRV-Cross prints the identifier bare and + // asks for nothing, exactly as it does for gl_ViewportIndex, so without this the stage + // fails to compile with "`gl_PointSize' undeclared" and the WHOLE program is replaced by + // program 0 - the draw renders nothing and any transform-feedback capture it was carrying + // is rejected outright. `extensionName` is the caller's answer, nullptr when the driver + // advertises neither spelling, because requesting an unadvertised extension is itself a + // compile error. A no-op when nullptr or already present. + String RequestPointSizeExtension(String glslCode, const char* extensionName); + // The extension name RequestPointSizeExtension should be given for `tier`, or nullptr for + // PointSizeTier::None. `tessellation` picks the tessellation spellings over the geometry + // ones; the two extensions are separate and neither implies the other. + const char* PointSizeExtensionName(MG_External::GLESCapabilities::PointSizeTier tier, Bool tessellation); // Writes a format layout qualifier into the image declarations named in // `esslFormatByUniformName` that still have none. The completion half of the image-format // bake, and ONLY that: the SPIR-V pass (BakeImageFormatsPass) is what normally puts the diff --git a/MobileGL/MG_Util/BackendLoaders/OpenGL/Loader.cpp b/MobileGL/MG_Util/BackendLoaders/OpenGL/Loader.cpp index 47fb349d..8499681e 100644 --- a/MobileGL/MG_Util/BackendLoaders/OpenGL/Loader.cpp +++ b/MobileGL/MG_Util/BackendLoaders/OpenGL/Loader.cpp @@ -984,6 +984,20 @@ namespace MobileGL::MG_Util::BackendLoader { if (std::strcmp(extension, "GL_OES_viewport_array") == 0) { caps.SupportsViewportArray = true; } + // EXT wins where both are advertised: it is the spelling the Android Extension + // Pack mandates, so it is the one a driver is most likely to have tested. + if (std::strcmp(extension, "GL_EXT_tessellation_point_size") == 0) { + caps.TessellationPointSizeSupport = MG_External::GLESCapabilities::PointSizeTier::ExtensionEXT; + } else if (std::strcmp(extension, "GL_OES_tessellation_point_size") == 0 && + caps.TessellationPointSizeSupport == MG_External::GLESCapabilities::PointSizeTier::None) { + caps.TessellationPointSizeSupport = MG_External::GLESCapabilities::PointSizeTier::ExtensionOES; + } + if (std::strcmp(extension, "GL_EXT_geometry_point_size") == 0) { + caps.GeometryPointSizeSupport = MG_External::GLESCapabilities::PointSizeTier::ExtensionEXT; + } else if (std::strcmp(extension, "GL_OES_geometry_point_size") == 0 && + caps.GeometryPointSizeSupport == MG_External::GLESCapabilities::PointSizeTier::None) { + caps.GeometryPointSizeSupport = MG_External::GLESCapabilities::PointSizeTier::ExtensionOES; + } } } // The pointer check on top of the extension check makes each flag sufficient on its own @@ -1055,6 +1069,19 @@ namespace MobileGL::MG_Util::BackendLoader { MGLOG_I(" clip distances (EXT_clip_cull_distance): %s", caps.SupportsClipDistance ? "yes" : "no"); MGLOG_I(" viewport array (OES_viewport_array; gl_ViewportIndex collapses to viewport 0 when absent): %s", caps.SupportsViewportArray ? "yes" : "no"); + { + const auto pointSizeTierName = [](MG_External::GLESCapabilities::PointSizeTier tier) { + switch (tier) { + case MG_External::GLESCapabilities::PointSizeTier::ExtensionEXT: return "EXT"; + case MG_External::GLESCapabilities::PointSizeTier::ExtensionOES: return "OES"; + default: return "no"; + } + }; + MGLOG_I(" tessellation gl_PointSize (EXT/OES_tessellation_point_size): %s", + pointSizeTierName(caps.TessellationPointSizeSupport)); + MGLOG_I(" geometry gl_PointSize (EXT/OES_geometry_point_size): %s", + pointSizeTierName(caps.GeometryPointSizeSupport)); + } // LOAD-BEARING STRING, not just a banner. android-plugin/trace-replay-ci.sh's // is_angle_surface_lost() greps mobilegl.log for exactly "OpenGL ES capabilities:" to diff --git a/MobileGL/MG_Util/BackendLoaders/OpenGL/Loader.h b/MobileGL/MG_Util/BackendLoaders/OpenGL/Loader.h index 03d28b0f..71b18dda 100644 --- a/MobileGL/MG_Util/BackendLoaders/OpenGL/Loader.h +++ b/MobileGL/MG_Util/BackendLoaders/OpenGL/Loader.h @@ -1118,6 +1118,23 @@ namespace MobileGL { ExtensionOES, // GL_OES_texture_buffer; ESSL below 320 must say GL_OES_texture_buffer }; TextureBufferTier TextureBufferSupport = TextureBufferTier::None; + // Which spelling of per-vertex point size a NON-VERTEX stage has, if any. In desktop + // GL gl_PointSize is an ordinary gl_PerVertex member that any vertex-processing stage + // may write and any program may capture by name; in ESSL it does not EXIST in a + // tessellation or geometry stage until GL_EXT/OES_tessellation_point_size (resp. + // ..._geometry_point_size) is requested - not even at 320, where the stages + // themselves are core. SPIRV-Cross prints the identifier bare and asks for nothing, + // exactly as it does for gl_ViewportIndex, so the directive has to be inserted into + // the emitted source (RequestPointSizeExtension) and a driver with neither spelling + // cannot compile such a stage at all. Extension string only: these add no entry + // points, so there is no pointer to require. + enum class PointSizeTier : Uint8 { + None = 0, // neither spelling; the stage cannot name gl_PointSize + ExtensionEXT, // GL_EXT_tessellation_point_size / GL_EXT_geometry_point_size + ExtensionOES, // GL_OES_tessellation_point_size / GL_OES_geometry_point_size + }; + PointSizeTier TessellationPointSizeSupport = PointSizeTier::None; + PointSizeTier GeometryPointSizeSupport = PointSizeTier::None; // GL_MAX_TEXTURE_BUFFER_SIZE actually came back from the driver. False means the value // below is MobileGL's own floor, not a driver answer: the pname is only legal once // buffer textures exist, and querying it on a driver without them raises