[gdal-dev] RFC 39

[Ari Jolma]

Even,

I've uploaded a version, which fixes these. I added a note that GEOS is 
required, as these probably segfault without it (the methods return 
NULLs and not geometries).

I added a simple test against a pre-computed layer envelope, and it 
speeds up the computation in my test case ~30%. Still the most of the 
time is spent in OGRLineString::getEnvelope as it goes through all 
linestrings of the input layer.

Ari

On 05/02/2012 09:14 PM, Even Rouault wrote:
>> 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