improved reverse geocoding + misc fixes

android/app/src/main/kotlin/deckers/thibault/aves/utils/StorageUtils.kt

@@ -149,7 +149,7 @@ object StorageUtils {
         return paths.map { ensureTrailingSeparator(it) }.toTypedArray()
     }
 
-    // return physicalPaths based on phone model
+    // returns physicalPaths based on phone model
     @SuppressLint("SdCardPath")
     private val physicalPaths = arrayOf(
         "/storage/sdcard0",

lib/geo/countries.dart

@@ -17,12 +17,12 @@ class CountryTopology {
 
   Future<Topology> getTopology() => _topology != null ? SynchronousFuture(_topology) : rootBundle.loadString(topoJsonAsset).then(TopoJson().parse);
 
-  // return the country containing given coordinates
+  // returns the country containing given coordinates
   Future<CountryCode> countryCode(LatLng position) async {
     return _countryOfNumeric(await numericCode(position));
   }
 
-  // return the ISO 3166-1 numeric code of the country containing given coordinates
+  // returns the ISO 3166-1 numeric code of the country containing given coordinates
   Future<int> numericCode(LatLng position) async {
     final topology = await getTopology();
     if (topology == null) return null;
@@ -31,7 +31,7 @@ class CountryTopology {
     return _getNumeric(topology, countries, position);
   }
 
-  // return a map of the given positions by country
+  // returns a map of the given positions by country
   Future<Map<CountryCode, Set<LatLng>>> countryCodeMap(Set<LatLng> positions) async {
     final numericMap = await numericCodeMap(positions);
     numericMap.remove(null);
@@ -43,7 +43,7 @@ class CountryTopology {
     return codeMap;
   }
 
-  // return a map of the given positions by the ISO 3166-1 numeric code of the country containing them
+  // returns a map of the given positions by the ISO 3166-1 numeric code of the country containing them
   Future<Map<int, Set<LatLng>>> numericCodeMap(Set<LatLng> positions) async {
     final topology = await getTopology();
     if (topology == null) return null;
@@ -68,10 +68,18 @@ class CountryTopology {
     return null;
   }
 
-  static int _getNumeric(Topology topology, List<Geometry> countries, LatLng position) {
+  static int _getNumeric(Topology topology, List<Geometry> mruCountries, LatLng position) {
     final point = [position.longitude, position.latitude];
-    final hit = countries.firstWhere((country) => country.containsPoint(topology, point), orElse: () => null);
+    final hit = mruCountries.firstWhere((country) => country.containsPoint(topology, point), orElse: () => null);
-    final idString = (hit?.id as String);
+    if (hit == null) return null;
+
+    // promote hit countries, assuming given positions are likely to come from the same countries
+    if (mruCountries.first != hit) {
+      mruCountries.remove(hit);
+      mruCountries.insert(0, hit);
+    }
+
+    final idString = (hit.id as String);
     final code = idString == null ? null : int.tryParse(idString);
     return code;
   }

lib/geo/format.dart

@@ -21,7 +21,7 @@ String _decimal2sexagesimal(final double degDecimal) {
   return '$deg° $min′ ${roundToPrecision(sec, decimals: 2).toStringAsFixed(2)}″';
 }
 
-// return coordinates formatted as DMS, e.g. ['41° 24′ 12.2″ N', '2° 10′ 26.5″ E']
+// returns coordinates formatted as DMS, e.g. ['41° 24′ 12.2″ N', '2° 10′ 26.5″ E']
 List<String> toDMS(LatLng latLng) {
   if (latLng == null) return [];
   final lat = latLng.latitude;

lib/model/entry.dart

@@ -50,7 +50,7 @@ class AvesEntry {
   final Future<List<Address>> Function(Coordinates coordinates) _findAddresses = Geocoder.local.findAddressesFromCoordinates;
 
   // TODO TLAD make it dynamic if it depends on OS/lib versions
-  static const List<String> undecodable = [MimeTypes.crw, MimeTypes.psd];
+  static const List<String> undecodable = [MimeTypes.crw, MimeTypes.djvu, MimeTypes.psd];
 
   AvesEntry({
     this.uri,
@@ -289,6 +289,8 @@ class AvesEntry {
   static const ratioSeparator = '\u2236';
   static const resolutionSeparator = ' \u00D7 ';
 
+  bool get isSized => (width ?? 0) > 0 && (height ?? 0) > 0;
+
   String get resolutionText {
     final ws = width ?? '?';
     final hs = height ?? '?';
@@ -369,11 +371,15 @@ class AvesEntry {
     return _durationText;
   }
 
-  bool get hasGps => _catalogMetadata?.latitude != null;
+  // returns whether this entry has GPS coordinates
+  // (0, 0) coordinates are considered invalid, as it is likely a default value
+  bool get hasGps => _catalogMetadata != null && _catalogMetadata.latitude != null && _catalogMetadata.longitude != null && (_catalogMetadata.latitude != 0 || _catalogMetadata.longitude != 0);
 
   bool get hasAddress => _addressDetails != null;
 
-  bool get hasPlace => _addressDetails?.place?.isNotEmpty == true;
+  // has a place, or at least the full country name
+  // derived from Google reverse geocoding addresses
+  bool get hasFineAddress => _addressDetails != null && (_addressDetails.place?.isNotEmpty == true || (_addressDetails.countryName?.length ?? 0) > 3);
 
   LatLng get latLng => hasGps ? LatLng(_catalogMetadata.latitude, _catalogMetadata.longitude) : null;
 
@@ -449,22 +455,23 @@ class AvesEntry {
     addressChangeNotifier.notifyListeners();
   }
 
-  Future<void> locate() async {
+  Future<void> locate({@required bool background}) async {
+    if (!hasGps) return;
     await _locateCountry();
     if (await availability.canLocatePlaces) {
-      await locatePlace(background: false);
+      await locatePlace(background: background);
     }
   }
 
   // quick reverse geocoding to find the country, using an offline asset
   Future<void> _locateCountry() async {
-    if (hasAddress) return;
+    if (!hasGps || hasAddress) return;
     final countryCode = await countryTopology.countryCode(latLng);
     setCountry(countryCode);
   }
 
   void setCountry(CountryCode countryCode) {
-    if (hasPlace || countryCode == null) return;
+    if (hasFineAddress || countryCode == null) return;
     addressDetails = AddressDetails(
       contentId: contentId,
       countryCode: countryCode.alpha2,
@@ -474,16 +481,10 @@ class AvesEntry {
 
   // full reverse geocoding, requiring Play Services and some connectivity
   Future<void> locatePlace({@required bool background}) async {
-    if (hasPlace) return;
+    if (!hasGps || hasFineAddress) return;
-
+    final coordinates = latLng;
-    await catalog(background: background);
-    final latitude = _catalogMetadata?.latitude;
-    final longitude = _catalogMetadata?.longitude;
-    if (latitude == null || longitude == null || (latitude == 0 && longitude == 0)) return;
-
-    final coordinates = Coordinates(latitude, longitude);
     try {
-      Future<List<Address>> call() => _findAddresses(coordinates);
+      Future<List<Address>> call() => _findAddresses(Coordinates(coordinates.latitude, coordinates.longitude));
       final addresses = await (background
           ? servicePolicy.call(
               call,
@@ -511,13 +512,11 @@ class AvesEntry {
   }
 
   Future<String> findAddressLine() async {
-    final latitude = _catalogMetadata?.latitude;
+    if (!hasGps) return null;
-    final longitude = _catalogMetadata?.longitude;
-    if (latitude == null || longitude == null || (latitude == 0 && longitude == 0)) return null;
 
-    final coordinates = Coordinates(latitude, longitude);
+    final coordinates = latLng;
     try {
-      final addresses = await _findAddresses(coordinates);
+      final addresses = await _findAddresses(Coordinates(coordinates.latitude, coordinates.longitude));
       if (addresses != null && addresses.isNotEmpty) {
         final address = addresses.first;
         return address.addressLine;

lib/model/source/collection_source.dart

@@ -23,6 +23,8 @@ mixin SourceBase {
 
   List<AvesEntry> get sortedEntriesByDate;
 
+  ValueNotifier<SourceState> stateNotifier = ValueNotifier(SourceState.ready);
+
   final StreamController<ProgressEvent> _progressStreamController = StreamController.broadcast();
 
   Stream<ProgressEvent> get progressStream => _progressStreamController.stream;
@@ -58,8 +60,6 @@ abstract class CollectionSource with SourceBase, AlbumMixin, LocationMixin, TagM
     return _sortedEntriesByDate;
   }
 
-  ValueNotifier<SourceState> stateNotifier = ValueNotifier(SourceState.ready);
-
   List<DateMetadata> _savedDates;
 
   Future<void> loadDates() async {

lib/model/source/location.dart

@@ -35,9 +35,14 @@ mixin LocationMixin on SourceBase {
 
   // quick reverse geocoding to find the countries, using an offline asset
   Future<void> _locateCountries() async {
-    final todo = visibleEntries.where((entry) => entry.hasGps && entry.addressDetails?.countryCode == null).toSet();
+    final todo = visibleEntries.where((entry) => entry.hasGps && !entry.hasAddress).toSet();
     if (todo.isEmpty) return;
 
+    stateNotifier.value = SourceState.locating;
+    var progressDone = 0;
+    final progressTotal = todo.length;
+    setProgress(done: progressDone, total: progressTotal);
+
     // final stopwatch = Stopwatch()..start();
     final countryCodeMap = await countryTopology.countryCodeMap(todo.map((entry) => entry.latLng).toSet());
     final newAddresses = <AddressDetails>[];
@@ -48,12 +53,13 @@ mixin LocationMixin on SourceBase {
       if (entry.hasAddress) {
         newAddresses.add(entry.addressDetails);
       }
+      setProgress(done: ++progressDone, total: progressTotal);
     });
     if (newAddresses.isNotEmpty) {
       await metadataDb.saveAddresses(List.unmodifiable(newAddresses));
       onAddressMetadataChanged();
     }
-    // debugPrint('$runtimeType _locateCountries complete in ${stopwatch.elapsed.inSeconds}s');
+    // debugPrint('$runtimeType _locateCountries complete in ${stopwatch.elapsed.inMilliseconds}ms');
   }
 
   // full reverse geocoding, requiring Play Services and some connectivity
@@ -61,7 +67,7 @@ mixin LocationMixin on SourceBase {
     if (!(await availability.canLocatePlaces)) return;
 
     // final stopwatch = Stopwatch()..start();
-    final byLocated = groupBy<AvesEntry, bool>(visibleEntries.where((entry) => entry.hasGps), (entry) => entry.hasPlace);
+    final byLocated = groupBy<AvesEntry, bool>(visibleEntries.where((entry) => entry.hasGps), (entry) => entry.hasFineAddress);
     final todo = byLocated[false] ?? [];
     if (todo.isEmpty) return;
 
@@ -85,6 +91,7 @@ mixin LocationMixin on SourceBase {
     final knownLocations = <Tuple2, AddressDetails>{};
     byLocated[true]?.forEach((entry) => knownLocations.putIfAbsent(approximateLatLng(entry), () => entry.addressDetails));
 
+    stateNotifier.value = SourceState.locating;
     var progressDone = 0;
     final progressTotal = todo.length;
     setProgress(done: progressDone, total: progressTotal);
@@ -100,7 +107,7 @@ mixin LocationMixin on SourceBase {
         // so that we skip geocoding of following entries with the same coordinates
         knownLocations[latLng] = entry.addressDetails;
       }
-      if (entry.hasPlace) {
+      if (entry.hasFineAddress) {
         newAddresses.add(entry.addressDetails);
         if (newAddresses.length >= _commitCountThreshold) {
           await metadataDb.saveAddresses(List.unmodifiable(newAddresses));
@@ -147,7 +154,8 @@ mixin LocationMixin on SourceBase {
       _filterEntryCountMap.clear();
       _filterRecentEntryMap.clear();
     } else {
-      final countryCodes = entries.where((entry) => entry.hasPlace).map((entry) => entry.addressDetails.countryCode).toSet();
+      final countryCodes = entries.where((entry) => entry.hasAddress).map((entry) => entry.addressDetails.countryCode).toSet();
+      countryCodes.remove(null);
       countryCodes.forEach(_filterEntryCountMap.remove);
     }
   }

lib/model/source/media_store_source.dart

@@ -13,7 +13,6 @@ import 'package:aves/utils/android_file_utils.dart';
 import 'package:aves/utils/math_utils.dart';
 import 'package:firebase_analytics/firebase_analytics.dart';
 import 'package:flutter/material.dart';
-import 'package:pedantic/pedantic.dart';
 
 class MediaStoreSource extends CollectionSource {
   bool _initialized = false;
@@ -103,25 +102,25 @@ class MediaStoreSource extends CollectionSource {
           updateDirectories();
         }
 
-        final analytics = FirebaseAnalytics();
-        unawaited(analytics.setUserProperty(name: 'local_item_count', value: (ceilBy(allEntries.length, 3)).toString()));
-        unawaited(analytics.setUserProperty(name: 'album_count', value: (ceilBy(rawAlbums.length, 1)).toString()));
-
-        stateNotifier.value = SourceState.cataloguing;
         await catalogEntries();
-        unawaited(analytics.setUserProperty(name: 'tag_count', value: (ceilBy(sortedTags.length, 1)).toString()));
-
-        stateNotifier.value = SourceState.locating;
         await locateEntries();
-        unawaited(analytics.setUserProperty(name: 'country_count', value: (ceilBy(sortedCountries.length, 1)).toString()));
-
         stateNotifier.value = SourceState.ready;
+
+        _reportCollectionDimensions();
         debugPrint('$runtimeType refresh done, elapsed=${stopwatch.elapsed}');
       },
       onError: (error) => debugPrint('$runtimeType stream error=$error'),
     );
   }
 
+  void _reportCollectionDimensions() {
+    final analytics = FirebaseAnalytics();
+    analytics.setUserProperty(name: 'local_item_count', value: (ceilBy(allEntries.length, 3)).toString());
+    analytics.setUserProperty(name: 'album_count', value: (ceilBy(rawAlbums.length, 1)).toString());
+    analytics.setUserProperty(name: 'tag_count', value: (ceilBy(sortedTags.length, 1)).toString());
+    analytics.setUserProperty(name: 'country_count', value: (ceilBy(sortedCountries.length, 1)).toString());
+  }
+
   // returns URIs to retry later. They could be URIs that are:
   // 1) currently being processed during bulk move/deletion
   // 2) registered in the Media Store but still being processed by their owner in a temporary location
@@ -178,13 +177,8 @@ class MediaStoreSource extends CollectionSource {
       addEntries(newEntries);
       await metadataDb.saveEntries(newEntries);
       cleanEmptyAlbums(existingDirectories);
-
-      stateNotifier.value = SourceState.cataloguing;
       await catalogEntries();
-
-      stateNotifier.value = SourceState.locating;
       await locateEntries();
-
       stateNotifier.value = SourceState.ready;
     }
 

lib/model/source/tag.dart

@@ -27,6 +27,7 @@ mixin TagMixin on SourceBase {
     final todo = visibleEntries.where((entry) => !entry.isCatalogued).toList();
     if (todo.isEmpty) return;
 
+    stateNotifier.value = SourceState.cataloguing;
     var progressDone = 0;
     final progressTotal = todo.length;
     setProgress(done: progressDone, total: progressTotal);

lib/ref/mime_types.dart

@@ -18,6 +18,7 @@ class MimeTypes {
   static const cr2 = 'image/x-canon-cr2';
   static const crw = 'image/x-canon-crw';
   static const dcr = 'image/x-kodak-dcr';
+  static const djvu = 'image/vnd.djvu';
   static const dng = 'image/x-adobe-dng';
   static const erf = 'image/x-epson-erf';
   static const k25 = 'image/x-kodak-k25';

lib/services/android_debug_service.dart

@@ -28,7 +28,7 @@ class AndroidDebugService {
 
   static Future<Map> getBitmapFactoryInfo(AvesEntry entry) async {
     try {
-      // return map with all data available when decoding image bounds with `BitmapFactory`
+      // returns map with all data available when decoding image bounds with `BitmapFactory`
       final result = await platform.invokeMethod('getBitmapFactoryInfo', <String, dynamic>{
         'uri': entry.uri,
       }) as Map;
@@ -41,7 +41,7 @@ class AndroidDebugService {
 
   static Future<Map> getContentResolverMetadata(AvesEntry entry) async {
     try {
-      // return map with all data available from the content resolver
+      // returns map with all data available from the content resolver
       final result = await platform.invokeMethod('getContentResolverMetadata', <String, dynamic>{
         'mimeType': entry.mimeType,
         'uri': entry.uri,
@@ -55,7 +55,7 @@ class AndroidDebugService {
 
   static Future<Map> getExifInterfaceMetadata(AvesEntry entry) async {
     try {
-      // return map with all data available from the `ExifInterface` library
+      // returns map with all data available from the `ExifInterface` library
       final result = await platform.invokeMethod('getExifInterfaceMetadata', <String, dynamic>{
         'mimeType': entry.mimeType,
         'uri': entry.uri,
@@ -70,7 +70,7 @@ class AndroidDebugService {
 
   static Future<Map> getMediaMetadataRetrieverMetadata(AvesEntry entry) async {
     try {
-      // return map with all data available from `MediaMetadataRetriever`
+      // returns map with all data available from `MediaMetadataRetriever`
       final result = await platform.invokeMethod('getMediaMetadataRetrieverMetadata', <String, dynamic>{
         'uri': entry.uri,
       }) as Map;
@@ -83,7 +83,7 @@ class AndroidDebugService {
 
   static Future<Map> getMetadataExtractorSummary(AvesEntry entry) async {
     try {
-      // return map with the mime type and tag count for each directory found by `metadata-extractor`
+      // returns map with the mime type and tag count for each directory found by `metadata-extractor`
       final result = await platform.invokeMethod('getMetadataExtractorSummary', <String, dynamic>{
         'mimeType': entry.mimeType,
         'uri': entry.uri,

lib/services/android_file_service.dart

@@ -95,7 +95,7 @@ class AndroidFileService {
     return false;
   }
 
-  // return media URI
+  // returns media URI
   static Future<Uri> scanFile(String path, String mimeType) async {
     debugPrint('scanFile with path=$path, mimeType=$mimeType');
     try {

lib/services/image_file_service.dart

@@ -248,7 +248,7 @@ class ImageFileService {
 
   static Future<Map> rename(AvesEntry entry, String newName) async {
     try {
-      // return map with: 'contentId' 'path' 'title' 'uri' (all optional)
+      // returns map with: 'contentId' 'path' 'title' 'uri' (all optional)
       final result = await platform.invokeMethod('rename', <String, dynamic>{
         'entry': _toPlatformEntryMap(entry),
         'newName': newName,
@@ -262,7 +262,7 @@ class ImageFileService {
 
   static Future<Map> rotate(AvesEntry entry, {@required bool clockwise}) async {
     try {
-      // return map with: 'rotationDegrees' 'isFlipped'
+      // returns map with: 'rotationDegrees' 'isFlipped'
       final result = await platform.invokeMethod('rotate', <String, dynamic>{
         'entry': _toPlatformEntryMap(entry),
         'clockwise': clockwise,
@@ -276,7 +276,7 @@ class ImageFileService {
 
   static Future<Map> flip(AvesEntry entry) async {
     try {
-      // return map with: 'rotationDegrees' 'isFlipped'
+      // returns map with: 'rotationDegrees' 'isFlipped'
       final result = await platform.invokeMethod('flip', <String, dynamic>{
         'entry': _toPlatformEntryMap(entry),
       }) as Map;

lib/services/metadata_service.dart

@@ -11,7 +11,7 @@ import 'package:flutter/services.dart';
 class MetadataService {
   static const platform = MethodChannel('deckers.thibault/aves/metadata');
 
-  // return Map<Map<Key, Value>> (map of directories, each directory being a map of metadata label and value description)
+  // returns Map<Map<Key, Value>> (map of directories, each directory being a map of metadata label and value description)
   static Future<Map> getAllMetadata(AvesEntry entry) async {
     if (entry.isSvg) return null;
 
@@ -33,7 +33,7 @@ class MetadataService {
 
     Future<CatalogMetadata> call() async {
       try {
-        // return map with:
+        // returns map with:
         // 'mimeType': MIME type as reported by metadata extractors, not Media Store (string)
         // 'dateMillis': date taken in milliseconds since Epoch (long)
         // 'isAnimated': animated gif/webp (bool)
@@ -69,7 +69,7 @@ class MetadataService {
     if (entry.isSvg) return null;
 
     try {
-      // return map with values for: 'aperture' (double), 'exposureTime' (description), 'focalLength' (double), 'iso' (int)
+      // returns map with values for: 'aperture' (double), 'exposureTime' (description), 'focalLength' (double), 'iso' (int)
       final result = await platform.invokeMethod('getOverlayMetadata', <String, dynamic>{
         'mimeType': entry.mimeType,
         'uri': entry.uri,
@@ -98,7 +98,7 @@ class MetadataService {
 
   static Future<PanoramaInfo> getPanoramaInfo(AvesEntry entry) async {
     try {
-      // return map with values for:
+      // returns map with values for:
       // 'croppedAreaLeft' (int), 'croppedAreaTop' (int), 'croppedAreaWidth' (int), 'croppedAreaHeight' (int),
       // 'fullPanoWidth' (int), 'fullPanoHeight' (int)
       final result = await platform.invokeMethod('getPanoramaInfo', <String, dynamic>{

lib/services/viewer_service.dart

@@ -6,7 +6,7 @@ class ViewerService {
 
   static Future<Map> getIntentData() async {
     try {
-      // return nullable map with 'action' and possibly 'uri' 'mimeType'
+      // returns nullable map with 'action' and possibly 'uri' 'mimeType'
       return await platform.invokeMethod('getIntentData') as Map;
     } on PlatformException catch (e) {
       debugPrint('getIntentData failed with code=${e.code}, exception=${e.message}, details=${e.details}');

lib/widgets/debug/app_debug_page.dart

@@ -64,7 +64,7 @@ class _AppDebugPageState extends State<AppDebugPage> {
     final catalogued = visibleEntries.where((entry) => entry.isCatalogued);
     final withGps = catalogued.where((entry) => entry.hasGps);
     final withAddress = withGps.where((entry) => entry.hasAddress);
-    final withPlace = withGps.where((entry) => entry.hasPlace);
+    final withFineAddress = withGps.where((entry) => entry.hasFineAddress);
     return AvesExpansionTile(
       title: 'General',
       children: [
@@ -106,7 +106,7 @@ class _AppDebugPageState extends State<AppDebugPage> {
               'Catalogued': '${catalogued.length}',
               'With GPS': '${withGps.length}',
               'With address': '${withAddress.length}',
-              'With place': '${withPlace.length}',
+              'With fine address': '${withFineAddress.length}',
             },
           ),
         ),

lib/widgets/viewer/debug_page.dart

@@ -107,7 +107,7 @@ class ViewerDebugPage extends StatelessWidget {
         InfoRowGroup({
           'hasGps': '${entry.hasGps}',
           'hasAddress': '${entry.hasAddress}',
-          'hasPlace': '${entry.hasPlace}',
+          'hasFineAddress': '${entry.hasFineAddress}',
           'latLng': '${entry.latLng}',
           'geoUri': '${entry.geoUri}',
         }),

lib/widgets/viewer/entry_vertical_pager.dart

@@ -151,7 +151,7 @@ class _ViewerVerticalPageViewState extends State<ViewerVerticalPageView> {
       // make sure to locate the entry,
       // so that we can display the address instead of coordinates
       // even when initial collection locating has not reached this entry yet
-      entry.locate();
+      entry.catalog(background: false).then((_) => entry.locate(background: false));
     } else {
       Navigator.pop(context);
     }

lib/widgets/viewer/info/basic_section.dart

@@ -55,7 +55,7 @@ class BasicSection extends StatelessWidget {
           'Title': title,
           'Date': dateText,
           if (entry.isVideo) ..._buildVideoRows(),
-          if (!entry.isSvg) 'Resolution': rasterResolutionText,
+          if (!entry.isSvg && entry.isSized) 'Resolution': rasterResolutionText,
           'Size': entry.sizeBytes != null ? formatFilesize(entry.sizeBytes) : Constants.infoUnknown,
           'URI': uri,
           if (path != null) 'Path': path,

lib/widgets/viewer/overlay/bottom.dart

@@ -386,7 +386,7 @@ class _DateRow extends StatelessWidget {
   Widget build(BuildContext context) {
     final date = entry.bestDate;
     final dateText = date != null ? '${DateFormat.yMMMd().format(date)} • ${DateFormat.Hm().format(date)}' : Constants.overlayUnknown;
-    final resolutionText = entry.isSvg ? entry.aspectRatioText : entry.resolutionText;
+    final resolutionText = entry.isSvg ? entry.aspectRatioText : entry.isSized ? entry.resolutionText : '';
 
     return Row(
       children: [