From 636fedff122dc254ec77ea5504cf8d94acd8b351 Mon Sep 17 00:00:00 2001 From: Mike Morris Date: Wed, 10 Sep 2014 14:24:29 -0400 Subject: [PATCH 1/5] make context active before clearing buffers --- common/headless_view.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/common/headless_view.cpp b/common/headless_view.cpp index f790d90ce..0fce22398 100644 --- a/common/headless_view.cpp +++ b/common/headless_view.cpp @@ -113,6 +113,8 @@ void HeadlessView::resize(int width, int height) { } void HeadlessView::clear_buffers() { + make_active(); + #if MBGL_USE_CGL glBindFramebufferEXT(GL_FRAMEBUFFER_EXT, 0); From c836bc2b01c1c8642875d86126c1ef2a76dfd359 Mon Sep 17 00:00:00 2001 From: Mike Morris Date: Wed, 10 Sep 2014 15:11:17 -0400 Subject: [PATCH 2/5] Revert "make context active before clearing buffers" This reverts commit 636fedff122dc254ec77ea5504cf8d94acd8b351. --- common/headless_view.cpp | 2 -- 1 file changed, 2 deletions(-) diff --git a/common/headless_view.cpp b/common/headless_view.cpp index 0fce22398..f790d90ce 100644 --- a/common/headless_view.cpp +++ b/common/headless_view.cpp @@ -113,8 +113,6 @@ void HeadlessView::resize(int width, int height) { } void HeadlessView::clear_buffers() { - make_active(); - #if MBGL_USE_CGL glBindFramebufferEXT(GL_FRAMEBUFFER_EXT, 0); From c004c036964e9012b08aa4a2f8b16094630d62be Mon Sep 17 00:00:00 2001 From: Leith Bade Date: Tue, 24 Jun 2014 00:25:20 +1000 Subject: [PATCH 3/5] Add View::make_inactive() to remove context from rendering thread View::make_inactive() is called just before termination of the rendering thread. The implementation must clear the thread's GL context. This ensures there is no context still bound to a thread during GL shutdown. This is needed as GL will not actually delete a context until it is not active on any thread. Fixes mapbox/mapbox-gl-native#340 Conflicts: common/glfw_view.hpp include/llmr/map/view.hpp test/headless.cpp --- common/glfw_view.cpp | 4 ++++ common/glfw_view.hpp | 1 + common/headless_view.cpp | 13 +++++++++++++ common/headless_view.hpp | 1 + include/mbgl/map/view.hpp | 4 ++++ src/map/map.cpp | 2 ++ 6 files changed, 25 insertions(+) diff --git a/common/glfw_view.cpp b/common/glfw_view.cpp index f53090a00..724b8bf18 100644 --- a/common/glfw_view.cpp +++ b/common/glfw_view.cpp @@ -192,6 +192,10 @@ void GLFWView::make_active() { glfwMakeContextCurrent(window); } +void GLFWView::make_inactive() { + glfwMakeContextCurrent(nullptr); +} + void GLFWView::swap() { glfwPostEmptyEvent(); diff --git a/common/glfw_view.hpp b/common/glfw_view.hpp index d2f6872fc..04085f775 100644 --- a/common/glfw_view.hpp +++ b/common/glfw_view.hpp @@ -17,6 +17,7 @@ public: void initialize(mbgl::Map *map); void swap(); void make_active(); + void make_inactive(); void notify_map_change(mbgl::MapChange change, mbgl::timestamp delay = 0); static void key(GLFWwindow *window, int key, int scancode, int action, int mods); diff --git a/common/headless_view.cpp b/common/headless_view.cpp index f790d90ce..34326ae29 100644 --- a/common/headless_view.cpp +++ b/common/headless_view.cpp @@ -179,6 +179,19 @@ void HeadlessView::make_active() { #endif } +void HeadlessView::make_inactive() { +#if MBGL_USE_CGL + CGLError error = CGLSetCurrentContext(nullptr); + if (error) { + fprintf(stderr, "Removing OpenGL context failed\n"); + } +#endif + +#if MBGL_USE_GLX + // no-op +#endif +} + void HeadlessView::swap() {} unsigned int HeadlessView::root_fbo() { diff --git a/common/headless_view.hpp b/common/headless_view.hpp index 0b255b4a3..a4432bbae 100644 --- a/common/headless_view.hpp +++ b/common/headless_view.hpp @@ -23,6 +23,7 @@ public: void notify_map_change(MapChange change, timestamp delay = 0); void make_active(); + void make_inactive(); void swap(); unsigned int root_fbo(); diff --git a/include/mbgl/map/view.hpp b/include/mbgl/map/view.hpp index 92d60d4d0..3e2f1a4b5 100644 --- a/include/mbgl/map/view.hpp +++ b/include/mbgl/map/view.hpp @@ -35,6 +35,10 @@ public: // renderer setup since the render thread doesn't switch the contexts. virtual void make_active() = 0; + // Called from the render thread. Makes the GL context inactive in the current + // thread. This is called once just before the rendering thread terminates. + virtual void make_inactive() = 0; + // Returns the base framebuffer object, if any, and 0 if using the system // provided framebuffer. virtual unsigned int root_fbo() { diff --git a/src/map/map.cpp b/src/map/map.cpp index 9984f1b18..de7a7032d 100644 --- a/src/map/map.cpp +++ b/src/map/map.cpp @@ -114,6 +114,8 @@ void Map::run() { prepare(); render(); } + + view.make_inactive(); } void Map::rerender() { From 71549e7b93b6b44ed38e8915d8735c136889c3f1 Mon Sep 17 00:00:00 2001 From: Leith Bade Date: Tue, 24 Jun 2014 00:41:28 +1000 Subject: [PATCH 4/5] Adds the function Map::terminate() to allow all GL resources held by the map to be released immediately. Map::terminate() will call any GL functions on the current thread without blocking or defering to the rendering thread. This function allows a GL context to be destroyed and recreated safely without having to delete the Map object. Fixes mapbox/mapbox-gl-native#341 Conflicts: include/llmr/renderer/painter.hpp test/headless.cpp --- common/glfw_view.cpp | 5 ++++- include/mbgl/map/map.hpp | 3 +++ include/mbgl/renderer/painter.hpp | 2 ++ src/map/map.cpp | 6 ++++++ src/renderer/painter.cpp | 19 +++++++++++++++++++ 5 files changed, 34 insertions(+), 1 deletion(-) diff --git a/common/glfw_view.cpp b/common/glfw_view.cpp index 724b8bf18..0908786bb 100644 --- a/common/glfw_view.cpp +++ b/common/glfw_view.cpp @@ -8,7 +8,10 @@ GLFWView::GLFWView(bool fullscreen) : fullscreen(fullscreen) { #endif } -GLFWView::~GLFWView() { glfwTerminate(); } +GLFWView::~GLFWView() { + map->terminate(); + glfwTerminate(); +} void GLFWView::initialize(mbgl::Map *map) { View::initialize(map); diff --git a/include/mbgl/map/map.hpp b/include/mbgl/map/map.hpp index fba38879e..0d2926d04 100644 --- a/include/mbgl/map/map.hpp +++ b/include/mbgl/map/map.hpp @@ -53,6 +53,9 @@ public: // Triggers a cleanup that releases resources. void cleanup(); + // Releases resources immediately + void terminate(); + // Controls buffer swapping. bool needsSwap(); void swapped(); diff --git a/include/mbgl/renderer/painter.hpp b/include/mbgl/renderer/painter.hpp index 75529d113..15331b9b6 100644 --- a/include/mbgl/renderer/painter.hpp +++ b/include/mbgl/renderer/painter.hpp @@ -63,6 +63,7 @@ public: // lazy initialization) in case rendering continues. void cleanup(); + void terminate(); // Renders the backdrop of the OpenGL view. This also paints in areas where we don't have any // tiles whatsoever. @@ -123,6 +124,7 @@ public: private: void setupShaders(); + void deleteShaders(); mat4 translatedMatrix(const mat4& matrix, const std::array &translation, const Tile::ID &id, TranslateAnchorType anchor); void prepareTile(const Tile& tile); diff --git a/src/map/map.cpp b/src/map/map.cpp index de7a7032d..00f0ac91c 100644 --- a/src/map/map.cpp +++ b/src/map/map.cpp @@ -153,6 +153,12 @@ void Map::cleanup(uv_async_t *async) { map->painter.cleanup(); } +void Map::terminate() { + view.make_active(); + painter.terminate(); + view.make_inactive(); +} + void Map::render(uv_async_t *async) { Map *map = static_cast(async->data); diff --git a/src/renderer/painter.cpp b/src/renderer/painter.cpp index c1705cd7c..b1522a56b 100644 --- a/src/renderer/painter.cpp +++ b/src/renderer/painter.cpp @@ -82,9 +82,28 @@ void Painter::setupShaders() { if (!gaussianShader) gaussianShader = std::make_unique(); } +void Painter::deleteShaders() { + plainShader = nullptr; + outlineShader = nullptr; + lineShader = nullptr; + linejoinShader = nullptr; + patternShader = nullptr; + iconShader = nullptr; + rasterShader = nullptr; + textShader = nullptr; + dotShader = nullptr; + compositeShader = nullptr; + gaussianShader = nullptr; +} + void Painter::cleanup() { } +void Painter::terminate() { + cleanup(); + deleteShaders(); +} + void Painter::resize() { const TransformState &state = map.getState(); if (gl_viewport != state.getFramebufferDimensions()) { From 7a4c914d05cab81ca55ac3cde478997c6d32477e Mon Sep 17 00:00:00 2001 From: Leith Bade Date: Tue, 24 Jun 2014 16:19:47 +1000 Subject: [PATCH 5/5] Move notify_map_change() from a global callback, to a callback in View. This allows for their to me more than one Map object alive at a time. Fixes mapbox/mapbox-gl-native#346 Conflicts: common/glfw_view.hpp include/llmr/map/view.hpp include/mbgl/map/transform.hpp src/map/map.cpp src/map/transform.cpp test/headless.cpp --- src/map/map.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/src/map/map.cpp b/src/map/map.cpp index 00f0ac91c..e6c4e9d03 100644 --- a/src/map/map.cpp +++ b/src/map/map.cpp @@ -156,7 +156,6 @@ void Map::cleanup(uv_async_t *async) { void Map::terminate() { view.make_active(); painter.terminate(); - view.make_inactive(); } void Map::render(uv_async_t *async) {