From 945ffff22958416168a819d9c3220e1fa47d3b3a Mon Sep 17 00:00:00 2001 From: Sargun Vohra Date: Mon, 13 Jul 2026 09:51:24 -0700 Subject: [PATCH] core: Tolerate stencil clipping setup failures (#4317) --- platform/linux/src/gl_functions.cpp | 1 - src/mbgl/gl/attribute.hpp | 28 +++++-- src/mbgl/gl/context.cpp | 6 ++ src/mbgl/gl/context.hpp | 1 + src/mbgl/gl/drawable_gl.cpp | 2 +- src/mbgl/gl/layer_group_gl.cpp | 18 ++++- src/mbgl/renderer/paint_parameters.cpp | 81 +++++++++++++-------- src/mbgl/renderer/paint_parameters.hpp | 15 +++- src/mbgl/renderer/render_target.cpp | 3 + src/mbgl/renderer/renderer_impl.cpp | 6 ++ src/mbgl/shaders/gl/legacy/program.hpp | 63 ++++++++-------- src/mbgl/shaders/gl/legacy/program_base.hpp | 17 ++++- 12 files changed, 168 insertions(+), 73 deletions(-) diff --git a/platform/linux/src/gl_functions.cpp b/platform/linux/src/gl_functions.cpp index 3fcc0ae47..f06e6941a 100644 --- a/platform/linux/src/gl_functions.cpp +++ b/platform/linux/src/gl_functions.cpp @@ -3,7 +3,6 @@ #define GL_GLEXT_PROTOTYPES #include -#include namespace mbgl { namespace platform { diff --git a/src/mbgl/gl/attribute.hpp b/src/mbgl/gl/attribute.hpp index 9dbd96343..c69102680 100644 --- a/src/mbgl/gl/attribute.hpp +++ b/src/mbgl/gl/attribute.hpp @@ -5,10 +5,11 @@ #include #include -#include -#include +#include #include +#include #include +#include namespace mbgl { namespace gl { @@ -34,9 +35,12 @@ public: void queryLocations(const ProgramID& id) { locations = Locations{ queryLocation(id, concat_literals<&string_literal<'a', '_'>::value, &As::name>::value())...}; + } + + bool hasExpectedFirstLocation() const { using TypeOfFirst = typename std::tuple_element_t<0, std::tuple>; - [[maybe_unused]] auto first = locations.template get(); - assert(first && first.value() == 0); + const auto location = locations.template get(); + return location && *location == 0; } static constexpr const char* getFirstAttribName() { @@ -67,12 +71,24 @@ public: AttributeBindingArray toBindingArray(const gfx::AttributeBindings>& bindings) const { AttributeBindingArray result; - result.resize(sizeof...(As)); + + std::optional maxLocation; + auto updateMaxLocation = [&](const std::optional& location) { + if (location) { + maxLocation = maxLocation ? std::max(*maxLocation, *location) : *location; + } + }; + + util::ignore({(updateMaxLocation(locations.template get()), 0)...}); + + if (maxLocation) { + result.resize(*maxLocation + 1); + } auto maybeAddBinding = [&](const std::optional& location, const std::optional& binding) { if (location) { - result.at(*location) = binding; + result[*location] = binding; } }; diff --git a/src/mbgl/gl/context.cpp b/src/mbgl/gl/context.cpp index ab2eb63fe..5e7749013 100644 --- a/src/mbgl/gl/context.cpp +++ b/src/mbgl/gl/context.cpp @@ -693,6 +693,12 @@ void Context::setStencilMode(const gfx::StencilMode& stencil) { } } +bool Context::hasStencilBuffer() const { + GLint bits = 0; + MBGL_CHECK_ERROR(glGetIntegerv(GL_STENCIL_BITS, &bits)); + return bits > 0; +} + void Context::setColorMode(const gfx::ColorMode& color) { if (color.blendFunction.is()) { blend = false; diff --git a/src/mbgl/gl/context.hpp b/src/mbgl/gl/context.hpp index b83625fe3..0d654c4ba 100644 --- a/src/mbgl/gl/context.hpp +++ b/src/mbgl/gl/context.hpp @@ -79,6 +79,7 @@ public: void setColorMode(const gfx::ColorMode&); void setCullFaceMode(const gfx::CullFaceMode&); void setScissorTest(const gfx::ScissorRect&); + bool hasStencilBuffer() const; void draw(const gfx::DrawMode&, std::size_t indexOffset, std::size_t indexLength); diff --git a/src/mbgl/gl/drawable_gl.cpp b/src/mbgl/gl/drawable_gl.cpp index 7fca47b45..7d1710bf9 100644 --- a/src/mbgl/gl/drawable_gl.cpp +++ b/src/mbgl/gl/drawable_gl.cpp @@ -243,7 +243,7 @@ gfx::ColorMode DrawableGL::makeColorMode(PaintParameters& parameters) const { } gfx::StencilMode DrawableGL::makeStencilMode(PaintParameters& parameters) const { - if (enableStencil) { + if (enableStencil && parameters.stencilClippingAvailable) { if (!is3D && tileID) { return parameters.stencilModeForClipping(tileID->toUnwrapped()); } diff --git a/src/mbgl/gl/layer_group_gl.cpp b/src/mbgl/gl/layer_group_gl.cpp index 6dab46f05..08ec7b234 100644 --- a/src/mbgl/gl/layer_group_gl.cpp +++ b/src/mbgl/gl/layer_group_gl.cpp @@ -64,7 +64,9 @@ void TileLayerGroupGL::render(RenderOrchestrator&, PaintParameters& parameters) bool stencil3d = false; gfx::StencilMode stencilMode3d; - if (getDrawableCount()) { + parameters.stencilClippingAvailable = parameters.renderTargetHasStencilBuffer; + + if (getDrawableCount() && parameters.stencilClippingAvailable) { MLN_TRACE_ZONE(clip masks); #if !defined(NDEBUG) const auto label_clip = getName() + (getName().empty() ? "" : "-") + "tile-clip-masks"; @@ -90,10 +92,16 @@ void TileLayerGroupGL::render(RenderOrchestrator&, PaintParameters& parameters) if (features3d) { stencilMode3d = stencil3d ? parameters.stencilModeFor3D() : gfx::StencilMode::disabled(); } else if (stencilTiles && !stencilTiles->empty()) { - parameters.renderTileClippingMasks(stencilTiles); + if (!parameters.renderTileClippingMasks(stencilTiles)) { + parameters.stencilClippingAvailable = false; + } } } + if (!parameters.stencilClippingAvailable) { + context.setStencilMode(gfx::StencilMode::disabled()); + } + #if !defined(NDEBUG) const auto label_render = getName() + (getName().empty() ? "" : "-") + "render"; const auto debugGroupRender = parameters.encoder->createDebugGroup(label_render.c_str()); @@ -167,6 +175,12 @@ void LayerGroupGL::render(RenderOrchestrator&, PaintParameters& parameters) { return; } + auto& context = static_cast(parameters.context); + parameters.stencilClippingAvailable = parameters.renderTargetHasStencilBuffer; + if (!parameters.stencilClippingAvailable) { + context.setStencilMode(gfx::StencilMode::disabled()); + } + bool bindUBOs = false; visitDrawables([&](gfx::Drawable& drawable) { if (!drawable.getEnabled() || !drawable.hasRenderPass(parameters.pass)) { diff --git a/src/mbgl/renderer/paint_parameters.cpp b/src/mbgl/renderer/paint_parameters.cpp index c5a6eb5bb..6ca276bf9 100644 --- a/src/mbgl/renderer/paint_parameters.cpp +++ b/src/mbgl/renderer/paint_parameters.cpp @@ -12,6 +12,7 @@ #include #if MLN_RENDER_BACKEND_OPENGL +#include #include #endif @@ -95,6 +96,18 @@ PaintParameters::PaintParameters(gfx::Context& context_, PaintParameters::~PaintParameters() = default; +#if MLN_RENDER_BACKEND_OPENGL +void PaintParameters::updateStencilBufferAvailability() { + auto& glContext = static_cast(context); + renderTargetHasStencilBuffer = glContext.hasStencilBuffer(); + stencilClippingAvailable = renderTargetHasStencilBuffer; + + if (!stencilClippingAvailable) { + glContext.setStencilMode(gfx::StencilMode::disabled()); + } +} +#endif + mat4 PaintParameters::matrixForTile(const UnwrappedTileID& tileID, bool aligned) const { mat4 matrix; state.matrixFor(matrix, tileID); @@ -169,10 +182,14 @@ void PaintParameters::clearStencil() { #endif } -void PaintParameters::renderTileClippingMasks(const RenderTiles& renderTiles) { +bool PaintParameters::renderTileClippingMasks(const RenderTiles& renderTiles) { // We can avoid updating the mask if it already contains the same set of tiles. - if (!renderTiles || !renderPass || tileIDsCovered(renderTiles, tileClippingMaskIDs)) { - return; + if (!renderTiles || tileIDsCovered(renderTiles, tileClippingMaskIDs)) { + return true; + } + + if (!renderPass) { + return false; } tileClippingMaskIDs.clear(); @@ -292,11 +309,9 @@ void PaintParameters::renderTileClippingMasks(const RenderTiles& renderTiles) { auto program = staticData.shaders->getLegacyGroup().get(); if (!program) { - return; + return false; } - static_cast(context).renderingStats().stencilUpdates++; - const style::Properties<>::PossiblyEvaluated properties{}; const ClippingMaskProgram::Binders paintAttributeData(properties, 0); @@ -313,32 +328,38 @@ void PaintParameters::renderTileClippingMasks(const RenderTiles& renderTiles) { continue; } - program->draw(context, - *renderPass, - gfx::Triangles(), - gfx::DepthMode::disabled(), - gfx::StencilMode{gfx::StencilMode::Always{}, - stencilID, - 0b11111111, - gfx::StencilOpType::Keep, - gfx::StencilOpType::Keep, - gfx::StencilOpType::Replace}, - gfx::ColorMode::disabled(), - gfx::CullFaceMode::disabled(), - *staticData.quadTriangleIndexBuffer, - staticData.clippingMaskSegments, - ClippingMaskProgram::computeAllUniformValues( - ClippingMaskProgram::LayoutUniformValues{ - uniforms::matrix::Value(matrixForTile(tileID)), - }, - paintAttributeData, - properties, - static_cast(state.getZoom())), - ClippingMaskProgram::computeAllAttributeBindings( - *staticData.tileVertexBuffer, paintAttributeData, properties), - "clipping/" + util::toString(stencilID)); + if (!program->draw(context, + *renderPass, + gfx::Triangles(), + gfx::DepthMode::disabled(), + gfx::StencilMode{gfx::StencilMode::Always{}, + stencilID, + 0b11111111, + gfx::StencilOpType::Keep, + gfx::StencilOpType::Keep, + gfx::StencilOpType::Replace}, + gfx::ColorMode::disabled(), + gfx::CullFaceMode::disabled(), + *staticData.quadTriangleIndexBuffer, + staticData.clippingMaskSegments, + ClippingMaskProgram::computeAllUniformValues( + ClippingMaskProgram::LayoutUniformValues{ + uniforms::matrix::Value(matrixForTile(tileID)), + }, + paintAttributeData, + properties, + static_cast(state.getZoom())), + ClippingMaskProgram::computeAllAttributeBindings( + *staticData.tileVertexBuffer, paintAttributeData, properties), + "clipping/" + util::toString(stencilID))) { + tileClippingMaskIDs.clear(); + return false; + } } + static_cast(context).renderingStats().stencilUpdates++; #endif // MLN_RENDER_BACKEND_OPENGL + + return true; } gfx::StencilMode PaintParameters::stencilModeForClipping(const UnwrappedTileID& tileID) const { diff --git a/src/mbgl/renderer/paint_parameters.hpp b/src/mbgl/renderer/paint_parameters.hpp index abcac59d8..a585ba07a 100644 --- a/src/mbgl/renderer/paint_parameters.hpp +++ b/src/mbgl/renderer/paint_parameters.hpp @@ -97,7 +97,20 @@ public: // Stencil handling public: - void renderTileClippingMasks(const RenderTiles&); +#if MLN_RENDER_BACKEND_OPENGL + /// Update cached stencil availability for the currently bound GL framebuffer. + void updateStencilBufferAvailability(); + + /// Whether the current GL render target has a stencil buffer. + bool renderTargetHasStencilBuffer = true; + + /// Transient GL draw state for the current layer group. GL layer groups reset this from + /// `renderTargetHasStencilBuffer`, then disable it if clipping-mask setup fails. + bool stencilClippingAvailable = true; +#endif + + /// @return True when clipping mask state is ready or no update was needed. + bool renderTileClippingMasks(const RenderTiles&); /// Clear the stencil buffer, even if there are no tile masks (for 3D) void clearStencil(); diff --git a/src/mbgl/renderer/render_target.cpp b/src/mbgl/renderer/render_target.cpp index 447d57879..81254ba07 100644 --- a/src/mbgl/renderer/render_target.cpp +++ b/src/mbgl/renderer/render_target.cpp @@ -70,6 +70,9 @@ void RenderTarget::render(RenderOrchestrator& orchestrator, const RenderTree& re .clearColor = Color{0.0f, 0.0f, 0.0f, 1.0f}, .clearDepth = {}, .clearStencil = {}}); +#if MLN_RENDER_BACKEND_OPENGL + parameters.updateStencilBufferAvailability(); +#endif const gfx::ScissorRect prevScissorRect = parameters.scissorRect; const auto& size = getTexture()->getSize(); diff --git a/src/mbgl/renderer/renderer_impl.cpp b/src/mbgl/renderer/renderer_impl.cpp index 1ba686915..500773570 100644 --- a/src/mbgl/renderer/renderer_impl.cpp +++ b/src/mbgl/renderer/renderer_impl.cpp @@ -318,6 +318,9 @@ void Renderer::Impl::render(const RenderTree& renderTree, const std::shared_ptr< const auto debugGroup(parameters.encoder->createDebugGroup("common-3d")); parameters.pass = RenderPass::Pass3D; +#if MLN_RENDER_BACKEND_OPENGL + parameters.updateStencilBufferAvailability(); +#endif // TODO is this needed? // if (!parameters.staticData.depthRenderbuffer || @@ -368,6 +371,9 @@ void Renderer::Impl::render(const RenderTree& renderTree, const std::shared_ptr< .clearColor = color, .clearDepth = 1.0f, .clearStencil = 0}); +#if MLN_RENDER_BACKEND_OPENGL + parameters.updateStencilBufferAvailability(); +#endif } }; diff --git a/src/mbgl/shaders/gl/legacy/program.hpp b/src/mbgl/shaders/gl/legacy/program.hpp index 4a01420b8..dedbd81ab 100644 --- a/src/mbgl/shaders/gl/legacy/program.hpp +++ b/src/mbgl/shaders/gl/legacy/program.hpp @@ -91,7 +91,7 @@ public: } template - void draw(gfx::Context& context, + bool draw(gfx::Context& context, gfx::RenderPass& renderPass, const DrawMode& drawMode, const gfx::DepthMode& depthMode, @@ -106,7 +106,7 @@ public: static_assert(Primitive == gfx::PrimitiveTypeOf::value, "incompatible draw mode"); if (!programBase) { - return; + return false; } auto drawScopeIt = segment.drawScopes.find(layerID); @@ -114,23 +114,23 @@ public: drawScopeIt = segment.drawScopes.emplace(layerID, context.createDrawScope()).first; } - programBase->draw(context, - renderPass, - drawMode, - depthMode, - stencilMode, - colorMode, - cullFaceMode, - uniformValues, - drawScopeIt->second, - allAttributeBindings.offset(segment.vertexOffset), - indexBuffer, - segment.indexOffset, - segment.indexLength); + return programBase->draw(context, + renderPass, + drawMode, + depthMode, + stencilMode, + colorMode, + cullFaceMode, + uniformValues, + drawScopeIt->second, + allAttributeBindings.offset(segment.vertexOffset), + indexBuffer, + segment.indexOffset, + segment.indexLength); } template - void draw(gfx::Context& context, + bool draw(gfx::Context& context, gfx::RenderPass& renderPass, const DrawMode& drawMode, const gfx::DepthMode& depthMode, @@ -145,7 +145,7 @@ public: static_assert(Primitive == gfx::PrimitiveTypeOf::value, "incompatible draw mode"); if (!programBase) { - return; + return false; } for (auto& segment : segments) { @@ -155,20 +155,23 @@ public: drawScopeIt = segment.drawScopes.emplace(layerID, context.createDrawScope()).first; } - programBase->draw(context, - renderPass, - drawMode, - depthMode, - stencilMode, - colorMode, - cullFaceMode, - uniformValues, - drawScopeIt->second, - allAttributeBindings.offset(segment.vertexOffset), - indexBuffer, - segment.indexOffset, - segment.indexLength); + if (!programBase->draw(context, + renderPass, + drawMode, + depthMode, + stencilMode, + colorMode, + cullFaceMode, + uniformValues, + drawScopeIt->second, + allAttributeBindings.offset(segment.vertexOffset), + indexBuffer, + segment.indexOffset, + segment.indexLength)) { + return false; + } } + return true; } }; diff --git a/src/mbgl/shaders/gl/legacy/program_base.hpp b/src/mbgl/shaders/gl/legacy/program_base.hpp index bda40764f..a8654b3f3 100644 --- a/src/mbgl/shaders/gl/legacy/program_base.hpp +++ b/src/mbgl/shaders/gl/legacy/program_base.hpp @@ -15,6 +15,8 @@ #include #include +#include +#include #include namespace mbgl { @@ -45,6 +47,10 @@ public: context.createShader(ShaderType::Fragment, fragmentSource), attributeLocations.getFirstAttribName())) { attributeLocations.queryLocations(program); + if (!attributeLocations.hasExpectedFirstLocation()) { + throw std::runtime_error("Shader is missing active attribute at location 0: " + + std::string(attributeLocations.getFirstAttribName())); + } uniformStates.queryLocations(program); } @@ -80,7 +86,7 @@ public: gl::UniformStates uniformStates; }; - void draw(gfx::Context& genericContext, + bool draw(gfx::Context& genericContext, gfx::RenderPass&, const gfx::DrawMode& drawMode, const gfx::DepthMode& depthMode, @@ -101,6 +107,10 @@ public: context.setCullFaceMode(cullFaceMode); const uint32_t key = gl::AttributeKey::compute(attributeBindings); + if (failedInstances.contains(key)) { + return false; + } + auto it = instances.find(key); if (it == instances.end()) { try { @@ -112,7 +122,8 @@ public: .first; } catch (const std::runtime_error& e) { Log::Error(Event::OpenGL, e.what()); - return; + failedInstances.insert(key); + return false; } } @@ -125,10 +136,12 @@ public: vertexArray.bind(context, indexBuffer, instance.attributeLocations.toBindingArray(attributeBindings)); context.draw(drawMode, indexOffset, indexLength); + return true; } private: std::map> instances; + std::set failedInstances; }; } // namespace gl