Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[pull] main from mapbox:main #387

Merged
merged 2 commits into from
Jan 11, 2023
Merged

[pull] main from mapbox:main #387

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
0cad995
[viewport] Fix transition to FollowPuckViewportState when no new loca…
pengdev Jan 10, 2023
e4d0c2e
Make 'Cluster points within a layer' example code compatible with GL …
OttyLab Jan 10, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ Mapbox welcomes participation and contributions from everyone.
## Features ✨ and improvements 🏁
* Skip redundant `MapboxMap.setCamera` updates in `CameraAnimationsPlugin`. ([1909](https://github.com/mapbox/mapbox-maps-android/pull/1909))
* Improve performance by setting geojson data directly. ([1920](https://github.com/mapbox/mapbox-maps-android/pull/1920))
* Fix viewport hang when transition to `FollowPuckViewportState` and no new location update is available. ([1929](https://github.com/mapbox/mapbox-maps-android/pull/1929))

## Bug fixes 🐞
* Fix crash due to invalid distance when panning the map. ([1906](https://github.com/mapbox/mapbox-maps-android/pull/1906))
Expand Down
63 changes: 23 additions & 40 deletions app/src/main/java/com/mapbox/maps/testapp/examples/CircleLayerClusteringActivity.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import com.mapbox.maps.extension.style.layers.generated.circleLayer
import com.mapbox.maps.extension.style.layers.generated.symbolLayer
import com.mapbox.maps.extension.style.sources.addSource
import com.mapbox.maps.extension.style.sources.generated.geoJsonSource
import com.mapbox.maps.extension.style.utils.ColorUtils
import com.mapbox.maps.plugin.animation.flyTo
import com.mapbox.maps.testapp.R
import com.mapbox.maps.testapp.utils.BitmapUtils.bitmapFromDrawableRes
Expand Down Expand Up @@ -140,47 +141,29 @@ class CircleLayerClusteringActivity : AppCompatActivity() {
intArrayOf(0, ContextCompat.getColor(this, R.color.blue))
)

for (i in layers.indices) {

// Add clusters' circles
style.addLayer(
circleLayer("cluster-$i", GEOJSON_SOURCE_ID) {
circleColor(layers[i][1])
circleRadius(18.0)
filter(
if (i == 0) {
has {
literal("point_count")
}
gte {
toNumber {
get { literal("point_count") }
}
literal(layers[i][0].toLong())
}
} else {
all {
has {
literal("point_count")
}
gte {
toNumber {
get { literal("point_count") }
}
literal(layers[i][0].toLong())
}
lt {
toNumber {
get { literal("point_count") }
}
literal(layers[i - 1][0].toLong())
}
}
// Add clusters' circles
style.addLayer(
circleLayer("clusters", GEOJSON_SOURCE_ID) {
circleColor(
step {
get("point_count")
literal(ColorUtils.colorToRgbaString(layers[2][1]))
stop {
literal(layers[1][0].toDouble())
literal(ColorUtils.colorToRgbaString(layers[1][1]))
}
)
}
)
}
stop {
literal(layers[0][0].toDouble())
literal(ColorUtils.colorToRgbaString(layers[0][1]))
}
}
)
circleRadius(18.0)
filter(
has("point_count")
)
}
)

style.addLayer(
symbolLayer("count", GEOJSON_SOURCE_ID) {
Expand Down
25 changes: 21 additions & 4 deletions ...port/src/main/kotlin/com/mapbox/maps/plugin/viewport/state/FollowPuckViewportStateImpl.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,20 +59,30 @@ internal class FollowPuckViewportStateImpl(
}

private fun notifyLatestViewportData() {
if (lastLocation != null && (options.bearing is FollowPuckViewportStateBearing.Constant || lastBearing != null)) {
if (shouldNotifyLatestViewportData()) {
val viewportData = evaluateViewportData()
if (isFollowingStateRunning) {
// Use instant update here since the location updates are already interpolated by the location component plugin
updateFrame(viewportData, true)
}
dataSourceUpdateObservers.forEach {
if (!it.onNewData(viewportData)) {
dataSourceUpdateObservers.remove(it)
}
notifyViewportStateDataObserver(it, viewportData)
}
}
}

private fun shouldNotifyLatestViewportData() =
lastLocation != null && (options.bearing is FollowPuckViewportStateBearing.Constant || lastBearing != null)

private fun notifyViewportStateDataObserver(
observer: ViewportStateDataObserver,
cameraOptions: CameraOptions
) {
if (!observer.onNewData(cameraOptions)) {
dataSourceUpdateObservers.remove(observer)
}
}

private fun evaluateViewportData(): CameraOptions {
return CameraOptions.Builder()
.center(lastLocation)
Expand Down Expand Up @@ -103,6 +113,10 @@ internal class FollowPuckViewportStateImpl(
locationComponent.removeOnIndicatorPositionChangedListener(indicatorPositionChangedListener)
locationComponent.removeOnIndicatorBearingChangedListener(indicatorBearingChangedListener)
isObservingLocationUpdates = false
// when unsubscribed from the location updates, we don't want to keep an outdated location, so
// when user transition to the FollowPuckViewportState, there wouldn't be any unintentional jump.
lastBearing = null
lastLocation = null
}
}

Expand All @@ -126,6 +140,9 @@ internal class FollowPuckViewportStateImpl(
checkLocationComponentEnablement()
addIndicatorListenerIfNeeded()
dataSourceUpdateObservers.add(viewportStateDataObserver)
if (shouldNotifyLatestViewportData()) {
notifyViewportStateDataObserver(viewportStateDataObserver, evaluateViewportData())
}
return Cancelable {
dataSourceUpdateObservers.remove(viewportStateDataObserver)
removeIndicatorListenerIfNeeded()
Expand Down
80 changes: 69 additions & 11 deletions .../src/test/kotlin/com/mapbox/maps/plugin/viewport/state/FollowPuckViewportStateImplTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,16 +21,7 @@ import com.mapbox.maps.plugin.viewport.LOCATION_COMPONENT_UTILS
import com.mapbox.maps.plugin.viewport.data.FollowPuckViewportStateBearing
import com.mapbox.maps.plugin.viewport.data.FollowPuckViewportStateOptions
import com.mapbox.maps.plugin.viewport.transition.MapboxViewportTransitionFactory
import io.mockk.every
import io.mockk.just
import io.mockk.mockk
import io.mockk.mockkStatic
import io.mockk.runs
import io.mockk.slot
import io.mockk.unmockkAll
import io.mockk.unmockkStatic
import io.mockk.verify
import io.mockk.verifySequence
import io.mockk.*
import org.junit.After
import org.junit.Assert.assertFalse
import org.junit.Assert.assertTrue
Expand Down Expand Up @@ -135,7 +126,9 @@ class FollowPuckViewportStateImplTest {

// set the bearing to be constant
followingState.apply {
options = options.toBuilder().bearing(FollowPuckViewportStateBearing.Constant(constantBearing)).build()
options =
options.toBuilder().bearing(FollowPuckViewportStateBearing.Constant(constantBearing))
.build()
}
// stop observing after the first data point
every { dataObserver.onNewData(any()) } returns false
Expand Down Expand Up @@ -180,6 +173,71 @@ class FollowPuckViewportStateImplTest {
}
}

@Test
fun testObserveDataSourceWithOnlyExistingLocation() {
val indicatorBearingChangedListenerSlot = slot<OnIndicatorBearingChangedListener>()
val indicatorPositionChangedListenerSlot = slot<OnIndicatorPositionChangedListener>()
val dataObserver = mockk<ViewportStateDataObserver>()
val testBearing = 10.0
val testCenter = Point.fromLngLat(0.0, 0.0)

// keep observing after the first data point
every { dataObserver.onNewData(any()) } returns true

// immediately emmit location update when listener is added.
every {
locationPlugin.addOnIndicatorBearingChangedListener(
capture(
indicatorBearingChangedListenerSlot
)
)
} answers {
indicatorBearingChangedListenerSlot.captured.onIndicatorBearingChanged(testBearing)
}
every {
locationPlugin.addOnIndicatorPositionChangedListener(
capture(
indicatorPositionChangedListenerSlot
)
)
} answers {
indicatorPositionChangedListenerSlot.captured.onIndicatorPositionChanged(testCenter)
}

followingState.observeDataSource(dataObserver)
verify {
locationPlugin.addOnIndicatorBearingChangedListener(any())
}
verify {
locationPlugin.addOnIndicatorPositionChangedListener(any())
}
verify(exactly = 1) {
dataObserver.onNewData(
cameraOptions {
bearing(testBearing)
center(testCenter)
pitch(DEFAULT_FOLLOW_PUCK_VIEWPORT_STATE_PITCH)
zoom(DEFAULT_FOLLOW_PUCK_VIEWPORT_STATE_ZOOM)
padding(EdgeInsets(0.0, 0.0, 0.0, 0.0))
}
)
}

// observing the data source again should only emit exactly one data point.
followingState.observeDataSource(dataObserver)
verify(exactly = 2) {
dataObserver.onNewData(
cameraOptions {
bearing(testBearing)
center(testCenter)
pitch(DEFAULT_FOLLOW_PUCK_VIEWPORT_STATE_PITCH)
zoom(DEFAULT_FOLLOW_PUCK_VIEWPORT_STATE_ZOOM)
padding(EdgeInsets(0.0, 0.0, 0.0, 0.0))
}
)
}
}

@Test
fun testObserveDataSourceContinuously() {
val indicatorBearingChangedListenerSlot = slot<OnIndicatorBearingChangedListener>()
Expand Down