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

setZoom overwrites setLatLng #10928

Closed
Nols1000 opened this issue Jan 16, 2018 · 9 comments
Closed

setZoom overwrites setLatLng #10928

Nols1000 opened this issue Jan 16, 2018 · 9 comments
Labels
Android Mapbox Maps SDK for Android documentation good first issue Good for newcomers

Comments

@Nols1000
Copy link

Nols1000 commented Jan 16, 2018

Hi,

I've found that when calling setLatLng and then setZoom. The position set in setLatLng is not centered anymore. When calling setZoom first it works as expected. It would be nice if this bug could be documented in the javadoc of MapboxMap.

Platform: Android
Mapbox SDK version: 5.3.1

Steps to trigger behavior

  1. Call MapboxMap.setLatLng(position)
  2. Call MapboxMap.setZoom(zoom)

Expected behavior

The mapview shows the map in the set zoom and at the position centered.

Actual behavior

The mapview shows the map in the set zoom and at some location near the position.

PS: I've seen this issue on the OnePlus 5 and OnePlus 5t. I've also seen it on an emulator of a colleague but was not able to replicate it by myself. Also my Motorola G 2014 (titan) wasn't showing bug.

@Nols1000 Nols1000 changed the title setZoom setZoom overwrites setLatLng Jan 16, 2018
@tobrun tobrun added the Android Mapbox Maps SDK for Android label Jan 16, 2018
@tobrun tobrun added this to the android-v5.3.2 milestone Jan 16, 2018
@tobrun
Copy link
Member

tobrun commented Jan 16, 2018

I was able to reproduce this issue:

The following code will position the camera correctly above the U.S. capitol:

    mapView.getMapAsync(new OnMapReadyCallback() {
      @Override
      public void onMapReady(MapboxMap mapboxMap) {
        mapboxMap.setZoom(20);
        mapboxMap.setLatLng(new LatLng(38.889783, -77.009037));
        mapboxMap.setZoom(16);
      }
    });

While the following code places us somewhere in Ecuador:

    mapView.getMapAsync(new OnMapReadyCallback() {
      @Override
      public void onMapReady(MapboxMap mapboxMap) {
        mapboxMap.setLatLng(new LatLng(38.889783, -77.009037));
        mapboxMap.setZoom(16);
      }
    });

@tobrun
Copy link
Member

tobrun commented Jan 16, 2018

Been debugging our C++ code and I'm seeing the folllowing easeTo invocations in transform.cpp:

First code block
  • latlng 0,0 & zoom 0
  • latlng 2.6703413245275276E-312, 0 & zoom 20
  • latlng 38.889783000000001, -77.009037000000006 & zoom 2.6703613717351733E-312
  • latlng 2.711533251710973E-312, 2.7115332538898025E-312 & zoom 16
Second code block
  • latlng 0,0 & zoom 0
  • latlng 38.889783000000001,-77.009037000000006 & zoom 2.6703613717351733E-312
  • latlng 2.711533251710973E-312,2.7115332538898025E-312 & zoom 16

While the intermediate results are the same, it's not resulting in the same transformation.

Note: above latlng values equal to the camera definition passed into easeTo.

@tobrun
Copy link
Member

tobrun commented Jan 16, 2018

Looked more into the calculated values of easeTo, below you can see the start LatLng value for each easeTo invocation:

First code block
  • latlng 0,-0 & zoom 0.2360141919000848
  • latlng 0,0 & zoom 20
  • latlng -0.12793672399876976, 0.12792772061916935 & zoom 20
  • latlng 38.889783000000008, -77.009037000000035 & zoom 16
Second code block
  • latlng 0,-0 & zoom 0.2360141919000848
  • latlng 0,0 & zoom 0.2360141919000848
  • latlng 0, -77.009037000000035 & zoom 16

@tobrun
Copy link
Member

tobrun commented Jan 16, 2018

I identified that the 0 latitude value is coming from unproject in projection.hpp:

 static LatLng unproject(const Point<double>& p, double scale, LatLng::WrapMode wrapMode = LatLng::Unwrapped) {
        auto p2 = p * util::DEGREES_MAX / worldSize(scale);
        return LatLng {
            util::DEGREES_MAX / M_PI * std::atan(std::exp((util::LONGITUDE_MAX - p2.y) * util::DEG2RAD)) - 90.0,
            p2.x - util::LONGITUDE_MAX,
            wrapMode // unwrapped
        };
  }

Since this method is called multiple times, it's hard to tell where the calculation is going wrong.

FWIW when the 0 occurs: p2.y = 180

@tobrun tobrun added the Core The cross-platform C++ core, aka mbgl label Jan 16, 2018
@tobrun tobrun removed this from the android-v5.3.2 milestone Jan 23, 2018
@jfirebaugh
Copy link
Contributor

This is expected behavior; at low zoom levels, setLatLng is constrained so that the entire viewport shows map data (no empty areas at the top or bottom of the viewport).

@tobrun, can you ensure that this is mentioned in the Android documentation?

@jfirebaugh jfirebaugh added documentation and removed Core The cross-platform C++ core, aka mbgl labels Feb 12, 2018
@Nols1000
Copy link
Author

Nols1000 commented Feb 12, 2018

@jfirebaugh How is that expected behavior. If you dont change the behavior be sure you add this information at a prominent position in the javadoc and in the getting started.

@tobrun
Copy link
Member

tobrun commented Feb 13, 2018

@jfirebaugh I had a hunch this was expected behaviour. On top of adding documentation, I think exposing a method as setLatLngZoom makes sense to avoid users running into this. Thank you for 👀 on this issue.

@tobrun tobrun added this to the android-v6.0.0 milestone Feb 13, 2018
@tobrun tobrun added the good first issue Good for newcomers label Feb 13, 2018
@jfirebaugh
Copy link
Contributor

You can already set the center and zoom simultaneously with moveCamera, right?

@tobrun
Copy link
Member

tobrun commented Mar 2, 2018

The api's from OP have been removed, issue is not longer actionable.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Android Mapbox Maps SDK for Android documentation good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

3 participants