From 8e5214144ec4f3a4fb40b7a7e4d8f09fd10dbb78 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Konstantin=20K=C3=A4fer?= Date: Mon, 27 Feb 2017 18:33:16 +0100 Subject: [PATCH] [core] Ensure that a BackendScope exists when doing GL calls --- benchmark/api/query.benchmark.cpp | 2 ++ bin/render.cpp | 2 ++ include/mbgl/map/backend_scope.hpp | 15 ++++++++++++++- platform/glfw/glfw_view.cpp | 2 ++ platform/ios/src/MGLMapView.mm | 4 ++++ platform/macos/src/MGLMapView.mm | 6 +++++- platform/node/src/node_map.cpp | 2 ++ platform/qt/src/qmapboxgl.cpp | 4 ++++ src/mbgl/map/backend.cpp | 2 ++ src/mbgl/map/backend_scope.cpp | 19 +++++++++++++++---- src/mbgl/map/map.cpp | 3 ++- test/api/annotations.test.cpp | 2 ++ test/api/api_misuse.test.cpp | 3 +++ test/api/custom_layer.test.cpp | 2 ++ test/api/query.test.cpp | 2 ++ test/api/render_missing.test.cpp | 2 ++ test/api/repeated_render.test.cpp | 2 ++ test/gl/object.test.cpp | 2 +- test/map/map.test.cpp | 4 +++- test/util/memory.test.cpp | 2 ++ 20 files changed, 73 insertions(+), 9 deletions(-) diff --git a/benchmark/api/query.benchmark.cpp b/benchmark/api/query.benchmark.cpp index 53a524b45..197103f06 100644 --- a/benchmark/api/query.benchmark.cpp +++ b/benchmark/api/query.benchmark.cpp @@ -2,6 +2,7 @@ #include #include +#include #include #include #include @@ -34,6 +35,7 @@ public: util::RunLoop loop; HeadlessBackend backend; + BackendScope scope { backend }; OffscreenView view{ backend.getContext(), { 1000, 1000 } }; DefaultFileSource fileSource{ "benchmark/fixtures/api/cache.db", "." }; ThreadPool threadPool{ 4 }; diff --git a/bin/render.cpp b/bin/render.cpp index d275b445d..d4090cfa9 100644 --- a/bin/render.cpp +++ b/bin/render.cpp @@ -1,4 +1,5 @@ #include +#include #include #include #include @@ -85,6 +86,7 @@ int main(int argc, char *argv[]) { } HeadlessBackend backend; + BackendScope scope { backend }; OffscreenView view(backend.getContext(), { width * pixelRatio, height * pixelRatio }); ThreadPool threadPool(4); Map map(backend, mbgl::Size { width, height }, pixelRatio, fileSource, threadPool, MapMode::Still); diff --git a/include/mbgl/map/backend_scope.hpp b/include/mbgl/map/backend_scope.hpp index 5a207e6ac..4985cd197 100644 --- a/include/mbgl/map/backend_scope.hpp +++ b/include/mbgl/map/backend_scope.hpp @@ -6,13 +6,26 @@ class Backend; class BackendScope { public: - BackendScope(Backend&); + // There are two types of scopes: Creating an "Implicit" scope tells Mapbox GL that the + // supporting windowing system has already activated the GL Backend and that no further actions + // are required. Creating an "Explicit" scope actually enables the GL Backend, and disables it + // when the BackendScope is destroyed. + enum class ScopeType : bool { + Implicit, + Explicit, + }; + + BackendScope(Backend&, ScopeType = ScopeType::Explicit); ~BackendScope(); + // Returns true when there is currently a BackendScope active in this thread. + static bool exists(); + private: BackendScope* priorScope; BackendScope* nextScope; Backend& backend; + const ScopeType scopeType; }; } // namespace mbgl diff --git a/platform/glfw/glfw_view.cpp b/platform/glfw/glfw_view.cpp index 39dca8080..d73b147de 100644 --- a/platform/glfw/glfw_view.cpp +++ b/platform/glfw/glfw_view.cpp @@ -10,6 +10,7 @@ #include #include #include +#include #include #include @@ -462,6 +463,7 @@ void GLFWView::run() { const double started = glfwGetTime(); glfwMakeContextCurrent(window); + mbgl::BackendScope scope { *this, mbgl::BackendScope::ScopeType::Implicit }; updateViewBinding(); map->render(*this); diff --git a/platform/ios/src/MGLMapView.mm b/platform/ios/src/MGLMapView.mm index 1ccd5ce35..6bd309244 100644 --- a/platform/ios/src/MGLMapView.mm +++ b/platform/ios/src/MGLMapView.mm @@ -21,6 +21,7 @@ #include #include #include +#include #include #include #include @@ -881,6 +882,9 @@ public: { if ( ! self.dormant) { + // The OpenGL implementation automatically enables the OpenGL context for us. + mbgl::BackendScope scope { *_mbglView, mbgl::BackendScope::ScopeType::Implicit }; + _mbglView->updateViewBinding(); _mbglMap->render(*_mbglView); diff --git a/platform/macos/src/MGLMapView.mm b/platform/macos/src/MGLMapView.mm index 028d41ced..ef1c0f3bc 100644 --- a/platform/macos/src/MGLMapView.mm +++ b/platform/macos/src/MGLMapView.mm @@ -29,6 +29,7 @@ #import #import #import +#import #import #import #import @@ -780,6 +781,8 @@ public: return reinterpret_cast(symbol); }); + // The OpenGL implementation automatically enables the OpenGL context for us. + mbgl::BackendScope scope { *_mbglView, mbgl::BackendScope::ScopeType::Implicit }; _mbglView->updateViewBinding(); _mbglMap->render(*_mbglView); @@ -2872,7 +2875,8 @@ public: fbo = mbgl::gl::value::BindFramebuffer::Get(); getContext().bindFramebuffer.setCurrentValue(fbo); getContext().viewport.setCurrentValue(getViewport()); - assert(mbgl::gl::value::Viewport::Get() == getContext().viewport.getCurrentValue()); + auto actualViewport = mbgl::gl::value::Viewport::Get(); + assert(actualViewport == getContext().viewport.getCurrentValue()); } void bind() override { diff --git a/platform/node/src/node_map.cpp b/platform/node/src/node_map.cpp index 881c1b2fb..52d659b11 100644 --- a/platform/node/src/node_map.cpp +++ b/platform/node/src/node_map.cpp @@ -10,6 +10,7 @@ #include #include #include +#include #include #include @@ -369,6 +370,7 @@ void NodeMap::startRender(NodeMap::RenderOptions options) { static_cast(options.height * pixelRatio) }; if (!view || view->getSize() != fbSize) { view.reset(); + mbgl::BackendScope scope { backend }; view = std::make_unique(backend.getContext(), fbSize); } diff --git a/platform/qt/src/qmapboxgl.cpp b/platform/qt/src/qmapboxgl.cpp index 3d5da10c3..384bdc8eb 100644 --- a/platform/qt/src/qmapboxgl.cpp +++ b/platform/qt/src/qmapboxgl.cpp @@ -8,6 +8,7 @@ #include #include #include +#include #include #include #include @@ -1495,6 +1496,9 @@ void QMapboxGL::render() } #endif + // The OpenGL implementation automatically enables the OpenGL context for us. + mbgl::BackendScope scope { *d_ptr, mbgl::BackendScope::ScopeType::Implicit }; + d_ptr->dirty = false; d_ptr->updateViewBinding(); d_ptr->mapObj->render(*d_ptr); diff --git a/src/mbgl/map/backend.cpp b/src/mbgl/map/backend.cpp index 2a171cdb7..a1a2f8f3a 100644 --- a/src/mbgl/map/backend.cpp +++ b/src/mbgl/map/backend.cpp @@ -1,4 +1,5 @@ #include +#include #include #include @@ -9,6 +10,7 @@ Backend::Backend() : context(std::make_unique()) { } gl::Context& Backend::getContext() { + assert(BackendScope::exists()); return *context; } diff --git a/src/mbgl/map/backend_scope.cpp b/src/mbgl/map/backend_scope.cpp index 98775cead..824ad4498 100644 --- a/src/mbgl/map/backend_scope.cpp +++ b/src/mbgl/map/backend_scope.cpp @@ -8,15 +8,19 @@ namespace mbgl { static util::ThreadLocal currentScope; -BackendScope::BackendScope(Backend& backend_) +BackendScope::BackendScope(Backend& backend_, ScopeType scopeType_) : priorScope(currentScope.get()), nextScope(nullptr), - backend(backend_) { + backend(backend_), + scopeType(scopeType_) { if (priorScope) { assert(priorScope->nextScope == nullptr); priorScope->nextScope = this; } - backend.activate(); + if (scopeType == ScopeType::Explicit) { + backend.activate(); + } + currentScope.set(this); } @@ -28,9 +32,16 @@ BackendScope::~BackendScope() { assert(priorScope->nextScope == this); priorScope->nextScope = nullptr; } else { - backend.deactivate(); + if (scopeType == ScopeType::Explicit) { + backend.deactivate(); + } + currentScope.set(nullptr); } } +bool BackendScope::exists() { + return currentScope.get(); +} + } // namespace mbgl diff --git a/src/mbgl/map/map.cpp b/src/mbgl/map/map.cpp index 4c787eb66..a883a6928 100644 --- a/src/mbgl/map/map.cpp +++ b/src/mbgl/map/map.cpp @@ -267,8 +267,9 @@ void Map::Impl::render(View& view) { updateFlags = Update::Nothing; + gl::Context& context = backend.getContext(); if (!painter) { - painter = std::make_unique(backend.getContext(), transform.getState(), pixelRatio, programCacheDir); + painter = std::make_unique(context, transform.getState(), pixelRatio, programCacheDir); } if (mode == MapMode::Continuous) { diff --git a/test/api/annotations.test.cpp b/test/api/annotations.test.cpp index 87d64d53d..4bcaa0389 100644 --- a/test/api/annotations.test.cpp +++ b/test/api/annotations.test.cpp @@ -5,6 +5,7 @@ #include #include #include +#include #include #include #include @@ -24,6 +25,7 @@ class AnnotationTest { public: util::RunLoop loop; HeadlessBackend backend { test::sharedDisplay() }; + BackendScope scope { backend }; OffscreenView view { backend.getContext() }; StubFileSource fileSource; ThreadPool threadPool { 4 }; diff --git a/test/api/api_misuse.test.cpp b/test/api/api_misuse.test.cpp index 1a61872f7..af703fddf 100644 --- a/test/api/api_misuse.test.cpp +++ b/test/api/api_misuse.test.cpp @@ -3,6 +3,7 @@ #include #include +#include #include #include #include @@ -22,6 +23,7 @@ TEST(API, RenderWithoutCallback) { util::RunLoop loop; HeadlessBackend backend { test::sharedDisplay() }; + BackendScope scope { backend }; OffscreenView view { backend.getContext(), { 128, 512 } }; StubFileSource fileSource; ThreadPool threadPool(4); @@ -47,6 +49,7 @@ TEST(API, RenderWithoutStyle) { util::RunLoop loop; HeadlessBackend backend { test::sharedDisplay() }; + BackendScope scope { backend }; OffscreenView view { backend.getContext(), { 128, 512 } }; StubFileSource fileSource; ThreadPool threadPool(4); diff --git a/test/api/custom_layer.test.cpp b/test/api/custom_layer.test.cpp index dd5619746..658c5333c 100644 --- a/test/api/custom_layer.test.cpp +++ b/test/api/custom_layer.test.cpp @@ -2,6 +2,7 @@ #include #include +#include #include #include #include @@ -86,6 +87,7 @@ TEST(CustomLayer, Basic) { util::RunLoop loop; HeadlessBackend backend { test::sharedDisplay() }; + BackendScope scope { backend }; OffscreenView view { backend.getContext() }; #ifdef MBGL_ASSET_ZIP diff --git a/test/api/query.test.cpp b/test/api/query.test.cpp index 45e6b1926..532cc14d2 100644 --- a/test/api/query.test.cpp +++ b/test/api/query.test.cpp @@ -1,4 +1,5 @@ #include +#include #include #include #include @@ -29,6 +30,7 @@ public: util::RunLoop loop; HeadlessBackend backend { test::sharedDisplay() }; + BackendScope scope { backend }; OffscreenView view { backend.getContext() }; StubFileSource fileSource; ThreadPool threadPool { 4 }; diff --git a/test/api/render_missing.test.cpp b/test/api/render_missing.test.cpp index c1bf7e570..8637462cb 100644 --- a/test/api/render_missing.test.cpp +++ b/test/api/render_missing.test.cpp @@ -2,6 +2,7 @@ #include #include +#include #include #include #include @@ -26,6 +27,7 @@ TEST(API, TEST_REQUIRES_SERVER(RenderMissingTile)) { const auto style = util::read_file("test/fixtures/api/water_missing_tiles.json"); HeadlessBackend backend { test::sharedDisplay() }; + BackendScope scope { backend }; OffscreenView view { backend.getContext(), { 256, 512 } }; #ifdef MBGL_ASSET_ZIP // Regenerate with `cd test/fixtures/api/ && zip -r assets.zip assets/` diff --git a/test/api/repeated_render.test.cpp b/test/api/repeated_render.test.cpp index 800813075..6648a7752 100644 --- a/test/api/repeated_render.test.cpp +++ b/test/api/repeated_render.test.cpp @@ -2,6 +2,7 @@ #include #include +#include #include #include #include @@ -22,6 +23,7 @@ TEST(API, RepeatedRender) { const auto style = util::read_file("test/fixtures/api/water.json"); HeadlessBackend backend { test::sharedDisplay() }; + BackendScope scope { backend }; OffscreenView view { backend.getContext(), { 256, 512 } }; #ifdef MBGL_ASSET_ZIP // Regenerate with `cd test/fixtures/api/ && zip -r assets.zip assets/` diff --git a/test/gl/object.test.cpp b/test/gl/object.test.cpp index 85ae45708..5d2bf6735 100644 --- a/test/gl/object.test.cpp +++ b/test/gl/object.test.cpp @@ -62,8 +62,8 @@ TEST(GLObject, Value) { TEST(GLObject, Store) { HeadlessBackend backend { test::sharedDisplay() }; - OffscreenView view(backend.getContext()); BackendScope scope { backend }; + OffscreenView view(backend.getContext()); gl::Context context; EXPECT_TRUE(context.empty()); diff --git a/test/map/map.test.cpp b/test/map/map.test.cpp index 2d995fc59..c5017aeb0 100644 --- a/test/map/map.test.cpp +++ b/test/map/map.test.cpp @@ -47,6 +47,7 @@ public: struct MapTest { util::RunLoop runLoop; BackendTest backend; + BackendScope scope { backend }; OffscreenView view { backend.getContext() }; StubFileSource fileSource; ThreadPool threadPool { 4 }; @@ -552,6 +553,7 @@ public: TEST(Map, TEST_DISABLED_ON_CI(ContinuousRendering)) { util::RunLoop runLoop; MockBackend backend { test::sharedDisplay() }; + BackendScope scope { backend }; OffscreenView view { backend.getContext() }; ThreadPool threadPool { 4 }; @@ -585,7 +587,7 @@ TEST(Map, TEST_DISABLED_ON_CI(ContinuousRendering)) { }); } - BackendScope scope(backend); + BackendScope scope2(backend); map.render(view); }}; diff --git a/test/util/memory.test.cpp b/test/util/memory.test.cpp index 79a3c43db..065d024be 100644 --- a/test/util/memory.test.cpp +++ b/test/util/memory.test.cpp @@ -3,6 +3,7 @@ #include #include +#include #include #include #include @@ -35,6 +36,7 @@ public: util::RunLoop runLoop; HeadlessBackend backend { test::sharedDisplay() }; + BackendScope scope { backend }; OffscreenView view{ backend.getContext(), { 512, 512 } }; StubFileSource fileSource; ThreadPool threadPool { 4 };