Merge pull request #148 from dougborg/fix/node-render-prioritization

Prioritize closest nodes to viewport center when render limit is active

lib/widgets/map/map_data_manager.dart

@@ -10,6 +10,13 @@ import '../../dev_config.dart';
 /// Manages data fetching, filtering, and node limit logic for the map.
 /// Handles profile changes, zoom level restrictions, and node rendering limits.
 class MapDataManager {
+  final List<OsmNode> Function(LatLngBounds bounds) _getNodesForBounds;
+
+  MapDataManager({
+    List<OsmNode> Function(LatLngBounds bounds)? getNodesForBounds,
+  }) : _getNodesForBounds = getNodesForBounds ??
+           ((bounds) => NodeProviderWithCache.instance.getCachedNodesForBounds(bounds));
+
   // Track node limit state for parent notification
   bool _lastNodeLimitState = false;
 
@@ -51,28 +58,42 @@ class MapDataManager {
     List<OsmNode> allNodes;
     List<OsmNode> nodesToRender;
     bool isLimitActive = false;
-    
+    int validNodesCount = 0;
+
     if (currentZoom >= minZoom) {
       // Above minimum zoom - get cached nodes with expanded bounds to prevent edge blinking
       if (mapBounds != null) {
         final expandedBounds = _expandBounds(mapBounds, kNodeRenderingBoundsExpansion);
-        allNodes = NodeProviderWithCache.instance.getCachedNodesForBounds(expandedBounds);
+        allNodes = _getNodesForBounds(expandedBounds);
       } else {
         allNodes = <OsmNode>[];
       }
-      
+
       // Filter out invalid coordinates before applying limit
       final validNodes = allNodes.where((node) {
         return (node.coord.latitude != 0 || node.coord.longitude != 0) &&
-               node.coord.latitude.abs() <= 90 && 
+               node.coord.latitude.abs() <= 90 &&
                node.coord.longitude.abs() <= 180;
       }).toList();
-      
-      // Apply rendering limit to prevent UI lag
-      if (validNodes.length > maxNodes) {
+      validNodesCount = validNodes.length;
+
+      // Apply rendering limit to prevent UI lag.
+      // Sort by distance from viewport center so the most visible nodes
+      // always make the cut, preventing gaps that shift as you pan.
+      if (validNodesCount > maxNodes) {
+        final bounds = mapBounds!;
+        final centerLat = (bounds.north + bounds.south) / 2;
+        final centerLng = (bounds.east + bounds.west) / 2;
+        validNodes.sort((a, b) {
+          final distA = (a.coord.latitude - centerLat) * (a.coord.latitude - centerLat) +
+                        (a.coord.longitude - centerLng) * (a.coord.longitude - centerLng);
+          final distB = (b.coord.latitude - centerLat) * (b.coord.latitude - centerLat) +
+                        (b.coord.longitude - centerLng) * (b.coord.longitude - centerLng);
+          final cmp = distA.compareTo(distB);
+          return cmp != 0 ? cmp : a.id.compareTo(b.id);
+        });
         nodesToRender = validNodes.take(maxNodes).toList();
         isLimitActive = true;
-        debugPrint('[MapDataManager] Node limit active: rendering ${nodesToRender.length} of ${validNodes.length} devices');
       } else {
         nodesToRender = validNodes;
         isLimitActive = false;
@@ -87,6 +108,9 @@ class MapDataManager {
     // Notify parent if limit state changed (for button disabling)
     if (isLimitActive != _lastNodeLimitState) {
       _lastNodeLimitState = isLimitActive;
+      if (isLimitActive) {
+        debugPrint('[MapDataManager] Node limit active: rendering ${nodesToRender.length} of $validNodesCount valid devices');
+      }
       // Schedule callback after build completes to avoid setState during build
       WidgetsBinding.instance.addPostFrameCallback((_) {
         onNodeLimitChanged?.call(isLimitActive);
@@ -97,11 +121,7 @@ class MapDataManager {
       allNodes: allNodes,
       nodesToRender: nodesToRender,
       isLimitActive: isLimitActive,
-      validNodesCount: isLimitActive ? allNodes.where((node) {
-        return (node.coord.latitude != 0 || node.coord.longitude != 0) &&
-               node.coord.latitude.abs() <= 90 && 
-               node.coord.longitude.abs() <= 180;
-      }).length : 0,
+      validNodesCount: isLimitActive ? validNodesCount : 0,
     );
   }
 

test/widgets/map_data_manager_test.dart

@@ -0,0 +1,164 @@
+import 'package:flutter/widgets.dart';
+import 'package:flutter_test/flutter_test.dart';
+import 'package:flutter_map/flutter_map.dart';
+import 'package:latlong2/latlong.dart';
+
+import 'package:deflockapp/models/osm_node.dart';
+import 'package:deflockapp/app_state.dart';
+import 'package:deflockapp/widgets/map/map_data_manager.dart';
+
+void main() {
+  OsmNode nodeAt(int id, double lat, double lng) {
+    return OsmNode(id: id, coord: LatLng(lat, lng), tags: {'surveillance': 'outdoor'});
+  }
+
+  group('Node render prioritization', () {
+    late MapDataManager dataManager;
+    late List<OsmNode> testNodes;
+
+    setUp(() {
+      WidgetsFlutterBinding.ensureInitialized();
+      testNodes = [];
+      dataManager = MapDataManager(
+        getNodesForBounds: (_) => testNodes,
+      );
+    });
+
+    test('closest nodes to viewport center are kept', () {
+      final bounds = LatLngBounds(LatLng(38.0, -78.0), LatLng(39.0, -77.0));
+      // Center is (38.5, -77.5)
+      testNodes = [
+        nodeAt(1, 38.9, -77.9),   // far from center
+        nodeAt(2, 38.5, -77.5),   // at center
+        nodeAt(3, 38.1, -77.1),   // far from center
+        nodeAt(4, 38.51, -77.49), // very close to center
+        nodeAt(5, 38.0, -78.0),   // corner — farthest
+      ];
+
+      final result = dataManager.getNodesForRendering(
+        currentZoom: 14,
+        mapBounds: bounds,
+        uploadMode: UploadMode.production,
+        maxNodes: 3,
+      );
+
+      expect(result.isLimitActive, isTrue);
+      expect(result.nodesToRender.length, 3);
+      final ids = result.nodesToRender.map((n) => n.id).toSet();
+      expect(ids.contains(2), isTrue, reason: 'Node at center should be kept');
+      expect(ids.contains(4), isTrue, reason: 'Node near center should be kept');
+      expect(ids.contains(5), isFalse, reason: 'Node at corner should be dropped');
+    });
+
+    test('returns all nodes when under the limit', () {
+      final bounds = LatLngBounds(LatLng(38.0, -78.0), LatLng(39.0, -77.0));
+      testNodes = [
+        nodeAt(1, 38.5, -77.5),
+        nodeAt(2, 38.6, -77.6),
+      ];
+
+      final result = dataManager.getNodesForRendering(
+        currentZoom: 14,
+        mapBounds: bounds,
+        uploadMode: UploadMode.production,
+        maxNodes: 10,
+      );
+
+      expect(result.isLimitActive, isFalse);
+      expect(result.nodesToRender.length, 2);
+    });
+
+    test('returns empty when below minimum zoom', () {
+      final bounds = LatLngBounds(LatLng(38.0, -78.0), LatLng(39.0, -77.0));
+      testNodes = [nodeAt(1, 38.5, -77.5)];
+
+      final result = dataManager.getNodesForRendering(
+        currentZoom: 5,
+        mapBounds: bounds,
+        uploadMode: UploadMode.production,
+        maxNodes: 10,
+      );
+
+      expect(result.nodesToRender, isEmpty);
+    });
+
+    test('panning viewport changes which nodes are prioritized', () {
+      final nodes = [
+        nodeAt(1, 38.0, -78.0), // SW
+        nodeAt(2, 38.5, -77.5), // middle
+        nodeAt(3, 39.0, -77.0), // NE
+      ];
+
+      // Viewport centered near SW
+      testNodes = List.from(nodes);
+      final swBounds = LatLngBounds(LatLng(37.5, -78.5), LatLng(38.5, -77.5));
+      final swResult = dataManager.getNodesForRendering(
+        currentZoom: 14,
+        mapBounds: swBounds,
+        uploadMode: UploadMode.production,
+        maxNodes: 1,
+      );
+      expect(swResult.nodesToRender.first.id, 1,
+          reason: 'SW node closest to SW-centered viewport');
+
+      // Viewport centered near NE
+      testNodes = List.from(nodes);
+      final neBounds = LatLngBounds(LatLng(38.5, -77.5), LatLng(39.5, -76.5));
+      final neResult = dataManager.getNodesForRendering(
+        currentZoom: 14,
+        mapBounds: neBounds,
+        uploadMode: UploadMode.production,
+        maxNodes: 1,
+      );
+      expect(neResult.nodesToRender.first.id, 3,
+          reason: 'NE node closest to NE-centered viewport');
+    });
+
+    test('order is stable for repeated calls with same viewport', () {
+      final bounds = LatLngBounds(LatLng(38.0, -78.0), LatLng(39.0, -77.0));
+      makeNodes() => [
+        nodeAt(1, 38.9, -77.9),
+        nodeAt(2, 38.5, -77.5),
+        nodeAt(3, 38.1, -77.1),
+        nodeAt(4, 38.51, -77.49),
+        nodeAt(5, 38.0, -78.0),
+      ];
+
+      testNodes = makeNodes();
+      final result1 = dataManager.getNodesForRendering(
+        currentZoom: 14, mapBounds: bounds,
+        uploadMode: UploadMode.production, maxNodes: 3,
+      );
+
+      testNodes = makeNodes();
+      final result2 = dataManager.getNodesForRendering(
+        currentZoom: 14, mapBounds: bounds,
+        uploadMode: UploadMode.production, maxNodes: 3,
+      );
+
+      expect(
+        result1.nodesToRender.map((n) => n.id).toList(),
+        result2.nodesToRender.map((n) => n.id).toList(),
+      );
+    });
+
+    test('filters out invalid coordinates before prioritizing', () {
+      final bounds = LatLngBounds(LatLng(38.0, -78.0), LatLng(39.0, -77.0));
+      testNodes = [
+        nodeAt(1, 0, 0),          // invalid (0,0)
+        nodeAt(2, 38.5, -77.5),   // valid, at center
+        nodeAt(3, 200, -77.5),    // invalid lat
+      ];
+
+      final result = dataManager.getNodesForRendering(
+        currentZoom: 14,
+        mapBounds: bounds,
+        uploadMode: UploadMode.production,
+        maxNodes: 10,
+      );
+
+      expect(result.nodesToRender.length, 1);
+      expect(result.nodesToRender.first.id, 2);
+    });
+  });
+}