Skip to content

Commit 1f9fcef

Browse files
NewmanjackNewmanjack
committed
Address PR feedback for pointOnFeature implementation
1 parent f6c0e41 commit 1f9fcef

File tree

3 files changed

+62
-123
lines changed

3 files changed

+62
-123
lines changed

‎benchmark/point_on_feature_benchmark.dart‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,12 @@ import 'package:benchmark/benchmark.dart';
22
import 'package:turf/turf.dart';
33

44
// Create some test features for benchmarking
5-
var point = Feature(
5+
final point = Feature(
66
geometry: Point(coordinates: Position.of([5.0, 10.0])),
77
properties: {'name': 'Test Point'},
88
);
99

10-
var polygon = Feature<Polygon>(
10+
final polygon = Feature<Polygon>(
1111
geometry: Polygon(coordinates: [
1212
[
1313
Position.of([-10.0, 0.0]),
@@ -19,7 +19,7 @@ var polygon = Feature<Polygon>(
1919
properties: {'name': 'Triangle Polygon'},
2020
);
2121

22-
var lineString = Feature<LineString>(
22+
final lineString = Feature<LineString>(
2323
geometry: LineString(coordinates: [
2424
Position.of([0.0, 0.0]),
2525
Position.of([10.0, 10.0]),
@@ -28,7 +28,7 @@ var lineString = Feature<LineString>(
2828
properties: {'name': 'Line String Example'},
2929
);
3030

31-
var featureCollection = FeatureCollection<GeometryObject>(features: [
31+
final featureCollection = FeatureCollection<GeometryObject>(features: [
3232
Feature(geometry: Point(coordinates: Position.of([0.0, 0.0]))),
3333
Feature<Polygon>(
3434
geometry: Polygon(coordinates: [

‎lib/src/point_on_feature.dart‎

Lines changed: 52 additions & 115 deletions
Original file line numberDiff line numberDiff line change
@@ -1,20 +1,24 @@
11
import 'dart:math' as math;
2-
import 'package:geotypes/geotypes.dart'; // We still need the GeoJSON types, as they're used throughout the package
2+
import 'package:geotypes/geotypes.dart';
3+
import 'package:turf/centroid.dart';
4+
import 'package:turf/boolean.dart';
5+
import 'package:turf/midpoint.dart';
36

4-
/// Returns a Feature<Point> that represents a point guaranteed to be on the feature.
7+
/// Returns a [Feature<Point>] that represents a point guaranteed to be on the feature.
58
///
6-
/// - For Point geometries: returns the original point
7-
/// - For Polygon geometries: computes a point inside the polygon (preference to centroid)
8-
/// - For MultiPolygon geometries: uses the first polygon to compute a point
9-
/// - For LineString geometries: computes the midpoint along the line
10-
/// - For FeatureCollection: returns a point on the largest feature
9+
/// - For [Point] geometries: returns the original point
10+
/// - For [Polygon] geometries: computes a point inside the polygon (preference to centroid)
11+
/// - For [MultiPolygon] geometries: uses the first polygon to compute a point
12+
/// - For [LineString] geometries: computes the midpoint along the line
13+
/// - For [FeatureCollection]: returns a point on the largest feature
1114
///
1215
/// The resulting point is guaranteed to be on the feature.
13-
Feature<Point>? pointOnFeature(dynamic featureInput) {
16+
/// Throws an exception for unsupported input types.
17+
Feature<Point> pointOnFeature(dynamic featureInput) {
1418
// Handle FeatureCollection
1519
if (featureInput is FeatureCollection) {
1620
if (featureInput.features.isEmpty) {
17-
return null;
21+
throw Exception('Empty FeatureCollection: Cannot compute a point on an empty collection');
1822
}
1923

2024
// Find the largest feature in the collection
@@ -41,129 +45,62 @@ Feature<Point>? pointOnFeature(dynamic featureInput) {
4145
// Already a point: return it.
4246
return Feature<Point>(geometry: geometry, properties: featureInput.properties);
4347
} else if (geometry is LineString) {
44-
// For LineString: compute the midpoint
45-
return _midpointOnLine(geometry, featureInput.properties);
48+
// For LineString: compute the midpoint using the first segment
49+
final coords = geometry.coordinates;
50+
if (coords.isEmpty) {
51+
throw Exception('Empty LineString: Cannot compute a point on an empty line');
52+
}
53+
54+
if (coords.length == 1) {
55+
// Only one point in the LineString, return it
56+
return Feature<Point>(
57+
geometry: Point(coordinates: coords.first),
58+
properties: featureInput.properties
59+
);
60+
}
61+
62+
// Calculate the midpoint of the first segment
63+
final pt1 = Point(coordinates: coords[0]);
64+
final pt2 = Point(coordinates: coords[1]);
65+
final mid = midpoint(pt1, pt2);
66+
67+
return Feature<Point>(
68+
geometry: mid,
69+
properties: featureInput.properties
70+
);
4671
} else if (geometry is Polygon) {
47-
final centroid = calculateCentroid(geometry);
48-
// Convert Point to Position for boolean check
49-
final pointPos = Position(centroid.coordinates[0] ?? 0.0, centroid.coordinates[1] ?? 0.0);
50-
if (_pointInPolygon(pointPos, geometry)) {
51-
return Feature<Point>(geometry: centroid, properties: featureInput.properties);
72+
// For Polygon: try to use the centroid, and fall back to a vertex if needed
73+
final centroidFeature = centroid(geometry);
74+
final cent = centroidFeature.geometry!;
75+
76+
if (booleanPointInPolygon(cent.coordinates!, geometry)) {
77+
return Feature<Point>(geometry: cent, properties: featureInput.properties);
5278
} else {
5379
// Try each vertex of the outer ring.
5480
final outerRing = geometry.coordinates.first;
5581
for (final pos in outerRing) {
5682
final candidate = Point(coordinates: pos);
57-
// Convert Point to Position for boolean check
58-
final candidatePos = Position(candidate.coordinates[0] ?? 0.0, candidate.coordinates[1] ?? 0.0);
59-
if (_pointInPolygon(candidatePos, geometry)) {
83+
if (booleanPointInPolygon(candidate.coordinates!, geometry)) {
6084
return Feature<Point>(geometry: candidate, properties: featureInput.properties);
6185
}
6286
}
6387
// Fallback: return the centroid.
64-
return Feature<Point>(geometry: centroid, properties: featureInput.properties);
88+
return Feature<Point>(geometry: cent, properties: featureInput.properties);
6589
}
6690
} else if (geometry is MultiPolygon) {
6791
// Use the first polygon from the MultiPolygon.
68-
if (geometry.coordinates.isNotEmpty && geometry.coordinates.first.isNotEmpty) {
69-
final firstPoly = Polygon(coordinates: geometry.coordinates.first);
70-
return pointOnFeature(Feature(
71-
geometry: firstPoly, properties: featureInput.properties));
92+
if (geometry.coordinates.isEmpty || geometry.coordinates.first.isEmpty) {
93+
throw Exception('Empty MultiPolygon: Cannot compute a point on an empty geometry');
7294
}
95+
96+
final firstPoly = Polygon(coordinates: geometry.coordinates.first);
97+
return pointOnFeature(Feature(
98+
geometry: firstPoly, properties: featureInput.properties));
7399
}
74100
}
75101

76-
// Unsupported input type.
77-
return null;
78-
}
79-
80-
/// Calculates the arithmetic centroid of a Polygon's outer ring.
81-
Point calculateCentroid(Polygon polygon) {
82-
final outerRing = polygon.coordinates.first;
83-
double sumX = 0.0;
84-
double sumY = 0.0;
85-
final count = outerRing.length;
86-
for (final pos in outerRing) {
87-
sumX += pos[0] ?? 0.0;
88-
sumY += pos[1] ?? 0.0;
89-
}
90-
return Point(coordinates: Position(sumX / count, sumY / count));
91-
}
92-
93-
/// Calculates a representative midpoint on a LineString.
94-
Feature<Point> _midpointOnLine(LineString line, Map<String, dynamic>? properties) {
95-
final coords = line.coordinates;
96-
if (coords.isEmpty) {
97-
// Fallback for empty LineString - should not happen with valid GeoJSON
98-
return Feature<Point>(
99-
geometry: Point(coordinates: Position(0, 0)),
100-
properties: properties
101-
);
102-
}
103-
104-
if (coords.length == 1) {
105-
// Only one point in the LineString
106-
return Feature<Point>(
107-
geometry: Point(coordinates: coords.first),
108-
properties: properties
109-
);
110-
}
111-
112-
// Calculate the midpoint of the first segment for simplicity
113-
// Note: This matches the test expectations
114-
final start = coords[0];
115-
final end = coords[1];
116-
117-
// Calculate the midpoint
118-
final midX = (start[0] ?? 0.0) + ((end[0] ?? 0.0) - (start[0] ?? 0.0)) / 2;
119-
final midY = (start[1] ?? 0.0) + ((end[1] ?? 0.0) - (start[1] ?? 0.0)) / 2;
120-
121-
return Feature<Point>(
122-
geometry: Point(coordinates: Position(midX, midY)),
123-
properties: properties
124-
);
125-
}
126-
127-
/// Checks if a point is inside a polygon using a ray-casting algorithm.
128-
bool _pointInPolygon(Position point, Polygon polygon) {
129-
final outerRing = polygon.coordinates.first;
130-
final int numVertices = outerRing.length;
131-
bool inside = false;
132-
final num pxNum = point[0] ?? 0.0;
133-
final num pyNum = point[1] ?? 0.0;
134-
final double px = pxNum.toDouble();
135-
final double py = pyNum.toDouble();
136-
137-
for (int i = 0, j = numVertices - 1; i < numVertices; j = i++) {
138-
final num xiNum = outerRing[i][0] ?? 0.0;
139-
final num yiNum = outerRing[i][1] ?? 0.0;
140-
final num xjNum = outerRing[j][0] ?? 0.0;
141-
final num yjNum = outerRing[j][1] ?? 0.0;
142-
final double xi = xiNum.toDouble();
143-
final double yi = yiNum.toDouble();
144-
final double xj = xjNum.toDouble();
145-
final double yj = yjNum.toDouble();
146-
147-
// Check if point is on a polygon vertex
148-
if ((xi == px && yi == py) || (xj == px && yj == py)) {
149-
return true;
150-
}
151-
152-
// Check if point is on a polygon edge
153-
if (yi == yj && yi == py &&
154-
((xi <= px && px <= xj) || (xj <= px && px <= xi))) {
155-
return true;
156-
}
157-
158-
// Ray-casting algorithm for checking if point is inside polygon
159-
final bool intersect = ((yi > py) != (yj > py)) &&
160-
(px < (xj - xi) * (py - yi) / (yj - yi + 0.0) + xi);
161-
if (intersect) {
162-
inside = !inside;
163-
}
164-
}
165-
166-
return inside;
102+
// Throw an exception for unsupported input types
103+
throw Exception('Unsupported input type: Cannot compute a point on ${featureInput.runtimeType}');
167104
}
168105

169106
/// Helper to estimate the "size" of a feature for comparison.

‎test/components/point_on_feature_test.dart‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,10 @@ void main() {
8989
final result = pointOnFeature(lineString);
9090

9191
expect(result, isNotNull);
92-
expect(result!.geometry!.coordinates!.toList(), equals([5.0, 5.0]));
92+
expect(result.geometry!.coordinates!.toList().length, equals(2));
93+
// The midpoint is calculated geodesically (not just simple averaging)
94+
expect(result.geometry!.coordinates![0], closeTo(5.0, 0.1));
95+
expect(result.geometry!.coordinates![1], closeTo(5.0, 0.1));
9396
});
9497

9598
test('FeatureCollection - returns point on largest feature', () {
@@ -121,10 +124,9 @@ void main() {
121124
expect(coords[1], lessThanOrEqualTo(10.0));
122125
});
123126

124-
test('Empty FeatureCollection returns null', () {
127+
test('Empty FeatureCollection throws exception', () {
125128
final emptyFC = FeatureCollection<GeometryObject>(features: []);
126-
final result = pointOnFeature(emptyFC);
127-
expect(result, isNull);
129+
expect(() => pointOnFeature(emptyFC), throwsException);
128130
});
129131
});
130132
}

0 commit comments

Comments
 (0)