map: cluster entry fetch fix

2021-08-19 10:36:08 +09:00 · 2021-08-19 10:36:08 +09:00 · 91ab2ef17f
commit 91ab2ef17f
parent fd33904658
5 changed files with 86 additions and 61 deletions
--- a/lib/utils/constants.dart
+++ b/lib/utils/constants.dart
@ -26,6 +26,16 @@ class Constants {

  static final pointNemo = LatLng(-48.876667, -123.393333);

+  static final wonders = [
+    LatLng(29.979167, 31.134167),
+    LatLng(36.451000, 28.223615),
+    LatLng(32.5355, 44.4275),
+    LatLng(31.213889, 29.885556),
+    LatLng(37.0379, 27.4241),
+    LatLng(37.637861, 21.63),
+    LatLng(37.949722, 27.363889),
+  ];
+
  static const int infoGroupMaxValueLength = 140;

  static const List<Dependency> androidDependencies = [
--- a/lib/widgets/common/map/geo_map.dart
+++ b/lib/widgets/common/map/geo_map.dart
@ -6,6 +6,8 @@ import 'package:aves/model/settings/map_style.dart';
 import 'package:aves/model/settings/settings.dart';
 import 'package:aves/services/services.dart';
 import 'package:aves/theme/durations.dart';
+import 'package:aves/utils/constants.dart';
+import 'package:aves/utils/math_utils.dart';
 import 'package:aves/widgets/common/map/attribution.dart';
 import 'package:aves/widgets/common/map/buttons.dart';
 import 'package:aves/widgets/common/map/decorator.dart';
@ -18,7 +20,6 @@ import 'package:equatable/equatable.dart';
 import 'package:fluster/fluster.dart';
 import 'package:flutter/foundation.dart';
 import 'package:flutter/material.dart';
-import 'package:latlong2/latlong.dart';
 import 'package:provider/provider.dart';

 class GeoMap extends StatefulWidget {
@ -27,7 +28,7 @@ class GeoMap extends StatefulWidget {
  final double? mapHeight;
  final ValueNotifier<bool> isAnimatingNotifier;
  final UserZoomChangeCallback? onUserZoomChange;
-  final GeoEntryTapCallback? onEntryTap;
+  final MarkerTapCallback? onMarkerTap;

  static const markerImageExtent = 48.0;
  static const pointerSize = Size(8, 6);
@ -39,7 +40,7 @@ class GeoMap extends StatefulWidget {
    this.mapHeight,
    required this.isAnimatingNotifier,
    this.onUserZoomChange,
-    this.onEntryTap,
+    this.onMarkerTap,
  }) : super(key: key);

  @override
@ -52,7 +53,9 @@ class _GeoMapState extends State<GeoMap> with TickerProviderStateMixin {
  // it is especially severe the first time, but still significant afterwards
  // so we prevent loading it while scrolling or animating
  bool _googleMapsLoaded = false;
-  late ValueNotifier<ZoomedBounds> boundsNotifier;
+  late final ValueNotifier<ZoomedBounds> _boundsNotifier;
+  late final Fluster<GeoEntry> _defaultMarkerCluster;
+  Fluster<GeoEntry>? _slowMarkerCluster;

  List<AvesEntry> get entries => widget.entries;

@ -60,50 +63,40 @@ class _GeoMapState extends State<GeoMap> with TickerProviderStateMixin {

  double? get mapHeight => widget.mapHeight;

-  static final wonders = [
-    LatLng(29.979167, 31.134167),
-    LatLng(36.451000, 28.223615),
-    LatLng(32.5355, 44.4275),
-    LatLng(31.213889, 29.885556),
-    LatLng(37.0379, 27.4241),
-    LatLng(37.637861, 21.63),
-    LatLng(37.949722, 27.363889),
-  ];
-
  @override
  void initState() {
    super.initState();
    final points = entries.map((v) => v.latLng!).toSet();
-    boundsNotifier = ValueNotifier(ZoomedBounds.fromPoints(
-      points: points.isNotEmpty ? points : {wonders[Random().nextInt(wonders.length)]},
+    _boundsNotifier = ValueNotifier(ZoomedBounds.fromPoints(
+      points: points.isNotEmpty ? points : {Constants.wonders[Random().nextInt(Constants.wonders.length)]},
      collocationZoom: settings.infoMapZoom,
    ));
+    _defaultMarkerCluster = _buildFluster();
  }

  @override
  Widget build(BuildContext context) {
-    final markers = entries.map((entry) {
-      var latLng = entry.latLng!;
-      return GeoEntry(
-        entry: entry,
-        latitude: latLng.latitude,
-        longitude: latLng.longitude,
-        markerId: entry.uri,
-      );
-    }).toList();
-    final markerCluster = Fluster<GeoEntry>(
-      // we keep clustering on the whole range of zooms (including the maximum)
-      // to avoid collocated entries overlapping
-      minZoom: 0,
-      maxZoom: 22,
-      // TODO TLAD [map] derive `radius` / `extent`, from device pixel ratio and marker extent?
-      // (radius=120, extent=2 << 8) is equivalent to (radius=240, extent=2 << 9)
-      radius: 240,
-      extent: 2 << 9,
-      nodeSize: 64,
-      points: markers,
-      createCluster: (base, lng, lat) => GeoEntry.createCluster(base, lng, lat),
-    );
+    void _onMarkerTap(GeoEntry geoEntry) {
+      final onTap = widget.onMarkerTap;
+      if (onTap == null) return;
+
+      final geoEntries = <GeoEntry>[];
+      final clusterId = geoEntry.clusterId;
+      if (clusterId != null) {
+        var points = _defaultMarkerCluster.points(clusterId);
+        if (points.length != geoEntry.pointsSize) {
+          // `Fluster.points()` method does not always return all the points contained in a cluster
+          // the higher `nodeSize` is, the higher the chance to get all the points (i.e. as many as the cluster `pointsSize`)
+          _slowMarkerCluster ??= _buildFluster(nodeSize: smallestPowerOf2(widget.entries.length));
+          points = _slowMarkerCluster!.points(clusterId);
+          assert(points.length == geoEntry.pointsSize);
+        }
+        geoEntries.addAll(points);
+      } else {
+        geoEntries.add(geoEntry);
+      }
+      onTap(geoEntries.map((geoEntry) => geoEntry.entry!).toList());
+    }

    return FutureBuilder<bool>(
      future: availability.isConnected,
@ -125,28 +118,28 @@ class _GeoMapState extends State<GeoMap> with TickerProviderStateMixin {

            Widget child = isGoogleMaps
                ? EntryGoogleMap(
-                    boundsNotifier: boundsNotifier,
+                    boundsNotifier: _boundsNotifier,
                    interactive: interactive,
                    style: mapStyle,
                    markerBuilder: _buildMarker,
-                    markerCluster: markerCluster,
+                    markerCluster: _defaultMarkerCluster,
                    markerEntries: entries,
                    onUserZoomChange: widget.onUserZoomChange,
-                    onEntryTap: widget.onEntryTap,
+                    onMarkerTap: _onMarkerTap,
                  )
                : EntryLeafletMap(
-                    boundsNotifier: boundsNotifier,
+                    boundsNotifier: _boundsNotifier,
                    interactive: interactive,
                    style: mapStyle,
                    markerBuilder: _buildMarker,
-                    markerCluster: markerCluster,
+                    markerCluster: _defaultMarkerCluster,
                    markerEntries: entries,
                    markerSize: Size(
                      GeoMap.markerImageExtent + ImageMarker.outerBorderWidth * 2,
                      GeoMap.markerImageExtent + ImageMarker.outerBorderWidth * 2 + GeoMap.pointerSize.height,
                    ),
                    onUserZoomChange: widget.onUserZoomChange,
-                    onEntryTap: widget.onEntryTap,
+                    onMarkerTap: _onMarkerTap,
                  );

            child = Column(
@ -179,7 +172,7 @@ class _GeoMapState extends State<GeoMap> with TickerProviderStateMixin {
                        interactive: interactive,
                      ),
                      MapButtonPanel(
-                        latLng: boundsNotifier.value.center,
+                        latLng: _boundsNotifier.value.center,
                      ),
                    ],
                  );
@ -203,6 +196,33 @@ class _GeoMapState extends State<GeoMap> with TickerProviderStateMixin {
      },
    );
  }
+
+  Fluster<GeoEntry> _buildFluster({int nodeSize = 64}) {
+    final markers = entries.map((entry) {
+      final latLng = entry.latLng!;
+      return GeoEntry(
+        entry: entry,
+        latitude: latLng.latitude,
+        longitude: latLng.longitude,
+        markerId: entry.uri,
+      );
+    }).toList();
+
+    return Fluster<GeoEntry>(
+      // we keep clustering on the whole range of zooms (including the maximum)
+      // to avoid collocated entries overlapping
+      minZoom: 0,
+      maxZoom: 22,
+      // TODO TLAD [map] derive `radius` / `extent`, from device pixel ratio and marker extent?
+      // (radius=120, extent=2 << 8) is equivalent to (radius=240, extent=2 << 9)
+      radius: 240,
+      extent: 2 << 9,
+      // node size: 64 by default, higher means faster indexing but slower search
+      nodeSize: nodeSize,
+      points: markers,
+      createCluster: (base, lng, lat) => GeoEntry.createCluster(base, lng, lat),
+    );
+  }
 }

@immutable
@ -218,4 +238,4 @@ class MarkerKey extends LocalKey with EquatableMixin {

 typedef EntryMarkerBuilder = Widget Function(MarkerKey key);
 typedef UserZoomChangeCallback = void Function(double zoom);
-typedef GeoEntryTapCallback = void Function(List<GeoEntry> geoEntries);
+typedef MarkerTapCallback = void Function(List<AvesEntry> entries);
--- a/lib/widgets/common/map/google/map.dart
+++ b/lib/widgets/common/map/google/map.dart
@ -25,7 +25,7 @@ class EntryGoogleMap extends StatefulWidget {
  final Fluster<GeoEntry> markerCluster;
  final List<AvesEntry> markerEntries;
  final UserZoomChangeCallback? onUserZoomChange;
-  final GeoEntryTapCallback? onEntryTap;
+  final void Function(GeoEntry geoEntry)? onMarkerTap;

  const EntryGoogleMap({
    Key? key,
@ -36,7 +36,7 @@ class EntryGoogleMap extends StatefulWidget {
    required this.markerCluster,
    required this.markerEntries,
    this.onUserZoomChange,
-    this.onEntryTap,
+    this.onMarkerTap,
  }) : super(key: key);

  @override
@ -134,7 +134,7 @@ class _EntryGoogleMapState extends State<EntryGoogleMap> with WidgetsBindingObse
      animation: _markerBitmapChangeNotifier,
      builder: (context, child) {
        final markers = <Marker>{};
-        final onTap = widget.onEntryTap;
+        final onEntryTap = widget.onMarkerTap;
        geoEntryByMarkerKey.forEach((markerKey, geoEntry) {
          final bytes = _markerBitmaps[markerKey];
          if (bytes != null) {
@ -143,12 +143,7 @@ class _EntryGoogleMapState extends State<EntryGoogleMap> with WidgetsBindingObse
              markerId: MarkerId(geoEntry.markerId!),
              icon: BitmapDescriptor.fromBytes(bytes),
              position: latLng,
-              onTap: onTap != null
-                  ? () {
-                      final clusterId = geoEntry.clusterId;
-                      onTap(clusterId != null ? widget.markerCluster.points(clusterId) : [geoEntry]);
-                    }
-                  : null,
+              onTap: onEntryTap != null ? () => onEntryTap(geoEntry) : null,
            ));
          }
        });
--- a/lib/widgets/common/map/leaflet/map.dart
+++ b/lib/widgets/common/map/leaflet/map.dart
@ -24,7 +24,7 @@ class EntryLeafletMap extends StatefulWidget {
  final List<AvesEntry> markerEntries;
  final Size markerSize;
  final UserZoomChangeCallback? onUserZoomChange;
-  final GeoEntryTapCallback? onEntryTap;
+  final void Function(GeoEntry geoEntry)? onMarkerTap;

  const EntryLeafletMap({
    Key? key,
@ -36,7 +36,7 @@ class EntryLeafletMap extends StatefulWidget {
    required this.markerEntries,
    required this.markerSize,
    this.onUserZoomChange,
-    this.onEntryTap,
+    this.onMarkerTap,
  }) : super(key: key);

  @override
@ -118,8 +118,7 @@ class _EntryLeafletMapState extends State<EntryLeafletMap> with TickerProviderSt
        point: latLng,
        builder: (context) => GestureDetector(
          onTap: () {
-            final clusterId = geoEntry.clusterId;
-            widget.onEntryTap?.call(clusterId != null ? widget.markerCluster.points(clusterId) : [geoEntry]);
+            widget.onMarkerTap?.call(geoEntry);
            _moveTo(latLng);
          },
          child: widget.markerBuilder(markerKey),
--- a/lib/widgets/map/map_page.dart
+++ b/lib/widgets/map/map_page.dart
@ -6,6 +6,7 @@ import 'package:aves/widgets/common/extensions/build_context.dart';
 import 'package:aves/widgets/common/map/geo_map.dart';
 import 'package:aves/widgets/common/providers/media_query_data_provider.dart';
 import 'package:aves/widgets/common/thumbnail/scroller.dart';
+import 'package:collection/collection.dart';
 import 'package:flutter/foundation.dart';
 import 'package:flutter/material.dart';
 import 'package:flutter/scheduler.dart';
@ -61,8 +62,8 @@ class _MapPageState extends State<MapPage> {
                  entries: entries,
                  interactive: true,
                  isAnimatingNotifier: _isAnimatingNotifier,
-                  onEntryTap: (geoEntries) {
-                    debugPrint('TLAD count=${geoEntries.length} entry=${geoEntries.first.entry}');
+                  onMarkerTap: (entries) {
+                    debugPrint('TLAD count=${entries.length} entry=${entries.firstOrNull?.bestTitle}');
                  },
                ),
              ),