CompoundCurve: Add validateConstruction method

[dbaston]

Resolves #1103

[dr-jts]

Would this make sense to add to Geometry.isValid()? And possibly to IsValidOp.

[rouault]

I may miss something, but it is not obvious to me that you can't insert an empty curve in a CompoundCurve, in which case back() and front() are returning something invalid. At least if using directly the CompoundCurve::CompoundCurve(std::vector<std::unique_ptr>&& p_curves, const GeometryFactory& gf) constructor. That might be hard/impossible to trigger through WKT. But maybe through WKB ?

[dbaston]

You're right, this is possible and would crash. Of course it couldn't be so easy.

[dbaston]

Would this make sense to add to Geometry.isValid()? And possibly to IsValidOp.

I was aiming to handle it consistently with how we handle linear rings, where we simply disallow the construction of a ring with a single point, or a ring that doesn't close.

geos/src/geom/LinearRing.cpp

Lines 47 to 68 in bf2257a

	void
	LinearRing::validateConstruction()
	{
	// Empty ring is valid
	if(points->isEmpty()) {
	return;
	}
	if(!LineString::isClosed()) {
	throw util::IllegalArgumentException(
	"Points of LinearRing do not form a closed linestring"
	);
	}
	if(points->getSize() < MINIMUM_VALID_SIZE) {
	std::ostringstream os;
	os << "Invalid number of points in LinearRing found "
	<< points->getSize() << " - must be 0 or >= " << MINIMUM_VALID_SIZE;
	throw util::IllegalArgumentException(os.str());
	}
	}

[dr-jts]

Would this make sense to add to Geometry.isValid()? And possibly to IsValidOp.

I was aiming to handle it consistently with how we handle linear rings, where we simply disallow the construction of a ring with a single point, or a ring that doesn't close.

So would this mean that a WKT or WKB serialization of an invalid compound curve could not be read? Sometimes users like to be able to read a structurally invalid geometry, so the library can be used to test validity and possible allow repair.

I wonder how PostGIS handles this?

[pramsey]

Probably the same as LinearRing, which is (wait for it) to disallow non-closed WKT but allow non-closed WKB.

select 'CompoundCurve((0 0, 0 1, 1 1), (1 2, 1 0, 0 0))'::geometry; 
ERROR:  incontinuous compound curve

The point of being liberal in the binary is to allow dump/restore to work, even when highly invalid geometries have found their way into a table.

[rouault]

So would this mean that a WKT or WKB serialization of an invalid compound curve could not be read? Sometimes users like to be able to read a structurally invalid geometry, so the library can be used to test validity and possible allow repair

I was wondering about that too. I've checked that GDAL allows for an extremely tiny gap of 1e-14, which probably is never met in practice.

# 1e-13 difference: rejected:
>>> g = ogr.CreateGeometryFromWkt('CompoundCurve((0 0,1 1),(1 1.0000000000001,3 3))')
ERROR 1: Non contiguous curves
ERROR 6: OGR Error: General Error

# 1e-14 difference: tolerated
>>> g = ogr.CreateGeometryFromWkt('CompoundCurve((0 0,1 1),(1 1.00000000000001,3 3))')
>>> print(g)
COMPOUNDCURVE ((0 0,1 1),(1 1,3 3))

What happens in practice though is when ingesting GML curves, and in particular when the compound curve chaining straight lines with circular portions where the circular portions are not defined as a CircularString, but as a ArcByCenterPoint, where you need to recompute the start and ending points from the center point and the start and end angles, which easily leads to precision issues. But I've ended up compensating for that in the GML import logic itself, because the gap is generally much larger than 1e-14.

[dbaston]

So would this mean that a WKT or WKB serialization of an invalid compound curve could not be read?

Correct, GEOS does not allow the construction of linear rings that are not closed, linear rings that have fewer than 3 points, or linestrings with a single point.