Fixed bug in bulk downloader where the final tile event was repeated …

…without `isRepeat` set Undeprecated `DownloadableRegion.(maybe)When`
JaffaKetchup · Oct 14, 2024 · eae1ed7 · eae1ed7
1 parent 458a6c3
commit eae1ed7
Show file tree

Hide file tree

Showing 9 changed files with 75 additions and 102 deletions.
diff --git a/example/lib/main.dart b/example/lib/main.dart
@@ -9,10 +9,10 @@ import 'src/screens/initialisation_error/initialisation_error.dart';
 import 'src/screens/main/main.dart';
 import 'src/screens/main/secondary_view/contents/home/components/stores_list/state/export_selection_provider.dart';
 import 'src/screens/old/configure_download/configure_download.dart';
-import 'src/shared/state/download_configuration_provider.dart';
 import 'src/screens/old/download/download.dart';
 import 'src/screens/store_editor/store_editor.dart';
 import 'src/shared/misc/shared_preferences.dart';
+import 'src/shared/state/download_configuration_provider.dart';
 import 'src/shared/state/general_provider.dart';
 import 'src/shared/state/region_selection_provider.dart';
 

diff --git a/...b/src/screens/main/map_view/components/download_progress/components/greyscale_masker.dart b/...b/src/screens/main/map_view/components/download_progress/components/greyscale_masker.dart
@@ -246,18 +246,12 @@ class _GreyscaleMaskerRenderer extends RenderProxyBox {
 
         final _TileMappingValue tmv;
         if (_tileMapping[intermediateZoomTile] case final existingTMV?) {
-          try {
-            assert(
-              existingTMV.subtilesCount < maxSubtilesCount,
-              'Existing subtiles count must be smaller than max subtiles count '
-              '($intermediateZoomTile: ${existingTMV.subtilesCount} !< '
-              '$maxSubtilesCount)',
-            );
-          } catch (e) {
-            print(tile);
-            print(intermediateZoomTile);
-            rethrow;
-          }
+          assert(
+            existingTMV.subtilesCount < maxSubtilesCount,
+            'Existing subtiles count must be smaller than max subtiles count '
+            '($intermediateZoomTile: ${existingTMV.subtilesCount} !< '
+            '$maxSubtilesCount)',
+          );
 
           existingTMV.subtilesCount += 1;
           tmv = existingTMV;

diff --git a/.../lib/src/screens/main/map_view/components/download_progress/download_progress_masker.dart b/.../lib/src/screens/main/map_view/components/download_progress/download_progress_masker.dart
@@ -6,21 +6,22 @@ import 'dart:typed_data';
 import 'package:flutter/material.dart';
 import 'package:flutter/rendering.dart';
 import 'package:flutter_map/flutter_map.dart';
+import 'package:flutter_map_tile_caching/flutter_map_tile_caching.dart';
 import 'package:latlong2/latlong.dart' hide Path;
 
 part 'components/greyscale_masker.dart';
 
 class DownloadProgressMasker extends StatefulWidget {
   const DownloadProgressMasker({
     super.key,
-    required this.tileCoordinatesStream,
+    required this.downloadProgressStream,
     required this.minZoom,
     required this.maxZoom,
     this.tileSize = 256,
     required this.child,
   });
 
-  final Stream<TileCoordinates>? tileCoordinatesStream;
+  final Stream<DownloadProgress>? downloadProgressStream;
   final int minZoom;
   final int maxZoom;
   final int tileSize;
@@ -33,19 +34,13 @@ class DownloadProgressMasker extends StatefulWidget {
 class _DownloadProgressMaskerState extends State<DownloadProgressMasker> {
   @override
   Widget build(BuildContext context) {
-    if (widget.tileCoordinatesStream case final tcs?) {
+    if (widget.downloadProgressStream case final dps?) {
       return RepaintBoundary(
         child: GreyscaleMasker(
-          /*key: ObjectKey(
-            (
-              widget.minZoom,
-              widget.maxZoom,
-              widget.tileCoordinatesStream,
-              widget.tileSize,
-            ),
-          ),*/
           mapCamera: MapCamera.of(context),
-          tileCoordinatesStream: tcs,
+          tileCoordinatesStream: dps
+              .where((e) => !e.latestTileEvent.isRepeat)
+              .map((e) => e.latestTileEvent.coordinates),
           minZoom: widget.minZoom,
           maxZoom: widget.maxZoom,
           tileSize: widget.tileSize,

diff --git a/example/lib/src/screens/main/map_view/map_view.dart b/example/lib/src/screens/main/map_view/map_view.dart
@@ -122,7 +122,7 @@ class _MapViewState extends State<MapView> with TickerProviderStateMixin {
     ),
   ];*/
 
-  Stream<TileCoordinates>? _testingDownloadTileCoordsStream;
+  Stream<DownloadProgress>? _testingDownloadTileCoordsStream;
 
   bool _isInRegionSelectMode() =>
       widget.mode == MapViewMode.downloadRegion &&
@@ -149,15 +149,13 @@ class _MapViewState extends State<MapView> with TickerProviderStateMixin {
         if (!_isInRegionSelectMode()) {
           setState(
             () => _testingDownloadTileCoordsStream =
-                const FMTCStore('Local Tile Server')
-                    .download
-                    .startForeground(
+                const FMTCStore('Local Tile Server').download.startForeground(
                       region: const CircleRegion(
                         LatLng(45.3052535669648, 14.476223064038985),
                         5,
                       ).toDownloadable(
-                        minZoom: 11,
-                        maxZoom: 16,
+                        minZoom: 0,
+                        maxZoom: 12,
                         options: TileLayer(
                           //urlTemplate: 'http://127.0.0.1:7070/{z}/{x}/{y}',
                           urlTemplate:
@@ -172,11 +170,7 @@ class _MapViewState extends State<MapView> with TickerProviderStateMixin {
                         keys: ['access_token'],
                       ),
                       rateLimit: 20,
-                    )
-                    .map(
-                      (event) => event.latestTileEvent.coordinates,
-                    )
-                    .asBroadcastStream(),
+                    ),
           );
           return;
         }
@@ -416,13 +410,13 @@ class _MapViewState extends State<MapView> with TickerProviderStateMixin {
               options: mapOptions,
               children: [
                 DownloadProgressMasker(
-                  tileCoordinatesStream: _testingDownloadTileCoordsStream,
+                  downloadProgressStream: _testingDownloadTileCoordsStream,
                   /*tileCoordinates: Stream.periodic(
                         const Duration(seconds: 5),
                         _testingCoordsList.elementAtOrNull,
                       ).whereNotNull(),*/
-                  minZoom: 11,
-                  maxZoom: 16,
+                  minZoom: 0,
+                  maxZoom: 12,
                   child: tileLayer,
                 ),
                 PolygonLayer(

diff --git a/lib/src/backend/impls/objectbox/models/src/recovery_region.dart b/lib/src/backend/impls/objectbox/models/src/recovery_region.dart
@@ -34,13 +34,13 @@ class ObjectBoxRecoveryRegion {
   /// If representing a [MultiRegion], then [multiLinkedRegions] must be filled
   /// manually.
   ObjectBoxRecoveryRegion.fromRegion({required BaseRegion region})
-      : typeId = region.when(
-          rectangle: (_) => 0,
-          circle: (_) => 1,
-          line: (_) => 2,
-          customPolygon: (_) => 3,
-          multi: (_) => 4,
-        ),
+      : typeId = switch (region) {
+          RectangleRegion() => 0,
+          CircleRegion() => 1,
+          LineRegion() => 2,
+          CustomPolygonRegion() => 3,
+          MultiRegion() => 4,
+        },
         rectNwLat =
             region is RectangleRegion ? region.bounds.northWest.latitude : null,
         rectNwLng = region is RectangleRegion

diff --git a/lib/src/bulk_download/external/download_progress.dart b/lib/src/bulk_download/external/download_progress.dart
@@ -201,7 +201,7 @@ class DownloadProgress {
     required int? rateLimit,
   }) =>
       DownloadProgress.__(
-        latestTileEvent: latestTileEvent._repeat(),
+        latestTileEvent: latestTileEvent._copyWithRepeat(),
         cachedTiles: cachedTiles,
         cachedSize: cachedSize,
         bufferedTiles: bufferedTiles,
@@ -225,38 +225,40 @@ class DownloadProgress {
     required double tilesPerSecond,
     required int? rateLimit,
     bool isComplete = false,
-  }) =>
-      DownloadProgress.__(
-        latestTileEvent: newTileEvent ?? latestTileEvent,
-        cachedTiles: newTileEvent != null &&
-                newTileEvent.result.category == TileEventResultCategory.cached
-            ? cachedTiles + 1
-            : cachedTiles,
-        cachedSize: newTileEvent != null &&
-                newTileEvent.result.category == TileEventResultCategory.cached
-            ? cachedSize + (newTileEvent.tileImage!.lengthInBytes / 1024)
-            : cachedSize,
-        bufferedTiles: newBufferedTiles,
-        bufferedSize: newBufferedSize,
-        skippedTiles: newTileEvent != null &&
-                newTileEvent.result.category == TileEventResultCategory.skipped
-            ? skippedTiles + 1
-            : skippedTiles,
-        skippedSize: newTileEvent != null &&
-                newTileEvent.result.category == TileEventResultCategory.skipped
-            ? skippedSize + (newTileEvent.tileImage!.lengthInBytes / 1024)
-            : skippedSize,
-        failedTiles: newTileEvent != null &&
-                newTileEvent.result.category == TileEventResultCategory.failed
-            ? failedTiles + 1
-            : failedTiles,
-        maxTiles: maxTiles,
-        elapsedDuration: newDuration,
-        tilesPerSecond: tilesPerSecond,
-        isTPSArtificiallyCapped:
-            tilesPerSecond >= (rateLimit ?? double.infinity) - 0.5,
-        isComplete: isComplete,
-      );
+  }) {
+    final isNewTile = newTileEvent != null;
+    return DownloadProgress.__(
+      latestTileEvent: newTileEvent ?? latestTileEvent._copyWithRepeat(),
+      cachedTiles: isNewTile &&
+              newTileEvent.result.category == TileEventResultCategory.cached
+          ? cachedTiles + 1
+          : cachedTiles,
+      cachedSize: isNewTile &&
+              newTileEvent.result.category == TileEventResultCategory.cached
+          ? cachedSize + (newTileEvent.tileImage!.lengthInBytes / 1024)
+          : cachedSize,
+      bufferedTiles: newBufferedTiles,
+      bufferedSize: newBufferedSize,
+      skippedTiles: isNewTile &&
+              newTileEvent.result.category == TileEventResultCategory.skipped
+          ? skippedTiles + 1
+          : skippedTiles,
+      skippedSize: isNewTile &&
+              newTileEvent.result.category == TileEventResultCategory.skipped
+          ? skippedSize + (newTileEvent.tileImage!.lengthInBytes / 1024)
+          : skippedSize,
+      failedTiles: isNewTile &&
+              newTileEvent.result.category == TileEventResultCategory.failed
+          ? failedTiles + 1
+          : failedTiles,
+      maxTiles: maxTiles,
+      elapsedDuration: newDuration,
+      tilesPerSecond: tilesPerSecond,
+      isTPSArtificiallyCapped:
+          tilesPerSecond >= (rateLimit ?? double.infinity) - 0.5,
+      isComplete: isComplete,
+    );
+  }
 
   @override
   bool operator ==(Object other) =>

diff --git a/lib/src/bulk_download/external/tile_event.dart b/lib/src/bulk_download/external/tile_event.dart
@@ -141,7 +141,7 @@ class TileEvent {
 
   final bool _wasBufferReset;
 
-  TileEvent _repeat() => TileEvent._(
+  TileEvent _copyWithRepeat() => TileEvent._(
         result,
         url: url,
         coordinates: coordinates,

diff --git a/lib/src/bulk_download/internal/manager.dart b/lib/src/bulk_download/internal/manager.dart
@@ -256,7 +256,6 @@ Future<void> _downloadManager(
               .then((sp) => sp.send(null)),
         );
 
-        // TODO: Debug whether multiple threads are downloading the same tile
         downloadThreadReceivePort.listen(
           (evt) async {
             // Thread is sending tile data

diff --git a/lib/src/regions/downloadable_region.dart b/lib/src/regions/downloadable_region.dart
@@ -75,10 +75,6 @@ class DownloadableRegion<R extends BaseRegion> {
   ///
   /// Requires all region types to have a defined handler. See [maybeWhen] for
   /// the equivalent where this is not required.
-  @Deprecated(
-    'Prefer using a pattern matching selection (such as `if case` or '
-    '`switch`). This will be removed in a future version.',
-  )
   T when<T>({
     required T Function(DownloadableRegion<RectangleRegion> rectangle)
         rectangle,
@@ -100,10 +96,6 @@ class DownloadableRegion<R extends BaseRegion> {
   ///
   /// If the specified method is not defined for the type of region which this
   /// region is, `null` will be returned.
-  @Deprecated(
-    'Prefer using a pattern matching selection (such as `if case` or '
-    '`switch`). This will be removed in a future version.',
-  )
   T? maybeWhen<T>({
     T Function(DownloadableRegion<RectangleRegion> rectangle)? rectangle,
     T Function(DownloadableRegion<CircleRegion> circle)? circle,
@@ -121,19 +113,16 @@ class DownloadableRegion<R extends BaseRegion> {
       };
 
   @override
-  bool operator ==(Object other) {
-    final e = identical(this, other) ||
-        (other is DownloadableRegion &&
-            other.originalRegion == originalRegion &&
-            other.minZoom == minZoom &&
-            other.maxZoom == maxZoom &&
-            other.options == options &&
-            other.start == start &&
-            other.end == end &&
-            other.crs == crs);
-    print((other as DownloadableRegion).options == options);
-    return e;
-  }
+  bool operator ==(Object other) =>
+      identical(this, other) ||
+      (other is DownloadableRegion &&
+          other.originalRegion == originalRegion &&
+          other.minZoom == minZoom &&
+          other.maxZoom == maxZoom &&
+          other.options == options && //! Will never be equal
+          other.start == start &&
+          other.end == end &&
+          other.crs == crs);
 
   @override
   int get hashCode => Object.hashAllUnordered([