Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow GeoJSONWriter to force polygons to be CCW #694

Merged
merged 16 commits into from
Feb 25, 2021
Merged
Show file tree
Hide file tree
Changes from 13 commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
49284e7
correct parameter order on assertEquals, avoiding confusion when test…
ruibritopt Feb 15, 2021
3cdc88a
fix tests that would fail the new right hand rule
ruibritopt Feb 15, 2021
0054db6
right hand rules tests for polygon with exterior and polygon with hole
ruibritopt Feb 15, 2021
f6fd350
enforce right hand rule when writing the geojson string
ruibritopt Feb 15, 2021
934bac8
Merge branch 'master' into feat/RFC-7946-geojson-spec-right-hand-rule
ruibritopt Feb 16, 2021
9ab1f57
Merge branch 'master' into feat/RFC-7946-geojson-spec-right-hand-rule
ruibritopt Feb 17, 2021
e4f26a1
creation of a TransformationUtils class that holds the right hand rul…
ruibritopt Feb 17, 2021
aa689ba
added isRHREnforced option to GeoJsonWriter class which invokes the t…
ruibritopt Feb 17, 2021
63b4708
i was getting index out of bounds with an implementation that uses ar…
ruibritopt Feb 17, 2021
4650572
resolved potential aliases errors && renamed right hand rule methods …
ruibritopt Feb 18, 2021
6aa05b4
renamed right hand rule method args to CCW
ruibritopt Feb 18, 2021
666a775
extended javadoc information
ruibritopt Feb 19, 2021
8bca609
Merge branch 'master' into feat/RFC-7946-geojson-spec-right-hand-rule
ruibritopt Feb 19, 2021
d027011
renamed arguments for the sake of brevity
ruibritopt Feb 22, 2021
77300ae
Merge remote-tracking branch 'origin/feat/RFC-7946-geojson-spec-right…
ruibritopt Feb 22, 2021
4a2e129
Merge branch 'master' into feat/RFC-7946-geojson-spec-right-hand-rule
ruibritopt Feb 23, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -11,14 +11,6 @@
*/
package org.locationtech.jts.io.geojson;

import java.io.IOException;
import java.io.StringWriter;
import java.io.Writer;
import java.util.ArrayList;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;

import org.json.simple.JSONAware;
import org.json.simple.JSONObject;
import org.locationtech.jts.geom.CoordinateSequence;
Expand All @@ -32,6 +24,14 @@
import org.locationtech.jts.geom.Polygon;
import org.locationtech.jts.util.Assert;

import java.io.IOException;
import java.io.StringWriter;
import java.io.Writer;
import java.util.ArrayList;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;


/**
* Writes {@link Geometry}s as JSON fragments in GeoJSON format.
Expand Down Expand Up @@ -59,6 +59,7 @@ public class GeoJsonWriter {

private double scale;
private boolean isEncodeCRS = true;
private boolean isEnforceCCW = false;

/**
* Constructs a GeoJsonWriter instance.
Expand Down Expand Up @@ -88,6 +89,16 @@ public void setEncodeCRS(boolean isEncodeCRS) {
this.isEncodeCRS = isEncodeCRS;
}

/**
* Sets whether the GeoJSON should be output following counter-clockwise orientation aka Right Hand Rule defined in RFC7946
* See <a href="https://tools.ietf.org/html/rfc7946#section-3.1.6">RFC 7946 Specification</a> for more context.
*
* @param enforceCCW true if the GeoJSON should be output following the RFC7946 counter-clockwise orientation aka Right Hand Rule
*/
public void setEnforceCCW(boolean enforceCCW) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One final request. In the interest of brevity, can this be changed to setForceCCW and isForceCCW?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done and done. Don't hold back on the requests, they have all been on the money and I'm a big fan of things done right 😄👍👍

this.isEnforceCCW = enforceCCW;
}

/**
* Writes a {@link Geometry} in GeoJson format to a String.
*
Expand Down Expand Up @@ -158,6 +169,10 @@ public String toJSONString() {
} else if (geometry instanceof Polygon) {
Polygon polygon = (Polygon) geometry;

if (isEnforceCCW) {
polygon = (Polygon) OrientationTransformer.transformCCW(polygon);
}

result.put(GeoJsonConstants.NAME_COORDINATES, makeJsonAware(polygon));

} else if (geometry instanceof MultiPoint) {
Expand All @@ -173,6 +188,10 @@ public String toJSONString() {
} else if (geometry instanceof MultiPolygon) {
MultiPolygon multiPolygon = (MultiPolygon) geometry;

if (isEnforceCCW) {
multiPolygon = (MultiPolygon) OrientationTransformer.transformCCW(multiPolygon);
}

result.put(GeoJsonConstants.NAME_COORDINATES, makeJsonAware(multiPolygon));

} else if (geometry instanceof GeometryCollection) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
package org.locationtech.jts.io.geojson;

import org.locationtech.jts.algorithm.Orientation;
import org.locationtech.jts.geom.*;

import java.util.ArrayList;
import java.util.List;

/**
* Utilities to modify the ring orientation of polygonal geometries.
*/
public class OrientationTransformer {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add class Javadoc. Can be something simple like

Utilities to modify the ring orientation of polygonal geometries.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Took your recommendation verbatim


/**
* Transforms a geometry using the Right Hand Rule specifications defined
* in the latest GeoJSON specification.
* See <a href="https://tools.ietf.org/html/rfc7946#section-3.1.6">RFC-7946 Specification</a> for more context.
*
* @param geometry to be transformed
* @return Geometry under the Right Hand Rule specifications
*/
public static Geometry transformCCW(final Geometry geometry) {

if (geometry instanceof MultiPolygon) {
MultiPolygon multiPolygon = (MultiPolygon) geometry;

List<Polygon> polygons = new ArrayList<>();
for (int i = 0; i < multiPolygon.getNumGeometries(); i++) {
final Geometry polygon = multiPolygon.getGeometryN(i);
polygons.add((Polygon) transformCCW(polygon));
}

return new GeometryFactory().createMultiPolygon(polygons.toArray(new Polygon[0]));

} else if (geometry instanceof Polygon) {
return transformCCW((Polygon) geometry);

} else {
return geometry;
}
}

/**
* Transforms a polygon using the Right Hand Rule specifications defined
* in the latest GeoJSON specification.
* See <a href="https://tools.ietf.org/html/rfc7946#section-3.1.6">RFC-7946 Specification</a> for more context.
*
* @param polygon to be transformed
* @return Polygon under the Right Hand Rule specifications
*/
public static Polygon transformCCW(Polygon polygon) {
LinearRing exteriorRing = polygon.getExteriorRing();
LinearRing exteriorRingEnforced = transformCCW(exteriorRing, true);

List<LinearRing> interiorRings = new ArrayList<>();
for (int i = 0; i < polygon.getNumInteriorRing(); i++) {
interiorRings.add(transformCCW(polygon.getInteriorRingN(i), false));
}

return new GeometryFactory(polygon.getPrecisionModel(), polygon.getSRID())
.createPolygon(exteriorRingEnforced, interiorRings.toArray(new LinearRing[0]));
}

/**
* Transforms a polygon using the Right Hand Rule specifications defined
* in the latest GeoJSON specification.
* A linear ring MUST follow the right-hand rule with respect to the
* area it bounds, i.e., exterior rings are counterclockwise, and
* holes are clockwise.
*
* See <a href="https://tools.ietf.org/html/rfc7946#section-3.1.6">RFC 7946 Specification</a> for more context.
*
* @param ring the LinearRing, a constraint specific to Polygons
* @param isExteriorRing true if the LinearRing is the exterior polygon ring, the one that defines the boundary
* @return LinearRing under the Right Hand Rule specifications
*/
public static LinearRing transformCCW(LinearRing ring, boolean isExteriorRing) {
final boolean isRingClockWise = !Orientation.isCCW(ring.getCoordinateSequence());

final LinearRing rightHandRuleRing;
if (isExteriorRing) {
rightHandRuleRing = isRingClockWise? ring.reverse() : (LinearRing) ring.copy();
} else {
rightHandRuleRing = isRingClockWise? (LinearRing) ring.copy() : ring.reverse();
}
return rightHandRuleRing;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,16 @@ public void testPolygonWithHole() throws ParseException {
"{'type':'Polygon','coordinates':[[[0.0,0.0],[100,0.0],[100,100],[0.0,100],[0.0,0.0]],[[1,1],[1,10],[10,10],[10,1],[1,1]]]}");
}

public void testPolygonRightHandRule() throws ParseException {
runTest("POLYGON ((0 0, 0 100, 100 100, 100 0, 0 0))", true,
"{'type':'Polygon','coordinates':[[[0.0,0.0],[100,0.0],[100,100],[0.0,100],[0.0,0.0]]]}");
}

public void testPolygonWithHoleRightHandRule() throws ParseException {
runTest("POLYGON ((0 0, 0 100, 100 100, 100 0, 0 0), (1 1, 10 1, 10 10, 1 10, 1 1) )", true,
"{'type':'Polygon','coordinates':[[[0.0,0.0],[100,0.0],[100,100],[0.0,100],[0.0,0.0]],[[1,1],[1,10],[10,10],[10,1],[1,1]]]}");
}

public void testMultiPoint() throws ParseException {
runTest("MULTIPOINT ((0 0), (1 4), (100 200))",
"{'type':'MultiPoint','coordinates':[[0.0,0.0],[1,4],[100,200]]}");
Expand Down Expand Up @@ -125,20 +135,25 @@ private void runTest(String wkt) throws ParseException {
}

private void runTest(String wkt, String expectedGeojson) throws ParseException {
runTest(wkt, 0, false, expectedGeojson);
runTest(wkt, 0, false, false, expectedGeojson);
}

private void runTest(String wkt, int srid, String expectedGeojson) throws ParseException {
runTest(wkt, srid, true, expectedGeojson);
runTest(wkt, srid, true, false, expectedGeojson);
}

private void runTest(String wkt, boolean enforceRHR, String expectedGeojson) throws ParseException {
runTest(wkt, 0, false, enforceRHR, expectedGeojson);
}

private void runTest(String wkt, int srid, boolean encodeCRS, String expectedGeojson) throws ParseException {
private void runTest(String wkt, int srid, boolean encodeCRS, boolean enforceRHR, String expectedGeojson) throws ParseException {
Geometry geom = read(wkt);
geom.setSRID(srid);
geoJsonWriter.setEncodeCRS(encodeCRS);
geoJsonWriter.setEnforceCCW(enforceRHR);
String json = this.geoJsonWriter.write(geom);
json = json.replace('"', '\'');
assertEquals(json, expectedGeojson);
assertEquals(expectedGeojson, json);
}

}