Skip to content

Commit c1694fe

Browse files
jushgithub-actions[bot]
jush
authored and
github-actions[bot]
committed
Improve camera for coordinates performance (#4224)
It turns out that we're passing a `List<Point>` to `cameraForCoordinates` and that would result in many JNI calls which slows it down. This PR calculates the bounding box for the list of points before sending it to native for processing. GitOrigin-RevId: e450e7ddadda255ab07c3ee8d5d797a6f4cf44ab
1 parent 669a863 commit c1694fe

File tree

4 files changed

+233
-25
lines changed

4 files changed

+233
-25
lines changed

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ Mapbox welcomes participation and contributions from everyone.
66

77
## Features ✨ and improvements 🏁
88
* Added new `FillLayer.fillPatternCrossFade`, `FillExtrusionLayer.fillExtrusionPatternCrossFade`, `LineLayer.fillExtrusionPatternCrossFade` properties.
9+
* Improve the performance of `MapboxMap.cameraForCoordinates(...)` for large amounts of points.
910

1011

1112
# 11.13.0-rc.1 June 03, 2025

‎maps-sdk/src/androidTest/java/com/mapbox/maps/CameraForCoordinatesTest.kt‎

Lines changed: 138 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,138 @@
1+
package com.mapbox.maps
2+
3+
import androidx.test.annotation.UiThreadTest
4+
import androidx.test.ext.junit.rules.ActivityScenarioRule
5+
import androidx.test.ext.junit.runners.AndroidJUnit4
6+
import com.mapbox.geojson.Point
7+
import com.mapbox.maps.dsl.cameraOptions
8+
import com.mapbox.maps.util.isEmpty
9+
import org.junit.After
10+
import org.junit.Assert
11+
import org.junit.Before
12+
import org.junit.Rule
13+
import org.junit.Test
14+
import org.junit.runner.RunWith
15+
import java.util.concurrent.CountDownLatch
16+
17+
@RunWith(AndroidJUnit4::class)
18+
class CameraForCoordinatesTest {
19+
20+
@get:Rule
21+
val rule = ActivityScenarioRule(EmptyActivity::class.java)
22+
23+
private lateinit var mapView: MapView
24+
private lateinit var mapboxMap: MapboxMap
25+
private lateinit var countDownLatch: CountDownLatch
26+
27+
@Before
28+
fun setUp() {
29+
val latch = CountDownLatch(2)
30+
rule.scenario.onActivity {
31+
mapView = MapView(it, MapInitOptions(it))
32+
mapboxMap = mapView.mapboxMap
33+
// Hardcoded width and height to avoid issues with different screen sizes
34+
it.frameLayout.addView(mapView, 200, 200)
35+
mapboxMap.getStyle {
36+
latch.countDown()
37+
}
38+
mapboxMap.subscribeMapIdle {
39+
latch.countDown()
40+
}
41+
}
42+
latch.throwExceptionOnTimeoutMs(timeoutMs = 30_000L)
43+
}
44+
45+
@OptIn(MapboxDelicateApi::class)
46+
@Test
47+
fun basicCameraForCoordinates() {
48+
countDownLatch = CountDownLatch(1)
49+
rule.scenario.onActivity {
50+
it.runOnUiThread {
51+
mapView.onStart()
52+
val points: List<Point> = listOf(
53+
Point.fromLngLat(0.0, 0.0),
54+
Point.fromLngLat(1.0, 1.0),
55+
Point.fromLngLat(2.0, 2.0),
56+
Point.fromLngLat(4.0, 4.0),
57+
)
58+
val cameraForCoordinates = mapboxMap.cameraForCoordinates(
59+
coordinates = points,
60+
camera = cameraOptions { },
61+
coordinatesPadding = null,
62+
maxZoom = null,
63+
offset = null,
64+
)
65+
Assert.assertFalse(cameraForCoordinates.isEmpty)
66+
Assert.assertEquals(2.001, cameraForCoordinates.center!!.latitude(), 0.001)
67+
Assert.assertEquals(2.001, cameraForCoordinates.center!!.longitude(), 0.001)
68+
Assert.assertEquals(3.738, cameraForCoordinates.zoom!!, 0.001)
69+
countDownLatch.countDown()
70+
}
71+
}
72+
countDownLatch.throwExceptionOnTimeoutMs()
73+
}
74+
75+
@OptIn(MapboxDelicateApi::class)
76+
@Test
77+
fun antimeridianCameraForCoordinates() {
78+
countDownLatch = CountDownLatch(1)
79+
rule.scenario.onActivity {
80+
it.runOnUiThread {
81+
mapView.onStart()
82+
val points: List<Point> = listOf(
83+
Point.fromLngLat(-180.0, 10.0),
84+
Point.fromLngLat(180.0, 20.0),
85+
)
86+
val cameraForCoordinates = mapboxMap.cameraForCoordinates(
87+
coordinates = points,
88+
camera = cameraOptions { },
89+
coordinatesPadding = null,
90+
maxZoom = null,
91+
offset = null,
92+
)
93+
Assert.assertFalse(cameraForCoordinates.isEmpty)
94+
Assert.assertEquals(-180.0, cameraForCoordinates.center!!.longitude(), 0.001)
95+
Assert.assertEquals(85.902, cameraForCoordinates.center!!.latitude(), 0.001)
96+
Assert.assertEquals(0.0, cameraForCoordinates.zoom!!, 0.001)
97+
countDownLatch.countDown()
98+
}
99+
}
100+
countDownLatch.throwExceptionOnTimeoutMs()
101+
}
102+
103+
@OptIn(MapboxDelicateApi::class)
104+
@Test
105+
fun antimeridian2CameraForCoordinates() {
106+
countDownLatch = CountDownLatch(1)
107+
rule.scenario.onActivity {
108+
it.runOnUiThread {
109+
mapView.onStart()
110+
val points: List<Point> = listOf(
111+
Point.fromLngLat(-185.0, 19.5),
112+
Point.fromLngLat(-150.0, 20.0),
113+
)
114+
val cameraForCoordinates = mapboxMap.cameraForCoordinates(
115+
coordinates = points,
116+
camera = cameraOptions { },
117+
coordinatesPadding = null,
118+
maxZoom = null,
119+
offset = null,
120+
)
121+
Assert.assertFalse(cameraForCoordinates.isEmpty)
122+
Assert.assertEquals(-167.5, cameraForCoordinates.center!!.longitude(), 0.001)
123+
Assert.assertEquals(20.179, cameraForCoordinates.center!!.latitude(), 0.001)
124+
Assert.assertEquals(0.478, cameraForCoordinates.zoom!!, 0.001)
125+
mapboxMap.setCamera(cameraForCoordinates)
126+
countDownLatch.countDown()
127+
}
128+
}
129+
countDownLatch.throwExceptionOnTimeoutMs()
130+
}
131+
132+
@After
133+
@UiThreadTest
134+
fun teardown() {
135+
mapView.onStop()
136+
mapView.onDestroy()
137+
}
138+
}

‎maps-sdk/src/main/java/com/mapbox/maps/MapboxMap.kt‎

Lines changed: 67 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -793,7 +793,12 @@ class MapboxMap :
793793
pitch: Double?
794794
): CameraOptions {
795795
checkNativeMap("cameraForCoordinates")
796-
return nativeMap.cameraForCoordinates(coordinates, coordinatesPadding, bearing, pitch)
796+
return nativeMap.cameraForCoordinates(
797+
calculateBoundingBox(coordinates),
798+
coordinatesPadding,
799+
bearing,
800+
pitch
801+
)
797802
}
798803

799804
/**
@@ -827,7 +832,7 @@ class MapboxMap :
827832
box: ScreenBox
828833
): CameraOptions {
829834
checkNativeMap("cameraForCoordinates")
830-
return nativeMap.cameraForCoordinates(coordinates, camera, box)
835+
return nativeMap.cameraForCoordinates(calculateBoundingBox(coordinates), camera, box)
831836
}
832837

833838
/**
@@ -869,7 +874,13 @@ class MapboxMap :
869874
if (!nativeMap.sizeSet) {
870875
return cameraOptions { }
871876
}
872-
return nativeMap.cameraForCoordinates(coordinates, camera, coordinatesPadding, maxZoom, offset).getValueOrElse {
877+
return nativeMap.cameraForCoordinates(
878+
calculateBoundingBox(coordinates),
879+
camera,
880+
coordinatesPadding,
881+
maxZoom,
882+
offset
883+
).getValueOrElse {
873884
logE(
874885
TAG,
875886
"Error occurred in synchronous cameraForCoordinates(coordinates: $coordinates, camera: $camera, coordinatesPadding: $coordinatesPadding, maxZoom: $maxZoom, offset: $offset, mapSize: ${nativeMap.getSize()}): $it, empty cameraState will be returned"
@@ -900,14 +911,19 @@ class MapboxMap :
900911
checkNativeMap("cameraForCoordinates")
901912
nativeMap.whenMapSizeReady {
902913
result.invoke(
903-
nativeMap.cameraForCoordinates(coordinates, camera, coordinatesPadding, maxZoom, offset)
904-
.getValueOrElse {
905-
logE(
906-
TAG,
907-
"Error occurred in asynchronous cameraForCoordinates(coordinates: $coordinates, camera: $camera, coordinatesPadding: $coordinatesPadding, maxZoom: $maxZoom, offset: $offset, mapSize: ${nativeMap.getSize()}): $it, empty cameraState will be returned"
908-
)
909-
return@getValueOrElse cameraOptions { }
910-
}
914+
nativeMap.cameraForCoordinates(
915+
calculateBoundingBox(coordinates),
916+
camera,
917+
coordinatesPadding,
918+
maxZoom,
919+
offset
920+
).getValueOrElse {
921+
logE(
922+
TAG,
923+
"Error occurred in asynchronous cameraForCoordinates(coordinates: $coordinates, camera: $camera, coordinatesPadding: $coordinatesPadding, maxZoom: $maxZoom, offset: $offset, mapSize: ${nativeMap.getSize()}): $it, empty cameraState will be returned"
924+
)
925+
return@getValueOrElse cameraOptions { }
926+
}
911927
)
912928
}
913929
}
@@ -2957,5 +2973,45 @@ class MapboxMap :
29572973
nativeObserver: NativeObserver,
29582974
pixelRatio: Float
29592975
) = MapboxMap(nativeMap, nativeObserver, pixelRatio)
2976+
2977+
/**
2978+
* Calculates the bounding box for a list of points.
2979+
*
2980+
* @return the bounding box as a list of two points or the original [points] if the size is less than or equal to 2.
2981+
*/
2982+
private fun calculateBoundingBox(points: List<Point>): List<Point> {
2983+
if (points.size <= 2) {
2984+
return points
2985+
}
2986+
val bbox = DoubleArray(4)
2987+
bbox[0] = Double.POSITIVE_INFINITY
2988+
bbox[1] = Double.POSITIVE_INFINITY
2989+
bbox[2] = Double.NEGATIVE_INFINITY
2990+
bbox[3] = Double.NEGATIVE_INFINITY
2991+
2992+
@Suppress("ReplaceManualRangeWithIndicesCalls")
2993+
for (i in 0 until points.size) {
2994+
val point: Point = points[i]
2995+
val longitude = point.longitude()
2996+
val latitude = point.latitude()
2997+
2998+
if (bbox[0] > longitude) {
2999+
bbox[0] = longitude
3000+
}
3001+
if (bbox[1] > latitude) {
3002+
bbox[1] = latitude
3003+
}
3004+
if (bbox[2] < longitude) {
3005+
bbox[2] = longitude
3006+
}
3007+
if (bbox[3] < latitude) {
3008+
bbox[3] = latitude
3009+
}
3010+
}
3011+
return listOf(
3012+
Point.fromLngLat(bbox[0], bbox[1]),
3013+
Point.fromLngLat(bbox[2], bbox[3])
3014+
)
3015+
}
29603016
}
29613017
}

‎maps-sdk/src/test/java/com/mapbox/maps/MapboxMapTest.kt‎

Lines changed: 27 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -712,60 +712,60 @@ class MapboxMapTest {
712712

713713
@Test
714714
fun cameraForCoordinatesOne() {
715-
val points = mockk<List<Point>>()
715+
val points = defaultPointsList
716716
val camera = mockk<CameraOptions>()
717717
val box = mockk<ScreenBox>()
718718
mapboxMap.cameraForCoordinates(points, camera, box)
719-
verify { nativeMap.cameraForCoordinates(points, camera, box) }
719+
verify { nativeMap.cameraForCoordinates(defaultPointsBbox, camera, box) }
720720
}
721721

722722
@Test
723723
fun cameraForCoordinatesTwo() {
724-
val points = mockk<List<Point>>()
724+
val points = defaultPointsList
725725
val edgeInsets = mockk<EdgeInsets>()
726726
mapboxMap.cameraForCoordinates(points, edgeInsets, 0.0, 1.0)
727-
verify { nativeMap.cameraForCoordinates(points, edgeInsets, 0.0, 1.0) }
727+
verify { nativeMap.cameraForCoordinates(defaultPointsBbox, edgeInsets, 0.0, 1.0) }
728728
}
729729

730730
@Test
731731
fun cameraForCoordinatesOverload() {
732-
val points = mockk<List<Point>>()
732+
val points = defaultPointsList
733733
mapboxMap.cameraForCoordinates(points)
734-
verify { nativeMap.cameraForCoordinates(points, null, null, null) }
734+
verify { nativeMap.cameraForCoordinates(defaultPointsBbox, null, null, null) }
735735
}
736736

737737
@Test
738738
fun cameraForCoordinatesWithoutPadding() {
739-
val points = mockk<List<Point>>()
739+
val points = defaultPointsList
740740
mapboxMap.cameraForCoordinates(points)
741-
verify { nativeMap.cameraForCoordinates(points, null, null, null) }
741+
verify { nativeMap.cameraForCoordinates(defaultPointsBbox, null, null, null) }
742742
}
743743

744744
@Test
745745
fun cameraForCoordinatesOverloadBearing() {
746-
val points = mockk<List<Point>>()
746+
val points = defaultPointsList
747747
mapboxMap.cameraForCoordinates(points, bearing = 2.0)
748-
verify { nativeMap.cameraForCoordinates(points, null, 2.0, null) }
748+
verify { nativeMap.cameraForCoordinates(defaultPointsBbox, null, 2.0, null) }
749749
}
750750

751751
@Test
752752
fun cameraForCoordinatesOverloadPitch() {
753-
val points = mockk<List<Point>>()
753+
val points = defaultPointsList
754754
mapboxMap.cameraForCoordinates(points, pitch = 2.0)
755-
verify { nativeMap.cameraForCoordinates(points, null, null, 2.0) }
755+
verify { nativeMap.cameraForCoordinates(defaultPointsBbox, null, null, 2.0) }
756756
}
757757

758758
@Test
759759
fun cameraForCoordinatesWithOffset() {
760-
val points = mockk<List<Point>>()
760+
val points = defaultPointsList
761761
val camera = mockk<CameraOptions>()
762762
val offset = mockk<ScreenCoordinate>()
763763
every { nativeMap.sizeSet } returns true
764764
every { nativeMap.cameraForCoordinates(any(), any(), any(), any(), any()) } returns
765765
ExpectedFactory.createValue(mockk())
766766

767767
mapboxMap.cameraForCoordinates(points, camera, ZERO_EDGE_INSETS, 1.0, offset)
768-
verify { nativeMap.cameraForCoordinates(points, camera, ZERO_EDGE_INSETS, 1.0, offset) }
768+
verify { nativeMap.cameraForCoordinates(defaultPointsBbox, camera, ZERO_EDGE_INSETS, 1.0, offset) }
769769
}
770770

771771
@Test
@@ -1849,6 +1849,19 @@ class MapboxMapTest {
18491849
every { nativeMap.map } returns map
18501850
assertEquals(mapboxMap.getNativeMap(), map)
18511851
}
1852+
1853+
private companion object {
1854+
private val defaultPointsList = listOf(
1855+
Point.fromLngLat(0.0, 0.0),
1856+
Point.fromLngLat(1.1, 1.1),
1857+
Point.fromLngLat(2.2, 2.2),
1858+
Point.fromLngLat(3.3, 3.3),
1859+
)
1860+
private val defaultPointsBbox = listOf(
1861+
Point.fromLngLat(0.0, 0.0),
1862+
Point.fromLngLat(3.3, 3.3),
1863+
)
1864+
}
18521865
}
18531866

18541867
@RunWith(ParameterizedRobolectricTestRunner::class)

0 commit comments

Comments
 (0)