> What would it take to adopt this RFC? Ari,
I'm pretty satisfied with the current state of the RFC. From a recent discussion with Frank, I think he has perhaps some comments to add ? I've looked at the latest version of the patch. A few comments : * You should not use GetFeaturesRead() to know how many features have been iterated to compute the progress rate. It is not a reliable way of knowing it, since it is up to the driver to do the proper incrementation (and many drivers don't : this counter is usually only used for debugging purposes). But the more fundamental reason is that this counter is global to the session. So if the user has already iterated over the input layer before calling the new functions, the counter will not be 0. Also, some implementations of GetFeatureCount() (actually the default non specialized implementation of GetFeatureCount()) might also cause the counter to be incremented. So I'd suggest maintaining your own local counter instead. * At the end of the loop, it would also be appropriate to call the progress callback with a progress rate of 1.0. That will make, for example, the output of the GDALTermProgress implementation to be nicer. * In set_filter_from(), pLayer->SetSpatialFilter(geom- >Intersection(pGeometryExistingFilter)) likely leaks the passed geometry. You should assign it to a temporary variable and delete it afterwards. * In the code block triggered if the pfnProgress returns FALSE, there should be a delete x; * In the Doxygen, it would be appropriate to mention that these routines use geometric operations that are only available if GDAL is compiled with GEOS support. I've also read your note in the RFC about performance. It is interesting to see that envelope computation is the limiting factor when the method layer is in memory. It is an interesting track for a later optimization where envelope would be cached instead of being computed each time. Spatial indexing would also do the trick. It reminds me that a few years ago I've added a cpl_quad_tree.cpp implementation that is a direct transposition of the shapelib implementation, but not tied with shapefiles. It has never been seriously used AFAIR, but it could perhaps be a starting point to do in-memory spatial indexing. Even _______________________________________________ gdal-dev mailing list [email protected] http://lists.osgeo.org/mailman/listinfo/gdal-dev
