Re-initialize map view after changing style about maputnik HOT 22 CLOSED

Hopefully the last patch I'm doing to fix these issues: #889

from maputnik.

Hmm... interesting, might be related to style diff capabilities or limitations.

from maputnik.

FWIW, cannot reproduce this with v1.7.0 at https://maputnik.github.io/editor/#13/31.27563/35.12306

from maputnik.

Looks like there's wrong handling of style update somewhere in the code.
The following is reported in the console:

In the past (1.7), the style diff was not able to perform the diff and the map was reloaded with the following warning:
Unable to perform style diff: Unimplemented: setSprite.. Rebuilding the style from scratch.
In recent version of maplibre-gl-js there is a more eager attempt to allow style diffing, but it might have bugs.
So this might be a bug in maplibre and not maputnik.
I'll need to see if I can create a minimal reproduction.
Thanks for reporting this issue!

from maputnik.

Ok, so I tried to see if I can reproduce this on "plain" maplibre, and I was not able to.
I also looked at the diff code in both maplibre-gl and maplibre-style-spec to see if I see some problems there and it is both covered with tests and runs as expected.
So I dug deeper and it seems that the reason for this is the following:

1. When replacing a style, the style diff method is called.
2. This in turn removes all the layers and then removes the sources.
3. After the first source is removed, an event of data is raised to let anyone who listens know that the source was removed.
4. This apparently is picked up by mapbox-inspect which forces the original style back to the map.

mapbox-inspect is updating the map style here:
https://github.com/lukasmartinelli/mapbox-gl-inspect/blob/dab82f2f83670bfeea93195da5db0f35dae99921/lib/MapboxInspect.js#L130

It might make sense since mapbox-inspect has its own style, but I'm not sure it should be applied when the mode in not "inspect" mode.

Bottom line, this is a maputnik related, I'm not sure I fully understand how to solve it though...

from maputnik.

Seems like https://github.com/lukasmartinelli/mapbox-gl-inspect is completely abandoned, not sure it's worth opening a PR there.
@nyurik do you know if we can get access to that repo?
The other option is simply copy the code into this repo and use it here, I'm sure it can be simplified dramatically to only support the use case of maputnik, maybe.

from maputnik.

We could ask @lukasmartinelli if he would be interested in transferring it here too - seems like a worthy component to maintain.

from maputnik.

Happy to transfer it!

from maputnik.

Awesome, thanks @lukasmartinelli! I think the easiest is to grant me or @HarelM admin rights on that repo, and we can do the rest. Note that we will probably want to continue publishing it under the original npm name, so may need to add harel as an admin there too. Thx!

from maputnik.

Actually, I just realized it has mapbox in its name - so perhaps we should rename it and/or fork it, and republish under maplibre name?

from maputnik.

Yeah, I was thinking about @maplibre/maplibre-inspect or something similar like we did with the other libraries.
We can keep the original repo as is for legacy mapbox users.

from maputnik.

I can probably commit to maintaining it, as it is an integral part of maputnik, and I don't expect a lot of changes there (It's also like 5 files in total...).
@nyurik will you do the honor of opening a PR to fork the repo under the maplibre umbrella?

from maputnik.

Yeah, perhaps it really does make sense to have an official fork that does not show up as a "github fork" (under the name at the top), but instead mentions in the readme that it was adapted from the other repo and renamed. There are not that many issues/pull requests it seems to do the transfer...

from maputnik.

I created maplibre/maplibre#359 - I guess we should do a proper fork (without the link to upstream). Leaving it to @HarelM :)

from maputnik.

@zstadler let me know if you can reproduce this issue in latest version, I have deployed a temporary fix until we migrate maplibre-gl-inspect under maplibre org.

from maputnik.

@HarelM,
The original steps to reproductions do not trigger this issue.

However, since inspect was mentioned above, I've tried the following steps, and the style does change as expected.

1. Open Maplibre's maputnik in this location
2. Open -> Gallery Styles -> Empty Style
3. Reload the page to be on the safe side
4. Open -> Open URL -> https://raw.githubusercontent.com/IsraelHikingMap/VectorMap/master/Styles/IHM.json
5. Change to inspect view
6. Open -> Gallery Styles -> OSM Liberty
7. Change to map view
8. The Israel Hiking style is shown rather than OSM Liberty
9. Reload the page
10. The OSM Liberty style map is shown

from maputnik.

The reason for the above, which is similar but different to the original issue is that the inspect library adds metadata to the style to indicate that this style is from the inspect library.
When doing the diff, this flag is kept and the inspect library doesn't change the "original" style.
So when turning off inspect, the original style is incorrect.
Seems like there are a lot of assumptions in both maputnik and inspect that are preventing this from working correctly, I'm not sure I'll be able to solve those...

from maputnik.

After an archaeological digging I found some hacks that were added in order to fix some odd behavior between mapbox-gl and mapbox-gl-inspect.
Here is the relevant link: https://github.com/maplibre/maputnik/blob/5f54dd0ccf34d205c8598186bd41eaae328f238b/src/components/MapMaplibreGl.tsx#L120L132

We now own maplibre-inspect and these should be fixed properly now in there, the problem is the issues they link to, and making sure everything is working as expected after the hacks are removed.
In general there are some race conditions between setting the inspect style and regular style, and it causes a mayham.

Relevant issues to the above HACK comments:
#576
#647

I'm not sure I know how to solve all this "properly" as it seems there's a fundamental problem with how maputnik works with the store and event drivern and how maplibre-inspect works - listening to map events.

from maputnik.

I hope both issues reported here are fixed now.
I have tested both on the local dev machine and now the deployed site I can't reproduce the issues.
Due to the complexity of the events and sync, the solution caused the transition to and from inspect is a bit more sluggish, but I'm OK with that assuming the data is correct.

from maputnik.

Well, almost...

The second scenario is leaves Maputnik in an inconsistent state

1. Open Maplibre's maputnik in this location
2. Open -> Gallery Styles -> Empty Style
3. Reload the page to be on the safe side
4. Open -> Gallery Styles -> OSM Liberty
5. Change to inspect view
6. Open -> Gallery Styles -> Positron
7. The Positron style map is shown rather inspect view
   

from maputnik.

In the above case, the sources are equal, so after the style change, which sets the "regular" map style, the inspect code, which listens to style changes compares the sources and doesn't apply the inspect style.
I'll check what can be done, thanks for all the testing!!

from maputnik.

LGTM!

from maputnik.