From 9a94dc2548aa54cf994773ee6308fd0a038b9029 Mon Sep 17 00:00:00 2001 From: Jason-Whitmore Date: Sun, 29 Jun 2025 02:30:10 -0700 Subject: [PATCH] Fixes Issue 6312: GPS has huge error and does not update (in Nearby) (#6352) * NearbyParentFragment.kt: add helper methods for user location overlays and accuracy data. Before this commit, the code used to create the user location overlays was in multiple places. Additionally, there was no easy way to access the location accuracy. This commit places the user location overlay creation code into helper methods, as well as adding a new location accuracy getter method. These methods can now be used to refactor other parts of the file. * NearbyParentFragment.kt: create method to update user location overlays Before this commit, there was no easy way to update the user location overlays. This commit adds the updateUserLocationOverlays() method, which will properly replace the old user location overlays with new ones. It will also add the overlays if they do not already exist. * NearbyParentFragment.kt: replace old code with calls to updateUserLocationOverlays() This commit completes the refactor and fixes the issue of the user overlays not updating. The new method updateUserLocationOverlays is called to refactor and simplify old code. * Removal of file is not related to the issue, but is needed for project to compile and run. * NearbyParentFragment.kt: fix bug where multiple user location overlays would appear Before this commit, the user could see multiple user location overlays if they paused the app and reopened it when there are no Places/pins on the map. This was caused by a linear search failing to identify the target overlay because it compared Drawables between two Overlays, which was unreliable. This commit contains a better solution for replacing existing user location overlays by adding 2 instance variables to keep track of the overlays. The position of these overlays in the overlay list can then be found by using indexOf() with these instance variables rather than the linear search that was implemented before. Some refactoring was also done. --------- Co-authored-by: Nicolas Raoul --- .../nearby/fragments/NearbyParentFragment.kt | 226 ++++++++++++------ 1 file changed, 154 insertions(+), 72 deletions(-) diff --git a/app/src/main/java/fr/free/nrw/commons/nearby/fragments/NearbyParentFragment.kt b/app/src/main/java/fr/free/nrw/commons/nearby/fragments/NearbyParentFragment.kt index 5b52c0ce5..26875927e 100644 --- a/app/src/main/java/fr/free/nrw/commons/nearby/fragments/NearbyParentFragment.kt +++ b/app/src/main/java/fr/free/nrw/commons/nearby/fragments/NearbyParentFragment.kt @@ -123,6 +123,7 @@ import org.osmdroid.views.CustomZoomButtonsController import org.osmdroid.views.MapView import org.osmdroid.views.overlay.MapEventsOverlay import org.osmdroid.views.overlay.Marker +import org.osmdroid.views.overlay.Overlay import org.osmdroid.views.overlay.ScaleBarOverlay import org.osmdroid.views.overlay.ScaleDiskOverlay import org.osmdroid.views.overlay.TilesOverlay @@ -267,6 +268,9 @@ class NearbyParentFragment : CommonsDaggerSupportFragment(), private var dataList: MutableList? = null private var bottomSheetAdapter: BottomSheetAdapter? = null + private var userLocationOverlay: Overlay? = null + private var userLocationErrorOverlay: Overlay? = null + private val galleryPickLauncherForResult = registerForActivityResult(ActivityResultContracts.StartActivityForResult()) { result -> controller?.handleActivityResultWithCallback( @@ -724,7 +728,7 @@ class NearbyParentFragment : CommonsDaggerSupportFragment(), val targetP = GeoPoint(target.latitude, target.longitude) mapCenter = targetP binding?.map?.controller?.setCenter(targetP) - recenterMarkerToPosition(targetP) + updateUserLocationOverlays(targetP, true) if (!isCameFromExploreMap()) { moveCameraToPosition(targetP) } @@ -1863,6 +1867,8 @@ class NearbyParentFragment : CommonsDaggerSupportFragment(), lastKnownLocation = latLng NearbyController.currentLocation = lastKnownLocation presenter!!.updateMapAndList(locationChangeType) + + updateUserLocationOverlays(GeoPoint(latLng.latitude, latLng.longitude), true) } override fun onLocationChangedSignificantly(latLng: LatLng) { @@ -2644,43 +2650,14 @@ class NearbyParentFragment : CommonsDaggerSupportFragment(), */ override fun clearAllMarkers() { binding!!.map.overlayManager.clear() - binding!!.map.invalidate() - val geoPoint = mapCenter - if (geoPoint != null) { - val diskOverlay = - ScaleDiskOverlay( - this.context, - geoPoint, 2000, UnitOfMeasure.foot - ) - val circlePaint = Paint() - circlePaint.color = Color.rgb(128, 128, 128) - circlePaint.style = Paint.Style.STROKE - circlePaint.strokeWidth = 2f - diskOverlay.setCirclePaint2(circlePaint) - val diskPaint = Paint() - diskPaint.color = Color.argb(40, 128, 128, 128) - diskPaint.style = Paint.Style.FILL_AND_STROKE - diskOverlay.setCirclePaint1(diskPaint) - diskOverlay.setDisplaySizeMin(900) - diskOverlay.setDisplaySizeMax(1700) - binding!!.map.overlays.add(diskOverlay) - val startMarker = Marker( - binding!!.map - ) - startMarker.position = geoPoint - startMarker.setAnchor( - Marker.ANCHOR_CENTER, - Marker.ANCHOR_BOTTOM - ) - startMarker.icon = - getDrawable( - this.requireContext(), - fr.free.nrw.commons.R.drawable.current_location_marker - ) - startMarker.title = "Your Location" - startMarker.textLabelFontSize = 24 - binding!!.map.overlays.add(startMarker) + + var geoPoint = mapCenter + val lastLatLng = locationManager.getLastLocation() + if (lastLatLng != null) { + geoPoint = GeoPoint(lastLatLng.latitude, lastLatLng.longitude) } + updateUserLocationOverlays(geoPoint, false) + val scaleBarOverlay = ScaleBarOverlay(binding!!.map) scaleBarOverlay.setScaleBarOffset(15, 25) val barPaint = Paint() @@ -2690,6 +2667,7 @@ class NearbyParentFragment : CommonsDaggerSupportFragment(), binding!!.map.overlays.add(scaleBarOverlay) binding!!.map.overlays.add(mapEventsOverlay) binding!!.map.setMultiTouchControls(true) + binding!!.map.invalidate() } /** @@ -2700,45 +2678,149 @@ class NearbyParentFragment : CommonsDaggerSupportFragment(), private fun recenterMarkerToPosition(geoPoint: GeoPoint?) { geoPoint?.let { binding?.map?.controller?.setCenter(it) - val overlays = binding?.map?.overlays ?: return@let - // Remove markers and disks using index-based removal - var i = 0 - while (i < overlays.size) { - when (overlays[i]) { - is Marker, is ScaleDiskOverlay -> overlays.removeAt(i) - else -> i++ - } - } - - // Add disk overlay - ScaleDiskOverlay(context, it, 2000, UnitOfMeasure.foot).apply { - setCirclePaint2(Paint().apply { - color = Color.rgb(128, 128, 128) - style = Paint.Style.STROKE - strokeWidth = 2f - }) - setCirclePaint1(Paint().apply { - color = Color.argb(40, 128, 128, 128) - style = Paint.Style.FILL_AND_STROKE - }) - setDisplaySizeMin(900) - setDisplaySizeMax(1700) - overlays.add(this) - } - - // Add marker - Marker(binding?.map).apply { - position = it - setAnchor(Marker.ANCHOR_CENTER, Marker.ANCHOR_BOTTOM) - icon = getDrawable(context, R.drawable.current_location_marker) - title = "Your Location" - textLabelFontSize = 24 - overlays.add(this) - } + updateUserLocationOverlays(it, true); } } + /** + * Updates the user current location overlays (both the location and error overlays) by + * replacing any existing location overlays with new overlays at the given GeoPoint. If there + * are no existing location and error overlays, then new overlays are added. + * + * @param geoPoint The GeoPoint representing the user's current location. + * @param invalidate If true, the map overlays will be invalidated after the user + * location overlays are updated/added. If false, the overlays will not be invalidated. + */ + private fun updateUserLocationOverlays(geoPoint: GeoPoint?, invalidate: Boolean) { + geoPoint?.let{ + updateUserLocationOverlay(geoPoint) + updateUserLocationErrorOverlay(geoPoint) + } + + if (invalidate) { + binding!!.map.invalidate() + } + } + + /** + * Updates the user location error overlay by either replacing it with or adding a new one. + * + * If the user location error overlay is null, the new overlay is added. If the + * overlay is not null, it is replaced by the new overlay. + * + * @param geoPoint The GeoPoint representing the user's location + */ + private fun updateUserLocationErrorOverlay(geoPoint: GeoPoint) { + val overlays = binding?.map?.overlays ?: return + + // Multiply accuracy by 2 to get 95% confidence interval + val accuracy = getCurrentLocationAccuracy() * 2 + val overlay = createCurrentLocationErrorOverlay(this.context, geoPoint, + (accuracy).toInt(), UnitOfMeasure.meter) + + val index = overlays.indexOf(userLocationErrorOverlay) + + if (userLocationErrorOverlay == null || index == -1) { + overlays.add(overlay) + } else { + overlays[index] = overlay + } + + userLocationErrorOverlay = overlay + } + + /** + * Updates the user location overlay by either replacing it with or adding a new one. + * + * If the user location overlay is null, the new overlay is added. If the + * overlay is not null, it is replaced by the new overlay. + * + * @param geoPoint The GeoPoint representing the user's location + */ + private fun updateUserLocationOverlay(geoPoint: GeoPoint) { + val overlays = binding?.map?.overlays ?: return + + val overlay = createCurrentLocationOverlay(geoPoint) + + val index = overlays.indexOf(userLocationOverlay) + + if (userLocationOverlay == null || index == -1) { + overlays.add(overlay) + } else { + overlays[index] = overlay + } + + userLocationOverlay = overlay + } + + /** + * @return The accuracy of the current location with a confidence at the 68th percentile. + * Units are in meters. Returning 0 may indicate failure. + */ + private fun getCurrentLocationAccuracy(): Float { + var accuracy = 0f + val lastLocation = locationManager.getLastLocation() + if (lastLocation != null) { + accuracy = lastLocation.accuracy + } + + return accuracy + } + + /** + * Creates the current location overlay + * + * @param geoPoint The GeoPoint where the current location overlay will be placed. + * + * @return The current location overlay as a Marker + */ + private fun createCurrentLocationOverlay(geoPoint: GeoPoint): Marker { + val currentLocationOverlay = Marker( + binding!!.map + ) + currentLocationOverlay.position = geoPoint + currentLocationOverlay.icon = + getDrawable( + this.requireContext(), + fr.free.nrw.commons.R.drawable.current_location_marker + ) + currentLocationOverlay.title = "Your Location" + currentLocationOverlay.textLabelFontSize = 24 + currentLocationOverlay.setAnchor(0.5f, 0.5f) + + return currentLocationOverlay + } + + /** + * Creates the location error overlay to show the user how accurate the current location + * overlay is. The edge of the disk is the 95% confidence interval. + * + * @param context The Android context + * @param point The user's location as a GeoPoint + * @param value The radius of the disk + * @param unitOfMeasure The unit of measurement of the value/disk radius. + * + * @return The location error overlay as a ScaleDiskOverlay. + */ + private fun createCurrentLocationErrorOverlay(context: Context?, point: GeoPoint, value: Int, + unitOfMeasure: UnitOfMeasure): ScaleDiskOverlay { + val scaleDisk = ScaleDiskOverlay(context, point, value, unitOfMeasure) + + scaleDisk.setCirclePaint2(Paint().apply { + color = Color.rgb(128, 128, 128) + style = Paint.Style.STROKE + strokeWidth = 2f + }) + + scaleDisk.setCirclePaint1(Paint().apply { + color = Color.argb(40, 128, 128, 128) + style = Paint.Style.FILL_AND_STROKE + }) + + return scaleDisk + } + private fun moveCameraToPosition(geoPoint: GeoPoint) { binding!!.map.controller.animateTo(geoPoint) }