Hi Nyall
>This potentially has huge impacts on the performance of common tasks
such as selecting all features which intersect a geometry.
Are you really sure the impact is huge? If you select features on the
map, you get a new one from provider each time. So also with the old
geometry class, it was necessary to convert to geos for each intersect.
But yes, if several geos operations are done in sequence on really the
same geometry instance, there is an overhead now. However, I would
assume the geos conversion takes far less time than the geos operation
itself. Did you observe cases where the geos conversion really takes
very long time?
Lets assume the geos conversion indeed takes a long time. Couldn't we
just store the pointer to QgsGeos (or better QgsGeometryEngine class)
inside QgsGeometry? This would be straightforward to do. Furthermore, it
is possible to prepare QgsGeos. A prepared geos geometry can do _much_
faster intersects/touches/ etc. operations if called repeatedly. We can
cache a prepared QgsGeos now with the new engine, but we couldn't do
that with the old QgsGeometry class. So for repeated predicate
calculations, the new geometry is much better, also performance wise.
Regards,
Marco
On 23.06.2015 04:09, Nyall Dawson wrote:
Hi all,
Unfortunately, we've become aware of a serious performance regression
caused by the new geometry engine. Basically, the situation is that
for all geometry operations which rely on geos (think buffers, splits,
spatial relation operations such as intersects and within,... ) the
geometry now needs to be converted into a geos representation with
*every* operation. In the old geometry engine this conversion was done
once, and the result stored so that follow up operations would not
need to recalculate it. This potentially has huge impacts on the
performance of common tasks such as selecting all features which
intersect a geometry.
I've had a look, and unfortunately it's not trivial to fix this. I
think the correct solution to this is to:
- make QgsGeometryEngine accept and return QgsGeometry containers, not
QgsAbstractGeometryV2
- store the generated geos representation of geometries within
QgsGeometryPrivate inside the QgsGeometry container. This way it will
be reusable between different geometry operations, and shared when
QgsGeometry objects are copied. This will also have the benefit that
if a geometry is prepared using geos then subsequent geos operations
performed on that QgsGeometry and its shared copies will be much faster.
- make QgsGeometry a friend class of QgsGeo, so that it can access
QgsGeometryPrivate to retrieve or set the geos representation of the
geometry as required
An alternative (short term) solution would be to just cache the geos
representation when geometry operations are called through the older
QgsGeometry modification/relationship operations. This would be
easier, but means that the API of QgsGeometryEngine will be stuck with
the current design, and we won't be able to properly fix this until
breaking the api for 3.0.
Either way, I doubt this can be addressed within the remaining 3 days
we have until release. Should we delay to address this? Release with
the regression? Or am I missing something and there's an easier
solution we could implement? Or even possibly this additional cost of
recalculating the geos representation is trivial and can be ignored
(maybe someone could test this with a little repeated intersection
script)?
Thoughts?
Nyall
_______________________________________________
Qgis-developer mailing list
Qgis-developer@lists.osgeo.org
http://lists.osgeo.org/mailman/listinfo/qgis-developer
--
Dr. Marco Hugentobler
Sourcepole - Linux & Open Source Solutions
Weberstrasse 5, CH-8004 Zürich, Switzerland
marco.hugentob...@sourcepole.ch http://www.sourcepole.ch
Technical Advisor QGIS Project Steering Committee
_______________________________________________
Qgis-developer mailing list
Qgis-developer@lists.osgeo.org
http://lists.osgeo.org/mailman/listinfo/qgis-developer
