From 5cf918cf27736b9a9e0adfa1b0ce4239343a57d6 Mon Sep 17 00:00:00 2001 From: Shobhit Agarwal Date: Mon, 2 Oct 2023 08:25:46 +0530 Subject: [PATCH] Only animate the camera when device location is updated or user clicks (#1944) --- .../ui/common/AbstractMapContainerFragment.kt | 15 ++++++----- .../ground/ui/common/BaseMapViewModel.kt | 25 +++++++++++-------- .../HomeScreenMapContainerFragment.kt | 9 ++++++- .../ground/ui/map/CameraUpdateRequest.kt | 18 +++++++++++++ .../android/ground/ui/map/MapFragment.kt | 6 ++--- .../ground/ui/map/gms/GoogleMapsFragment.kt | 25 +++++++++++-------- 6 files changed, 67 insertions(+), 31 deletions(-) create mode 100644 ground/src/main/java/com/google/android/ground/ui/map/CameraUpdateRequest.kt diff --git a/ground/src/main/java/com/google/android/ground/ui/common/AbstractMapContainerFragment.kt b/ground/src/main/java/com/google/android/ground/ui/common/AbstractMapContainerFragment.kt index 2d4ef677ca..d77939b3a5 100644 --- a/ground/src/main/java/com/google/android/ground/ui/common/AbstractMapContainerFragment.kt +++ b/ground/src/main/java/com/google/android/ground/ui/common/AbstractMapContainerFragment.kt @@ -27,7 +27,7 @@ import com.google.android.ground.system.GeocodingManager import com.google.android.ground.system.PermissionDeniedException import com.google.android.ground.system.SettingsChangeRequestCanceled import com.google.android.ground.ui.home.mapcontainer.MapTypeDialogFragmentDirections -import com.google.android.ground.ui.map.CameraPosition +import com.google.android.ground.ui.map.CameraUpdateRequest import com.google.android.ground.ui.map.MapFragment import javax.inject.Inject import kotlin.math.max @@ -47,6 +47,7 @@ abstract class AbstractMapContainerFragment : AbstractFragment() { super.onViewCreated(view, savedInstanceState) map.attachToParent(this, R.id.map) { onMapAttached(it) } } + private fun launchWhenStarted(fn: suspend () -> Unit) { lifecycleScope.launch { repeatOnLifecycle(Lifecycle.State.STARTED) { fn.invoke() } } } @@ -125,8 +126,10 @@ abstract class AbstractMapContainerFragment : AbstractFragment() { Toast.makeText(context, messageId, Toast.LENGTH_LONG).show() } - private fun onCameraUpdateRequest(newPosition: CameraPosition, map: MapFragment) { - Timber.v("Update camera: %s", newPosition) + private fun onCameraUpdateRequest(cameraUpdateRequest: CameraUpdateRequest, map: MapFragment) { + Timber.v("Update camera: $cameraUpdateRequest") + val newPosition = cameraUpdateRequest.cameraPosition + val shouldAnimate = cameraUpdateRequest.shouldAnimate val bounds = newPosition.bounds val target = newPosition.target var zoomLevel = newPosition.zoomLevel @@ -137,11 +140,11 @@ abstract class AbstractMapContainerFragment : AbstractFragment() { // TODO(#1712): Fix this once CameraPosition is refactored to not contain duplicated state if (bounds != null) { - map.moveCamera(bounds) + map.moveCamera(bounds, shouldAnimate) } else if (target != null && zoomLevel != null) { - map.moveCamera(target, zoomLevel) + map.moveCamera(target, zoomLevel, shouldAnimate) } else if (target != null) { - map.moveCamera(target) + map.moveCamera(target, shouldAnimate) } else { error("Must have either target or bounds set") } diff --git a/ground/src/main/java/com/google/android/ground/ui/common/BaseMapViewModel.kt b/ground/src/main/java/com/google/android/ground/ui/common/BaseMapViewModel.kt index 857c72c75a..42e18486b4 100644 --- a/ground/src/main/java/com/google/android/ground/ui/common/BaseMapViewModel.kt +++ b/ground/src/main/java/com/google/android/ground/ui/common/BaseMapViewModel.kt @@ -40,6 +40,7 @@ import com.google.android.ground.system.PermissionDeniedException import com.google.android.ground.system.PermissionsManager import com.google.android.ground.system.SettingsManager import com.google.android.ground.ui.map.CameraPosition +import com.google.android.ground.ui.map.CameraUpdateRequest import com.google.android.ground.ui.map.MapType import com.google.android.ground.ui.map.gms.GmsExt.toBounds import com.google.android.ground.ui.map.gms.toCoordinates @@ -71,7 +72,7 @@ constructor( @IoDispatcher private val ioDispatcher: CoroutineDispatcher, ) : AbstractViewModel() { - private val _cameraPosition = MutableStateFlow(null) + private val _cameraUpdateRequests = MutableStateFlow(null) val locationLock: MutableStateFlow> = MutableStateFlow(Result.success(mapStateRepository.isLocationLockEnabled)) @@ -191,7 +192,7 @@ constructor( } /** Emits a stream of camera update requests. */ - fun getCameraUpdateRequests(): Flow = _cameraPosition.filterNotNull() + fun getCameraUpdateRequests(): Flow = _cameraUpdateRequests.filterNotNull() /** Emits a stream of current camera position. */ fun getCurrentCameraPosition(): Flow = currentCameraPosition.filterNotNull() @@ -220,7 +221,7 @@ constructor( surveyRepository.activeSurveyFlow .filterNotNull() .transform { getLastSavedPositionOrDefaultBounds(it)?.apply { emit(this) } } - .collect { updateMapCamera(it) } + .collect { setCameraPosition(it, false) } } private suspend fun getLastSavedPositionOrDefaultBounds(survey: Survey): CameraPosition? { @@ -235,18 +236,22 @@ constructor( return geometries.toBounds()?.let { CameraPosition(bounds = it) } } - /** Requests moving the map camera to [coordinates]. */ private fun panCamera(coordinates: Coordinates) { - updateMapCamera(CameraPosition(coordinates)) + setCameraPosition(CameraPosition(coordinates), true) } - /** Requests moving the map camera to [coordinates] with zoom level [DEFAULT_LOI_ZOOM_LEVEL]. */ - fun panAndZoomCamera(coordinates: Coordinates) { - updateMapCamera(CameraPosition(coordinates, DEFAULT_LOI_ZOOM_LEVEL)) + private fun panAndZoomCamera(coordinates: Coordinates) { + setCameraPosition(CameraPosition(coordinates, DEFAULT_LOI_ZOOM_LEVEL), true) } - private fun updateMapCamera(cameraPosition: CameraPosition) { - _cameraPosition.value = cameraPosition + /** + * Requests moving the map camera to the given position. + * + * @param cameraPosition new position + * @param shouldAnimate whether to animate the map camera or not + */ + fun setCameraPosition(cameraPosition: CameraPosition, shouldAnimate: Boolean) { + _cameraUpdateRequests.value = CameraUpdateRequest(cameraPosition, shouldAnimate) } /** Called when the map camera is moved. */ diff --git a/ground/src/main/java/com/google/android/ground/ui/home/mapcontainer/HomeScreenMapContainerFragment.kt b/ground/src/main/java/com/google/android/ground/ui/home/mapcontainer/HomeScreenMapContainerFragment.kt index 0f268d80e6..0f56a0931f 100644 --- a/ground/src/main/java/com/google/android/ground/ui/home/mapcontainer/HomeScreenMapContainerFragment.kt +++ b/ground/src/main/java/com/google/android/ground/ui/home/mapcontainer/HomeScreenMapContainerFragment.kt @@ -24,6 +24,7 @@ import androidx.recyclerview.widget.LinearLayoutManager import androidx.recyclerview.widget.PagerSnapHelper import androidx.recyclerview.widget.RecyclerView import androidx.recyclerview.widget.SnapHelper +import com.google.android.ground.Config import com.google.android.ground.R import com.google.android.ground.coroutines.IoDispatcher import com.google.android.ground.databinding.BasemapLayoutBinding @@ -41,6 +42,7 @@ import com.google.android.ground.ui.home.HomeScreenFragmentDirections import com.google.android.ground.ui.home.HomeScreenViewModel import com.google.android.ground.ui.home.mapcontainer.cards.MapCardAdapter import com.google.android.ground.ui.home.mapcontainer.cards.MapCardUiData +import com.google.android.ground.ui.map.CameraPosition import com.google.android.ground.ui.map.MapFragment import dagger.hilt.android.AndroidEntryPoint import javax.inject.Inject @@ -216,7 +218,12 @@ class HomeScreenMapContainerFragment : Hilt_HomeScreenMapContainerFragment() { state.locationOfInterest ?.geometry ?.takeIf { it is Point } - ?.let { mapContainerViewModel.panAndZoomCamera(it.center()) } + ?.let { + mapContainerViewModel.setCameraPosition( + CameraPosition(it.center(), Config.DEFAULT_LOI_ZOOM_LEVEL), + false + ) + } } BottomSheetState.Visibility.HIDDEN -> { map.enableGestures() diff --git a/ground/src/main/java/com/google/android/ground/ui/map/CameraUpdateRequest.kt b/ground/src/main/java/com/google/android/ground/ui/map/CameraUpdateRequest.kt new file mode 100644 index 0000000000..5ff0ff66b8 --- /dev/null +++ b/ground/src/main/java/com/google/android/ground/ui/map/CameraUpdateRequest.kt @@ -0,0 +1,18 @@ +/* + * Copyright 2023 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.google.android.ground.ui.map + +data class CameraUpdateRequest(val cameraPosition: CameraPosition, val shouldAnimate: Boolean) diff --git a/ground/src/main/java/com/google/android/ground/ui/map/MapFragment.kt b/ground/src/main/java/com/google/android/ground/ui/map/MapFragment.kt index 5cf7b3de1b..ba9d43e39a 100644 --- a/ground/src/main/java/com/google/android/ground/ui/map/MapFragment.kt +++ b/ground/src/main/java/com/google/android/ground/ui/map/MapFragment.kt @@ -66,18 +66,18 @@ interface MapFragment { fun disableGestures() /** Centers the map viewport around the specified [Coordinates]. */ - fun moveCamera(coordinates: Coordinates) + fun moveCamera(coordinates: Coordinates, shouldAnimate: Boolean) /** * Centers the map viewport around the specified [Coordinates] and updates the map's current zoom * level. */ - fun moveCamera(coordinates: Coordinates, zoomLevel: Float) + fun moveCamera(coordinates: Coordinates, zoomLevel: Float, shouldAnimate: Boolean) /** * Centers the map viewport around the specified [Bounds] are included at the least zoom level. */ - fun moveCamera(bounds: Bounds) + fun moveCamera(bounds: Bounds, shouldAnimate: Boolean) /** Displays user location indicator on the map. */ @SuppressLint("MissingPermission") fun enableCurrentLocationIndicator() diff --git a/ground/src/main/java/com/google/android/ground/ui/map/gms/GoogleMapsFragment.kt b/ground/src/main/java/com/google/android/ground/ui/map/gms/GoogleMapsFragment.kt index d775e58819..3dc7f994d6 100644 --- a/ground/src/main/java/com/google/android/ground/ui/map/gms/GoogleMapsFragment.kt +++ b/ground/src/main/java/com/google/android/ground/ui/map/gms/GoogleMapsFragment.kt @@ -26,6 +26,7 @@ import androidx.annotation.IdRes import androidx.core.view.ViewCompat import androidx.core.view.WindowInsetsCompat import androidx.lifecycle.lifecycleScope +import com.google.android.gms.maps.CameraUpdate import com.google.android.gms.maps.CameraUpdateFactory import com.google.android.gms.maps.GoogleMap import com.google.android.gms.maps.GoogleMap.OnCameraMoveStartedListener @@ -41,7 +42,6 @@ import com.google.android.ground.model.imagery.TileSource.Type.TILED_WEB_MAP import com.google.android.ground.ui.common.AbstractFragment import com.google.android.ground.ui.map.* import com.google.android.ground.ui.map.CameraPosition -import com.google.android.ground.ui.map.MapFragment import com.google.android.ground.ui.map.gms.GmsExt.toBounds import com.google.android.ground.ui.map.gms.mog.MogCollection import com.google.android.ground.ui.map.gms.mog.MogTileProvider @@ -152,7 +152,7 @@ class GoogleMapsFragment : Hilt_GoogleMapsFragment(), MapFragment { viewGroup: ViewGroup?, bundle: Bundle? ): View = - super.onCreateView(layoutInflater, viewGroup, bundle)!!.apply { + super.onCreateView(layoutInflater, viewGroup, bundle).apply { ViewCompat.setOnApplyWindowInsetsListener(this) { view, insets -> onApplyWindowInsets(view, insets) } @@ -209,7 +209,7 @@ class GoogleMapsFragment : Hilt_GoogleMapsFragment(), MapFragment { private fun onClusterItemClick(cluster: Cluster): Boolean { // Move the camera to point to LOIs within the current cluster - cluster.items.map { it.feature.geometry }.toBounds()?.let { moveCamera(it) } + cluster.items.map { it.feature.geometry }.toBounds()?.let { moveCamera(it, true) } return true } @@ -226,17 +226,20 @@ class GoogleMapsFragment : Hilt_GoogleMapsFragment(), MapFragment { override fun disableGestures() = map.uiSettings.setAllGesturesEnabled(false) - override fun moveCamera(coordinates: Coordinates) = - map.animateCamera(CameraUpdateFactory.newLatLng(coordinates.toGoogleMapsObject())) + override fun moveCamera(coordinates: Coordinates, shouldAnimate: Boolean) = + moveCamera(CameraUpdateFactory.newLatLng(coordinates.toGoogleMapsObject()), shouldAnimate) - override fun moveCamera(coordinates: Coordinates, zoomLevel: Float) = - map.animateCamera( - CameraUpdateFactory.newLatLngZoom(coordinates.toGoogleMapsObject(), zoomLevel) + override fun moveCamera(coordinates: Coordinates, zoomLevel: Float, shouldAnimate: Boolean) = + moveCamera( + CameraUpdateFactory.newLatLngZoom(coordinates.toGoogleMapsObject(), zoomLevel), + shouldAnimate ) - override fun moveCamera(bounds: Bounds) { - map.animateCamera(CameraUpdateFactory.newLatLngBounds(bounds.toGoogleMapsObject(), 100)) - } + override fun moveCamera(bounds: Bounds, shouldAnimate: Boolean) = + moveCamera(CameraUpdateFactory.newLatLngBounds(bounds.toGoogleMapsObject(), 100), shouldAnimate) + + private fun moveCamera(cameraUpdate: CameraUpdate, shouldAnimate: Boolean) = + if (shouldAnimate) map.animateCamera(cameraUpdate) else map.moveCamera(cameraUpdate) private fun getCustomCap(): CustomCap { if (customCap == null) {