From ee94520bffeae2f6246284e1931d0cc1d615f708 Mon Sep 17 00:00:00 2001 From: Jukka Hietanen Date: Mon, 29 Mar 2021 21:52:57 +0300 Subject: [PATCH] [ios] Cherry pick relevant changes from from https://github.com/mapbox/mapbox-gl-native-ios/pull/411/files (#60) Co-authored-by: Jukka Hietanen --- .../ios/platform/ios/src/MGLMapView+OpenGL.mm | 29 +++----- platform/ios/platform/ios/src/MGLMapView.mm | 66 +++++++++++++++---- 2 files changed, 64 insertions(+), 31 deletions(-) diff --git a/platform/ios/platform/ios/src/MGLMapView+OpenGL.mm b/platform/ios/platform/ios/src/MGLMapView+OpenGL.mm index c96bfa9e8..340f2a9fd 100644 --- a/platform/ios/platform/ios/src/MGLMapView+OpenGL.mm +++ b/platform/ios/platform/ios/src/MGLMapView+OpenGL.mm @@ -97,24 +97,14 @@ void MGLMapViewOpenGLImpl::setPresentsWithTransaction(const bool value) { void MGLMapViewOpenGLImpl::display() { auto& resource = getResource(); - // See https://github.com/mapbox/mapbox-gl-native/issues/14232 - // glClear can be blocked for 1 second. This code is an "escape hatch", - // an attempt to detect this situation and rebuild the GL views. - if (mapView.enablePresentsWithTransaction && resource.atLeastiOS_12_2_0) { - CFTimeInterval before = CACurrentMediaTime(); - [resource.glView display]; - CFTimeInterval after = CACurrentMediaTime(); - - if (after - before >= 1.0) { -#ifdef MGL_RECREATE_GL_IN_AN_EMERGENCY - dispatch_async(dispatch_get_main_queue(), ^{ - emergencyRecreateGL(); - }); -#endif - } - } else { - [resource.glView display]; - } + // Calling `display` here directly causes the stuttering bug (if + // `presentsWithTransaction` is `YES` - see above) + // as reported in https://github.com/mapbox/mapbox-gl-native-ios/issues/350 + // + // Since we use `presentsWithTransaction` to synchronize with UIView + // annotations, we now let the system handle when the view is rendered. This + // has the potential to increase latency + [resource.glView setNeedsDisplay]; } void MGLMapViewOpenGLImpl::createView() { @@ -137,7 +127,7 @@ void MGLMapViewOpenGLImpl::createView() { resource.glView.drawableDepthFormat = GLKViewDrawableDepthFormat16; resource.glView.opaque = mapView.opaque; resource.glView.layer.opaque = mapView.opaque; - resource.glView.enableSetNeedsDisplay = NO; + resource.glView.enableSetNeedsDisplay = YES; CAEAGLLayer* eaglLayer = MGL_OBJC_DYNAMIC_CAST(resource.glView.layer, CAEAGLLayer); eaglLayer.presentsWithTransaction = NO; @@ -155,6 +145,7 @@ void MGLMapViewOpenGLImpl::deleteView() { } #ifdef MGL_RECREATE_GL_IN_AN_EMERGENCY +// TODO: Fix or remove // See https://github.com/mapbox/mapbox-gl-native/issues/14232 void MGLMapViewOpenGLImpl::emergencyRecreateGL() { auto& resource = getResource(); diff --git a/platform/ios/platform/ios/src/MGLMapView.mm b/platform/ios/platform/ios/src/MGLMapView.mm index f54a97598..58732c302 100644 --- a/platform/ios/platform/ios/src/MGLMapView.mm +++ b/platform/ios/platform/ios/src/MGLMapView.mm @@ -974,12 +974,31 @@ public: } } +- (void)updateViewsPostMapRendering { + // Update UIKit elements, prior to rendering + [self updateUserLocationAnnotationView]; + [self updateAnnotationViews]; + [self updateCalloutView]; + + // Call any pending completion blocks. This is primarily to ensure + // that annotations are in the expected position after core rendering + // and map update. + // + // TODO: Consider using this same mechanism for delegate callbacks. + [self processPendingBlocks]; +} + - (void)renderSync { if ( ! self.dormant && _rendererFrontend) { _rendererFrontend->render(); } + + // TODO: This should be moved from what's essentially the UIView rendering + // To do this, add view models that can be updated separately, before the + // UIViews can be updated to match + [self updateViewsPostMapRendering]; } // This gets called when the view dimension changes, e.g. because the device is being rotated. @@ -1172,6 +1191,37 @@ public: - (void)updateFromDisplayLink:(CADisplayLink *)displayLink { + // CADisplayLink's call interval closely matches the that defined by, + // preferredFramesPerSecond, however it is NOT called on the vsync and + // can fire some time after the vsync, and the duration can often exceed + // the expected period. + // + // The `timestamp` property should represent (or be very close to) the vsync, + // so for any kind of frame rate measurement, it can be important to record + // the time upon entry to this method. + // + // This start time, coupled with the `targetTimestamp` gives you a measure + // of how long you have to do work before the next vsync. + // + // Note that CADisplayLink's duration property is interval between vsyncs at + // the device's natural frequency (60, 120). Instead, for the duration of a + // frame, use the two timestamps instead. This is especially important if + // you have set preferredFramesPerSecond to something other than the default. + // + // │ remaining duration ┃ + // │◀ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ▶┃ + // ┌ ─ ─ ─ ─ ─ ┼───────────────────────╋───────────────────────────────────┳─────── + // │ ┃ ┃ + // │ │ ┃ ┃ + // │ ┃ ┃ + // ▼ │ ▼ ▼ + // timestamp │ target + // (vsync?) │ timestamp + // │ + // ▼ + // display link + // start time + MGLAssertIsMainThread(); // Not "visible" - this isn't a full definition of visibility, but if @@ -1196,21 +1246,13 @@ public: { _needsDisplayRefresh = NO; - // Update UIKit elements, prior to rendering - [self updateUserLocationAnnotationView]; - [self updateAnnotationViews]; - [self updateCalloutView]; - - // Call any pending completion blocks. This is primarily to ensure - // that annotations are in the expected position after core rendering - // and map update. - // - // TODO: Consider using this same mechanism for delegate callbacks. - [self processPendingBlocks]; - + // UIView update logic has moved into `renderSync` above, which now gets + // triggered by a call to setNeedsDisplay. + // See MGLMapViewOpenGLImpl::display() for more details _mbglView->display(); } + // TODO: Fix if (self.experimental_enableFrameRateMeasurement) { CFTimeInterval now = CACurrentMediaTime();