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

Backport setBearing with new Camera api #3397

Closed
tobrun opened this issue Dec 23, 2015 · 18 comments
Closed

Backport setBearing with new Camera api #3397

tobrun opened this issue Dec 23, 2015 · 18 comments
Assignees
Labels
Android Mapbox Maps SDK for Android

Comments

@tobrun
Copy link
Member

tobrun commented Dec 23, 2015

This type of backport for setCenterCoordinate will land in #3396.
We need to backport equivalent for setBearing.
This is needed to fix issue #2049.

@tobrun tobrun added the Android Mapbox Maps SDK for Android label Dec 23, 2015
@tobrun tobrun added this to the android-v3.1.0 milestone Dec 23, 2015
@tobrun tobrun added the P1 label Dec 23, 2015
@tobrun
Copy link
Member Author

tobrun commented Dec 23, 2015

The same fix for setCenterCoordinate is not working.
The map is not updating the bearing of the map,
I validated that the values from the sensors are correct.
The code used is working in a separate activity.

@tobrun
Copy link
Member Author

tobrun commented Dec 23, 2015

Figured out that the new camera api doesn't like negative degrees for usage in bearing.
That was the reason for not updating the bearing of the map.

@tobrun
Copy link
Member Author

tobrun commented Dec 23, 2015

Here you can see the problem.
The rotation update is not smooth. It happens instantly, same as non-camera api implementation.
Strange enough the code for updating bearing works in a separate activity.
Maybe a cancelTransition causes the animation to end sooner as expected.

@tobrun
Copy link
Member Author

tobrun commented Dec 23, 2015

Going to add log messages in related cpp code

#include <mbgl/platform/log.hpp>

and

mbgl::Log::Error(mbgl::Event::JNI, "canceltransitions in transform.cpp", "canceltransitions");

and see if a cancel interrupts our animation.

@tobrun
Copy link
Member Author

tobrun commented Dec 25, 2015

I have added log messages to multiple methods in transform.cpp.
Now we can easily see if any other method is pre-empting the animation.
Also I'm building with BUILDTYPE=Debug make android-lib-x86 to actually see the log output.

@tobrun
Copy link
Member Author

tobrun commented Dec 25, 2015

This results in following output.

I'm noticing the following things:

  • UserLocationView is triggering too much updates
  • the difference between two flyTo calls is less then 0.200 seconds, which indicates that de animation is preempt to execute the next one.

Need to investigate/solve why the UserLocationView is doing too much work.

@tobrun
Copy link
Member Author

tobrun commented Jan 4, 2016

Investigating the relation of flyTo calls with updates coming from sensor data results in following ouput.

The update sensor data is marked as UPDATE COMPASSVALUE. This call is followed by a nativeSetBearing and multiple blocks containing flyTo + cancelTransitions.

The cause is not coming from the sensor data. Will now look into the update/redraw menthods.

@tobrun
Copy link
Member Author

tobrun commented Jan 4, 2016

updateOnNextFrame log doesn't occur in the log trace.
invalidate log occurs multiple times as seen in following output and seems to be the cause for the multiple NativeMapView.invalidate()

It seems plausible that invalidate could be the cause, will test this out.

@tobrun
Copy link
Member Author

tobrun commented Jan 4, 2016

This is the output without invalidate, which seems the same as above.
Will try to catch output coming from a 'working' implementation.

@tobrun
Copy link
Member Author

tobrun commented Jan 4, 2016

This is the output of an animated setBearing.

Some things I'm noticing when comparing logs:

  • only one startTransition
  • same rendersyncs calls
  • no flyTo calls

The flyTo calls will probably come from a location update. Ideally we would like to update both location and bearing at the same time with the same method. First need to investigate if that is the actual cause.

@tobrun
Copy link
Member Author

tobrun commented Jan 4, 2016

This output indicates that the problem is originating from calling setCenterCoordinate.

This indication is confirmed by commenting out setCenterCoordinate,
you can see result here.

I believe the way to resolve this is to update the location and bearing in the NativeMapView in one place by using the new Camera API.

@tobrun
Copy link
Member Author

tobrun commented Jan 4, 2016

I was able to refactor old way of updating (setCenterCoordinate and setBearing)
to use the new Camera API. Current progress on this can be seen here.
We are on track closing this issue down!

@tobrun
Copy link
Member Author

tobrun commented Jan 4, 2016

Just noticed that the Camera API doesn't like negative degree values.
Will need to convert them before feeding them in the API.

@tobrun
Copy link
Member Author

tobrun commented Jan 4, 2016

Currently working on streamlining updates (work similar to #3398)

@tobrun
Copy link
Member Author

tobrun commented Jan 4, 2016

To tackle #3398, #3425, #3078 and #3189 I'm going to rewrite UserLocationView.
Currently thinking about dividing current implementation into 2 separate views (based on Tracking mode). This might also be the ideal way to start work for #3276.

For now I'm going to cleanup the code and PR current implementation,
the main gist behind this issue is to get setBearing to animate.
The requirements mentioned above will be tackled in other issues.

@tobrun
Copy link
Member Author

tobrun commented Jan 4, 2016

Been squashing some bugs before the actual PR.
One issue I wanted to highlight is the regression of View animations when location tracking is disabled.
Both bearing as location updates are happening instantly.

@tobrun
Copy link
Member Author

tobrun commented Jan 5, 2016

Previous mentioned issue is resolved, only remaining thing I'm currently not 💯 about is the direction updates coming from the Android sensors.. We are far from the same UX google is having in their Google Maps app.

@tobrun
Copy link
Member Author

tobrun commented Jan 5, 2016

I have been researching how Google is handling their sensor data.. currently without any results. I'm going to finish up current progress. It seems that Google is doing things differently than their examples in aosp or code samples found online.

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

No branches or pull requests

2 participants