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.

Sign up for GitHub

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

Switch to compact-SRID WKB format #664

Merged
merged 2 commits into from
Jan 24, 2021
Merged

Switch to compact-SRID WKB format #664

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f1261ac
Switch to compact WKB format
hisener Jan 15, 2021
b542462
PR comments
hisener Jan 19, 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
34 changes: 15 additions & 19 deletions modules/core/src/main/java/org/locationtech/jts/io/WKBReader.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,8 +105,6 @@ private static int hexToInt(char hex)
private PrecisionModel precisionModel;
// default dimension - will be set on read
private int inputDimension = 2;
private boolean hasSRID = false;
private int SRID = 0;
/**
* true if structurally invalid input should be reported rather than repaired.
* At some point this could be made client-controllable.
Expand Down Expand Up @@ -156,11 +154,10 @@ public Geometry read(InStream is)
throws IOException, ParseException
{
dis.setInStream(is);
Geometry g = readGeometry();
return g;
return readGeometry(0);
}

private Geometry readGeometry()
private Geometry readGeometry(int SRID)
throws IOException, ParseException
{

Expand Down Expand Up @@ -201,8 +198,7 @@ else if(isStrict)
inputDimension = 2 + (hasZ?1:0) + (hasM?1:0);

// determine if SRIDs are present
hasSRID = (typeInt & 0x20000000) != 0;
int SRID = 0;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

So that we use the class field. This might be problematic in case a geometry collection contains mixed SRIDs as @dr-jts pointed here.

Copy link
Contributor

@dr-jts dr-jts Jan 15, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Yes, definitely problematic. This is a bug in the GEOS implementation IMO.

This should just use the SRID of the top-most geometry (or 0 if not set). JTS does not support mixed SRIDs in collections.

Copy link
Contributor Author

@hisener hisener Jan 15, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Shouldn't we throw a ParseException in that case instead of using the SRID of top-most geometry?

Copy link
Contributor

Choose a reason for hiding this comment

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

It's nice to allow reading as many WKB inputs as possible. Even if they are invalid, they can only be fixed if they are able to be read.

To preserve the most information, the SRIDs should always be read, and then applied to any missing SRIDs in child geometries. This will require passing the most recent SRID down the call stack as a parameter.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added a commit, PTAL.

Copy link
Contributor

Choose a reason for hiding this comment

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

Looks good.

boolean hasSRID = (typeInt & 0x20000000) != 0;
if (hasSRID) {
SRID = dis.readInt();
}
Expand All @@ -223,16 +219,16 @@ else if(isStrict)
geom = readPolygon();
break;
case WKBConstants.wkbMultiPoint :
geom = readMultiPoint();
geom = readMultiPoint(SRID);
break;
case WKBConstants.wkbMultiLineString :
geom = readMultiLineString();
geom = readMultiLineString(SRID);
break;
case WKBConstants.wkbMultiPolygon :
geom = readMultiPolygon();
geom = readMultiPolygon(SRID);
break;
case WKBConstants.wkbGeometryCollection :
geom = readGeometryCollection();
geom = readGeometryCollection(SRID);
break;
default:
throw new ParseException("Unknown WKB type " + geometryType);
Expand Down Expand Up @@ -292,52 +288,52 @@ private Polygon readPolygon() throws IOException
return factory.createPolygon(shell, holes);
}

private MultiPoint readMultiPoint() throws IOException, ParseException
private MultiPoint readMultiPoint(int SRID) throws IOException, ParseException
{
int numGeom = dis.readInt();
Point[] geoms = new Point[numGeom];
for (int i = 0; i < numGeom; i++) {
Geometry g = readGeometry();
Geometry g = readGeometry(SRID);
if (! (g instanceof Point))
throw new ParseException(INVALID_GEOM_TYPE_MSG + "MultiPoint");
geoms[i] = (Point) g;
}
return factory.createMultiPoint(geoms);
}

private MultiLineString readMultiLineString() throws IOException, ParseException
private MultiLineString readMultiLineString(int SRID) throws IOException, ParseException
{
int numGeom = dis.readInt();
LineString[] geoms = new LineString[numGeom];
for (int i = 0; i < numGeom; i++) {
Geometry g = readGeometry();
Geometry g = readGeometry(SRID);
if (! (g instanceof LineString))
throw new ParseException(INVALID_GEOM_TYPE_MSG + "MultiLineString");
geoms[i] = (LineString) g;
}
return factory.createMultiLineString(geoms);
}

private MultiPolygon readMultiPolygon() throws IOException, ParseException
private MultiPolygon readMultiPolygon(int SRID) throws IOException, ParseException
{
int numGeom = dis.readInt();
Polygon[] geoms = new Polygon[numGeom];

for (int i = 0; i < numGeom; i++) {
Geometry g = readGeometry();
Geometry g = readGeometry(SRID);
if (! (g instanceof Polygon))
throw new ParseException(INVALID_GEOM_TYPE_MSG + "MultiPolygon");
geoms[i] = (Polygon) g;
}
return factory.createMultiPolygon(geoms);
}

private GeometryCollection readGeometryCollection() throws IOException, ParseException
private GeometryCollection readGeometryCollection(int SRID) throws IOException, ParseException
{
int numGeom = dis.readInt();
Geometry[] geoms = new Geometry[numGeom];
for (int i = 0; i < numGeom; i++) {
geoms[i] = readGeometry();
geoms[i] = readGeometry(SRID);
}
return factory.createGeometryCollection(geoms);
}
Expand Down
3 changes: 3 additions & 0 deletions modules/core/src/main/java/org/locationtech/jts/io/WKBWriter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -380,9 +380,12 @@ private void writeGeometryCollection(int geometryType, GeometryCollection gc,
writeByteOrder(os);
writeGeometryType(geometryType, gc, os);
writeInt(gc.getNumGeometries(), os);
boolean originalIncludeSRID = this.includeSRID;
this.includeSRID = false;
for (int i = 0; i < gc.getNumGeometries(); i++) {
write(gc.getGeometryN(i), os);
}
this.includeSRID = originalIncludeSRID;
}

private void writeByteOrder(OutStream os) throws IOException
Expand Down
58 changes: 51 additions & 7 deletions modules/core/src/test/java/org/locationtech/jts/io/WKBReaderTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,12 @@

import org.locationtech.jts.geom.CoordinateSequenceComparator;
import org.locationtech.jts.geom.Geometry;
import org.locationtech.jts.geom.GeometryCollection;
import org.locationtech.jts.geom.GeometryFactory;
import org.locationtech.jts.geom.Polygon;

import junit.framework.TestCase;
import junit.textui.TestRunner;
import org.locationtech.jts.geom.impl.PackedCoordinateSequence;
import org.locationtech.jts.geom.impl.PackedCoordinateSequenceFactory;


Expand Down Expand Up @@ -101,12 +102,18 @@ public void test2dSpatialiteWKB() throws ParseException {
// MultiLineString
checkWKBGeometry("0105000020E6100000020000000102000000020000000000000000000000000000000000F03F000000000000004000000000000008400102000000020000000000000000001040000000000000144000000000000018400000000000001C40",
"MULTILINESTRING((0 1,2 3),(4 5,6 7))");
// MultiPolygon
checkWKBGeometry("0106000020E61000000200000001030000000200000005000000000000000000000000000000000000000000000000000000000000000000244000000000000024400000000000002440000000000000244000000000000000000000000000000000000000000000000005000000000000000000F03F000000000000F03F000000000000F03F0000000000002240000000000000224000000000000022400000000000002240000000000000F03F000000000000F03F000000000000F03F0103000000010000000500000000000000000022C0000000000000000000000000000022C00000000000002440000000000000F0BF0000000000002440000000000000F0BF000000000000000000000000000022C00000000000000000",
"MULTIPOLYGON(((0 0,0 10,10 10,10 0,0 0),(1 1,1 9,9 9,9 1,1 1)),((-9 0,-9 10,-1 10,-1 0,-9 0)))");
// GeometryCollection
checkWKBGeometry("0107000020E61000000900000001010000000000000000000000000000000000F03F01010000000000000000000000000000000000F03F01010000000000000000000040000000000000084001020000000200000000000000000000400000000000000840000000000000104000000000000014400102000000020000000000000000000000000000000000F03F000000000000004000000000000008400102000000020000000000000000001040000000000000144000000000000018400000000000001C4001030000000200000005000000000000000000000000000000000000000000000000000000000000000000244000000000000024400000000000002440000000000000244000000000000000000000000000000000000000000000000005000000000000000000F03F000000000000F03F000000000000F03F0000000000002240000000000000224000000000000022400000000000002240000000000000F03F000000000000F03F000000000000F03F01030000000200000005000000000000000000000000000000000000000000000000000000000000000000244000000000000024400000000000002440000000000000244000000000000000000000000000000000000000000000000005000000000000000000F03F000000000000F03F000000000000F03F0000000000002240000000000000224000000000000022400000000000002240000000000000F03F000000000000F03F000000000000F03F0103000000010000000500000000000000000022C0000000000000000000000000000022C00000000000002440000000000000F0BF0000000000002440000000000000F0BF000000000000000000000000000022C00000000000000000",
"GEOMETRYCOLLECTION(POINT(0 1),POINT(0 1),POINT(2 3),LINESTRING(2 3,4 5),LINESTRING(0 1,2 3),LINESTRING(4 5,6 7),POLYGON((0 0,0 10,10 10,10 0,0 0),(1 1,1 9,9 9,9 1,1 1)),POLYGON((0 0,0 10,10 10,10 0,0 0),(1 1,1 9,9 9,9 1,1 1)),POLYGON((-9 0,-9 10,-1 10,-1 0,-9 0)))");

String multiPolygonWkt = "MULTIPOLYGON(((0 0,0 10,10 10,10 0,0 0),(1 1,1 9,9 9,9 1,1 1)),((-9 0,-9 10,-1 10,-1 0,-9 0)))";
// MultiPolygon with non-compact WKB
checkWKBGeometry("0106000020E6100000020000000103000020E61000000200000005000000000000000000000000000000000000000000000000000000000000000000244000000000000024400000000000002440000000000000244000000000000000000000000000000000000000000000000005000000000000000000F03F000000000000F03F000000000000F03F0000000000002240000000000000224000000000000022400000000000002240000000000000F03F000000000000F03F000000000000F03F0103000020E6100000010000000500000000000000000022C0000000000000000000000000000022C00000000000002440000000000000F0BF0000000000002440000000000000F0BF000000000000000000000000000022C00000000000000000", multiPolygonWkt);
// MultiPolygon with compact WKB
checkWKBGeometry("0106000020E61000000200000001030000000200000005000000000000000000000000000000000000000000000000000000000000000000244000000000000024400000000000002440000000000000244000000000000000000000000000000000000000000000000005000000000000000000F03F000000000000F03F000000000000F03F0000000000002240000000000000224000000000000022400000000000002240000000000000F03F000000000000F03F000000000000F03F0103000000010000000500000000000000000022C0000000000000000000000000000022C00000000000002440000000000000F0BF0000000000002440000000000000F0BF000000000000000000000000000022C00000000000000000", multiPolygonWkt);

String geometryCollectionWkt = "GEOMETRYCOLLECTION(POINT(0 1),POINT(0 1),POINT(2 3),LINESTRING(2 3,4 5),LINESTRING(0 1,2 3),LINESTRING(4 5,6 7),POLYGON((0 0,0 10,10 10,10 0,0 0),(1 1,1 9,9 9,9 1,1 1)),POLYGON((0 0,0 10,10 10,10 0,0 0),(1 1,1 9,9 9,9 1,1 1)),POLYGON((-9 0,-9 10,-1 10,-1 0,-9 0)))";
// GeometryCollection with non-compact WKB
checkWKBGeometry("0107000020E6100000090000000101000020E61000000000000000000000000000000000F03F0101000020E61000000000000000000000000000000000F03F0101000020E6100000000000000000004000000000000008400102000020E61000000200000000000000000000400000000000000840000000000000104000000000000014400102000020E6100000020000000000000000000000000000000000F03F000000000000004000000000000008400102000020E6100000020000000000000000001040000000000000144000000000000018400000000000001C400103000020E61000000200000005000000000000000000000000000000000000000000000000000000000000000000244000000000000024400000000000002440000000000000244000000000000000000000000000000000000000000000000005000000000000000000F03F000000000000F03F000000000000F03F0000000000002240000000000000224000000000000022400000000000002240000000000000F03F000000000000F03F000000000000F03F0103000020E61000000200000005000000000000000000000000000000000000000000000000000000000000000000244000000000000024400000000000002440000000000000244000000000000000000000000000000000000000000000000005000000000000000000F03F000000000000F03F000000000000F03F0000000000002240000000000000224000000000000022400000000000002240000000000000F03F000000000000F03F000000000000F03F0103000020E6100000010000000500000000000000000022C0000000000000000000000000000022C00000000000002440000000000000F0BF0000000000002440000000000000F0BF000000000000000000000000000022C00000000000000000", geometryCollectionWkt);
// GeometryCollection with compact WKB
checkWKBGeometry("0107000020E61000000900000001010000000000000000000000000000000000F03F01010000000000000000000000000000000000F03F01010000000000000000000040000000000000084001020000000200000000000000000000400000000000000840000000000000104000000000000014400102000000020000000000000000000000000000000000F03F000000000000004000000000000008400102000000020000000000000000001040000000000000144000000000000018400000000000001C4001030000000200000005000000000000000000000000000000000000000000000000000000000000000000244000000000000024400000000000002440000000000000244000000000000000000000000000000000000000000000000005000000000000000000F03F000000000000F03F000000000000F03F0000000000002240000000000000224000000000000022400000000000002240000000000000F03F000000000000F03F000000000000F03F01030000000200000005000000000000000000000000000000000000000000000000000000000000000000244000000000000024400000000000002440000000000000244000000000000000000000000000000000000000000000000005000000000000000000F03F000000000000F03F000000000000F03F0000000000002240000000000000224000000000000022400000000000002240000000000000F03F000000000000F03F000000000000F03F0103000000010000000500000000000000000022C0000000000000000000000000000022C00000000000002440000000000000F0BF0000000000002440000000000000F0BF000000000000000000000000000022C00000000000000000", geometryCollectionWkt);
}

public void testSpatialiteWKB_Z() throws ParseException {
Expand Down Expand Up @@ -180,6 +187,30 @@ public void testSpatialiteWKB_ZM() throws ParseException {
// "MULTIPOLYGONM(((0 0 100,0 10 100,10 10 100,10 0 100,0 0 100),(1 1 100,1 9 100,9 9 100,9 1 100,1 1 100)),((-9 0 50,-9 10 50,-1 10 50,-1 0 50,-9 0 50)))");
}

public void testSRIDInSubGeometry() throws ParseException {
// MultiPolygon
checkSRID("0106000020E61000000200000001030000000200000005000000000000000000000000000000000000000000000000000000000000000000244000000000000024400000000000002440000000000000244000000000000000000000000000000000000000000000000005000000000000000000F03F000000000000F03F000000000000F03F0000000000002240000000000000224000000000000022400000000000002240000000000000F03F000000000000F03F000000000000F03F0103000000010000000500000000000000000022C0000000000000000000000000000022C00000000000002440000000000000F0BF0000000000002440000000000000F0BF000000000000000000000000000022C00000000000000000", 4326);
// GeometryCollection
checkSRID("0107000020E61000000900000001010000000000000000000000000000000000F03F01010000000000000000000000000000000000F03F01010000000000000000000040000000000000084001020000000200000000000000000000400000000000000840000000000000104000000000000014400102000000020000000000000000000000000000000000F03F000000000000004000000000000008400102000000020000000000000000001040000000000000144000000000000018400000000000001C4001030000000200000005000000000000000000000000000000000000000000000000000000000000000000244000000000000024400000000000002440000000000000244000000000000000000000000000000000000000000000000005000000000000000000F03F000000000000F03F000000000000F03F0000000000002240000000000000224000000000000022400000000000002240000000000000F03F000000000000F03F000000000000F03F01030000000200000005000000000000000000000000000000000000000000000000000000000000000000244000000000000024400000000000002440000000000000244000000000000000000000000000000000000000000000000005000000000000000000F03F000000000000F03F000000000000F03F0000000000002240000000000000224000000000000022400000000000002240000000000000F03F000000000000F03F000000000000F03F0103000000010000000500000000000000000022C0000000000000000000000000000022C00000000000002440000000000000F0BF0000000000002440000000000000F0BF000000000000000000000000000022C00000000000000000", 4326);
}

public void testInvalidWkbShouldBeReadable() throws ParseException {
WKBReader wkbReader = new WKBReader(geomFactory);
// The last sub-geometry uses 2029 unlike others.
Geometry geometry = wkbReader.read(WKBReader.hexToBytes("0107000020E6100000090000000101000020E61000000000000000000000000000000000F03F0101000020E61000000000000000000000000000000000F03F0101000020E6100000000000000000004000000000000008400102000020E61000000200000000000000000000400000000000000840000000000000104000000000000014400102000020E6100000020000000000000000000000000000000000F03F000000000000004000000000000008400102000020E6100000020000000000000000001040000000000000144000000000000018400000000000001C400103000020E61000000200000005000000000000000000000000000000000000000000000000000000000000000000244000000000000024400000000000002440000000000000244000000000000000000000000000000000000000000000000005000000000000000000F03F000000000000F03F000000000000F03F0000000000002240000000000000224000000000000022400000000000002240000000000000F03F000000000000F03F000000000000F03F0103000020E61000000200000005000000000000000000000000000000000000000000000000000000000000000000244000000000000024400000000000002440000000000000244000000000000000000000000000000000000000000000000005000000000000000000F03F000000000000F03F000000000000F03F0000000000002240000000000000224000000000000022400000000000002240000000000000F03F000000000000F03F000000000000F03F0103000020ED070000010000000500000000000000000022C0000000000000000000000000000022C00000000000002440000000000000F0BF0000000000002440000000000000F0BF000000000000000000000000000022C00000000000000000"));

assertTrue(geometry instanceof GeometryCollection);
assertEquals(4326, geometry.getSRID());

GeometryCollection geometryCollection = (GeometryCollection) geometry;
for(int i = 0; i < geometryCollection.getNumGeometries() - 1; i++) {
assertEquals(4326, geometryCollection.getGeometryN(i).getSRID());
}
Geometry lastSubGeometry = geometryCollection.getGeometryN(geometryCollection.getNumGeometries() - 1);
assertTrue(lastSubGeometry instanceof Polygon);
assertEquals(2029, lastSubGeometry.getSRID());
}

/**
* Not yet implemented satisfactorily.
*
Expand Down Expand Up @@ -213,4 +244,17 @@ else if (expectedWKT.contains("M(") || expectedWKT.contains("M ("))
assertTrue(isEqual);

}

private void checkSRID(String wkbHex, int expectedSrid) throws ParseException {
WKBReader wkbReader = new WKBReader(geomFactory);
Geometry geometry = wkbReader.read(WKBReader.hexToBytes(wkbHex));

assertTrue(geometry instanceof GeometryCollection);
assertEquals(expectedSrid, geometry.getSRID());

GeometryCollection geometryCollection = (GeometryCollection) geometry;
for(int i = 0; i < geometryCollection.getNumGeometries(); i++) {
assertEquals(expectedSrid, geometryCollection.getGeometryN(i).getSRID());
}
}
}
Loading