Opened 14 years ago

Closed 14 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: main
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 by pramsey, 14 years ago

Milestone: 3.2.1

comment:2 by strk, 14 years ago

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 by strk, 14 years ago

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

comment:4 by warmerdam, 14 years ago

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 by strk, 14 years ago

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 by warmerdam, 14 years ago

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 by warmerdam, 14 years ago

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 by warmerdam, 14 years ago

GEOSGeom_getCoordinateDimension() added to C API (#2997)

Only outstanding is correcting behavior of GEOSGeom_getDimensions().

comment:9 by warmerdam, 14 years ago

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.