search

mapbox / mapbox-gl-native

Interactive, thoroughly customizable maps in native Android, iOS, macOS, Node.js, and Qt applications, powered by vector tiles and OpenGL
https://mapbox.com/mobile
Other
4.36k stars 1.33k forks source link

Android Camera API #3244

Closed bleege closed 8 years ago

bleege commented 8 years ago

The next iteration of the map interaction API should be to add a Camera API. The Camera API will be modeled on the Google Maps Android API to maximize familiarity. Key classes will be:

/cc @zugaldia @incanus @twbell

bleege commented 8 years ago

Related to #2805

bleege commented 8 years ago

Basic classes, method stubs, and a TestApp activity are built. The next step is wiring up Core GL to the method stubs and verifying via TestApp activity.

bleege commented 8 years ago

The initial animateCamera() API is complete as of 08b5c1771ef1465bfd60155835411990c8e0c9b5. The final 2 animateCamera() methods to build are related integrating Callback support and setting Animation duration. Will investigate the new flyTo() API in Core GL ( #3171 ) to see if that makes sense for these methods at this time.

bleege commented 8 years ago

I refactored the CameraActivity in the TestApp to provide a visual test harness for the 3 animateCamera() methods that do the work (aka move the map) of the Camera API. The next steps will be seeing if the existing Core CL API (setCenterCoordinate(), setZoomLevel(), setTilt(new Double(), setBearing()) will be enough to implement the "move the map" part of it we'll need to wait for the flyTo methods to mature (see #3171). The original implementation done on 10-Dec-2015 in 8f6d08a54bc996fed6e5a44f338eb22252d8417a did work, but since yesterday's discovery and fix of #3259 that will need to be re-verified as there's some "null island ghosting" going on when now tested.

bleege commented 8 years ago

Confirmed that the "null island ghosting" is still happening when "Animate" button is originally pressed. This appears to be an issue of setCoordinate() getting stomped on. Will review @1ec5's flyTo fixes from last night in #3301 to see if that can address it.

device-2015-12-15-100852 First Press Of Animate Button Upon CameraActivity Being Loaded

device-2015-12-15-100912 Second (and Subsequent) Presses of Animate Button In CameraActivity (Desired State)

bleege commented 8 years ago

Just did some quick testing and was able to isolate the "ghost issue" to setZoomLevel() in animateCamera().

    public final void animateCamera (CameraUpdate update) {

        // Order Matters!  Otherwise Core GL will stomp on things
        setCenterCoordinate(update.getTarget());
        setZoomLevel(update.getZoom());
        setTilt(new Double(update.getTilt()), 0L);
        setBearing(update.getBearing());
    }
bleege commented 8 years ago

I tested @1ec5's flyTo refactored code from last night this morning and think it'll be the route we should go for building out the animateCamer() methods in the Java API. @zugaldia and I discussed this and we're agreed that we'll go this route once @1ec5's PR is merged in master we will start building using it. This should happen later today. To be clear, the animateCamera() methods are the following in MapView:

public final void animateCamera (CameraUpdate update) {}
public final void animateCamera (CameraUpdate update, MapView.CancelableCallback callback) {}
public final void animateCamera (CameraUpdate update, int durationMs, MapView.CancelableCallback callback){}
bleege commented 8 years ago

@1ec5's flyTo() fix was merged this afternoon. As of 334f08255a4e0a2cc1f4fe7a8082043c5de9e7d4 the basic JNI wiring of the method has been implemented and the the MapView's anitmateCamera() methods have been refactored to use it. Testing proved no crashes.

The next part will be generating the proper CameraOptions to pass to the Core GL's flyTo() method in jni.cpp.

bleege commented 8 years ago

Status Update: The math conversion and wiring from the Java based developer API to the C++ Core GL is mostly in. All three animateCamera() test methods render in their test harnesses as expected (animation, pitch, target location, bearing of MapView). There are only 2 more issues to resolve that I can see. The first is getting zoom to be respected by Core GL. The second is getting the Compass to line up properly when Core GL changes the bearing. Currently North and South are fine, but East and West are backwards.

bleege commented 8 years ago

So the best way to understand the current zoom issue is to see if play out in the TestApp. When the CameraActivity (just "Camera" in the Navigation Drawer) loads it starts with the MapView in it's default state high above Null Island. When tapping on the Animate button (which triggers a 0 nanosecond animation) the rendered display is as it should be (see Screenshot 1 below). When tapping the Animate button a second time (or as many times as you'd like afterwards) the rendered display is correct for everything except that the zoom level is at it's highest level (see Screenshot 2 below). In an ideal state, tapping the Animate button multiple times should always result in the Screenshot 1.

As for the Compass lining up properly, the Animate button should result in render facing East which is what it does. However, the Compass points West (red arrow is the direction that it's pointing towards).

Here's the germane code for these issues.

  1. CameraActivity's Animate button logic
  2. MapView's animateCamera()
  3. jni.cpp
  4. MapView's getDirection()

device-2015-12-16-180139 Screen Shot 1

device-2015-12-16-180740 Screen Shot 2

/cc @adam-mapbox @1ec5

bleege commented 8 years ago

I was just able to get C++ code to make it's Log messages visible in Android (aka redirect from StdOut to Android LogCat). This will make it easier to compare values as they move through the code.

The first example (see below) shows the CameraOptions values in Android's Camera API just before it sends them off to cross the JNI bridge into Core GL. This is the first line of the log below. The 4 lines below show what these values look like in jni.cpp's flyTo() after they've crossed the JNI bridge and are about to be bundled up into what Core GL needs them to be.

Analyzing the logs, while there appears to be some truncation by in large the values are what they should be.

12-17 09:21:04.893 5207-5207/com.mapbox.mapboxsdk.testapp I/MapView: flyTo() called with angle = -1.5707963267948966; target = LatLng [longitude=-88.06216, latitude=44.50128, altitude=0.0]; durationNano = 0; Pitch = 0.5235987755982988; Zoom = 14.0
12-17 09:21:04.893 5207-5207/com.mapbox.mapboxsdk.testapp I/mbgl: {Main}[JNI]: nativeFlyTo - Angle: -1.570796
12-17 09:21:04.893 5207-5207/com.mapbox.mapboxsdk.testapp I/mbgl: {Main}[JNI]: nativeFlyTo - Duration: 0
12-17 09:21:04.893 5207-5207/com.mapbox.mapboxsdk.testapp I/mbgl: {Main}[JNI]: nativeFlyTo - Pitch: 0.523599
12-17 09:21:04.893 5207-5207/com.mapbox.mapboxsdk.testapp I/mbgl: {Main}[JNI]: nativeFlyTo - Zoom: 14.000000
bleege commented 8 years ago

Just added logging to transform.cpp's flyTo() method (aka what actually does the screen animation) to verify what values were coming in and what values it decided to work with based on the map's current state and what was passed in. The 2 logs below show the results from Press 1 and Press 2 of the Animate Button that produce the screenshots documented above.

Press 1 / Screen Shot 1

12-17 09:41:48.083 10680-10680/com.mapbox.mapboxsdk.testapp I/MapView: flyTo() called with angle = -1.5707963267948966; target = LatLng [longitude=-88.06216, latitude=44.50128, altitude=0.0]; durationNano = 0; Pitch = 0.5235987755982988; Zoom = 14.0
12-17 09:41:48.093 10680-10680/com.mapbox.mapboxsdk.testapp I/mbgl: {Main}[JNI]: nativeFlyTo - Angle: -1.570796
12-17 09:41:48.093 10680-10680/com.mapbox.mapboxsdk.testapp I/mbgl: {Main}[JNI]: nativeFlyTo - Duration: 0
12-17 09:41:48.093 10680-10680/com.mapbox.mapboxsdk.testapp I/mbgl: {Main}[JNI]: nativeFlyTo - Pitch: 0.523599
12-17 09:41:48.093 10680-10680/com.mapbox.mapboxsdk.testapp I/mbgl: {Main}[JNI]: nativeFlyTo - Zoom: 14.000000
12-17 09:41:48.093 10680-10680/com.mapbox.mapboxsdk.testapp I/mbgl: {Main}[JNI]: flyTo - getZoom(): 0.126704
12-17 09:41:48.093 10680-10680/com.mapbox.mapboxsdk.testapp I/mbgl: {Main}[JNI]: flyTo - getAngle(): 0.000000
12-17 09:41:48.093 10680-10680/com.mapbox.mapboxsdk.testapp I/mbgl: {Main}[JNI]: flyTo - getPitch(): 0.000000
12-17 09:41:48.093 10680-10680/com.mapbox.mapboxsdk.testapp I/mbgl: {Main}[JNI]: flyTo - zoom: 14.000000
12-17 09:41:48.093 10680-10680/com.mapbox.mapboxsdk.testapp I/mbgl: {Main}[JNI]: flyTo - angle: -1.570796
12-17 09:41:48.093 10680-10680/com.mapbox.mapboxsdk.testapp I/mbgl: {Main}[JNI]: flyTo - pitch: 0.523599

Press 2 / Screen Shot 2

12-17 09:42:52.723 10680-10680/com.mapbox.mapboxsdk.testapp D/ViewRootImpl: ViewPostImeInputStage ACTION_DOWN
12-17 09:42:52.903 10680-10680/com.mapbox.mapboxsdk.testapp I/MapView: flyTo() called with angle = -1.5707963267948966; target = LatLng [longitude=-88.06216, latitude=44.50128, altitude=0.0]; durationNano = 0; Pitch = 0.5235987755982988; Zoom = 14.0
12-17 09:42:52.903 10680-10680/com.mapbox.mapboxsdk.testapp I/mbgl: {Main}[JNI]: nativeFlyTo - Angle: -1.570796
12-17 09:42:52.903 10680-10680/com.mapbox.mapboxsdk.testapp I/mbgl: {Main}[JNI]: nativeFlyTo - Duration: 0
12-17 09:42:52.903 10680-10680/com.mapbox.mapboxsdk.testapp I/mbgl: {Main}[JNI]: nativeFlyTo - Pitch: 0.523599
12-17 09:42:52.903 10680-10680/com.mapbox.mapboxsdk.testapp I/mbgl: {Main}[JNI]: nativeFlyTo - Zoom: 14.000000
12-17 09:42:52.903 10680-10680/com.mapbox.mapboxsdk.testapp I/mbgl: {Main}[JNI]: flyTo - getZoom(): 14.126704
12-17 09:42:52.903 10680-10680/com.mapbox.mapboxsdk.testapp I/mbgl: {Main}[JNI]: flyTo - getAngle(): -1.570796
12-17 09:42:52.903 10680-10680/com.mapbox.mapboxsdk.testapp I/mbgl: {Main}[JNI]: flyTo - getPitch(): 0.523599
12-17 09:42:52.903 10680-10680/com.mapbox.mapboxsdk.testapp I/mbgl: {Main}[JNI]: flyTo - zoom: 14.000000
12-17 09:42:52.903 10680-10680/com.mapbox.mapboxsdk.testapp I/mbgl: {Main}[JNI]: flyTo - angle: -1.570796
12-17 09:42:52.903 10680-10680/com.mapbox.mapboxsdk.testapp I/mbgl: {Main}[JNI]: flyTo - pitch: 0.523599
bleege commented 8 years ago

Both Press 1 and Press 2 logs above resulted in the same data being chosen for the flyTo() method to work as needed (this is a good thing). I've not added some more logs to get details of what the scale variables look like right before they start to do the animation. Here's what they produce:

Press 1

12-17 10:03:09.303 16537-16537/com.mapbox.mapboxsdk.testapp D/ViewRootImpl: ViewPostImeInputStage ACTION_DOWN
12-17 10:03:09.423 16537-16537/com.mapbox.mapboxsdk.testapp I/MapView: flyTo() called with angle = -1.5707963267948966; target = LatLng [longitude=-88.06216, latitude=44.50128, altitude=0.0]; durationNano = 0; Pitch = 0.5235987755982988; Zoom = 14.0
12-17 10:03:09.423 16537-16537/com.mapbox.mapboxsdk.testapp I/mbgl: {Main}[JNI]: nativeFlyTo - Angle: -1.570796
12-17 10:03:09.423 16537-16537/com.mapbox.mapboxsdk.testapp I/mbgl: {Main}[JNI]: nativeFlyTo - Duration: 0
12-17 10:03:09.423 16537-16537/com.mapbox.mapboxsdk.testapp I/mbgl: {Main}[JNI]: nativeFlyTo - Pitch: 0.523599
12-17 10:03:09.423 16537-16537/com.mapbox.mapboxsdk.testapp I/mbgl: {Main}[JNI]: nativeFlyTo - Zoom: 14.000000
12-17 10:03:09.423 16537-16537/com.mapbox.mapboxsdk.testapp I/mbgl: {Main}[JNI]: flyTo - getZoom(): 0.126704
12-17 10:03:09.423 16537-16537/com.mapbox.mapboxsdk.testapp I/mbgl: {Main}[JNI]: flyTo - getAngle(): 0.000000
12-17 10:03:09.423 16537-16537/com.mapbox.mapboxsdk.testapp I/mbgl: {Main}[JNI]: flyTo - getPitch(): 0.000000
12-17 10:03:09.423 16537-16537/com.mapbox.mapboxsdk.testapp I/mbgl: {Main}[JNI]: flyTo - zoom: 14.000000
12-17 10:03:09.423 16537-16537/com.mapbox.mapboxsdk.testapp I/mbgl: {Main}[JNI]: flyTo - angle: -1.570796
12-17 10:03:09.423 16537-16537/com.mapbox.mapboxsdk.testapp I/mbgl: {Main}[JNI]: flyTo - pitch: 0.523599
12-17 10:03:09.423 16537-16537/com.mapbox.mapboxsdk.testapp I/mbgl: {Main}[JNI]: flyTo - new_scale: 16384.000000
12-17 10:03:09.423 16537-16537/com.mapbox.mapboxsdk.testapp I/mbgl: {Main}[JNI]: flyTo - scaled_tile_size: 8388608.000000
12-17 10:03:09.423 16537-16537/com.mapbox.mapboxsdk.testapp I/mbgl: {Main}[JNI]: flyTo - state.Bc: 23301.688889
12-17 10:03:09.423 16537-16537/com.mapbox.mapboxsdk.testapp I/mbgl: {Main}[JNI]: flyTo - state.Cc: 1335088.428860
12-17 10:03:09.423 16537-16537/com.mapbox.mapboxsdk.testapp I/mbgl: {Main}[JNI]: flyTo - startZ: 0.126704

Press 2

12-17 10:04:07.213 16537-16537/com.mapbox.mapboxsdk.testapp D/ViewRootImpl: ViewPostImeInputStage ACTION_DOWN
12-17 10:04:07.383 16537-16537/com.mapbox.mapboxsdk.testapp I/MapView: flyTo() called with angle = -1.5707963267948966; target = LatLng [longitude=-88.06216, latitude=44.50128, altitude=0.0]; durationNano = 0; Pitch = 0.5235987755982988; Zoom = 14.0
12-17 10:04:07.383 16537-16537/com.mapbox.mapboxsdk.testapp I/mbgl: {Main}[JNI]: nativeFlyTo - Angle: -1.570796
12-17 10:04:07.383 16537-16537/com.mapbox.mapboxsdk.testapp I/mbgl: {Main}[JNI]: nativeFlyTo - Duration: 0
12-17 10:04:07.383 16537-16537/com.mapbox.mapboxsdk.testapp I/mbgl: {Main}[JNI]: nativeFlyTo - Pitch: 0.523599
12-17 10:04:07.383 16537-16537/com.mapbox.mapboxsdk.testapp I/mbgl: {Main}[JNI]: nativeFlyTo - Zoom: 14.000000
12-17 10:04:07.383 16537-16537/com.mapbox.mapboxsdk.testapp I/mbgl: {Main}[JNI]: flyTo - getZoom(): 14.126704
12-17 10:04:07.383 16537-16537/com.mapbox.mapboxsdk.testapp I/mbgl: {Main}[JNI]: flyTo - getAngle(): -1.570796
12-17 10:04:07.383 16537-16537/com.mapbox.mapboxsdk.testapp I/mbgl: {Main}[JNI]: flyTo - getPitch(): 0.523599
12-17 10:04:07.383 16537-16537/com.mapbox.mapboxsdk.testapp I/mbgl: {Main}[JNI]: flyTo - zoom: 14.000000
12-17 10:04:07.383 16537-16537/com.mapbox.mapboxsdk.testapp I/mbgl: {Main}[JNI]: flyTo - angle: -1.570796
12-17 10:04:07.383 16537-16537/com.mapbox.mapboxsdk.testapp I/mbgl: {Main}[JNI]: flyTo - pitch: 0.523599
12-17 10:04:07.383 16537-16537/com.mapbox.mapboxsdk.testapp I/mbgl: {Main}[JNI]: flyTo - new_scale: 16384.000000
12-17 10:04:07.383 16537-16537/com.mapbox.mapboxsdk.testapp I/mbgl: {Main}[JNI]: flyTo - scaled_tile_size: 8388608.000000
12-17 10:04:07.383 16537-16537/com.mapbox.mapboxsdk.testapp I/mbgl: {Main}[JNI]: flyTo - state.Bc: 23301.688889
12-17 10:04:07.383 16537-16537/com.mapbox.mapboxsdk.testapp I/mbgl: {Main}[JNI]: flyTo - state.Cc: 1335088.428860
12-17 10:04:07.383 16537-16537/com.mapbox.mapboxsdk.testapp I/mbgl: {Main}[JNI]: flyTo - startZ: 14.126704
bleege commented 8 years ago

The scale logs previously look like they're supposed to (AFAICT) as the startZ (starting zoom) should originally be 0 (where the camera starts high above Null Island) in Press 1, while in Press 2 it's around 14 which makes sense because the Press 1's data told it to zoom into Zoom 14 and it did.

This leads me to believe that the "end zoom" calculation may be incorrect. Only one way to find out. Time for some more logs! :smile:

bleege commented 8 years ago

The results of the newly added logs for desiredZoom, desiredScale, and state.scale are below. At first glance this is looking promising as they should be the same (AFAICT) between both Press 1 and Press 2 but they clearly are not. Time to look for the Smoking :gun:

Press 1

12-17 10:15:03.493 22652-22652/com.mapbox.mapboxsdk.testapp D/ViewRootImpl: ViewPostImeInputStage ACTION_DOWN
12-17 10:15:03.563 22652-22652/com.mapbox.mapboxsdk.testapp I/MapView: flyTo() called with angle = -1.5707963267948966; target = LatLng [longitude=-88.06216, latitude=44.50128, altitude=0.0]; durationNano = 0; Pitch = 0.5235987755982988; Zoom = 14.0
12-17 10:15:03.563 22652-22652/com.mapbox.mapboxsdk.testapp I/mbgl: {Main}[JNI]: nativeFlyTo - Angle: -1.570796
12-17 10:15:03.563 22652-22652/com.mapbox.mapboxsdk.testapp I/mbgl: {Main}[JNI]: nativeFlyTo - Duration: 0
12-17 10:15:03.563 22652-22652/com.mapbox.mapboxsdk.testapp I/mbgl: {Main}[JNI]: nativeFlyTo - Pitch: 0.523599
12-17 10:15:03.563 22652-22652/com.mapbox.mapboxsdk.testapp I/mbgl: {Main}[JNI]: nativeFlyTo - Zoom: 14.000000
12-17 10:15:03.563 22652-22652/com.mapbox.mapboxsdk.testapp I/mbgl: {Main}[JNI]: flyTo - getZoom(): 0.126704
12-17 10:15:03.563 22652-22652/com.mapbox.mapboxsdk.testapp I/mbgl: {Main}[JNI]: flyTo - getAngle(): 0.000000
12-17 10:15:03.573 22652-22652/com.mapbox.mapboxsdk.testapp I/mbgl: {Main}[JNI]: flyTo - getPitch(): 0.000000
12-17 10:15:03.573 22652-22652/com.mapbox.mapboxsdk.testapp I/mbgl: {Main}[JNI]: flyTo - zoom: 14.000000
12-17 10:15:03.573 22652-22652/com.mapbox.mapboxsdk.testapp I/mbgl: {Main}[JNI]: flyTo - angle: -1.570796
12-17 10:15:03.573 22652-22652/com.mapbox.mapboxsdk.testapp I/mbgl: {Main}[JNI]: flyTo - pitch: 0.523599
12-17 10:15:03.573 22652-22652/com.mapbox.mapboxsdk.testapp I/mbgl: {Main}[JNI]: flyTo - new_scale: 16384.000000
12-17 10:15:03.573 22652-22652/com.mapbox.mapboxsdk.testapp I/mbgl: {Main}[JNI]: flyTo - scaled_tile_size: 8388608.000000
12-17 10:15:03.573 22652-22652/com.mapbox.mapboxsdk.testapp I/mbgl: {Main}[JNI]: flyTo - state.Bc: 23301.688889
12-17 10:15:03.573 22652-22652/com.mapbox.mapboxsdk.testapp I/mbgl: {Main}[JNI]: flyTo - state.Cc: 1335088.428860
12-17 10:15:03.573 22652-22652/com.mapbox.mapboxsdk.testapp I/mbgl: {Main}[JNI]: flyTo - startZ: 0.126704
12-17 10:15:03.573 22652-22652/com.mapbox.mapboxsdk.testapp I/mbgl: {Main}[JNI]: flyTo - desiredZoom: 14.126704
12-17 10:15:03.573 22652-22652/com.mapbox.mapboxsdk.testapp I/mbgl: {Main}[JNI]: flyTo - desiredScale: 17888.000010
12-17 10:15:03.573 22652-22652/com.mapbox.mapboxsdk.testapp I/mbgl: {Main}[JNI]: flyTo - state.scale: 17888.000010

Press 2

12-17 10:16:11.913 22652-22652/com.mapbox.mapboxsdk.testapp D/ViewRootImpl: ViewPostImeInputStage ACTION_DOWN
12-17 10:16:12.023 22652-22652/com.mapbox.mapboxsdk.testapp I/MapView: flyTo() called with angle = -1.5707963267948966; target = LatLng [longitude=-88.06216, latitude=44.50128, altitude=0.0]; durationNano = 0; Pitch = 0.5235987755982988; Zoom = 14.0
12-17 10:16:12.023 22652-22652/com.mapbox.mapboxsdk.testapp I/mbgl: {Main}[JNI]: nativeFlyTo - Angle: -1.570796
12-17 10:16:12.023 22652-22652/com.mapbox.mapboxsdk.testapp I/mbgl: {Main}[JNI]: nativeFlyTo - Duration: 0
12-17 10:16:12.023 22652-22652/com.mapbox.mapboxsdk.testapp I/mbgl: {Main}[JNI]: nativeFlyTo - Pitch: 0.523599
12-17 10:16:12.023 22652-22652/com.mapbox.mapboxsdk.testapp I/mbgl: {Main}[JNI]: nativeFlyTo - Zoom: 14.000000
12-17 10:16:12.023 22652-22652/com.mapbox.mapboxsdk.testapp I/mbgl: {Main}[JNI]: flyTo - getZoom(): 14.126704
12-17 10:16:12.023 22652-22652/com.mapbox.mapboxsdk.testapp I/mbgl: {Main}[JNI]: flyTo - getAngle(): -1.570796
12-17 10:16:12.023 22652-22652/com.mapbox.mapboxsdk.testapp I/mbgl: {Main}[JNI]: flyTo - getPitch(): 0.523599
12-17 10:16:12.023 22652-22652/com.mapbox.mapboxsdk.testapp I/mbgl: {Main}[JNI]: flyTo - zoom: 14.000000
12-17 10:16:12.023 22652-22652/com.mapbox.mapboxsdk.testapp I/mbgl: {Main}[JNI]: flyTo - angle: -1.570796
12-17 10:16:12.023 22652-22652/com.mapbox.mapboxsdk.testapp I/mbgl: {Main}[JNI]: flyTo - pitch: 0.523599
12-17 10:16:12.023 22652-22652/com.mapbox.mapboxsdk.testapp I/mbgl: {Main}[JNI]: flyTo - new_scale: 16384.000000
12-17 10:16:12.023 22652-22652/com.mapbox.mapboxsdk.testapp I/mbgl: {Main}[JNI]: flyTo - scaled_tile_size: 8388608.000000
12-17 10:16:12.023 22652-22652/com.mapbox.mapboxsdk.testapp I/mbgl: {Main}[JNI]: flyTo - state.Bc: 23301.688889
12-17 10:16:12.033 22652-22652/com.mapbox.mapboxsdk.testapp I/mbgl: {Main}[JNI]: flyTo - state.Cc: 1335088.428860
12-17 10:16:12.033 22652-22652/com.mapbox.mapboxsdk.testapp I/mbgl: {Main}[JNI]: flyTo - startZ: 14.126704
12-17 10:16:12.033 22652-22652/com.mapbox.mapboxsdk.testapp I/mbgl: {Main}[JNI]: flyTo - desiredZoom: 28.126704
12-17 10:16:12.033 22652-22652/com.mapbox.mapboxsdk.testapp I/mbgl: {Main}[JNI]: flyTo - desiredScale: 293076992.323449
12-17 10:16:12.033 22652-22652/com.mapbox.mapboxsdk.testapp I/mbgl: {Main}[JNI]: flyTo - state.scale: 262144.000000
bleege commented 8 years ago

I noticed the zoom issue had to do with the the startZ being used to calculate the desiredZoom value for the animation process. When 0 length animation was being calculated it always took into account the current zoom level which would skew the desiredZoom to be higher than what it should be. Conversely, this is exactly what's needed when doing a longer than 0 length animation as the Camera needs to first "pull back" from it's current view before it can then "fly to" the target destination. To solve this I added a duration check so that the desiredZoomStart would not factor in for non 0 length animated "fly to" operations, but would still be used for greater than 0 length animated "fly to" operations. Admittedly, this may be only treating the symptom for the zoom issue, but so far this has produced solid results.

Would love :eyes: from @1ec5 @jfirebaugh on this.

1ec5 commented 8 years ago

Interesting, most likely a zero-second duration means the frame function is called once at t=0 but not at t=1. The iOS SDK never calls Transform::flyTo() with a zero-second duration; instead, if a duration is non-positive, it allows flyTo() to use the default duration based on the internal curve and speed parameters (which I’d like to get rid of, but that’s a different story). With a zero-second duration, you really should be calling Map::jumpTo() anyways. Calling Transform::easeTo() with a zero-second duration works because of a special case that synchronously jumps to the final state. I suppose Transform::flyTo() could detect this situation and call _easeTo() instead.

bleege commented 8 years ago

With a zero-second duration, you really should be calling Map::jumpTo() anyways. Calling Transform::easeTo() with a zero-second duration works because of a special case that synchronously jumps to the final state.

Ah ha! That sounds like a much better idea to me. flyTo should be all about flying from Point A to Point B over time. Let me do some refactoring and put the zero-second duration checks higher up the food chain so that it'll route to jumpTo for those cases and flyTo for the others. Then I can remove the duration checking "fix" from flyTo().

Thanks @1ec5!

bleege commented 8 years ago

Thanks to @1ec5's insight I just wired up both jumpTo() and easeTo() from Core GL to Android. I also added logic to the MapView.animateCamera() API for zero length duration animateCamera() calls to use jumpTo() instead of flyTo(). When tested all three test cases in the TestApp's CameraActivity worked as expected with regards to zoom.

Next up is to resolve the Compass issue so that it points in the proper directions after a jumpTo(), easeTo(), and flyTo() call is made.

zugaldia commented 8 years ago

Oh yaay. Great job @bleege.

bleege commented 8 years ago

According to GL JS, bearing or angle (your choice for term) is supposed to be Map rotation bearing in degrees counter-clockwise from north. Based on past experiences ^^, I already know that Core GL's CameraOptions.angle is in Radians. Now the trick will be to figure out it's range too.

bleege commented 8 years ago

I added some basic logging to help get to the bottom of the Bearing / Angle misalignment. I then ran the Animate button test (aka the same Screen Shot / Press 1 from above) as it's known to set the Camera to look East (which it does). The Compass shows West however (see screenshot). The following logs are produced:

12-17 14:25:15.623 26091-26091/com.mapbox.mapboxsdk.testapp I/MapView: CameraOptions.getBearing() == 90.0; angle = -1.5707963267948966
12-17 14:25:15.623 26091-26091/com.mapbox.mapboxsdk.testapp I/MapView: jumpTo() called with angle = -1.5707963267948966; target = LatLng [longitude=-88.06216, latitude=44.50128, altitude=0.0]; durationNano = 0; Pitch = 0.5235987755982988; Zoom = 14.0
12-17 14:25:15.673 26091-26091/com.mapbox.mapboxsdk.testapp I/MapView: direction = -90.0
12-17 14:25:15.673 26091-26091/com.mapbox.mapboxsdk.testapp I/MapView: final direction = 270.0
12-17 14:25:15.783 26091-26091/com.mapbox.mapboxsdk.testapp I/MapView: direction = -90.0
12-17 14:25:15.783 26091-26091/com.mapbox.mapboxsdk.testapp I/MapView: final direction = 270.0
12-17 14:25:15.813 26091-26091/com.mapbox.mapboxsdk.testapp I/MapView: direction = -90.0
12-17 14:25:15.813 26091-26091/com.mapbox.mapboxsdk.testapp I/MapView: final direction = 270.0
12-17 14:25:15.843 26091-26091/com.mapbox.mapboxsdk.testapp I/MapView: direction = -90.0
12-17 14:25:15.843 26091-26091/com.mapbox.mapboxsdk.testapp I/MapView: final direction = 270.0
12-17 14:25:15.883 26091-26091/com.mapbox.mapboxsdk.testapp I/MapView: direction = -90.0
12-17 14:25:15.883 26091-26091/com.mapbox.mapboxsdk.testapp I/MapView: final direction = 270.0
12-17 14:25:15.913 26091-26091/com.mapbox.mapboxsdk.testapp I/MapView: direction = -90.0
12-17 14:25:15.913 26091-26091/com.mapbox.mapboxsdk.testapp I/MapView: final direction = 270.0
12-17 14:25:15.943 26091-26091/com.mapbox.mapboxsdk.testapp I/MapView: direction = -90.0
12-17 14:25:15.943 26091-26091/com.mapbox.mapboxsdk.testapp I/MapView: final direction = 270.0

device-2015-12-17-142816

bleege commented 8 years ago

A pretty obvious first step (solution?) is the negated getBearing() in MapView.getDirection() which is what the CompassView uses to know which way to point. However, there a few other things that ALSO use this same method currently AND appear to be working correctly (see screenshot below plus Gesture Support). In other words, the concern is about fixing this one issue while breaking others.

screen shot 2015-12-17 at 2 36 50 pm Other Uses Of MapView.getDirection()

bleege commented 8 years ago

Ok, thanks to @friedbunny for straightening me out it turns out that the Compass is actually working as expected. It's job in life (as it is in the iOS SDK) is to always point North no matter what the Bearing is. In this case that means the the "red arrow" part always need to be pointing North. This is not as obvious as the iOS compass has a N in it's icon which drives home the point. Needless to say, my bad on this.

Ok, so the next part on this is to clean up the debug logs, investigate how to handle the callbacks, and test out use cases where not all CameraOptions are set (ie, bearing, tilt, and zoom) behave. Once these are done this iteration of the Camera API should be complete.

1ec5 commented 8 years ago

I already know that Core GL's CameraOptions.angle is in Radians. Now the trick will be to figure out it's range too.

CameraOptions.angle is in radians counter-clockwise from north. The value is wrapped to [−π rad, π rad), so you don’t have to worry about the allowable input range.

bleege commented 8 years ago

Discussed options for implementing Callbacks for animateCamera() with @zugaldia this afternoon. The best / only option available right now would be to listen for MapChange.REGION_DID_CHANGE_ANIMATED event and then fire off the Callback methods. I'll investigate using that, but if it doesn't work out we'll punt on Callbacks in the API for now.

bleege commented 8 years ago

Was able to get Callbacks working via OnMapChangedListener as previously hypothesized. Successfully tested (see screenshot below).

All that remains is to test the random / incomplete CameraOptions.

callback-test

bleege commented 8 years ago

Added support for optional values of bearing, tilt, and zoom in CameraOptions. Updated CameraActivity in TestApp and successfully tested.

All in all, things are looking good here. @1ec5 is working on some improvements to the flyTo() algorithm in #3296 that will improve performance (hopefully help with respect zoom issue). I'll want to look at this and the rest of the code again with some fresh eyes (including having @zugaldia give it a spin) before signing off on it and merging.

tobrun commented 8 years ago

@bleege Awesome progress on the feature + demonstration how to properly communicate.

Cheering from the sidelines \o/

bleege commented 8 years ago

Per chat, @zugaldia kicked the tires this morning on the branch and is happy with things. The only tweaks that we'll make are adding @UIThread annotations to the animateCamera() methods to remind everyone that these should only be used on the UI Thread as per regular Android UI conventions. As part of this we can also remove the View.post(new Runnable(){}) forced UI Thread wrapping of Callback firing command since everything will be on the UIThread.

Once these tweaks are made and tested that'll wrap up Iteration 0 on the Camera API and will then merge into master.

/cc @twbell

bleege commented 8 years ago

Thanks to @jfirebaugh's help with git rebasing the Camera API is now merged into master for everyone to enjoy!

mb12 commented 8 years ago

@bleege Is two finger drag gesture to tilt also available?

bleege commented 8 years ago

Is two finger drag gesture to tilt also available?

@mb12 Yep. That's been in there since 2.3.0.

KidusMT commented 6 years ago

My Camera Animation is not working on API 23 and higher...but works fine on lower...Have this issue been solved? Please help! here is my code...

// Marker
        mapboxMap.addMarker(new MarkerOptions()
                .position(new LatLng(latitude, longitude))
                .icon(icon));

        //41.900581, 12.487007 - Italy Rome
        CameraPosition cameraPosition = new CameraPosition.Builder()
                .target(new LatLng(latitude, longitude)) // Sets the new camera position
                .zoom(17) // Sets the zoom
                .bearing(180) // Rotate the camera
                .tilt(30) // Set the camera tilt
                .build(); // Creates a CameraPosition from the builder

        mapboxMap.animateCamera(CameraUpdateFactory
                .newCameraPosition(cameraPosition), 7000);

here is how it's been called in the onMapReady() method

@Override
    public void onMapReady(MapboxMap mapboxMap) {
        map = mapboxMap;

        mapboxMap.setOnMarkerClickListener(this);

        //Controlling the components on the mapBox
        mapboxMap.getUiSettings().setZoomControlsEnabled(false);//hide zoom control button
        mapboxMap.getUiSettings().setCompassEnabled(false);//hide compass
        mapboxMap.getUiSettings().setZoomGesturesEnabled(true);
        mapboxMap.getUiSettings().setScrollGesturesEnabled(true);
        mapboxMap.getUiSettings().setAllGesturesEnabled(true);

        // Create an Icon object for the marker to use
        Icon icon = IconFactory.getInstance(MapActivity.this).fromResource(R.drawable.marker);

        //This is the part where we check for the whether the permissions that are needed for this
        //feature is allowed or not.
        if (ActivityCompat.checkSelfPermission(this, Manifest.permission.ACCESS_FINE_LOCATION)
                != PackageManager.PERMISSION_GRANTED && ActivityCompat.checkSelfPermission(
                this, Manifest.permission.ACCESS_COARSE_LOCATION) != PackageManager.PERMISSION_GRANTED) {
            return;
        }

        //Animate the camera and locate the marker
        updateMap(41.900581, 12.487007, mapboxMap, icon);
    }
LukasPaczos commented 6 years ago

@KidusMT What SDK version are you using? Could you check if there are no exceptions printed in your Logcat after the map is loaded?

KidusMT commented 6 years ago

@LukasPaczos For some reason which I am not sure why but it worked like this now

 @Override
    protected void onCreate(Bundle savedInstanceState) {
        super.onCreate(savedInstanceState);
        // Mapbox access token is configured here. This needs to be called either in your application

        // object or in the same activity which contains the mapview.
        Mapbox.getInstance(this, BuildConfig.MAPBOX_API_KEY);

        // This contains the MapView in XML and needs to be called after the access token is configured.
        setContentView(R.layout.activity_map);

        mapView = findViewById(R.id.mapView);

        mapView.onCreate(savedInstanceState);
        mapView.getMapAsync(mapboxMap -> {

            //Controlling the components on the mapBox
            mapboxMap.getUiSettings().setZoomControlsEnabled(false);//hide zoom control button
            mapboxMap.getUiSettings().setCompassEnabled(false);//hide compass
            mapboxMap.getUiSettings().setZoomGesturesEnabled(true);
            mapboxMap.getUiSettings().setScrollGesturesEnabled(true);
            mapboxMap.getUiSettings().setAllGesturesEnabled(true);

            mapboxMap.setOnMarkerClickListener(this);

            // Create an Icon object for the marker to use
            Icon icon = IconFactory.getInstance(MapActivity.this).fromResource(R.drawable.marker);

            updateMap(41.900581, 12.487007, mapboxMap, icon);

            //Don't forget the setCoordinate chain method is reverse while taking arguments NOTE that!

            MapboxGeocoder client = new MapboxGeocoder.Builder()
                    .setAccessToken(BuildConfig.MAPBOX_API_KEY)
                    .setCoordinates(12.487007, 41.900581)
                    .setType(GeocoderCriteria.TYPE_POI)
                    .build();

            client.enqueue(new Callback<GeocoderResponse>() {

                @Override
                public void onResponse(Response<GeocoderResponse> response, Retrofit retrofit) {
                    if (response.isSuccess()) {
                        if (response.body().getFeatures().size() > 0) {
                            placeName = response.body().getFeatures().get(0).getText();
                            String tempLocation = response.body().getFeatures().get(0).getPlaceName();
                            if (tempLocation.contains(",")) {
                                String[] places = TextUtils.split(response.body().getFeatures().get(0).getPlaceName(), ",");
                                if (places.length > 1) {
                                    address = places[1];
                                }
                            }
                        }

                    } else {
                        //TODO add something later
                    }
                }

                @Override
                public void onFailure(Throwable t) {
                }
            });

            //TODO IMPROVE THE INFO WINDOW

            mapboxMap.setInfoWindowAdapter(marker1 -> {

                View v = getLayoutInflater().inflate(R.layout.layout_callout, null);
                TextView title = v.findViewById(R.id.title);
                TextView logo = v.findViewById(R.id.logoView);
                TextView subTitle = v.findViewById(R.id.sub_title);
                if (!placeName.isEmpty()) {
                    title.setText(placeName);
                    logo.setText(placeName.substring(0, 1));
                }

                subTitle.setText(address);
                return v;

            });

            mapboxMap.setOnInfoWindowClickListener(marker -> {
                startActivity(new Intent(MapActivity.this, DetailActivity.class));
                return true;
            });

        });

    }

    private void updateMap(double latitude, double longitude, MapboxMap mapboxMap, Icon icon) {

        // Marker
        mapboxMap.addMarker(new MarkerOptions()
                .position(new LatLng(latitude, longitude))
                .icon(icon));

        //41.900581, 12.487007 - Italy Rome
        CameraPosition cameraPosition = new CameraPosition.Builder()
                .target(new LatLng(latitude, longitude)) // Sets the new camera position
                .zoom(17) // Sets the zoom
                .bearing(180) // Rotate the camera
                .tilt(30) // Set the camera tilt
                .build(); // Creates a CameraPosition from the builder

        mapboxMap.animateCamera(CameraUpdateFactory
                .newCameraPosition(cameraPosition), 7000);

    }

this is my SDK version defined in the build config

ext.configuration = [   
        compileSdkVersion: 27,
        targetSdkVersion : 27,
        buildToolsVersion: "27.0.3"
]

now its working on all API level I am targeting to work the app for.