Skip to content

Commit

Permalink
address review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
@ank27
ank27 committed Nov 28, 2022
1 parent b9e01a4 commit 8a09463
Show file tree
Hide file tree
Showing 3 changed files with 39 additions and 30 deletions.
51 changes: 30 additions & 21 deletions sdk/src/main/java/com/mapbox/maps/ViewAnnotationManagerImpl.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ internal class ViewAnnotationManagerImpl(
) : ViewAnnotationManager, ViewAnnotationPositionsUpdateListener {
private val mapboxMap: MapboxMap = mapView.getMapboxMap()
private val renderThread = mapView.mapController.renderer.renderThread
private var pixelRatio = mapView.resources.displayMetrics.density
private val pixelRatio = mapView.resources.displayMetrics.density

init {
viewAnnotationsLayout.layoutParams = FrameLayout.LayoutParams(
Expand Down Expand Up @@ -213,36 +213,42 @@ internal class ViewAnnotationManagerImpl(
val zoom = cameraOptionForCoordinates.zoom
isCorrectBound = true
viewAnnotationOptions.forEach { options ->
val frame = getViewAnnotationOptionsFrame(options)
val annotationBounds = calculateCoordinateBoundForAnnotation(options, zoom)
if (north == null || calculateCoordinateBoundForAnnotation(north!!.first, zoom).north() < annotationBounds.north()) {
val frame = getViewAnnotationOptionsFrame(options) ?: Rect(0, 0, 0, 0)
val annotationBounds = calculateCoordinateBoundForAnnotation(options, frame, zoom)
if (north == null || calculateCoordinateBoundForAnnotation(north!!.first, frame, zoom).north() < annotationBounds.north()) {
north = Pair(options, frame)
isCorrectBound = false
}
if (east == null || calculateCoordinateBoundForAnnotation(east!!.first, zoom).east() < annotationBounds.east()) {
if (east == null || calculateCoordinateBoundForAnnotation(east!!.first, frame, zoom).east() < annotationBounds.east()) {
east = Pair(options, frame)
isCorrectBound = false
}
if (south == null || calculateCoordinateBoundForAnnotation(south!!.first, zoom).south() > annotationBounds.south()) {
if (south == null || calculateCoordinateBoundForAnnotation(south!!.first, frame, zoom).south() > annotationBounds.south()) {
south = Pair(options, frame)
isCorrectBound = false
}
if (west == null || calculateCoordinateBoundForAnnotation(west!!.first, zoom).west() > annotationBounds.west()) {
if (west == null || calculateCoordinateBoundForAnnotation(west!!.first, frame, zoom).west() > annotationBounds.west()) {
west = Pair(options, frame)
isCorrectBound = false
}
}

// adding extra checks for nullability (this shouldn't execute normally as we are checking nullability in loop of viewannotations).
if (north == null || east == null || south == null || west == null) {
logW(TAG, "ViewAnnotation options framing is null. Returning null camera option")
return null
}

val coordinateBoundsForCamera = CoordinateBounds(
Point.fromLngLat((west!!.first.geometry as Point).longitude(), (south!!.first.geometry as Point).latitude()),
Point.fromLngLat((east!!.first.geometry as Point).longitude(), (north!!.first.geometry as Point).latitude())
Point.fromLngLat(west!!.first.coordinate().longitude(), south!!.first.coordinate().latitude()),
Point.fromLngLat(east!!.first.coordinate().longitude(), north!!.first.coordinate().latitude())
)

val paddings = EdgeInsets(
(edgeInsets?.top ?: 0).toDouble().plus(abs((north!!.second?.top ?: 0.0).toDouble())),
(edgeInsets?.left ?: 0).toDouble().plus(abs((west!!.second?.left ?: 0.0).toDouble())),
(edgeInsets?.bottom ?: 0).toDouble().plus(abs((south!!.second?.bottom ?: 0.0).toDouble())),
(edgeInsets?.right ?: 0).toDouble().plus(abs((east!!.second?.right ?: 0.0).toDouble()))
(edgeInsets?.top ?: 0).toDouble() + abs((north!!.second?.top ?: 0.0).toDouble()),
(edgeInsets?.left ?: 0).toDouble() + abs((west!!.second?.left ?: 0.0).toDouble()),
(edgeInsets?.bottom ?: 0).toDouble() + abs((south!!.second?.bottom ?: 0.0).toDouble()),
(edgeInsets?.right ?: 0).toDouble() + abs((east!!.second?.right ?: 0.0).toDouble())
)

cameraOptionForCoordinates = mapboxMap.cameraForCoordinateBounds(
Expand All @@ -255,6 +261,8 @@ internal class ViewAnnotationManagerImpl(
return cameraOptionForCoordinates
}

private fun ViewAnnotationOptions.coordinate() = geometry as Point

/**
* Function to calculate coordinatebound associated with annotation option.
* this uses [MapboxMap.projectedMetersForCoordinate] and [MapboxMap.coordinateForProjectedMeters]
Expand All @@ -263,19 +271,19 @@ internal class ViewAnnotationManagerImpl(
@VisibleForTesting(otherwise = VisibleForTesting.PRIVATE)
internal fun calculateCoordinateBoundForAnnotation(
viewAnnotationOptions: ViewAnnotationOptions,
annotationFrame: Rect,
zoom: Double?
): CoordinateBounds {
val frame = getViewAnnotationOptionsFrame(viewAnnotationOptions)
val metersPerPixelAtLatitude =
if (zoom == null) mapboxMap.getMetersPerPixelAtLatitude((viewAnnotationOptions.geometry as Point).latitude()) else
mapboxMap.getMetersPerPixelAtLatitude((viewAnnotationOptions.geometry as Point).latitude(), zoom)
if (zoom == null) mapboxMap.getMetersPerPixelAtLatitude(viewAnnotationOptions.coordinate().latitude()) else
mapboxMap.getMetersPerPixelAtLatitude(viewAnnotationOptions.coordinate().latitude(), zoom)

val projectedMeterForCoordinate = mapboxMap.projectedMetersForCoordinate(viewAnnotationOptions.geometry as Point)
val projectedMeterForCoordinate = mapboxMap.projectedMetersForCoordinate(viewAnnotationOptions.coordinate())
val metersPerPixelDensity = metersPerPixelAtLatitude / pixelRatio
val northing = projectedMeterForCoordinate.northing + (abs(frame?.top ?: 0).toDouble() * metersPerPixelDensity)
val easting = projectedMeterForCoordinate.easting + (abs(frame?.right ?: 0).toDouble() * metersPerPixelDensity)
val southing = projectedMeterForCoordinate.northing - (abs(frame?.bottom ?: 0).toDouble() * metersPerPixelDensity)
val westing = projectedMeterForCoordinate.easting - (abs(frame?.left ?: 0).toDouble() * metersPerPixelDensity)
val northing = projectedMeterForCoordinate.northing + (abs(annotationFrame.top).toDouble() * metersPerPixelDensity)
val easting = projectedMeterForCoordinate.easting + (abs(annotationFrame.right).toDouble() * metersPerPixelDensity)
val southing = projectedMeterForCoordinate.northing - (abs(annotationFrame.bottom).toDouble() * metersPerPixelDensity)
val westing = projectedMeterForCoordinate.easting - (abs(annotationFrame.left).toDouble() * metersPerPixelDensity)

val projectedMeterNorthEast = ProjectedMeters(northing, easting)
val projectedMeterSouthWest = ProjectedMeters(southing, westing)
Expand Down Expand Up @@ -653,6 +661,7 @@ internal class ViewAnnotationManagerImpl(
internal const val EXCEPTION_TEXT_GEOMETRY_IS_NULL = "Geometry can not be null!"
internal const val EXCEPTION_TEXT_ASSOCIATED_FEATURE_ID_ALREADY_EXISTS =
"View annotation with associatedFeatureId=%s already exists!"
private const val TAG = "ViewAnnotationImpl"

@VisibleForTesting(otherwise = VisibleForTesting.PRIVATE)
internal fun needToReorderZ(
Expand Down
10 changes: 5 additions & 5 deletions sdk/src/test/java/com/mapbox/maps/ViewAnnotationManagerTest.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -488,10 +488,10 @@ class ViewAnnotationManagerTest {
)
} answers { expectedCameraOptions }
val coordinateBounds = mockk<CoordinateBounds>(relaxUnitFun = true)
every { coordinateBounds.north() } returns 0.0
every { coordinateBounds.east() } returns 0.0
every { coordinateBounds.west() } returns 0.0
every { coordinateBounds.south() } returns 0.0
every { coordinateBounds.north() } returns 20.0
every { coordinateBounds.east() } returns 20.0
every { coordinateBounds.west() } returns 10.0
every { coordinateBounds.south() } returns 10.0

every { mapboxMap.coordinateBoundsForCamera(any()) } returns coordinateBounds
every { mapboxMap.getMetersPerPixelAtLatitude(any(), any()) } returns 1.0
Expand Down Expand Up @@ -549,7 +549,7 @@ class ViewAnnotationManagerTest {
CoordinateBounds(Point.fromLngLat(0.0, 0.0), Point.fromLngLat(0.0, 0.0))
annotationManagerSpyk.addViewAnnotation(view, viewAnnotationOptions)
val actualCoordinateBounds =
annotationManagerSpyk.calculateCoordinateBoundForAnnotation(viewAnnotationOptions, 1.0)
annotationManagerSpyk.calculateCoordinateBoundForAnnotation(viewAnnotationOptions, rect,1.0)
assertEquals(expectedCoordinateBounds, actualCoordinateBounds)
}

Expand Down
8 changes: 4 additions & 4 deletions sdk/src/test/java/com/mapbox/maps/shadows/ShadowCoordinateBounds.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,18 +12,18 @@
public class ShadowCoordinateBounds {
@Implementation
public static double north() {
return 0.0;
return 20.0;
}
@Implementation
public static double east() {
return 0.0;
return 20.0;
}
@Implementation
public static double south() {
return 0.0;
return 10.0;
}
@Implementation
public static double west() {
return 0.0;
return 10.0;
}
}

0 comments on commit 8a09463

Please sign in to comment.