yocontra / node-gdal-next

Node.js bindings for GDAL (Geospatial Data Abstraction Library) [Fork]
https://contra.io/node-gdal-next/
Apache License 2.0
75 stars 36 forks source link

Support for Curve Geometry #8

Open zhangjinzhou opened 4 years ago

zhangjinzhou commented 4 years ago

According to this RFC, gdal is able to handle curve geometry since 2.0. How does the nodejs interface handle this? Currently, the lib returns not supported geometry error.

yocontra commented 4 years ago

@zhangjinzhou Can you post your example code that returns the error? Our interface to GDAL may need some updating

zhangjinzhou commented 4 years ago

@contra The code is straightforward.

const dataset = gdal.open("path to fgdb including curve geometry")
const layer = dataset.layers.get("layer including curve geometry")
layer.features.forEach((feature) => {
    const geom = feature.getGeometry();
});
yocontra commented 4 years ago

@zhangjinzhou Can you include the file then? I need a way to reproduce your error.

zhangjinzhou commented 4 years ago

@contra I generated a fgdb with one layer and one geometry. Hope it is enough to explain the issue. CurveGeom.gdb.zip

mmomtchev commented 3 years ago

In fact I would qualify this as a missing feature. Curved geometries are indeed currently not supported by node-gdal-next even if the underlying support in GDAL exists. There is a substantial amount of code that is missing - mostly the underlying C++ classes for the JS objects representing the curved geometries. @contra, I think you can add it to the roadmap, but once again, it is a substantial amount of work and not a simple bugfix. I have lots of free time at the moment, so maybe I will take a look.

mmomtchev commented 3 years ago

I just went through the underlying C++ classes for the various geometry types and I found a huge amount of copying and pasting. There are currently 7 geometry classes, with no common base, each of about 120 to 150 lines, with most of them being 100% identical.

Someone should completely rewrite that part, with one base class and some templating/inheritance, I think that the amount of code can be reduced with something like 80% to 90%, all while producing the missing curve geometries in the process. If there is someone with good understanding of C++ and GDAL geometries, I am willing to provide Node/V8 guidance.

Look in src/gdal_[multi]{line|point|polygon}string.cpp

mmomtchev commented 3 years ago

The geometries are a major mess. On the JS side, it is a hierarchy of classes that inherit from a base class, on the GDAL C++ side it is a hierarchy too and in node-gdal there are separate independent classes in which major care have been taken to always declare all the shared variables in the exact same order so that pointer casting works. It is total fubar and needs fixing otherwise I won't be able to async it.