[core] Privatize OpenGL/Context headers

cmake/benchmark.cmake

@@ -7,7 +7,6 @@ target_compile_options(mbgl-benchmark
 )
 
 target_include_directories(mbgl-benchmark
-    PRIVATE include
     PRIVATE src
     PRIVATE benchmark/include
     PRIVATE benchmark/src

cmake/core.cmake

@@ -9,7 +9,7 @@ target_compile_options(mbgl-core
 
 target_include_directories(mbgl-core
     PUBLIC include
-    PUBLIC src # TODO: make private
+    PRIVATE src
 )
 
 target_add_mason_package(mbgl-core PUBLIC geometry)

cmake/glfw.cmake

@@ -16,8 +16,6 @@ target_compile_options(mbgl-glfw
 )
 
 target_include_directories(mbgl-glfw
-    PRIVATE include
-    PRIVATE src # TODO: eliminate
     PRIVATE platform/default
 )
 

cmake/test.cmake

@@ -15,7 +15,6 @@ target_compile_options(mbgl-test
 set_source_files_properties(test/src/mbgl/test/util.cpp PROPERTIES COMPILE_FLAGS -DNODE_EXECUTABLE="${NodeJS_EXECUTABLE}")
 
 target_include_directories(mbgl-test
-    PRIVATE include
     PRIVATE src
     PRIVATE test/include
     PRIVATE test/src

include/mbgl/map/backend.hpp

@@ -1,6 +1,8 @@
 #pragma once
 
 #include <mbgl/map/map_observer.hpp>
+#include <mbgl/util/image.hpp>
+#include <mbgl/util/size.hpp>
 
 #include <memory>
 #include <mutex>
@@ -10,6 +12,7 @@ namespace mbgl {
 namespace gl {
 class Context;
 using ProcAddress = void (*)();
+using FramebufferID = uint32_t;
 } // namespace gl
 
 class BackendScope;
@@ -22,6 +25,9 @@ public:
     // Returns the backend's context which manages OpenGL state.
     gl::Context& getContext();
 
+    // Called prior to rendering to update the internally assumed OpenGL state.
+    virtual void updateAssumedState() = 0;
+
     // Called when the map needs to be rendered; the backend should call Map::render() at some point
     // in the near future. (Not called for Map::renderStill() mode.)
     virtual void invalidate() = 0;
@@ -45,6 +51,26 @@ protected:
     virtual void activate() = 0;
     virtual void deactivate() = 0;
 
+    // Reads the color pixel data from the currently bound framebuffer.
+    PremultipliedImage readFramebuffer(const Size&) const;
+
+    // A constant to signal that a framebuffer is bound, but with an unknown ID.
+    static constexpr const gl::FramebufferID ImplicitFramebufferBinding =
+        std::numeric_limits<gl::FramebufferID>::max();
+
+    // Tells the renderer that OpenGL state has already been set by the windowing toolkit.
+    // It sets the internal assumed state to the supplied values.
+    void assumeFramebufferBinding(gl::FramebufferID fbo);
+    void assumeViewportSize(const Size&);
+
+    // Returns true when assumed framebuffer binding hasn't changed from the implicit binding.
+    bool implicitFramebufferBound();
+
+    // Triggers an OpenGL state update if the internal assumed state doesn't match the
+    // supplied values.
+    void setFramebufferBinding(gl::FramebufferID fbo);
+    void setViewportSize(const Size&);
+
 protected:
     std::unique_ptr<gl::Context> context;
 

platform/android/src/native_map_view.cpp

@@ -14,7 +14,6 @@
 
 #include <jni/jni.hpp>
 
-#include <mbgl/gl/context.hpp>
 #include <mbgl/map/backend_scope.hpp>
 #include <mbgl/util/constants.hpp>
 #include <mbgl/util/event.hpp>
@@ -100,8 +99,8 @@ NativeMapView::~NativeMapView() {
  * From mbgl::View
  */
 void NativeMapView::bind() {
-    getContext().bindFramebuffer = 0;
-    getContext().viewport = { 0, 0, getFramebufferSize() };
+    setFramebufferBinding(0);
+    setViewportSize(getFramebufferSize());
 }
 
 /**
@@ -284,18 +283,17 @@ void NativeMapView::render(jni::JNIEnv& env) {
     BackendScope guard(*this);
 
     if (framebufferSizeChanged) {
-        getContext().viewport = { 0, 0, getFramebufferSize() };
+        setViewportSize(getFramebufferSize());
         framebufferSizeChanged = false;
     }
 
-    updateViewBinding();
     map->render(*this);
 
     if(snapshot){
          snapshot = false;
 
          // take snapshot
-         auto image = getContext().readFramebuffer<mbgl::PremultipliedImage>(getFramebufferSize());
+         auto image = readFramebuffer(getFramebufferSize());
          auto bitmap = Bitmap::CreateBitmap(env, std::move(image));
 
          // invoke Mapview#OnSnapshotReady
@@ -1421,11 +1419,9 @@ mbgl::Size NativeMapView::getFramebufferSize() const {
     return { static_cast<uint32_t>(fbWidth), static_cast<uint32_t>(fbHeight) };
 }
 
-void NativeMapView::updateViewBinding() {
-    getContext().bindFramebuffer.setCurrentValue(0);
-    assert(mbgl::gl::value::BindFramebuffer::Get() == getContext().bindFramebuffer.getCurrentValue());
-    getContext().viewport.setCurrentValue({ 0, 0, getFramebufferSize() });
-    assert(mbgl::gl::value::Viewport::Get() == getContext().viewport.getCurrentValue());
+void NativeMapView::updateAssumedState() {
+    assumeFramebufferBinding(0);
+    assumeViewportSize(getFramebufferSize());
 }
 
 void NativeMapView::updateFps() {

platform/android/src/native_map_view.hpp

@@ -58,6 +58,7 @@ public:
 
     // mbgl::Backend //
 
+    void updateAssumedState() override;
     void invalidate() override;
 
     // Deprecated //
@@ -281,7 +282,6 @@ private:
 
     EGLConfig chooseConfig(const EGLConfig configs[], EGLint numConfigs);
 
-    void updateViewBinding();
     mbgl::Size getFramebufferSize() const;
 
     void updateFps();

platform/default/mbgl/gl/headless_backend.cpp

@@ -41,6 +41,10 @@ void HeadlessBackend::deactivate() {
     active = false;
 }
 
+void HeadlessBackend::updateAssumedState() {
+    // no-op
+}
+
 void HeadlessBackend::invalidate() {
     assert(false);
 }

platform/default/mbgl/gl/headless_backend.hpp

@@ -15,6 +15,8 @@ public:
     HeadlessBackend(std::shared_ptr<HeadlessDisplay>);
     ~HeadlessBackend() override;
 
+    void updateAssumedState() override;
+
     void invalidate() override;
 
     struct Impl {

platform/glfw/glfw_view.cpp

@@ -3,8 +3,6 @@
 #include <mbgl/annotation/annotation.hpp>
 #include <mbgl/sprite/sprite_image.hpp>
 #include <mbgl/style/transition_options.hpp>
-#include <mbgl/gl/gl.hpp>
-#include <mbgl/gl/context.hpp>
 #include <mbgl/util/logging.hpp>
 #include <mbgl/util/platform.hpp>
 #include <mbgl/util/string.hpp>
@@ -129,16 +127,14 @@ void GLFWView::setMap(mbgl::Map *map_) {
     map->addAnnotationIcon("default_marker", makeSpriteImage(22, 22, 1));
 }
 
-void GLFWView::updateViewBinding() {
-    getContext().bindFramebuffer.setCurrentValue(0);
-    assert(mbgl::gl::value::BindFramebuffer::Get() == getContext().bindFramebuffer.getCurrentValue());
-    getContext().viewport.setCurrentValue({ 0, 0, getFramebufferSize() });
-    assert(mbgl::gl::value::Viewport::Get() == getContext().viewport.getCurrentValue());
+void GLFWView::updateAssumedState() {
+    assumeFramebufferBinding(0);
+    assumeViewportSize(getFramebufferSize());
 }
 
 void GLFWView::bind() {
-    getContext().bindFramebuffer = 0;
-    getContext().viewport = { 0, 0, getFramebufferSize() };
+    setFramebufferBinding(0);
+    setViewportSize(getFramebufferSize());
 }
 
 void GLFWView::onKey(GLFWwindow *window, int key, int /*scancode*/, int action, int mods) {
@@ -459,7 +455,6 @@ void GLFWView::run() {
             activate();
             mbgl::BackendScope scope { *this, mbgl::BackendScope::ScopeType::Implicit };
 
-            updateViewBinding();
             map->render(*this);
 
             glfwSwapBuffers(window);

platform/glfw/glfw_view.hpp

@@ -38,13 +38,13 @@ public:
     void run();
 
     // mbgl::View implementation
-    void updateViewBinding();
     void bind() override;
     mbgl::Size getSize() const;
     mbgl::Size getFramebufferSize() const;
 
     // mbgl::Backend implementation
     void invalidate() override;
+    void updateAssumedState() override;
 
 protected:
     // mbgl::Backend implementation

platform/ios/src/MGLMapView.mm

@@ -1,7 +1,6 @@
 #import "MGLMapView_Private.h"
 
 #include <mbgl/util/logging.hpp>
-#include <mbgl/gl/context.hpp>
 
 #import <GLKit/GLKit.h>
 #import <OpenGLES/EAGL.h>
@@ -869,7 +868,6 @@ public:
         // The OpenGL implementation automatically enables the OpenGL context for us.
         mbgl::BackendScope scope { *_mbglView, mbgl::BackendScope::ScopeType::Implicit };
 
-        _mbglView->updateViewBinding();
         _mbglMap->render(*_mbglView);
 
         [self updateUserLocationAnnotationView];
@@ -5322,31 +5320,25 @@ public:
     MBGLView(MGLMapView* nativeView_) : nativeView(nativeView_) {
     }
 
-    mbgl::gl::value::Viewport::Type getViewport() const {
-        return { 0, 0, nativeView.framebufferSize };
-    }
-
     /// This function is called before we start rendering, when iOS invokes our rendering method.
     /// iOS already sets the correct framebuffer and viewport for us, so we need to update the
     /// context state with the anticipated values.
-    void updateViewBinding() {
-        // We are using 0 as the placeholder value for the GLKView's framebuffer.
-        getContext().bindFramebuffer.setCurrentValue(0);
-        getContext().viewport.setCurrentValue(getViewport());
-        assert(mbgl::gl::value::Viewport::Get() == getContext().viewport.getCurrentValue());
+    void updateAssumedState() override {
+        assumeFramebufferBinding(ImplicitFramebufferBinding);
+        assumeViewportSize(nativeView.framebufferSize);
     }
 
     void bind() override {
-        if (getContext().bindFramebuffer != 0) {
+        if (!implicitFramebufferBound()) {
             // Something modified our state, and we need to bind the original drawable again.
             // Doing this also sets the viewport to the full framebuffer.
             // Note that in reality, iOS does not use the Framebuffer 0 (it's typically 1), and we
             // only use this is a placeholder value.
             [nativeView.glView bindDrawable];
-            updateViewBinding();
+            updateAssumedState();
         } else {
             // Our framebuffer is still bound, but the viewport might have changed.
-            getContext().viewport = getViewport();
+            setViewportSize(nativeView.framebufferSize);
         }
     }
 

platform/macos/src/MGLMapView.mm

@@ -26,9 +26,9 @@
 #import <mbgl/map/camera.hpp>
 #import <mbgl/storage/reachability.h>
 #import <mbgl/util/default_thread_pool.hpp>
-#import <mbgl/gl/context.hpp>
 #import <mbgl/map/backend.hpp>
 #import <mbgl/map/backend_scope.hpp>
+#import <mbgl/gl/gl.hpp>
 #import <mbgl/sprite/sprite_image.hpp>
 #import <mbgl/storage/default_file_source.hpp>
 #import <mbgl/storage/network_status.hpp>
@@ -770,7 +770,6 @@ public:
         // The OpenGL implementation automatically enables the OpenGL context for us.
         mbgl::BackendScope scope { *_mbglView, mbgl::BackendScope::ScopeType::Implicit };
 
-        _mbglView->updateViewBinding();
         _mbglMap->render(*_mbglView);
 
         if (_isPrinting) {
@@ -2866,25 +2865,19 @@ public:
         [NSOpenGLContext clearCurrentContext];
     }
 
-    mbgl::gl::value::Viewport::Type getViewport() const {
-        return { 0, 0, nativeView.framebufferSize };
-    }
-
-    void updateViewBinding() {
-        fbo = mbgl::gl::value::BindFramebuffer::Get();
-        getContext().bindFramebuffer.setCurrentValue(fbo);
-        getContext().viewport.setCurrentValue(getViewport());
-        auto actualViewport = mbgl::gl::value::Viewport::Get();
-        assert(actualViewport == getContext().viewport.getCurrentValue());
+    void updateAssumedState() override {
+        MBGL_CHECK_ERROR(glGetIntegerv(GL_FRAMEBUFFER_BINDING, &fbo));
+        assumeFramebufferBinding(fbo);
+        assumeViewportSize(nativeView.framebufferSize);
     }
 
     void bind() override {
-        getContext().bindFramebuffer = fbo;
-        getContext().viewport = getViewport();
+        setFramebufferBinding(fbo);
+        setViewportSize(nativeView.framebufferSize);
     }
 
     mbgl::PremultipliedImage readStillImage() {
-        return getContext().readFramebuffer<mbgl::PremultipliedImage>(nativeView.framebufferSize);
+        return readFramebuffer(nativeView.framebufferSize);
     }
 
 private:

platform/qt/src/qmapboxgl.cpp

@@ -7,7 +7,6 @@
 #include <mbgl/annotation/annotation.hpp>
 #include <mbgl/map/camera.hpp>
 #include <mbgl/map/map.hpp>
-#include <mbgl/gl/context.hpp>
 #include <mbgl/map/backend_scope.hpp>
 #include <mbgl/style/conversion.hpp>
 #include <mbgl/style/conversion/layer.hpp>
@@ -1500,7 +1499,6 @@ void QMapboxGL::render()
     mbgl::BackendScope scope { *d_ptr, mbgl::BackendScope::ScopeType::Implicit };
 
     d_ptr->dirty = false;
-    d_ptr->updateViewBinding();
     d_ptr->mapObj->render(*d_ptr);
 }
 
@@ -1575,16 +1573,14 @@ mbgl::Size QMapboxGLPrivate::framebufferSize() const {
     return { static_cast<uint32_t>(fbSize.width()), static_cast<uint32_t>(fbSize.height()) };
 }
 
-void QMapboxGLPrivate::updateViewBinding() {
-    getContext().bindFramebuffer.setCurrentValue(fbObject);
-    assert(mbgl::gl::value::BindFramebuffer::Get() == getContext().bindFramebuffer.getCurrentValue());
-    getContext().viewport.setCurrentValue({ 0, 0, framebufferSize() });
-    assert(mbgl::gl::value::Viewport::Get() == getContext().viewport.getCurrentValue());
+void QMapboxGLPrivate::updateAssumedState() {
+    assumeFramebufferBinding(fbObject);
+    assumeViewportSize(framebufferSize());
 }
 
 void QMapboxGLPrivate::bind() {
-    getContext().bindFramebuffer = fbObject;
-    getContext().viewport = { 0, 0, framebufferSize() };
+    setFramebufferBinding(fbObject);
+    setViewportSize(framebufferSize());
 }
 
 void QMapboxGLPrivate::invalidate()

platform/qt/src/qmapboxgl_p.hpp

@@ -21,12 +21,12 @@ public:
     virtual ~QMapboxGLPrivate();
 
     mbgl::Size framebufferSize() const;
-    void updateViewBinding();
 
     // mbgl::View implementation.
     void bind() final;
 
     // mbgl::Backend implementation.
+    void updateAssumedState() final;
     void invalidate() final;
     void activate() final {}
     void deactivate() final {}

src/mbgl/map/backend.cpp

@@ -21,6 +21,38 @@ gl::Context& Backend::getContext() {
     return *context;
 }
 
+PremultipliedImage Backend::readFramebuffer(const Size& size) const {
+    assert(context);
+    return context->readFramebuffer<PremultipliedImage>(size);
+}
+
+void Backend::assumeFramebufferBinding(const gl::FramebufferID fbo) {
+    getContext().bindFramebuffer.setCurrentValue(fbo);
+    if (fbo != ImplicitFramebufferBinding) {
+        assert(gl::value::BindFramebuffer::Get() == getContext().bindFramebuffer.getCurrentValue());
+    }
+}
+void Backend::assumeViewportSize(const Size& size) {
+    getContext().viewport.setCurrentValue({ 0, 0, size });
+    assert(gl::value::Viewport::Get() == getContext().viewport.getCurrentValue());
+}
+
+bool Backend::implicitFramebufferBound() {
+    return getContext().bindFramebuffer.getCurrentValue() == ImplicitFramebufferBinding;
+}
+
+void Backend::setFramebufferBinding(const gl::FramebufferID fbo) {
+    getContext().bindFramebuffer = fbo;
+    if (fbo != ImplicitFramebufferBinding) {
+        assert(gl::value::BindFramebuffer::Get() == getContext().bindFramebuffer.getCurrentValue());
+    }
+}
+
+void Backend::setViewportSize(const Size& size) {
+    getContext().viewport = { 0, 0, size };
+    assert(gl::value::Viewport::Get() == getContext().viewport.getCurrentValue());
+}
+
 Backend::~Backend() = default;
 
 } // namespace mbgl

src/mbgl/map/map.cpp

@@ -285,6 +285,8 @@ void Map::Impl::render(View& view) {
                               contextMode,
                               debugOptions };
 
+        backend.updateAssumedState();
+
         painter->render(*style,
                         frameData,
                         view,
@@ -323,6 +325,8 @@ void Map::Impl::render(View& view) {
                               contextMode,
                               debugOptions };
 
+        backend.updateAssumedState();
+
         try {
             painter->render(*style,
                             frameData,