Opened 11 years ago

Closed 10 years ago

#311 closed defect (fixed)

GEOSGeom_getDimensions returns only 3

Reported by: atrofast Owned by: warmerdam
Priority: major Milestone: 3.2.1
Component: C API Version: master
Severity: Significant Keywords: dimension
Cc: Charles.Thibert@…, pramsey

Description

The GEOSGeom_getDimensions (and _r) in the C API returns only 3, it seems that it never calls the Geometry object's getDimension but p->getCoordinateRO->getDimension() which I believe always leads to a 3 as seen in this file: source/headers/geos/geom/CoordinateArraySequence.h

Change History (9)

comment:1 Changed 11 years ago by pramsey

Milestone: 3.2.1

comment:2 Changed 11 years ago by strk

Documentation in the.h file also says that: Return 0 on exception (or empty geometry)

I'm afraid changing it would be the first ever C-API semantic change.

Where is it documented it should return other than always 3 ?

Lack of documentation, oops.

Unfortunately the *only* documentation says that 0 is an exception so converting this to a proper "geom dimension" extractor would break the only existing doc for it.

What about deprecating and exposing a new interface ?

comment:3 Changed 11 years ago by strk

a GEOSGeom_getDimensionType returning a newly-exposed GEOSDimensionType enum is my proposal. Not for 3.2 ofc, but for 3.3

comment:4 Changed 11 years ago by warmerdam

Keywords: dimension added

For #345 I have implemented support for preserving the coordinate dimension in the CoordinateArraySequence? class so as long as we call that we will start getting 2 in some cases.

In the last comment strk seems to suggest changing this function to return the GEOSDimensionType, presumably the same as Dimension::DimensionType? from the C++ code. In the normal OGC naming the "dimension" of a geometry is 1 for points, 2 for lines and 3 for areas while it seems we have previously been assuming that this method would return the "coordinate dimension", ie. 2 for 2D or 3 for 2.5D (with Z).

It does make me nervous to change the interpretation at this point, even at a major release point. But if we do, I do think we should add a GEOSGeom_getCoordinateDimension() method.

comment:5 Changed 11 years ago by strk

I was actually suggesting to _add_ a new method, deprecating the current one. The current one is useless anyway, so deprecation would hardly affect any user.

The need to add also arises from the fact that the _only_ documentation for current function is that it'd return 0 on exception, which is incompatible with returning 0 for a POINT.

In OGC point has 0 dimensions, line 1 (length), polygon 2 (area)

comment:6 Changed 11 years ago by warmerdam

Cc: pramsey added
Owner: changed from pramsey to warmerdam

I'm going to take on adding a getCoordinateDimension() method on Geometry which returns 2/3/4 depending on the dimension of the coordinates (as opposed to the dimension of the geometry).

Note, I'll only be doing this work in trunk for 3.3, not in the 3.2 branch which is currently set as the milestone for this ticket.

comment:7 Changed 11 years ago by warmerdam

Status: newassigned

Added getCoordinateDimension() method on the GEOSGeometry C++ class as part of the larger commit (r2995).

Still need to expose this in C and correct the action of the getDimension() method in C.

comment:8 Changed 10 years ago by warmerdam

GEOSGeom_getCoordinateDimension() added to C API (#2997)

Only outstanding is correcting behavior of GEOSGeom_getDimensions().

comment:9 Changed 10 years ago by warmerdam

Resolution: fixed
Status: assignedclosed

Make that (r2997), not #2997.

Also, it turns out I did change the behavior in r2997 such that GEOSGeom_getDimensions() now returns the geometry dimension rather than the coordinate dimension, but sticking with an integer return type instead of the C++ API enum.

Note: See TracTickets for help on using tickets.