Fix bearing of the puck being reset on settings change.

[yunikkk]

Summary of changes

The fix resolves the issue with puck bearing, that can jump to zero on when location component settings are changed. The wrong animation looks in the following way :

puck-bearing-jumping.mp4

There are two issues causing such behaviour.
First - we add layer on the map before applying current bearing value. Renderer, which is running in parallel on the background thread, may (and sometimes does) actually run rendering before bearing is applied, thus jumping to the default (0) bearing.
Second - bearing-transition property is not set and defaults to animating during 300 milliseconds. I'm not actually sure about this one since after the first fix it seems not necessary, but iOS for example resets this to zero https://github.com/mapbox/mapbox-maps-ios/blob/9f0e9605b081e45dce808a53f70b5eb0747858ab/Sources/MapboxMaps/Location/Puck/Puck2D.swift#L129. Seems it is some sort of outdated bearing animation in GL native that is not used since we do animations on platforms directly. @mapbox/maps-android please advice here

Pull request checklist:

• Briefly describe the changes in this PR.
• Include before/after visuals or gifs if this PR includes visual changes.
• Write tests for all new functionality. If tests were not written, please explain why.
• Optimize code for java consumption (@JvmOverloads, @file:JvmName, etc).
• Add example if relevant.
• Document any changes to public APIs.
• Apply changelog label ('breaking change', 'bug 🪲', 'build', 'docs', 'feature 🍏', 'performance ⚡', 'testing 💯') or use the label 'skip changelog'
• Add an entry inside this element for inclusion in the mapbox-maps-android changelog: <changelog></changelog>.
• If this PR is a v10.[version] release branch fix / enhancement, merge it to main firstly and then port to v10.[version] release branch.

PRs must be submitted under the terms of our Contributor License Agreement CLA.

[kiryldz]

so this should be called after updateCurrentBearing?

[yunikkk]

Exactly. Otherwise renderer may run render pass before the updateCurrentBearing will be called thus will use incorrect default zero bearing.
The call sequence becomes :

main thread : locationLayerRenderer.addLayers -> layers are added
render thread : render pass -> no bearing value is set, resets to default
main thread : updateCurrentBearing -> will be applied on the next render pass

[Chaoba]

This block will only be executed during init but in your screen recording, it's a long-lasting period. Wondering whether this is the root cause for it.

[yunikkk]

Init is called every time location settings are changed. For the recording I've triggered location component settings change every 700 milliseconds, e.g. in the LocationComponentActivity I've added

    val runnable = object : Runnable {
      override fun run() {
        binding.mapView.location.pulsingMaxRadius = 1f

        binding.mapView.postDelayed(this, 700L)
      }
    }

    binding.mapView.post(runnable)

[Chaoba]

Then it makes sense.

[yunikkk]

This is actually the change I'm not 100% sure about, though it seems that iOS does the same and it disable core bearing animations that otherwise are applied.
Waiting for the comments here

[kiryldz]

@alexshalamov perhaps you could comment here?

[alexshalamov]

Location indicator layer properties are transitionable. Bearing and position is interpolated if user sets new value. If interpolation will be always done on SDK level, we could consider making these properties non-transitionable.

[yunikkk]

@macdrevx could you please share how is it used on iOS https://github.com/mapbox/mapbox-maps-ios/blob/9f0e9605b081e45dce808a53f70b5eb0747858ab/Sources/MapboxMaps/Location/Puck/Puck2D.swift#L116 ?

[macdrevx]

We disable transitions on iOS.

[yunikkk]

@macdrevx is the user able to enable it though? Seems there's a public property for that https://github.com/mapbox/mapbox-maps-ios/blob/c95b14f16c9462f1a1241245ad6abd4d51d3b4f3/Sources/MapboxMaps/Style/Generated/Layers/LocationIndicatorLayer.swift#L94.

[pengdev]

the user able to enable it though? Seems there's a public property for that

@yunikkk the location component is built on top of the low level style APIs, so yes, user could overwrite whatever style properties in the runtime if they want to, however I think that's out of the scope of the LocationComponentPlugin.

Currently the bearing of the location indicator layer is driven by the platform animation system, and the gl-native transition should be set to 0, and I believe the default transition should be 0 already?

[pengdev]

Update: seems the default transition is not 0, as mentioned in the PR description, which is 300 milliseconds in duration. So we should set it specifically to 0.

[kiryldz]

Tests are still failing (as well as couple of other bots).

[kiryldz]

any comments what happens below?

[yunikkk]

Well since we can't compare the whole string (it contains formatted double values that may be slightly different - we compare start and end of the strings, then parse double values and compare relative to ULP (smallest possible double value increment)

[kiryldz]

LGTM with nits