[core] correctly set the framebuffer size on retina screens

Also asserts in Debug mode that the internal state and the OpenGL state are identical

7 files changed, 35 insertions, 14 deletions

diff --git a/platform/android/src/native_map_view.cpp b/platform/android/src/native_map_view.cpp
index 9943dafe36..65e39b198e 100755
--- a/platform/android/src/native_map_view.cpp
+++ b/platform/android/src/native_map_view.cpp
@@ -116,17 +116,20 @@ NativeMapView::~NativeMapView() {
     vm = nullptr;
 }
 
+mbgl::Size NativeMapView::getFramebufferSize() const {
+    return { static_cast<uint32_t>(fbWidth), static_cast<uint32_t>(fbHeight) };
+}
+
 void NativeMapView::updateViewBinding() {
     getContext().bindFramebuffer.setCurrentValue(0);
-    getContext().viewport.setCurrentValue(
-        { 0, 0, { static_cast<uint32_t>(fbWidth), static_cast<uint32_t>(fbHeight) } });
+    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::bind() {
     getContext().bindFramebuffer = 0;
-    getContext().viewport = { 0,
-                              0,
-                              { static_cast<uint32_t>(fbWidth), static_cast<uint32_t>(fbHeight) } };
+    getContext().viewport = { 0, 0, getFramebufferSize() };
 }
 
 void NativeMapView::activate() {
diff --git a/platform/android/src/native_map_view.hpp b/platform/android/src/native_map_view.hpp
index 43fb0c1ccd..09619802ce 100755
--- a/platform/android/src/native_map_view.hpp
+++ b/platform/android/src/native_map_view.hpp
@@ -20,6 +20,7 @@ public:
     NativeMapView(JNIEnv *env, jobject obj, float pixelRatio, int availableProcessors, size_t totalMemory);
     virtual ~NativeMapView();
 
+    mbgl::Size getFramebufferSize() const;
     void updateViewBinding();
     void bind() override;
 
diff --git a/platform/default/glfw_view.cpp b/platform/default/glfw_view.cpp
index e9b32b21c8..a8fa17548d 100644
--- a/platform/default/glfw_view.cpp
+++ b/platform/default/glfw_view.cpp
@@ -132,7 +132,9 @@ void GLFWView::setMap(mbgl::Map *map_) {
 
 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::bind() {
diff --git a/platform/ios/src/MGLMapView.mm b/platform/ios/src/MGLMapView.mm
index 43f357559f..d392849522 100644
--- a/platform/ios/src/MGLMapView.mm
+++ b/platform/ios/src/MGLMapView.mm
@@ -549,6 +549,12 @@ public:
              static_cast<uint32_t>(self.bounds.size.height) };
 }
 
+- (mbgl::Size)framebufferSize
+{
+    return { static_cast<uint32_t>(self.glView.drawableWidth),
+             static_cast<uint32_t>(self.glView.drawableHeight) };
+}
+
 - (void)createGLView
 {
     if (_context) return;
@@ -4953,7 +4959,7 @@ public:
     }
 
     mbgl::gl::value::Viewport::Type getViewport() const {
-        return { 0, 0, nativeView.size };
+        return { 0, 0, nativeView.framebufferSize };
     }
 
     /// This function is called before we start rendering, when iOS invokes our rendering method.
@@ -4963,6 +4969,7 @@ public:
         // 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 bind() override {
diff --git a/platform/macos/src/MGLMapView.mm b/platform/macos/src/MGLMapView.mm
index 2630a361a0..45208408fe 100644
--- a/platform/macos/src/MGLMapView.mm
+++ b/platform/macos/src/MGLMapView.mm
@@ -311,6 +311,11 @@ public:
              static_cast<uint32_t>(self.bounds.size.height) };
 }
 
+- (mbgl::Size)framebufferSize {
+    NSRect bounds = [self convertRectToBacking:self.bounds];
+    return { static_cast<uint32_t>(bounds.size.width), static_cast<uint32_t>(bounds.size.height) };
+}
+
 /// Adds zoom controls to the lower-right corner.
 - (void)installZoomControls {
     _zoomControls = [[NSSegmentedControl alloc] initWithFrame:NSZeroRect];
@@ -2563,13 +2568,14 @@ public:
     }
 
     mbgl::gl::value::Viewport::Type getViewport() const {
-        return { 0, 0, nativeView.size };
+        return { 0, 0, nativeView.framebufferSize };
     }
 
     void updateViewBinding() {
         fbo = mbgl::gl::value::BindFramebuffer::Get();
         getContext().bindFramebuffer.setCurrentValue(fbo);
         getContext().viewport.setCurrentValue(getViewport());
+        assert(mbgl::gl::value::Viewport::Get() == getContext().viewport.getCurrentValue());
     }
 
     void bind() override {
@@ -2578,17 +2584,14 @@ public:
     }
 
     mbgl::PremultipliedImage readStillImage() {
-        NSRect bounds = [nativeView convertRectToBacking:nativeView.bounds];
-        const uint32_t width = bounds.size.width;
-        const uint32_t height = bounds.size.height;
-        mbgl::PremultipliedImage image({ width, height });
-        MBGL_CHECK_ERROR(
-            glReadPixels(0, 0, width, height, GL_RGBA, GL_UNSIGNED_BYTE, image.data.get()));
+        mbgl::PremultipliedImage image(nativeView.framebufferSize);
+        MBGL_CHECK_ERROR(glReadPixels(0, 0, image.size.width, image.size.height, GL_RGBA,
+                                      GL_UNSIGNED_BYTE, image.data.get()));
 
         const size_t stride = image.stride();
         auto tmp = std::make_unique<uint8_t[]>(stride);
         uint8_t *rgba = image.data.get();
-        for (int i = 0, j = height - 1; i < j; i++, j--) {
+        for (int i = 0, j = image.size.height - 1; i < j; i++, j--) {
             std::memcpy(tmp.get(), rgba + i * stride, stride);
             std::memcpy(rgba + i * stride, rgba + j * stride, stride);
             std::memcpy(rgba + j * stride, tmp.get(), stride);
diff --git a/platform/qt/src/qmapboxgl.cpp b/platform/qt/src/qmapboxgl.cpp
index 4fdd9eb293..3253dd3251 100644
--- a/platform/qt/src/qmapboxgl.cpp
+++ b/platform/qt/src/qmapboxgl.cpp
@@ -854,6 +854,7 @@ void QMapboxGLPrivate::updateFramebufferBinding(QOpenGLFramebufferObject *fbo_)
             0, 0, { static_cast<uint32_t>(fbo->width()), static_cast<uint32_t>(fbo->height()) } };
     } else {
         getContext().bindFramebuffer.setCurrentValue(0);
+        assert(mbgl::gl::value::BindFramebuffer::Get() == getContext().bindFramebuffer.getCurrentValue());
         getContext().viewport = {
             0, 0, { static_cast<uint32_t>(fbSize.width()), static_cast<uint32_t>(fbSize.height()) } };
     }
diff --git a/src/mbgl/gl/value.hpp b/src/mbgl/gl/value.hpp
index 6296d64416..1f01d4e71a 100644
--- a/src/mbgl/gl/value.hpp
+++ b/src/mbgl/gl/value.hpp
@@ -183,6 +183,10 @@ constexpr bool operator!=(const Viewport::Type& a, const Viewport::Type& b) {
     return a.x != b.x || a.y != b.y || a.size != b.size;
 }
 
+constexpr bool operator==(const Viewport::Type& a, const Viewport::Type& b) {
+    return !(a != b);
+}
+
 struct BindFramebuffer {
     using Type = FramebufferID;
     static const constexpr Type Default = 0;