From 61da26fe1418fbfe297ee53cc1d7f21fa891a895 Mon Sep 17 00:00:00 2001
From: Tim Sylvester <tim@wetdogweather.com>
Date: Fri, 28 Apr 2023 15:07:03 -0700
Subject: [PATCH] Drawable Renderer (#1570), Eliminate GL drawable/builder
 dependencies from generic background layer

---
 include/mbgl/gfx/drawable.hpp                 |  7 +++-
 include/mbgl/gfx/drawable_builder.hpp         | 10 ++++--
 include/mbgl/gl/drawable_gl.hpp               |  2 +-
 include/mbgl/gl/drawable_gl_builder.hpp       |  6 ++--
 src/mbgl/gfx/context.hpp                      | 11 ++++++
 src/mbgl/gfx/drawable.cpp                     |  3 +-
 src/mbgl/gfx/drawable_builder.cpp             |  7 ++--
 src/mbgl/gl/context.cpp                       | 18 +++++++---
 src/mbgl/gl/context.hpp                       |  3 ++
 src/mbgl/gl/drawable_gl.cpp                   |  5 +--
 src/mbgl/gl/drawable_gl_builder.cpp           |  4 +--
 src/mbgl/renderer/render_orchestrator.cpp     |  5 +--
 src/mbgl/renderer/render_orchestrator.hpp     |  1 +
 src/mbgl/renderer/renderer.cpp                |  9 +++--
 src/mbgl/style/layer_impl.hpp                 |  5 ++-
 .../style/layers/background_layer_impl.cpp    | 36 ++++++++++++-------
 .../style/layers/background_layer_impl.hpp    |  4 ++-
 17 files changed, 97 insertions(+), 39 deletions(-)

diff --git a/include/mbgl/gfx/drawable.hpp b/include/mbgl/gfx/drawable.hpp
index 51cc93418..cb0a96c89 100644
--- a/include/mbgl/gfx/drawable.hpp
+++ b/include/mbgl/gfx/drawable.hpp
@@ -27,7 +27,7 @@ using DrawableTweakerPtr = std::shared_ptr<DrawableTweaker>;
 
 class Drawable {
 protected:
-    Drawable();
+    Drawable(std::string name);
 
 public:
     virtual ~Drawable() = default;
@@ -37,6 +37,10 @@ public:
     /// Draw the drawable
     virtual void draw(const PaintParameters &) const = 0;
 
+    /// Drawable name is used for debugging and troubleshooting
+    const std::string& getName() const { return name; }
+    void setName(std::string value) { name = std::move(value); }
+
     /// Which shader to use when rendering this drawable
     const gfx::ShaderProgramBasePtr& getShader() const { return shader; }
     void setShader(gfx::ShaderProgramBasePtr value) { shader = std::move(value); }
@@ -76,6 +80,7 @@ public:
     virtual void resetColor(const Color&) = 0;
 
 protected:
+    std::string name;
     util::SimpleIdentity uniqueID;
     gfx::ShaderProgramBasePtr shader;
     mat4 matrix; //= matrix::identity4();
diff --git a/include/mbgl/gfx/drawable_builder.hpp b/include/mbgl/gfx/drawable_builder.hpp
index 4a759adf5..5af3a921b 100644
--- a/include/mbgl/gfx/drawable_builder.hpp
+++ b/include/mbgl/gfx/drawable_builder.hpp
@@ -24,7 +24,7 @@ using ShaderProgramBasePtr = std::shared_ptr<ShaderProgramBase>;
  */
 class DrawableBuilder {
 protected:
-    DrawableBuilder();
+    DrawableBuilder(std::string name);
 
 public:
     virtual ~DrawableBuilder();
@@ -75,6 +75,10 @@ public:
     /// Get the vertex attributes that override default values in the shader program
     virtual const gfx::VertexAttributeArray& getVertexAttributes() const = 0;
 
+    /// Set the name given to new drawables
+    void setDrawableName(std::string value) { drawableName = std::move(value); }
+
+    /// Set the matrix applied to new drawables
     void setMatrix(mat4 value) { matrix = value; }
 
     /// Add a triangle
@@ -90,12 +94,14 @@ public:
 
 protected:
     /// Create an instance of the appropriate drawable type
-    virtual DrawablePtr createDrawable() const = 0;
+    virtual DrawablePtr createDrawable(std::string name) const = 0;
 
     /// Setup the SDK-specific aspects after all the values are present
     virtual void init() = 0;
 
 protected:
+    std::string name;
+    std::string drawableName;
     DrawPriority drawPriority = 0;
     DepthMaskType depthType = DepthMaskType::ReadOnly;
     gfx::ShaderProgramBasePtr shader;
diff --git a/include/mbgl/gl/drawable_gl.hpp b/include/mbgl/gl/drawable_gl.hpp
index 2de566a74..18da71d51 100644
--- a/include/mbgl/gl/drawable_gl.hpp
+++ b/include/mbgl/gl/drawable_gl.hpp
@@ -22,7 +22,7 @@ class VertexArray;
 
 class DrawableGL : public gfx::Drawable {
 public:
-    DrawableGL();
+    DrawableGL(std::string name);
     ~DrawableGL() override;
 
     void draw(const PaintParameters &) const override;
diff --git a/include/mbgl/gl/drawable_gl_builder.hpp b/include/mbgl/gl/drawable_gl_builder.hpp
index 500148742..5ab116673 100644
--- a/include/mbgl/gl/drawable_gl_builder.hpp
+++ b/include/mbgl/gl/drawable_gl_builder.hpp
@@ -11,16 +11,16 @@ namespace gl {
  */
 class DrawableGLBuilder final : public gfx::DrawableBuilder {
 public:
-    DrawableGLBuilder() {
+    DrawableGLBuilder(std::string name) : gfx::DrawableBuilder(std::move(name)) {
     }
     ~DrawableGLBuilder() override = default;
-    
+
     const gfx::VertexAttributeArray& getVertexAttributes() const override { return vertexAttributes; }
     void setVertexAttributes(const VertexAttributeArrayGL& value) { vertexAttributes = value; }
     void setVertexAttributes(VertexAttributeArrayGL&& value) { vertexAttributes = std::move(value); }
     
 protected:
-    gfx::DrawablePtr createDrawable() const override;
+    gfx::DrawablePtr createDrawable(std::string name) const override;
     
     /// Setup the SDK-specific aspects after all the values are present
     void init() override;
diff --git a/src/mbgl/gfx/context.hpp b/src/mbgl/gfx/context.hpp
index b580e0352..451b1e403 100644
--- a/src/mbgl/gfx/context.hpp
+++ b/src/mbgl/gfx/context.hpp
@@ -17,9 +17,14 @@ class ProgramParameters;
 namespace gfx {
 
 class Drawable;
+class DrawableBuilder;
+class DrawableTweaker;
 class OffscreenTexture;
 class ShaderProgramBase;
 
+using DrawablePtr = std::shared_ptr<Drawable>;
+using UniqueDrawableBuilder = std::unique_ptr<DrawableBuilder>;
+using DrawableTweakerPtr = std::shared_ptr<DrawableTweaker>;
 using ShaderProgramBasePtr = std::shared_ptr<ShaderProgramBase>;
 
 
@@ -92,6 +97,12 @@ public:
 public:
     /// Activate the shader, vertex attributes, etc., specified by the drawable
     virtual bool setupDraw(const PaintParameters&, const gfx::Drawable&) = 0;
+
+    /// Create a new drawable builder
+    virtual UniqueDrawableBuilder createDrawableBuilder(std::string name) = 0;
+    
+    /// Create a new drawable tweaker
+    virtual DrawableTweakerPtr createDrawableTweaker() = 0;
 };
 
 } // namespace gfx
diff --git a/src/mbgl/gfx/drawable.cpp b/src/mbgl/gfx/drawable.cpp
index 1fc6e5c2a..2c1b5043c 100644
--- a/src/mbgl/gfx/drawable.cpp
+++ b/src/mbgl/gfx/drawable.cpp
@@ -5,7 +5,8 @@
 namespace mbgl {
 namespace gfx {
 
-Drawable::Drawable() :
+Drawable::Drawable(std::string name_) :
+    name(name_),
     matrix(matrix::identity4()),
     depthType(DepthMaskType::ReadOnly)
 {
diff --git a/src/mbgl/gfx/drawable_builder.cpp b/src/mbgl/gfx/drawable_builder.cpp
index 3e457fe79..3bf0dbd59 100644
--- a/src/mbgl/gfx/drawable_builder.cpp
+++ b/src/mbgl/gfx/drawable_builder.cpp
@@ -4,8 +4,9 @@
 namespace mbgl {
 namespace gfx {
 
-DrawableBuilder::DrawableBuilder()
-    : impl(std::make_unique<Impl>()) {
+DrawableBuilder::DrawableBuilder(std::string name_) :
+    name(std::move(name_)),
+    impl(std::make_unique<Impl>()) {
 }
 
 DrawableBuilder::~DrawableBuilder() = default;
@@ -19,7 +20,7 @@ void DrawableBuilder::setColor(const Color& value) {
 
 DrawablePtr DrawableBuilder::getCurrentDrawable(bool createIfNone) {
     if (!currentDrawable && createIfNone) {
-        currentDrawable = createDrawable();
+        currentDrawable = createDrawable(drawableName.empty() ? name : drawableName);
     }
     return currentDrawable;
 }
diff --git a/src/mbgl/gl/context.cpp b/src/mbgl/gl/context.cpp
index d16e8832f..d950fba1a 100644
--- a/src/mbgl/gl/context.cpp
+++ b/src/mbgl/gl/context.cpp
@@ -1,15 +1,17 @@
 #include <mbgl/gl/context.hpp>
+#include <mbgl/gl/command_encoder.hpp>
+#include <mbgl/gl/defines.hpp>
 #include <mbgl/gl/drawable_gl.hpp>
+#include <mbgl/gl/drawable_gl_builder.hpp>
+#include <mbgl/gl/drawable_gl_tweaker.hpp>
+#include <mbgl/gl/draw_scope_resource.hpp>
 #include <mbgl/gl/enum.hpp>
 #include <mbgl/gl/renderer_backend.hpp>
-#include <mbgl/gl/texture_resource.hpp>
 #include <mbgl/gl/renderbuffer_resource.hpp>
-#include <mbgl/gl/draw_scope_resource.hpp>
+#include <mbgl/gl/texture_resource.hpp>
 #include <mbgl/gl/texture.hpp>
 #include <mbgl/gl/offscreen_texture.hpp>
-#include <mbgl/gl/command_encoder.hpp>
 #include <mbgl/gl/debugging_extension.hpp>
-#include <mbgl/gl/defines.hpp>
 #include <mbgl/renderer/paint_parameters.hpp>
 #include <mbgl/shaders/gl/shader_program_gl.hpp>
 #include <mbgl/util/traits.hpp>
@@ -477,6 +479,14 @@ bool Context::setupDraw(const PaintParameters& parameters, const gfx::Drawable&
     }
 }
 
+gfx::UniqueDrawableBuilder Context::createDrawableBuilder(std::string name) {
+    return std::make_unique<gl::DrawableGLBuilder>(std::move(name));
+}
+
+gfx::DrawableTweakerPtr Context::createDrawableTweaker() {
+    return std::make_shared<gl::DrawableGLTweaker>();
+}
+
 void Context::clear(std::optional<mbgl::Color> color,
                     std::optional<float> depth,
                     std::optional<int32_t> stencil) {
diff --git a/src/mbgl/gl/context.hpp b/src/mbgl/gl/context.hpp
index e5b943781..05b4f7cfe 100644
--- a/src/mbgl/gl/context.hpp
+++ b/src/mbgl/gl/context.hpp
@@ -106,6 +106,9 @@ public:
 
     bool setupDraw(const PaintParameters&, const gfx::Drawable&) override;
 
+    gfx::UniqueDrawableBuilder createDrawableBuilder(std::string name) override;
+    gfx::DrawableTweakerPtr createDrawableTweaker() override;
+
 private:
     RendererBackend& backend;
     bool cleanupOnDestruction = true;
diff --git a/src/mbgl/gl/drawable_gl.cpp b/src/mbgl/gl/drawable_gl.cpp
index 009c5bae9..cdfd1ee37 100644
--- a/src/mbgl/gl/drawable_gl.cpp
+++ b/src/mbgl/gl/drawable_gl.cpp
@@ -5,8 +5,9 @@
 namespace mbgl {
 namespace gl {
 
-DrawableGL::DrawableGL()
-    : impl(std::make_unique<Impl>()) {
+DrawableGL::DrawableGL(std::string name) :
+    Drawable(std::move(name)),
+    impl(std::make_unique<Impl>()) {
 }
 
 DrawableGL::~DrawableGL() {
diff --git a/src/mbgl/gl/drawable_gl_builder.cpp b/src/mbgl/gl/drawable_gl_builder.cpp
index c3c682fe0..979d39808 100644
--- a/src/mbgl/gl/drawable_gl_builder.cpp
+++ b/src/mbgl/gl/drawable_gl_builder.cpp
@@ -7,8 +7,8 @@
 namespace mbgl {
 namespace gl {
 
-gfx::DrawablePtr DrawableGLBuilder::createDrawable() const {
-    return std::make_shared<DrawableGL>();
+gfx::DrawablePtr DrawableGLBuilder::createDrawable(std::string name) const {
+    return std::make_shared<DrawableGL>(std::move(name));
 };
 
 void DrawableGLBuilder::init() {
diff --git a/src/mbgl/renderer/render_orchestrator.cpp b/src/mbgl/renderer/render_orchestrator.cpp
index fb81135aa..cf74adcc8 100644
--- a/src/mbgl/renderer/render_orchestrator.cpp
+++ b/src/mbgl/renderer/render_orchestrator.cpp
@@ -777,6 +777,7 @@ const gfx::DrawablePtr& RenderOrchestrator::getDrawable(const util::SimpleIdenti
 }
 
 void RenderOrchestrator::updateLayers(gfx::ShaderRegistry& shaders,
+                                      gfx::Context& context,
                                       const TransformState& state,
                                       const PropertyEvaluationParameters& evalParameters) {
     std::vector<std::unique_ptr<ChangeRequest>> changes;
@@ -785,11 +786,11 @@ void RenderOrchestrator::updateLayers(gfx::ShaderRegistry& shaders,
         kv.second->layerRemoved(changes);
     }
     for (auto& kv : layersAdded) {
-        kv.second->layerAdded(shaders, state, evalParameters, changes);
+        kv.second->layerAdded(shaders, context, state, evalParameters, changes);
     }
     
     for (auto& impl : *layerImpls) {
-        impl->update(state, evalParameters, changes);
+        impl->update(context, state, evalParameters, changes);
     }
 
     addChanges(changes);
diff --git a/src/mbgl/renderer/render_orchestrator.hpp b/src/mbgl/renderer/render_orchestrator.hpp
index 75bf81eba..9c9437711 100644
--- a/src/mbgl/renderer/render_orchestrator.hpp
+++ b/src/mbgl/renderer/render_orchestrator.hpp
@@ -102,6 +102,7 @@ public:
     const gfx::DrawablePtr& getDrawable(const util::SimpleIdentity&);
 
     void updateLayers(gfx::ShaderRegistry&,
+                      gfx::Context&,
                       const TransformState&,
                       const PropertyEvaluationParameters&);
 
diff --git a/src/mbgl/renderer/renderer.cpp b/src/mbgl/renderer/renderer.cpp
index 05c5ba05e..872b19fb3 100644
--- a/src/mbgl/renderer/renderer.cpp
+++ b/src/mbgl/renderer/renderer.cpp
@@ -1,12 +1,13 @@
 #include <mbgl/renderer/renderer.hpp>
 
+#include <mbgl/annotation/annotation_manager.hpp>
+#include <mbgl/gfx/backend_scope.hpp>
+#include <mbgl/gfx/renderer_backend.hpp>
 #include <mbgl/layermanager/layer_manager.hpp>
 #include <mbgl/renderer/renderer_impl.hpp>
 #include <mbgl/renderer/render_static_data.hpp>
 #include <mbgl/renderer/render_tree.hpp>
 #include <mbgl/renderer/update_parameters.hpp>
-#include <mbgl/gfx/backend_scope.hpp>
-#include <mbgl/annotation/annotation_manager.hpp>
 
 namespace mbgl {
 
@@ -46,7 +47,9 @@ void Renderer::render(const std::shared_ptr<UpdateParameters>& updateParameters)
         const auto& state = renderTreeParameters.transformParams.state;
 
         if (impl->staticData && impl->staticData->shaders) {
-            impl->orchestrator.updateLayers(*impl->staticData->shaders, state, evalParameters);
+            auto& context = impl->backend.getContext();
+            auto& shaders = *impl->staticData->shaders;
+            impl->orchestrator.updateLayers(shaders, context, state, evalParameters);
         }
 
         impl->render(*renderTree);
diff --git a/src/mbgl/style/layer_impl.hpp b/src/mbgl/style/layer_impl.hpp
index bee08c6de..12daafa14 100644
--- a/src/mbgl/style/layer_impl.hpp
+++ b/src/mbgl/style/layer_impl.hpp
@@ -23,6 +23,7 @@ using UniqueChangeRequest = std::unique_ptr<ChangeRequest>;
 using UniqueChangeRequestVec = std::vector<UniqueChangeRequest>;
 
 namespace gfx {
+class Context;
 class ShaderRegistry;
 } // namespace gfx
 
@@ -62,11 +63,13 @@ public:
     virtual void populateFontStack(std::set<FontStack>& fontStack) const;
 
     /// Generate any changes needed by the layer
-    virtual void update(const TransformState&,
+    virtual void update(gfx::Context&,
+                        const TransformState&,
                         const PropertyEvaluationParameters&,
                         UniqueChangeRequestVec&) const { }
 
     virtual void layerAdded(gfx::ShaderRegistry&,
+                            gfx::Context&,
                             const TransformState&,
                             const PropertyEvaluationParameters&,
                             UniqueChangeRequestVec&) const { }
diff --git a/src/mbgl/style/layers/background_layer_impl.cpp b/src/mbgl/style/layers/background_layer_impl.cpp
index 3dd87e96f..b59fb4c7a 100644
--- a/src/mbgl/style/layers/background_layer_impl.cpp
+++ b/src/mbgl/style/layers/background_layer_impl.cpp
@@ -1,12 +1,13 @@
 #include <mbgl/style/layers/background_layer_impl.hpp>
 
 #include <mbgl/gfx/shader_registry.hpp>
+#include <mbgl/gfx/drawable_builder.hpp>
 #include <mbgl/gl/context.hpp>
-#include <mbgl/gl/drawable_gl_builder.hpp>
-#include <mbgl/gl/drawable_gl_tweaker.hpp>
 #include <mbgl/renderer/change_request.hpp>
 #include <mbgl/renderer/paint_parameters.hpp>
-#include <mbgl/shaders/gl/shader_program_gl.hpp>
+//#include <mbgl/shaders/gl/shader_program_gl.hpp>
+//#include <mbgl/shaders/gl/shader_program_gl.hpp>
+#include <mbgl/shaders/shader_program_base.hpp>
 #include <mbgl/style/layer_properties.hpp>
 #include <mbgl/util/logging.hpp>
 #include <mbgl/util/tile_cover.hpp>
@@ -30,19 +31,27 @@ bool BackgroundLayer::Impl::hasLayoutDifference(const Layer::Impl&) const {
 
 constexpr auto shaderName = "background_generic";
 
+struct ShaderHack final : public gfx::ShaderProgramBase {
+    static constexpr std::string_view Name{"GenericGLShader"};
+    const std::string_view typeName() const noexcept override { return Name; }
+    const gfx::VertexAttributeArray& getUniforms() const override { return *dummy; }
+    const gfx::VertexAttributeArray& getVertexAttributes() const override { return *dummy; }
+    gfx::VertexAttributeArray& mutableUniforms() override { return *dummy; }
+    gfx::VertexAttributeArray& mutableVertexAttributes() override { return *dummy; }
+    gfx::VertexAttributeArray* dummy = nullptr;
+};
+
 void BackgroundLayer::Impl::layerAdded(gfx::ShaderRegistry& shaders,
-                                       const TransformState& state,
-                                       const PropertyEvaluationParameters& evalParameters,
-                                       UniqueChangeRequestVec& changes) const {
+                                       gfx::Context&,
+                                       const TransformState&,
+                                       const PropertyEvaluationParameters&,
+                                       UniqueChangeRequestVec&) const {
     {
         std::unique_lock<std::mutex> guard(mutex);
         if (!shader) {
-            shader = shaders.get<gl::ShaderProgramGL>(shaderName);
+            shader = shaders.get<ShaderHack>(shaderName);
         }
     }
-
-    // Add all the tiles
-    update(state, evalParameters, changes);
 }
 
 void BackgroundLayer::Impl::layerRemoved(UniqueChangeRequestVec& changes) const {
@@ -60,7 +69,8 @@ void BackgroundLayer::Impl::layerRemoved(UniqueChangeRequestVec& changes) const
         [](auto& ii) { return ii->second->getId(); });
 }
 
-void BackgroundLayer::Impl::update(const TransformState& state,
+void BackgroundLayer::Impl::update(gfx::Context& context,
+                                   const TransformState& state,
                                    const PropertyEvaluationParameters& evalParameters,
                                    UniqueChangeRequestVec& changes) const {
     std::unique_lock<std::mutex> guard(mutex);
@@ -148,9 +158,9 @@ void BackgroundLayer::Impl::update(const TransformState& state,
 
         // We actually need to build things, so set up a builder if we haven't already
         if (!builder) {
-            builder = std::make_unique<gl::DrawableGLBuilder>();   // from GL-specific code via virtual method on ...Context?
+            builder = context.createDrawableBuilder("background");
             builder->setShader(shader);
-            builder->addTweaker(std::make_shared<gl::DrawableGLTweaker>()); // generally shared across drawables
+            builder->addTweaker(context.createDrawableTweaker());
             builder->setColor(*color);
             builder->setColorMode(gfx::DrawableBuilder::ColorMode::PerDrawable);
             builder->setDepthType(gfx::DepthMaskType::ReadWrite);
diff --git a/src/mbgl/style/layers/background_layer_impl.hpp b/src/mbgl/style/layers/background_layer_impl.hpp
index d22dd1dc5..80bba1d9c 100644
--- a/src/mbgl/style/layers/background_layer_impl.hpp
+++ b/src/mbgl/style/layers/background_layer_impl.hpp
@@ -34,13 +34,15 @@ public:
     void stringifyLayout(rapidjson::Writer<rapidjson::StringBuffer>&) const override;
 
     void layerAdded(gfx::ShaderRegistry&,
+                    gfx::Context&,
                     const TransformState&,
                     const PropertyEvaluationParameters&,
                     UniqueChangeRequestVec&) const override;
     void layerRemoved(UniqueChangeRequestVec&) const override;
 
     /// Generate any changes needed by the layer
-    void update(const TransformState&,
+    void update(gfx::Context&,
+                const TransformState&,
                 const PropertyEvaluationParameters&,
                 UniqueChangeRequestVec&) const override;