Skip to content

Commit

Permalink
fix: fixed multiple issues in PolygonLayer and PolylineLayer (#1925)
Browse files Browse the repository at this point in the history
  • Loading branch information
JaffaKetchup authored Jul 2, 2024
1 parent c1ae4fb commit 374672d
Show file tree
Hide file tree
Showing 8 changed files with 46 additions and 122 deletions.
14 changes: 13 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,16 @@ Please consider [donating](https://docs.fleaflet.dev/supporters#support-us) or [

This CHANGELOG does not include every commit and/or PR - it is a hand picked selection of the most important ones. For a full list of changes, please check the GitHub repository releases/tags.

## [7.0.2] - 2024/07/XX

> Note that this version causes a technically breaking change by removing `PolygonLayer.useDynamicUpdate` & `PolylineLayer.useDynamicUpdate`, introduced in v7.0.1. However, the implementations for these was broken on introduction, and their intended purpose no longer exists. Therefore, these should not have been used in any capacity, and should not affect any projects.
Contains the following user-affecting bug fixes:

- Fixed significant performance issues with `PolygonLayer` & `PolylineLayer` inadvertently introduced by v7.0.1 - [#1925](https://github.com/fleaflet/flutter_map/pull/1925) for [#1921](https://github.com/fleaflet/flutter_map/issues/1921)
- Fixed bug where the holes of a `Polygon` would only appear if their winding direction was opposite to the direction of the `Polygon.points` - [#1925](https://github.com/fleaflet/flutter_map/pull/1925) for [#1924](https://github.com/fleaflet/flutter_map/issues/1924)
- Relaxed constraint on 'package:logger' dependency - [#1922](https://github.com/fleaflet/flutter_map/pull/1922) for [#1916](https://github.com/fleaflet/flutter_map/issues/1916)

## [7.0.1] - 2024/06/09

Contains the following user-affecting bug fixes:
Expand Down Expand Up @@ -62,10 +72,12 @@ Many thanks to these contributors (in no particular order):
- @monsieurtanuki
- ... and all the maintainers

## [6.2.0] - 2024/05/XX
## [6.2.1] - 2024/05/27

> If possible, prefer to update directly to v7. This version is provided only to enable Flutter 3.22 compatibility without requiring a breaking change.
> v6.2.0 was retracted from pub.dev due to a mistake in the release preparation. For more information, see [this comment](https://github.com/fleaflet/flutter_map/pull/1891#issuecomment-2134069848). v6.2.1 is the replacement without the issues.
Contains the following user-affecting changes:

- Added support for Flutter 2.22 - [#1883](https://github.com/fleaflet/flutter_map/pull/1883)
Expand Down
2 changes: 2 additions & 0 deletions example/lib/pages/polygon.dart
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,8 @@ class _PolygonPageState extends State<PolygonPage> {
(latlngs) => latlngs
.map((latlng) =>
LatLng(latlng.latitude - 6, latlng.longitude + 8))
.toList()
.reversed // Test that holes are always cut, no matter winding
.toList(),
)
.toList(),
Expand Down
8 changes: 7 additions & 1 deletion lib/src/layer/polygon_layer/painter.dart
Original file line number Diff line number Diff line change
Expand Up @@ -281,6 +281,11 @@ base class _PolygonPainter<R extends Object>
// https://github.com/fleaflet/flutter_map/issues/1898.
final holePointsList = polygon.holePointsList;
if (holePointsList != null && holePointsList.isNotEmpty) {
// See `Path.combine` comments below
// Avoids failing to cut holes if the winding directions of the holes
// and the normal points are the same
filledPath.fillType = PathFillType.evenOdd;

final holeOffsetsList = List<List<Offset>>.generate(
holePointsList.length,
(i) => getOffsets(camera, origin, holePointsList[i]),
Expand All @@ -293,8 +298,9 @@ base class _PolygonPainter<R extends Object>
// TODO: Potentially more efficient and may change the need to do
// opacity checking - needs testing. However,
// https://github.com/flutter/flutter/issues/44572 prevents this.
// Also need to verify if `xor` or `difference` is preferred.
/*filledPath = Path.combine(
PathOperation.difference,
PathOperation.xor,
filledPath,
Path()..addPolygon(holeOffsets, true),
);*/
Expand Down
1 change: 0 additions & 1 deletion lib/src/layer/polygon_layer/polygon_layer.dart
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,6 @@ base class PolygonLayer<R extends Object>
this.drawLabelsLast = false,
this.hitNotifier,
super.simplificationTolerance,
super.useDynamicUpdate,
}) : super();

@override
Expand Down
1 change: 0 additions & 1 deletion lib/src/layer/polyline_layer/polyline_layer.dart
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,6 @@ base class PolylineLayer<R extends Object>
this.cullingMargin = 10,
this.hitNotifier,
this.minimumHitbox = 10,
super.useDynamicUpdate,
super.simplificationTolerance,
}) : super();

Expand Down
122 changes: 23 additions & 99 deletions lib/src/layer/shared/layer_projection_simplification/state.dart
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
import 'dart:collection';

import 'package:flutter/material.dart';
import 'package:flutter_map/flutter_map.dart';
import 'package:flutter_map/src/layer/shared/layer_projection_simplification/widget.dart';
Expand Down Expand Up @@ -46,9 +44,8 @@ mixin ProjectionSimplificationManagement<
/// after [build] has been invoked.
late Iterable<ProjectedElement> simplifiedElements;

final _cachedProjectedElements = SplayTreeMap<int, ProjectedElement>();
final _cachedSimplifiedElements =
<int, SplayTreeMap<int, ProjectedElement>>{};
Iterable<ProjectedElement>? _cachedProjectedElements;
final _cachedSimplifiedElements = <int, Iterable<ProjectedElement>>{};

double? _devicePixelRatio;

Expand All @@ -57,88 +54,8 @@ mixin ProjectionSimplificationManagement<
void didUpdateWidget(W oldWidget) {
super.didUpdateWidget(oldWidget);

if (!widget.useDynamicUpdate) return;

final camera = MapCamera.of(context);

// If the simplification tolerance has changed, then clear all
// simplifications to allow `build` to re-simplify.
final hasSimplficationToleranceChanged =
oldWidget.simplificationTolerance != widget.simplificationTolerance;
if (hasSimplficationToleranceChanged) _cachedSimplifiedElements.clear();

final elements = getElements(widget);

// We specifically only use basic equality here, and not deep, since deep
// will always be equal.
if (getElements(oldWidget) == elements) return;

// Loop through all polygons in the new widget
// If not in the projection cache, then re-project. Also, do the same for
// the simplification cache, across all zoom levels for each polygon.
// Then, remove all polygons no longer in the new widget from each cache.
//
// This is an O(n^3) operation, assuming n is the number of polygons
// (assuming they are all similar, otherwise exact runtime will depend on
// existing cache lengths, etc.). However, compared to previous versions, it
// takes approximately the same duration, as it relieves the work from the
// `build` method.
for (final element in getElements(widget)) {
final existingProjection = _cachedProjectedElements[element.hashCode];

if (existingProjection == null) {
_cachedProjectedElements[element.hashCode] =
projectElement(projection: camera.crs.projection, element: element);

if (hasSimplficationToleranceChanged) continue;

for (final MapEntry(key: zoomLvl, value: simplifiedElements)
in _cachedSimplifiedElements.entries) {
final simplificationTolerance = getEffectiveSimplificationTolerance(
crs: camera.crs,
zoom: zoomLvl,
// When the tolerance changes, this method handles resetting and filling
pixelTolerance: widget.simplificationTolerance,
// When the DPR changes, the `build` method handles resetting and filling
devicePixelRatio: MediaQuery.devicePixelRatioOf(context),
);

final existingSimplification = simplifiedElements[element.hashCode];

if (existingSimplification == null) {
_cachedSimplifiedElements[zoomLvl]![element.hashCode] =
simplifyProjectedElement(
projectedElement: _cachedProjectedElements[element.hashCode]!,
tolerance: simplificationTolerance,
);
}
}
}
}

_cachedProjectedElements
.removeWhere((k, v) => !elements.map((p) => p.hashCode).contains(k));

for (final simplifiedElement in _cachedSimplifiedElements.values) {
simplifiedElement
.removeWhere((k, v) => !elements.map((p) => p.hashCode).contains(k));
}
}

@mustCallSuper
@override
void didChangeDependencies() {
super.didChangeDependencies();

// Performed once only, at load - projects all initial polygons
if (_cachedProjectedElements.isEmpty) {
final camera = MapCamera.of(context);

for (final element in getElements(widget)) {
_cachedProjectedElements[element.hashCode] =
projectElement(projection: camera.crs.projection, element: element);
}
}
_cachedProjectedElements = null;
_cachedSimplifiedElements.clear();
}

@mustBeOverridden
Expand All @@ -147,11 +64,22 @@ mixin ProjectionSimplificationManagement<
Widget build(BuildContext context) {
final camera = MapCamera.of(context);

final elements = getElements(widget);

final projected = _cachedProjectedElements ??= List.generate(
elements.length,
(i) => projectElement(
projection: camera.crs.projection,
element: elements.elementAt(i),
),
growable: false,
);

// The `build` method handles initial simplification, re-simplification only
// when the DPR has changed, and re-simplification implicitly when the
// tolerance is changed (and the cache is emptied by `didUpdateWidget`).
if (widget.simplificationTolerance == 0) {
simplifiedElements = _cachedProjectedElements.values;
simplifiedElements = projected;
} else {
// If the DPR has changed, invalidate the simplification cache
final newDPR = MediaQuery.devicePixelRatioOf(context);
Expand All @@ -160,17 +88,13 @@ mixin ProjectionSimplificationManagement<
_cachedSimplifiedElements.clear();
}

simplifiedElements = (_cachedSimplifiedElements[camera.zoom.floor()] ??=
SplayTreeMap.fromIterables(
_cachedProjectedElements.keys,
_simplifyElements(
camera: camera,
projectedElements: _cachedProjectedElements.values,
pixelTolerance: widget.simplificationTolerance,
devicePixelRatio: newDPR,
),
))
.values;
simplifiedElements =
(_cachedSimplifiedElements[camera.zoom.floor()] ??= _simplifyElements(
camera: camera,
projectedElements: projected,
pixelTolerance: widget.simplificationTolerance,
devicePixelRatio: newDPR,
));
}

return Builder(
Expand Down
18 changes: 0 additions & 18 deletions lib/src/layer/shared/layer_projection_simplification/widget.dart
Original file line number Diff line number Diff line change
@@ -1,28 +1,11 @@
import 'package:flutter/material.dart';
import 'package:flutter_map/flutter_map.dart';
import 'package:flutter_map/src/layer/shared/layer_projection_simplification/state.dart';

/// A [StatefulWidget] that includes the properties used by the [State] component
/// which mixes [ProjectionSimplificationManagement] in
@immutable
abstract base class ProjectionSimplificationManagementSupportedWidget
extends StatefulWidget {
/// Whether to apply the auto-update algorithm to re-paint the necessary
/// [Polygon]s when they change
///
/// It is recommended to leave this `true`, as default, otherwise changes to
/// child polygons may not update. It will detect which polygons have changed,
/// and only 'update' (re-project and re-simplify) those that are necessary.
///
/// However, where there are a large number of polygons, the majority (or more)
/// of which change at the same time, then it is recommended to set this
/// `false`. This will avoid a large unnecessary loop to detect changes, and
/// is likely to improve performance on state changes. If `false`, then the
/// layer will need to be manually rebuilt from scratch using new [Key]s
/// whenever necessary. Do not use a [UniqueKey] : this will cause the entire
/// widget to reset and rebuild every time the map camera changes.
final bool useDynamicUpdate;

/// Distance between two neighboring polyline points, in logical pixels scaled
/// to floored zoom
///
Expand All @@ -40,7 +23,6 @@ abstract base class ProjectionSimplificationManagementSupportedWidget
/// necessary assertions are made.
const ProjectionSimplificationManagementSupportedWidget({
super.key,
this.useDynamicUpdate = true,
this.simplificationTolerance = 0.3,
}) : assert(
simplificationTolerance >= 0,
Expand Down
2 changes: 1 addition & 1 deletion pubspec.yaml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
name: flutter_map
description: A versatile mapping package for Flutter, that's simple and easy to learn, yet completely customizable and configurable
version: 7.0.1
version: 7.0.2

repository: https://github.com/fleaflet/flutter_map
issue_tracker: https://github.com/fleaflet/flutter_map/issues
Expand Down

0 comments on commit 374672d

Please sign in to comment.