Skip to content

Commit b32ce9e

Browse files
dkhawkdkhawk
authored
chore: resolve ErrorProne, Lint, and Kotlinc warnings across library, clustering, data, heatmaps, and ui modules (#1757)
* fix: address lint issues in Java and Kotlin files * chore: safely handle projection nullability in cluster renderers and add documented unit tests for IconGenerator and PreCachingAlgorithmDecorator * test(ui): add persistent golden PNG reference files and pixel-by-pixel visual regression unit tests for IconGenerator * test(ui): replace dummy Robolectric bitmaps with real anti-aliased speech-bubble golden reference PNG images * test(ui): connect live iconGenerator state to renderToBitmap and assert pixel similarity against golden PNG files * test(ui): make assertBitmapsEqual robust to cross-platform headless CI font metrics * test(ui): add createScaledBitmap to assertBitmapsEqual for headless CI AWT font metric independence
1 parent 36e27de commit b32ce9e

22 files changed

Lines changed: 601 additions & 139 deletions

File tree

‎clustering/src/main/java/com/google/maps/android/clustering/ClusterManager.kt‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@
1616
package com.google.maps.android.clustering
1717

1818
import android.content.Context
19-
import android.os.AsyncTask
2019
import com.google.android.gms.maps.GoogleMap
2120
import com.google.android.gms.maps.GoogleMap.OnCameraIdleListener
2221
import com.google.android.gms.maps.GoogleMap.OnInfoWindowClickListener

‎clustering/src/main/java/com/google/maps/android/clustering/algo/NonHierarchicalDistanceBasedAlgorithm.kt‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@ import com.google.maps.android.geometry.Bounds
2222
import com.google.maps.android.geometry.Point
2323
import com.google.maps.android.projection.SphericalMercatorProjection
2424
import com.google.maps.android.quadtree.PointQuadTree
25-
import java.util.ArrayList
2625
import java.util.Collections
2726
import java.util.HashMap
2827
import java.util.HashSet

‎clustering/src/main/java/com/google/maps/android/clustering/algo/PreCachingAlgorithmDecorator.kt‎

Lines changed: 10 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import java.util.concurrent.Executor
2222
import java.util.concurrent.Executors
2323
import java.util.concurrent.locks.ReadWriteLock
2424
import java.util.concurrent.locks.ReentrantReadWriteLock
25+
import kotlin.concurrent.withLock
2526

2627
/**
2728
* Optimistically fetch clusters for adjacent zoom levels, caching them as necessary.
@@ -107,24 +108,18 @@ class PreCachingAlgorithmDecorator<T : ClusterItem>(
107108
}
108109

109110
private fun getClustersInternal(discreteZoom: Int): Set<Cluster<T>> {
110-
var results: Set<Cluster<T>>?
111-
mCacheLock.readLock().lock()
112-
results = mCache.get(discreteZoom)
113-
mCacheLock.readLock().unlock()
111+
val cached = mCacheLock.readLock().withLock {
112+
mCache.get(discreteZoom)
113+
}
114+
if (cached != null) {
115+
return cached
116+
}
114117

115-
if (results == null) {
116-
mCacheLock.writeLock().lock()
117-
try {
118-
results = mCache.get(discreteZoom)
119-
if (results == null) {
120-
results = algorithm.getClusters(discreteZoom.toFloat())
121-
mCache.put(discreteZoom, results)
122-
}
123-
} finally {
124-
mCacheLock.writeLock().unlock()
118+
return mCacheLock.writeLock().withLock {
119+
mCache.get(discreteZoom) ?: algorithm.getClusters(discreteZoom.toFloat()).also {
120+
mCache.put(discreteZoom, it)
125121
}
126122
}
127-
return results!!
128123
}
129124

130125
private inner class PrecacheRunnable(

‎clustering/src/main/java/com/google/maps/android/clustering/view/ClusterRenderer.kt‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@ package com.google.maps.android.clustering.view
1818
import androidx.annotation.StyleRes
1919
import com.google.maps.android.clustering.Cluster
2020
import com.google.maps.android.clustering.ClusterItem
21-
import com.google.maps.android.clustering.ClusterManager
2221
import com.google.maps.android.clustering.ClusterManager.OnClusterClickListener
2322
import com.google.maps.android.clustering.ClusterManager.OnClusterInfoWindowClickListener
2423
import com.google.maps.android.clustering.ClusterManager.OnClusterInfoWindowLongClickListener

‎clustering/src/main/java/com/google/maps/android/clustering/view/ClusterRendererMultipleItems.kt‎

Lines changed: 25 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,6 @@ import java.util.Queue
6363
import java.util.concurrent.ConcurrentHashMap
6464
import java.util.concurrent.Executor
6565
import java.util.concurrent.Executors
66-
import java.util.concurrent.locks.Condition
6766
import java.util.concurrent.locks.Lock
6867
import java.util.concurrent.locks.ReentrantLock
6968
import kotlin.math.abs
@@ -325,17 +324,19 @@ open class ClusterRendererMultipleItems<T : ClusterItem> @JvmOverloads construct
325324
}
326325
val projection = mMap.projection
327326

328-
var renderTask: RenderTask?
329-
synchronized(this) {
330-
renderTask = mNextClusters
327+
val renderTask = synchronized(this) {
328+
val task = mNextClusters
331329
mNextClusters = null
332330
mViewModificationInProgress = true
331+
task
333332
}
334333

335-
renderTask!!.setCallback { sendEmptyMessage(TASK_FINISHED) }
336-
renderTask!!.setProjection(projection)
337-
renderTask!!.setMapZoom(mMap.cameraPosition.zoom)
338-
mExecutor.execute(renderTask)
334+
renderTask?.let {
335+
it.setCallback { sendEmptyMessage(TASK_FINISHED) }
336+
it.setProjection(projection)
337+
it.setMapZoom(mMap.cameraPosition.zoom)
338+
mExecutor.execute(it)
339+
}
339340
}
340341

341342
fun queue(clusters: Set<Cluster<T>>) {
@@ -409,6 +410,8 @@ open class ClusterRendererMultipleItems<T : ClusterItem> @JvmOverloads construct
409410
val markerModifier = MarkerModifier()
410411
val zoom = mMapZoom
411412
val markersToRemove = mMarkers
413+
val sphericalMercatorProjection = mSphericalMercatorProjection
414+
val animate = mAnimate && sphericalMercatorProjection != null
412415
var visibleBounds: LatLngBounds
413416

414417
try {
@@ -421,11 +424,11 @@ open class ClusterRendererMultipleItems<T : ClusterItem> @JvmOverloads construct
421424

422425
// Find all of the existing clusters that are on-screen. These are candidates for markers to animate from.
423426
var existingClustersOnScreen: MutableList<Point>? = null
424-
if (this@ClusterRendererMultipleItems.mClusters != null && mAnimate) {
427+
if (this@ClusterRendererMultipleItems.mClusters != null && animate) {
425428
existingClustersOnScreen = ArrayList()
426429
for (c in this@ClusterRendererMultipleItems.mClusters!!) {
427430
if (shouldRenderAsCluster(c) && visibleBounds.contains(c.position)) {
428-
val point = mSphericalMercatorProjection!!.toPoint(c.position)
431+
val point = sphericalMercatorProjection.toPoint(c.position)
429432
existingClustersOnScreen.add(point)
430433
}
431434
}
@@ -436,11 +439,11 @@ open class ClusterRendererMultipleItems<T : ClusterItem> @JvmOverloads construct
436439
val newMarkers: MutableSet<MarkerWithPosition<T>> = Collections.newSetFromMap(ConcurrentHashMap())
437440
for (c in clusters) {
438441
val onScreen = visibleBounds.contains(c.position)
439-
if (mAnimate) {
440-
val point = mSphericalMercatorProjection!!.toPoint(c.position)
442+
if (animate) {
443+
val point = sphericalMercatorProjection.toPoint(c.position)
441444
val closest = findClosestCluster(existingClustersOnScreen, point)
442445
if (closest != null) {
443-
val animateFrom = mSphericalMercatorProjection!!.toLatLng(closest)
446+
val animateFrom = sphericalMercatorProjection.toLatLng(closest)
444447
markerModifier.add(true, CreateMarkerTask(c, newMarkers, animateFrom))
445448
RendererLogger.d("ClusterRenderer", "Animating cluster from closest cluster: " + c.position)
446449
} else {
@@ -463,26 +466,27 @@ open class ClusterRendererMultipleItems<T : ClusterItem> @JvmOverloads construct
463466

464467
// Find all of the new clusters that were added on-screen. These are candidates for markers to animate from.
465468
var newClustersOnScreen: MutableList<Point>? = null
466-
if (mAnimate) {
469+
if (animate) {
467470
newClustersOnScreen = ArrayList()
468471
for (c in clusters) {
469472
if (shouldRenderAsCluster(c) && visibleBounds.contains(c.position)) {
470-
val p = mSphericalMercatorProjection!!.toPoint(c.position)
473+
val p = sphericalMercatorProjection.toPoint(c.position)
471474
newClustersOnScreen.add(p)
472475
}
473476
}
474477
RendererLogger.d("ClusterRenderer", "New clusters on screen found: " + newClustersOnScreen.size)
475478
}
476479

477480
for (marker in markersToRemove) {
478-
val onScreen = marker.position?.let { visibleBounds.contains(it) } ?: false
481+
val position = marker.position
482+
val onScreen = position?.let { visibleBounds.contains(it) } ?: false
479483

480-
if (onScreen && mAnimate) {
481-
val point = mSphericalMercatorProjection!!.toPoint(marker.position!!)
484+
if (onScreen && animate) {
485+
val point = sphericalMercatorProjection.toPoint(position)
482486
val closest = findClosestCluster(newClustersOnScreen, point)
483487
if (closest != null) {
484-
val animateTo = mSphericalMercatorProjection!!.toLatLng(closest)
485-
markerModifier.animateThenRemove(marker, marker.position!!, animateTo!!)
488+
val animateTo = sphericalMercatorProjection.toLatLng(closest)
489+
markerModifier.animateThenRemove(marker, position, animateTo)
486490
RendererLogger.d("ClusterRenderer", "Animating then removing marker at position: " + marker.position)
487491
} else if (mClusterMarkerCache.mCache.keys
488492
.iterator()
@@ -1143,7 +1147,7 @@ open class ClusterRendererMultipleItems<T : ClusterItem> @JvmOverloads construct
11431147
val markerWithPosition: MarkerWithPosition<T>
11441148
if (marker == null) {
11451149
RendererLogger.d("ClusterRenderer", "Creating new cluster marker")
1146-
val markerOptions = MarkerOptions().position(if (animateFrom == null) cluster.position else animateFrom)
1150+
val markerOptions = MarkerOptions().position(animateFrom ?: cluster.position)
11471151
onBeforeClusterRendered(cluster, markerOptions)
11481152
marker = mClusterManager.clusterMarkerCollection.addMarker(markerOptions)
11491153
mClusterMarkerCache.put(cluster, marker)

‎clustering/src/main/java/com/google/maps/android/clustering/view/DefaultAdvancedMarkersClusterRenderer.kt‎

Lines changed: 35 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,6 @@ import java.util.Queue
6060
import java.util.concurrent.ConcurrentHashMap
6161
import java.util.concurrent.Executor
6262
import java.util.concurrent.Executors
63-
import java.util.concurrent.locks.Condition
6463
import java.util.concurrent.locks.ReentrantLock
6564
import kotlin.math.abs
6665
import kotlin.math.min
@@ -269,17 +268,19 @@ open class DefaultAdvancedMarkersClusterRenderer<T : ClusterItem> @JvmOverloads
269268
}
270269
val projection = mMap.projection
271270

272-
var renderTask: RenderTask?
273-
synchronized(this) {
274-
renderTask = mNextClusters
271+
val renderTask = synchronized(this) {
272+
val task = mNextClusters
275273
mNextClusters = null
276274
mViewModificationInProgress = true
275+
task
277276
}
278277

279-
renderTask!!.setCallback { sendEmptyMessage(TASK_FINISHED) }
280-
renderTask!!.setProjection(projection)
281-
renderTask!!.setMapZoom(mMap.cameraPosition.zoom)
282-
mExecutor.execute(renderTask)
278+
renderTask?.let {
279+
it.setCallback { sendEmptyMessage(TASK_FINISHED) }
280+
it.setProjection(projection)
281+
it.setMapZoom(mMap.cameraPosition.zoom)
282+
mExecutor.execute(it)
283+
}
283284
}
284285

285286
fun queue(clusters: Set<Cluster<T>>) {
@@ -409,14 +410,17 @@ open class DefaultAdvancedMarkersClusterRenderer<T : ClusterItem> @JvmOverloads
409410
}
410411
// TODO: Add some padding, so that markers can animate in from off-screen.
411412

413+
val sphericalMercatorProjection = mSphericalMercatorProjection
414+
val animate = mAnimate && sphericalMercatorProjection != null
415+
412416
// Find all of the existing clusters that are on-screen. These are candidates for
413417
// markers to animate from.
414418
var existingClustersOnScreen: MutableList<Point>? = null
415-
if (this@DefaultAdvancedMarkersClusterRenderer.mClusters != null && mAnimate) {
419+
if (this@DefaultAdvancedMarkersClusterRenderer.mClusters != null && animate) {
416420
existingClustersOnScreen = ArrayList()
417421
for (c in this@DefaultAdvancedMarkersClusterRenderer.mClusters!!) {
418422
if (shouldRenderAsCluster(c) && visibleBounds.contains(c.position)) {
419-
val point = mSphericalMercatorProjection!!.toPoint(c.position)
423+
val point = sphericalMercatorProjection.toPoint(c.position)
420424
existingClustersOnScreen.add(point)
421425
}
422426
}
@@ -429,11 +433,11 @@ open class DefaultAdvancedMarkersClusterRenderer<T : ClusterItem> @JvmOverloads
429433
)
430434
for (c in clusters) {
431435
val onScreen = visibleBounds.contains(c.position)
432-
if (zoomingIn && onScreen && mAnimate) {
433-
val point = mSphericalMercatorProjection!!.toPoint(c.position)
436+
if (zoomingIn && onScreen && animate) {
437+
val point = sphericalMercatorProjection.toPoint(c.position)
434438
val closest = findClosestCluster(existingClustersOnScreen, point)
435439
if (closest != null) {
436-
val animateTo = mSphericalMercatorProjection!!.toLatLng(closest)
440+
val animateTo = sphericalMercatorProjection.toLatLng(closest)
437441
markerModifier.add(true, CreateMarkerTask(c, newMarkers, animateTo))
438442
} else {
439443
markerModifier.add(true, CreateMarkerTask(c, newMarkers, null))
@@ -453,11 +457,11 @@ open class DefaultAdvancedMarkersClusterRenderer<T : ClusterItem> @JvmOverloads
453457
// Find all of the new clusters that were added on-screen. These are candidates for
454458
// markers to animate from.
455459
var newClustersOnScreen: MutableList<Point>? = null
456-
if (mAnimate) {
460+
if (animate) {
457461
newClustersOnScreen = ArrayList()
458462
for (c in clusters) {
459463
if (shouldRenderAsCluster(c) && visibleBounds.contains(c.position)) {
460-
val p = mSphericalMercatorProjection!!.toPoint(c.position)
464+
val p = sphericalMercatorProjection.toPoint(c.position)
461465
newClustersOnScreen.add(p)
462466
}
463467
}
@@ -468,12 +472,12 @@ open class DefaultAdvancedMarkersClusterRenderer<T : ClusterItem> @JvmOverloads
468472
val onScreen = visibleBounds.contains(marker.position)
469473
// Don't animate when zooming out more than 3 zoom levels.
470474
// TODO: drop animation based on speed of device & number of markers to animate.
471-
if (!zoomingIn && zoomDelta > -3 && onScreen && mAnimate) {
472-
val point = mSphericalMercatorProjection!!.toPoint(marker.position)
475+
if (!zoomingIn && zoomDelta > -3 && onScreen && animate) {
476+
val point = sphericalMercatorProjection.toPoint(marker.position)
473477
val closest = findClosestCluster(newClustersOnScreen, point)
474478
if (closest != null) {
475-
val animateTo = mSphericalMercatorProjection!!.toLatLng(closest)
476-
markerModifier.animateThenRemove(marker, marker.position, animateTo!!)
479+
val animateTo = sphericalMercatorProjection.toLatLng(closest)
480+
markerModifier.animateThenRemove(marker, marker.position, animateTo)
477481
} else {
478482
markerModifier.remove(true, marker.marker)
479483
}
@@ -1004,7 +1008,7 @@ open class DefaultAdvancedMarkersClusterRenderer<T : ClusterItem> @JvmOverloads
10041008
if (!shouldRenderAsCluster(cluster)) {
10051009
for (item in cluster.items) {
10061010
var marker = mMarkerCache[item] as AdvancedMarker?
1007-
var markerWithPosition: MarkerWithPosition
1011+
val markerWithPosition: MarkerWithPosition
10081012
if (marker == null) {
10091013
val advancedMarkerOptions = AdvancedMarkerOptions()
10101014
if (animateFrom != null) {
@@ -1016,9 +1020,10 @@ open class DefaultAdvancedMarkersClusterRenderer<T : ClusterItem> @JvmOverloads
10161020
}
10171021
}
10181022
onBeforeClusterItemRendered(item, advancedMarkerOptions)
1019-
marker = mClusterManager.markerCollection.addMarker(advancedMarkerOptions) as AdvancedMarker?
1020-
markerWithPosition = MarkerWithPosition(marker!!)
1021-
mMarkerCache.put(item, marker!!)
1023+
val newMarker = mClusterManager.markerCollection.addMarker(advancedMarkerOptions) as AdvancedMarker
1024+
marker = newMarker
1025+
markerWithPosition = MarkerWithPosition(newMarker)
1026+
mMarkerCache.put(item, newMarker)
10221027
if (animateFrom != null) {
10231028
markerModifier.animate(markerWithPosition, animateFrom, item.position)
10241029
}
@@ -1033,22 +1038,22 @@ open class DefaultAdvancedMarkersClusterRenderer<T : ClusterItem> @JvmOverloads
10331038
}
10341039

10351040
var marker = mClusterMarkerCache[cluster] as AdvancedMarker?
1036-
var markerWithPosition: MarkerWithPosition
1041+
val markerWithPosition: MarkerWithPosition
10371042
if (marker == null) {
1038-
val advancedMarkerOptions = AdvancedMarkerOptions().position(if (animateFrom == null) cluster.position else animateFrom)
1043+
val advancedMarkerOptions = AdvancedMarkerOptions().position(animateFrom ?: cluster.position)
10391044
onBeforeClusterRendered(cluster, advancedMarkerOptions)
1040-
val `object` = mClusterManager.clusterMarkerCollection.addMarker(advancedMarkerOptions)
1041-
marker = `object` as AdvancedMarker?
1042-
mClusterMarkerCache.put(cluster, marker!!)
1043-
markerWithPosition = MarkerWithPosition(marker)
1045+
val newMarker = mClusterManager.clusterMarkerCollection.addMarker(advancedMarkerOptions) as AdvancedMarker
1046+
marker = newMarker
1047+
mClusterMarkerCache.put(cluster, newMarker)
1048+
markerWithPosition = MarkerWithPosition(newMarker)
10441049
if (animateFrom != null) {
10451050
markerModifier.animate(markerWithPosition, animateFrom, cluster.position)
10461051
}
10471052
} else {
10481053
markerWithPosition = MarkerWithPosition(marker)
10491054
onClusterUpdated(cluster, marker)
10501055
}
1051-
onClusterRendered(cluster, marker!!)
1056+
onClusterRendered(cluster, marker)
10521057
newMarkers.add(markerWithPosition)
10531058
}
10541059
}

0 commit comments

Comments
 (0)