Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Style switching no longer invokes onStyleLoadedCallback on ios #771

Closed
felix-ht opened this issue Nov 16, 2021 · 11 comments
Closed

Style switching no longer invokes onStyleLoadedCallback on ios #771

felix-ht opened this issue Nov 16, 2021 · 11 comments

Comments

@felix-ht
Copy link
Collaborator

If i change the style on an active map the onStyleLoadedCallback does not get called again. (Rebuild a MapboxMap with a different styleString)
I assume that the cause of this is #690 @AAverin

@AAverin
Copy link
Contributor

AAverin commented Nov 16, 2021

I will investigate. But as I see so far a new instance of MapboxMapController is created every time build is called for MapboxMap dart class, so all the style loading should run again. Unless something else gets in the way.

@AAverin
Copy link
Contributor

AAverin commented Nov 16, 2021

@felix-ht I have just tried on latest master and onStyleLoadedCallback did execute correctly when I changed style, also visually style updated on the screen.

@felix-ht
Copy link
Collaborator Author

felix-ht commented Nov 16, 2021

huh i will try to reproduce - for now i switched back to a branch without the issue

@AAverin
Copy link
Contributor

AAverin commented Nov 16, 2021

If you have a commit in mind I can quickly switch to it and test. In my app there is a button to switch style that round-rotates over street, outdoor and satelite, so quite easy for me to check.

@felix-ht
Copy link
Collaborator Author

@AAverin so i did a git bisect and for me Feature/intermediate fixes is the one that breaks the style switching.

The styles still do switch fine, but on style loaded no longer gets invoked. As I add my vector data in that callback - it doesn't get added to the new style until i invoke the add vector data function by some other means.

@AAverin
Copy link
Contributor

AAverin commented Nov 17, 2021

@felix-ht I have logs in my onStyleLoaded callback and when I switch styles I see a new entry logged.
Can you reproduce it on some simplified example? You can use one I have been using for leaks testing #706 (comment)
I will try to reproduce again myself tomorrow

@felix-ht
Copy link
Collaborator Author

felix-ht commented Nov 17, 2021

@AAverin This patch fixes the issue for me:

diff --git a/ios/Classes/MapboxMapController.swift b/ios/Classes/MapboxMapController.swift
index 05d0e0c..4c31d6d 100644
--- a/ios/Classes/MapboxMapController.swift
+++ b/ios/Classes/MapboxMapController.swift
@@ -11,6 +11,7 @@ class MapboxMapController: NSObject, FlutterPlatformView, MGLMapViewDelegate, Ma
     private var mapView: MGLMapView
     private var isMapReady = false
     private var isStyleReady = false
+    private var isStyleReadyNotified = false
     private var mapReadyResult: FlutterResult?
     
     private var initialTilt: CGFloat?
@@ -94,7 +95,8 @@ class MapboxMapController: NSObject, FlutterPlatformView, MGLMapViewDelegate, Ma
                 result(nil)
                 //Style could happen to been ready before the map was ready due to the order of methods invoked
                 //We should invoke onStyleLoaded
-                if isStyleReady {
+                if isStyleReady && !isStyleReadyNotified {
+                    isStyleReadyNotified = true
                     if let channel = channel {
                         channel.invokeMethod("map#onStyleLoaded", arguments: nil)
                     }
@@ -849,6 +851,7 @@ class MapboxMapController: NSObject, FlutterPlatformView, MGLMapViewDelegate, Ma
      *  MGLMapViewDelegate
      */
     func mapView(_ mapView: MGLMapView, didFinishLoading style: MGLStyle) {
+        isStyleReadyNotified = false
         isMapReady = true
         updateMyLocationEnabled()
         
@@ -883,8 +886,8 @@ class MapboxMapController: NSObject, FlutterPlatformView, MGLMapViewDelegate, Ma
 
         //If map is ready and map#waitForMap was called, we invoke onStyleLoaded callback directly
         //If not, we will have to call it when map#waitForMap is done
-        if let mapReadyResult = mapReadyResult {
-            mapReadyResult(nil)
+        if !isStyleReadyNotified {
+            isStyleReadyNotified = true
             if let channel = channel {
                 channel.invokeMethod("map#onStyleLoaded", arguments: nil)
             }

For me onPlatformViewCreated and thus map#waitForMap only get called once. A style switch does not invoke it again. So mapReadyResult never gets set again and on style loaded does not get invoked after a style switch.

On a more general note assuming that the swift code runs in a single thread (on flutters ui thread) - your fix shouldn’t do anything as all of the code is synchronous so its actually impossible for map#waitForMap and 'mapView' to execute concurrently. As it stands right now i would actually be in favor of reverting this commit until be have a better understanding of whats going on with these callbacks.

Here is also a minimal example to reproduce this: https://github.com/flutter-mapbox-gl/maps/tree/style-switch-example
(i added the style switching to the Full screen map page, directly above the critical commit)

A snackbar is supposed to be shown whenever onStyleLoaded is called

@AAverin
Copy link
Contributor

AAverin commented Nov 17, 2021

Interesting, I'll try to investigate tomorrow. Thanks for the example.
The original fix is there because in some cases onStyleLoaded was never invoked on iOS. Debugging on real device, I came up with a fix and since then onStyleLoaded was always invoked.
Looks like there is some other edge-case situation that can happen I didn't take into account.

@AAverin
Copy link
Contributor

AAverin commented Nov 18, 2021

@felix-ht Debugging, it seems to relate to timing.
Running my app, I have more widgets in build method and a few futures to wait for (permissions, redux store), so there is a more delay and that's why onStyleLoaded calls back correctly.
Running your simplified example there is nothing to wait there for, so it doesn't call correctly.
Debugging your example everything calls back correctly too because of the debug delay.

@AAverin
Copy link
Contributor

AAverin commented Nov 18, 2021

The reason for the bug seems to be in didUpdateWidget. Changing style doesn't create a new instance of MapboxMapController and rather reuses an old one, updating it's options. Which is why your patch works, re-notifying onStyleLoaded every time style gets ready.

On the other hand I also tried completely removing my extra calls for onStyleLoaded reverting my PR, but then on the real phone I don't see onStyleLoaded called reliably anymore. Again, this could be related to delays in network and/or amount of futures one waits for building the map.

@tobrun
Copy link
Collaborator

tobrun commented Nov 22, 2021

#775 merged, closing

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants