Incorrect MyLocationView transformation

[justasm]

Platform: Android
Mapbox SDK version: 4.1.0-beta.3

Steps to trigger behavior

1. Enable MyBearingTracking.COMPASS tracking mode.
2. Tilt map around x axis (i.e. pitch) using two-finger shove gesture.
3. Rotate map around z axis (i.e. bearing, aka direction) using two-finger rotate gesture.

Expected behavior

Actual behavior

Discussion

The 3D transformation calculation for MyLocationView is

1. incorrect - the compass bearing / direction rotation is applied as M' = R(degrees) * M using Matrix#postRotate(float), whereas it should be calculated as M' = M * R(degrees), e.g. by using Matrix#preRotate(float).
2. incomplete - map rotation around the z axis, i.e. map bearing / direction, is not taken into account. The compass bearing indicator is therefore incorrect when the map is not pointing north. Map rotation gestures should be propagated to MyLocationView, similar to how tilt is, and accounted for when calculating the transformation.
3. needlessly complex - it uses both the Camera and Matrix classes, whereas the entire transformation could be calculated using just Camera and applied using Camera#applyToCanvas(canvas).

[tobrun]

@justasm thank you for writing this up. I was able to reproduce the issue shown in the screenshot and patch it up by applying Matrix#preRotate(float) instead of Matrix#postRotate(float) in 0fd450e. I'm now looking into your suggestions from 2) and 3).

[tobrun]

Also noticing that when compass bearing is active and we don't have a tracking mode active. That the rotate gesture is disabled, while it should be enabled.

[tobrun]

Noticing 2 JNI related items:

• getMeterPerPixelAtLatitude is called for each onDraw which is not good for performance.
• I'm thinking about caching this value, a not 100% correct value will not have that much impact since the value is used for the accuracy circle.
• The MyLocationView is sometimes stuck in an invalidation loop. Resulting in constantly invalidating through JNI. While the map is actually not changing and is stable.

[tobrun]

The MyLocationView is sometimes stuck in an invalidation loop.

This occurred only when we receive a location update and were trying to animate the location update through an animator. For non-tracking behaviour we don't need to invalidate the map, we only need to invalidate the MyLocationView.

[tobrun]

I'm thinking about caching this value, a not 100% correct value will not have that much impact since the value is used for the accuracy circle.

I was wrong here because getMetersPerPixelAtLatitude uses current zoom level. I'm going try moving it to another place in the code than the onDraw.

[tobrun]

I fixed the getMetersPerPixelAtLatitude by calling this only when zoom levels changes. This is currently results in a jump in accuracy circle width but it's more performant now. I'm going to punt on this and revisit the whole system later on but not for the 4.1.0 release (this includes the removal of Matrix as mentioned in the OP).

[tobrun]

Here you can see the less optimal UX but with improved performance since we are no longer calling a JNI method directly in onDraw.

[tobrun]

Looking into this issue has resulted me going into the rabbit hole and finding/fixing multiple things. To limit impact/regressions for the release we are planning next week I'm going to patch up a few things and revisit this issue after release.

-> simplified version of this ticket in: #5479

[tobrun]

I'm running back at this issue, starting this off by cherry picking some ideas from #5468

[cammace]

This ticket seems to be tracking multiple issues with the location view, If they haven't been fixed yet can we ticket them out separately and close this one @tobrun?