[core] BackendScope prevent double (de-)activation

- Guards against duplicate activations by checking wether the backend of the prior scope is the same as the current ones
- Makes sure that only the most outer backend scope deactivates by tracking activation state

5 files changed, 148 insertions, 8 deletions

diff --git a/cmake/test-files.cmake b/cmake/test-files.cmake
index ca7182581b..2d2ed7d35b 100644
--- a/cmake/test-files.cmake
+++ b/cmake/test-files.cmake
@@ -41,6 +41,7 @@ set(MBGL_TEST_FILES
     test/programs/symbol_program.test.cpp
 
     # renderer
+    test/renderer/backend_scope.test.cpp
     test/renderer/group_by_layout.test.cpp
     test/renderer/image_manager.test.cpp
 
diff --git a/include/mbgl/renderer/backend_scope.hpp b/include/mbgl/renderer/backend_scope.hpp
index f8a258f3df..73bafc84c7 100644
--- a/include/mbgl/renderer/backend_scope.hpp
+++ b/include/mbgl/renderer/backend_scope.hpp
@@ -22,10 +22,14 @@ public:
     static bool exists();
 
 private:
+    void activate();
+    void deactivate();
+
     BackendScope* priorScope;
     BackendScope* nextScope;
     RendererBackend& backend;
     const ScopeType scopeType;
+    bool activated = false;
 };
 
 } // namespace mbgl
diff --git a/include/mbgl/renderer/renderer_backend.hpp b/include/mbgl/renderer/renderer_backend.hpp
index 9d967fd51b..a383329395 100644
--- a/include/mbgl/renderer/renderer_backend.hpp
+++ b/include/mbgl/renderer/renderer_backend.hpp
@@ -4,6 +4,7 @@
 #include <mbgl/renderer/backend_scope.hpp>
 #include <mbgl/util/image.hpp>
 #include <mbgl/util/size.hpp>
+#include <mbgl/util/util.hpp>
 
 #include <memory>
 #include <mutex>
@@ -81,4 +82,8 @@ private:
     friend class BackendScope;
 };
 
+MBGL_CONSTEXPR bool operator==(const RendererBackend& a, const RendererBackend& b) {
+    return &a == &b;
+}
+
 } // namespace mbgl
diff --git a/src/mbgl/renderer/backend_scope.cpp b/src/mbgl/renderer/backend_scope.cpp
index 5d57421c48..fafeaabb39 100644
--- a/src/mbgl/renderer/backend_scope.cpp
+++ b/src/mbgl/renderer/backend_scope.cpp
@@ -16,30 +16,49 @@ BackendScope::BackendScope(RendererBackend& backend_, ScopeType scopeType_)
     if (priorScope) {
         assert(priorScope->nextScope == nullptr);
         priorScope->nextScope = this;
+        priorScope->deactivate();
     }
-    if (scopeType == ScopeType::Explicit) {
-        backend.activate();
-    }
+
+    activate();
 
     currentScope.set(this);
 }
 
 BackendScope::~BackendScope() {
     assert(nextScope == nullptr);
+    deactivate();
+
     if (priorScope) {
-        priorScope->backend.activate();
+        priorScope->activate();
         currentScope.set(priorScope);
         assert(priorScope->nextScope == this);
         priorScope->nextScope = nullptr;
     } else {
-        if (scopeType == ScopeType::Explicit) {
-            backend.deactivate();
-        }
-
         currentScope.set(nullptr);
     }
 }
 
+void BackendScope::activate() {
+    if (scopeType == ScopeType::Explicit &&
+            !(priorScope && this->backend == priorScope->backend) &&
+            !(nextScope && this->backend == nextScope->backend)) {
+        // Only activate when set to Explicit and
+        // only once per RenderBackend
+        backend.activate();
+        activated = true;
+    }
+}
+
+void BackendScope::deactivate() {
+    if (activated &&
+        !(nextScope && this->backend == nextScope->backend)) {
+        // Only deactivate when set to Explicit and
+        // only once per RenderBackend
+        backend.deactivate();
+        activated = false;
+    }
+}
+
 bool BackendScope::exists() {
     return currentScope.get();
 }
diff --git a/test/renderer/backend_scope.test.cpp b/test/renderer/backend_scope.test.cpp
new file mode 100644
index 0000000000..a8c92bb31f
--- /dev/null
+++ b/test/renderer/backend_scope.test.cpp
@@ -0,0 +1,111 @@
+#include <mbgl/test/util.hpp>
+
+#include <mbgl/renderer/renderer_backend.hpp>
+#include <mbgl/renderer/backend_scope.hpp>
+
+#include <functional>
+
+using namespace mbgl;
+
+class StubRendererBackend: public RendererBackend {
+public:
+
+    void activate() override {
+        if (activateFunction) activateFunction();
+    }
+
+    void deactivate() override {
+        if (deactivateFunction) deactivateFunction();
+    }
+
+    void updateAssumedState() override {
+        if (updateAssumedStateFunction) updateAssumedStateFunction();
+    }
+
+    gl::ProcAddress initializeExtension(const char* ext) override {
+        if (initializeExtensionFunction) {
+            return initializeExtensionFunction(ext);
+        } else {
+            return {};
+        }
+    }
+
+    std::function<void ()> activateFunction;
+    std::function<void ()> deactivateFunction;
+    std::function<void ()> updateAssumedStateFunction;
+    std::function<gl::ProcAddress (const char*)> initializeExtensionFunction;
+};
+
+// A scope should activate on construction
+// and deactivate on descruction (going out
+// of scope)
+TEST(BackendScope, SingleScope) {
+    bool activated;
+    bool deactivated;
+
+    StubRendererBackend backend;
+    backend.activateFunction = [&] { activated = true; };
+    backend.deactivateFunction = [&] { deactivated = true; };
+
+    {
+        BackendScope test { backend };
+    }
+
+    ASSERT_TRUE(activated);
+    ASSERT_TRUE(deactivated);
+}
+
+// With nested scopes, only the outer scope
+// should activate/deactivate
+TEST(BackendScope, NestedScopes) {
+    int activated = 0;
+    int deactivated = 0;
+
+    StubRendererBackend backend;
+    backend.activateFunction = [&] { activated++; };
+    backend.deactivateFunction = [&] { deactivated++; };
+
+    {
+        BackendScope outer { backend };
+        ASSERT_EQ(1, activated);
+        {
+            BackendScope inner { backend };
+            ASSERT_EQ(1, activated);
+        }
+        ASSERT_EQ(0, deactivated);
+    }
+
+    ASSERT_EQ(1, deactivated);
+}
+
+// With chained scopes, where scopes have
+// different backends, the scopes should each
+// activate the scope on entering and de-activating
+// on leaving the scope
+TEST(BackendScope, ChainedScopes) {
+    bool activatedA = false;
+    bool activatedB = false;
+    
+    StubRendererBackend backendA;
+    backendA.activateFunction = [&] { activatedA = true; };
+    backendA.deactivateFunction = [&] { activatedA = false; };
+    
+    StubRendererBackend backendB;
+    backendB.activateFunction = [&] { activatedB = true; };
+    backendB.deactivateFunction = [&] { activatedB = false; };
+    
+    {
+        BackendScope scopeA { backendA };
+        ASSERT_TRUE(activatedA);
+        {
+            BackendScope scopeB { backendB };
+            ASSERT_FALSE(activatedA);
+            ASSERT_TRUE(activatedB);
+        }
+        ASSERT_FALSE(activatedB);
+        ASSERT_TRUE(activatedA);
+    }
+    
+    ASSERT_FALSE(activatedA);
+    ASSERT_FALSE(activatedB);
+}