search

libgeos / geos

Geometry Engine, Open Source
https://libgeos.org
GNU Lesser General Public License v2.1
1.1k stars 339 forks source link

CoordinateSequence constructors change dimension to 3, fail to set hasZ #1090

Open tfiner opened 1 month ago

tfiner commented 1 month ago

  1. GEOS_COORDSEQ_PADZ is looked at in 3 constructors in CoordinateSequence.cpp, each time ignoring the hasz parameter, then heavy handedly bumping the m_stride manually to 1 more than the constructor's signature would indicate. The constructors also all fail to account for what they have done and don't set m_hasz(true). This has a cascading effect where the sequences then incorrectly report their number of dimensions. And it makes the type returned (XYZ) out of sync with hasZ too.

  2. CoordinateSequence(std::size_t sz, std:size_t dim): adds a 1 to dim, regardless of GEOS_COORDSEQ_PADZ, but then also doesn't set m_hasz(true).

The only constructor that doesn't lie is CoodinateSequence(const std::initializer_list<CoordinateXYZM>& list) which correctly sets all the members as expected.

It is surprising to me that the type system isn't being used here to greater effect. It is a bit surprising that the underlying data is being laid out manually (XYZ), and then it is wasting space. One of the reasons to choose C++ over other languages is that in C++ we don't pay for things we aren't using. From my own experience, I use 2D algorithms far more often than 3D and I suspect others using this library are as well.

dbaston commented 1 month ago

See https://github.com/libgeos/geos/issues/872 for discussion about why this is the case.

tfiner commented 1 month ago

I understand why you added it, I'm saying that to me, the it is inconsistent to allocate for Z, then say it doesn't have Z, return XYZ as a type, and set the stride to 3. 2 out of 4 things say that the coordinates have 3 dimensions: hasZ(), getDimension() vs. getCoordinateType() and stride(). Admittedly, stride() is private, but so was that comment in the code for the define that made everything XYZ.