CAPI functions do not preserve SRID

[dbaston]

For example, the following test fails:

    void object::test<11>()
    {
        geom1_ = GEOSGeomFromWKT("LINESTRING(0 0, 10 0)");
        GEOSSetSRID(geom1_, 32145);

        geom2_ = GEOSOffsetCurve(geom1_, 2, 0, GEOSBUF_JOIN_ROUND, 2);

        ensure_equals( GEOSGetSRID(geom1_), GEOSGetSRID(geom2_) );
    }

It's obvious that SRID could be preserved here. For binary operations, I'd say that it's not the job of GEOS to check SRID consistency, and that the result could have the SRID of the first argument.

Any thoughts about this?

[robe]

I would agree, but I think this would constitute a policy change so can't go in 3.6.3.

Could go in 3.7.0 even though we've called beta since it's not really an api change.

[strk]

Did you check what do Intersection/Union do ? I suspect OffsetCurve is just behind as being original code (rather than direct JTS port)

[dbaston]

No, I didn't check intersection or union. I started writing unit tests for this, and then it occurred to me that maybe this wasn't considered a bug. If there's agreement that it's a bug, I can go ahead with tests and fixes for all functions.

[dbaston]

It looks like the problem is unique to the C API, and affects all functions that produce a geometry as an output.

When a GEOS Geometry is created, it is given the SRID of its GeometryFactory. However, the C API does not use the concept of a GeometryFactory and provides no access to it. When the C API function GEOSSetSRID is called, it sets the SRID of an individual geometry, but leaves the GeometryFactory untouched.

Any objections to resolving this at the C API level, by setting the SRID of created geometries to match their inputs?

[robe]

I've got no objections. I don't even have objections if you want to back port to 3.6, 3.5 as it does sound like a bug.

[strk]

I don't think GeometryFactory should be affected by GEOSGeom_setSRID. JTS also doesn't set SRID of output geometry from operations, I wonder what Martin Davis thinks about this.

I'm +0 with adding SRID propagation in C-API (as I guess as of now all clients are already doing this in their converters)

[strk]

So it turns out Geometry#setSRID is supposed to be only used in exceptional cases, for backward compatibility: ​https://github.com/locationtech/jts/blob/master/modules/core/src/main/java/org/locationtech/jts/geom/Geometry.java#L264

Rather, a user should indeed set the SRID in the factory, which we're not doing from C-API.

Now that we DO have garbage-collected factories (for precision model) we might start doing something about SRID too. Sounds like a wider plan is needed though.

[dbaston]

Hi strk, I probably expressed myself poorly. I don't think the C API should modify the GeometryFactory.

Because it is not possible to manipulate a GeometryFactory in the C API, I think that the C API should set the SRID of the output geometries (after they are created) to match the SRID of the inputs.

I don't think we should try to retrofit GeometryFactories into the C API.

[dbaston]

Related: #724

[dbaston]

To attempt to further clarify, I am proposing that C API functions be modified like this (from GEOSUnaryUnion_r):

GeomPtr g3 ( g->Union() );
g3->setSRID( g->getSRID() ); // proposed modification
return g3.release();

[pramsey]

Thoughts, @dbaston? Now or push?

​https://github.com/libgeos/geos/pull/230

[dbaston]

I'm less and less understanding the use case for GEOS to carry a 4-byte SRID on every geometry (actually 8 bytes I think because of padding), with which it does nothing, especially given that we already provide a "userData" pointer which is probably also not used frequently. Given that the behavior has been incorrect for years and nobody besides me has complained, I'd be inclined to remove SRID in the next major release.

[Paul Ramsey <pramsey@…>]

In 76e23a2/git:

Preserve SRID in constructive geometry functions called via CAPI
Closes #896