Skip to content
This repository has been archived by the owner on Aug 8, 2023. It is now read-only.

Jump to style’s default viewport on initialization #6129

Closed
1ec5 opened this issue Aug 23, 2016 · 5 comments
Closed

Jump to style’s default viewport on initialization #6129

1ec5 opened this issue Aug 23, 2016 · 5 comments
Assignees
Labels
feature GL JS parity For feature parity with Mapbox GL JS

Comments

@1ec5
Copy link
Contributor

1ec5 commented Aug 23, 2016

The map should jump to the default center coordinates, zoom level, bearing, and pitch as soon as the first style loads after initialization. Switching from one style to another probably shouldn’t change the viewport.

Complicating matters, the initial viewport may be overridden for various reasons. For example, state restoration would take place upon initialization. Or the developer may programmatically set the viewport immediately after initializing the map view but before the style loads. The style shouldn’t be able to clobber a viewport set this way.

In the iOS and macOS SDKs, MGLMapView’s initial center coordinates, zoom level, and direction can be specified in a storyboard. Unfortunately, storyboard-defined properties are set asynchronously, in piecemeal fashion, after the MGLMapView object is initialized and thus sometime after mbgl::Map is initialized. The style’s default viewport shouldn’t clobber this asynchronously set viewport, either.

In #6127 (comment), @jfirebaugh suggests implementing the initial viewport behavior in core. That could work for the iOS and macOS SDKs, as long as we account for the cases above. Perhaps methods like mbgl::Map::setLatLng() and easeTo() could shut off any style-default-viewport functionality for the lifetime of the map.

/cc @friedbunny @tobrun

@1ec5 1ec5 added feature GL JS parity For feature parity with Mapbox GL JS labels Aug 23, 2016
@tobrun
Copy link
Member

tobrun commented Aug 24, 2016

Linking the related properties van style spec here

In #6127 (comment), @jfirebaugh suggests implementing the initial viewport behavior in core. That could work for the iOS and macOS SDKs, as long as we account for the cases above.

For Android this would work as well, this requirement fits well in the Android concept of OnMapReady, this is a callback that is called after the style has loaded. Until that point a user can't instantiate any animations or do camera manipulations other than the initial camera position (this can be set in layout xml or using the MapboxMapOptions using Lat, Lng, zoom en tilt). In case a user provides another initial camera position, it should override the data coming from the style.

API wise, how do you see this happening? add parameters to setStyleUrl to have an opptional moveViewport? make style loading a multi step process (calling multiple methods instead of one)?

update: dependency on the following PR #6046

@1ec5
Copy link
Contributor Author

1ec5 commented Aug 24, 2016

Ideally there would be no API change, even at the core level. Core would jump to the default viewport internally upon loading the first style. Only the first style ever loaded would ever cause a jump, and setters on mbgl::Map like setLatLng() would automatically turn off that functionality. So if the developer calls MapView.setLatLng() before the style finishes loading, the end result would be just as it is today.

@incanus
Copy link
Contributor

incanus commented Sep 12, 2016

One idea: this could be as simple as a Map property called e.g. initialCameraOptions, perhaps as a util::optional or boost::optional, that begins as the same default viewport that we are using now, gets set atomically by the style parse if non-NULL at that point, and that any of the relevant setters would just NULL out, preventing such auto-load (either style-backed or default) behavior.

@tobrun
Copy link
Member

tobrun commented Oct 6, 2016

Looked into where the values are being set:

Style::setJSON(const std::string& json)

This function is responsible for parsing a style and setting up Style values: eg.

defaultLatLng = parser.latLng;
defaultZoom = parser.zoom;
defaultBearing = parser.bearing;
defaultPitch = parser.pitch;

This function than notifies an observer letting map.ccp know that the style has been loaded.

@tobrun
Copy link
Member

tobrun commented Oct 6, 2016

I have something working:

Not manipulating the camera results in showing the default from the style:

device-2016-10-06-103047

Manipulating the camera in java:

        mapView.getMapAsync(new OnMapReadyCallback() {
            @Override
            public void onMapReady(@NonNull MapboxMap mapboxMap) {
                DynamicMarkerChangeActivity.this.mapboxMap = mapboxMap;
                mapboxMap.moveCamera(CameraUpdateFactory.newLatLngZoom(new LatLng(51.506675, -0.128699), 10));
            }
        });

Results in:

device-2016-10-06-102437

or manipulating in xml:

           <com.mapbox.mapboxsdk.maps.MapView
                app:center_latitude="51.506675"
                app:center_longitude="-0.128699"
                app:zoom="10" />

Results in:

device-2016-10-06-103433

or manipulating via MapboxMapOptions:

        MapboxMapOptions options = new MapboxMapOptions()
                .camera(new CameraPosition.Builder()
                        .target(new LatLng(51.506675, -0.128699))
                        .zoom(10)
                        .build());

Results in:

device-2016-10-06-104326

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
feature GL JS parity For feature parity with Mapbox GL JS
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants