Hi Even,

10.07.2015 00:19, Even Rouault пишет:
Hi Dmitry,

I have just finished the integration of GSoC2014 work into the
geographical network support in GDAL.
The major changes from original code:
1) The base network class (GNMNetwork) is inherited form GDALDataset. So
any GDAL utility, that works with GDALDataset can work with GNMNetwork.
2) Geographical network is a new type of data, additionally to the
DCAP_RASTER and DCAP_VECTOR, added DCAP_GNM
3) I created the generic implementation of GDAL geographical network
(GNMGenericNetwork). The generic network have some features: support of
connection rules, support virtual vertices and edges, etc. The
GNMNetwork API can be changed in future as common functions to generic
network and some other networks (pgRouting, ArcGIS, OSRM, etc.) should
be moved to the GNMNetwork class.
4) Two network drivers created for generic network: file based
datasource and db based datasource. The file network driver tested on
ESRI shapefile and db - on PostGIS. The another vector datasource which
are not applicable with this drivers should have their own drivers.
I could probably figure out that myself by looking at the code, but what are
the main difference between the generic file network driver and the db file
network driver ? Both should use the generic OGR API, no ?
The difference come from the OGR driver specific. For example, database can
store strings more than 255 characters, but shape file - not (I need to store the
spatial reference somethere), also in file based sources the layer usually
connect with it's own datasource, and database have a one datasource for all
layers. Also, in file based datasource the network is a sonamed folder, and
during deletion process the folder may be deleted if it empty (if user put to this folder it's own files, the folder will not deleted). In case of database the network is a database schema. So, the such differences force me to make the two drivers. Also this drivers show the example of implementation the generic network
support.

5) The main method of created and configured GNMNetwork is GetPath,
which return the temporary OGRLayer (similar to ExecuteSQL).
6) By default the GNM is switched off. The ./configure --with-gnm should
be run to enable GNM. Mayne set it on by default?
I think the decision depends mostly on how much we are confident that the API
is stable. While there's not a non-generic driver implemented, there's
potentially a risk (I think, but I've not a strong opinion on this.)
Ok, let leave it. Also I change nmake to not build GNM by default

7) The python binding is implemented and used in tests.

The RFC 48 in most cases is actual, except C API, which is not
implemented.  Do we need it?
The idea is that the SWIG bindings only needs to know the C API, which provide
some insulation from C++ ABI changes. So you could theoretically use the GDAL
2.0.0 Python bindings with a GDAL 2.1.0 shared object.
But the main reason might be to avoid issues related to C++ ABI if not using
the same compiler for the GDAL dll and building the Python bindings. Which can
happen on Windows since the Python bindings require the same MSVC version than
the one used to compile Python itself. When sticking to the C ABI, you can be
safe. With the C++ ABI, some bad things could potentially happen.
Done

https://trac.osgeo.org/gdal/wiki/rfc48_geographical_networks_support

Links:
Perhaps do you have a git branch somewhere (just for the sake of easier
applying) ?
Ok, I send the pull request.

- diff with binary test data -
https://drive.google.com/file/d/0BzlLlHyrgQHkNkktUmxYbWtjV3M/view?usp=shari
ng - diff without binary test data -
https://drive.google.com/file/d/0BzlLlHyrgQHkZ2lZN2wwTTVOQlE/view?usp=shari
ng - binary test data -
https://drive.google.com/file/d/0BzlLlHyrgQHkTTJjRFZNOEtfb3c/view?usp=shari
ng

Code reviewing and testing are welcome. Also the some decisions in
implementation can be discussed.
 From a random skimming, I'm wondering about the below :

+%inline %{
+  GNMNetworkShadow* CastToNetwork(GDALMajorObjectShadow* base) {
+    return static_cast<GNMNetworkShadow*>(base);
+  }
+%}
+
+%inline %{
+  GNMGenericNetworkShadow* CastToGenericNetwork(GDALMajorObjectShadow* base)
{
+    return static_cast<GNMGenericNetworkShadow*>(base);
+  }
+%}
+

Shouldn't that be a dynamic_cast ? Otherwise you'll manage to cast any
GDALMajorObject into a GNMxxxx even when it is not valid. You could probably
test that by trying to cast a TIFF dataset or something completely irrelevant.
The compiler didn't let me dynamic cast from void* to void* (as the
GDALMajorObjectShadow* is typedef void* and GNMNetworkShadow*,
GNMGenericNetworkShadow* also void). Any idea how to solve this?
The GDAL 2.0 provide ability to create datasets combine vector and
raster data (drivers support both types).
In case of PostGIS we can have the driver combined vector, raster and
networks (pgRouting). Maybe someone give an idea how it can be done.

Currently, from what I see from the generic network implementation, the whole
graph is loaded from storage into memory at Open() time
(GNMFileNetwork::Open() calling LoadGraphLayer()). Right ?  Wondering how much
a limitation it can be in practice. Couldn't the loading of the graph be
defered up to the point it is really needed, so that Open() is kept reasonably
fast ? I don't see any try catch around insertions into STL containers, so I
guess that things might blow up if a too big graph is loaded.
Yes, there is a note in graph description (doxygen comments) that this
implementation are limited and may be potential problems with big graphs.
The GNMGraph class is a subject of future improvements. But lazy loading of the
graph data make sense. Done it.

Even


Best regards,
    Dmitry

_______________________________________________
gdal-dev mailing list
gdal-dev@lists.osgeo.org
http://lists.osgeo.org/mailman/listinfo/gdal-dev

Reply via email to