Open m-kuhn opened 7 years ago
I would not be in favor of making ExactIntersect on by default, but we could go with an option for adding method to set exact filter rect (actually for exact intersection we should allow any geometry rather than just a rect).
I would not be in favor of making ExactIntersect on by default
Why? Because of possible performance impacts?
we could go with an option for adding method to set exact filter rect
Sounds good (if we don't change the default behaviour).
actually for exact intersection we should allow any geometry rather than just a rect
That can be a simple filterExpression
- or we can add a series of shorthand methods to QgsFeatureRequest
that transparently create a filterExpression
in the background.
I would not be in favor of making ExactIntersect on by default
Why? Because of possible performance impacts?
Yes and also because most of the time the exact intersection is not necessary. Also, it is the same behavior as with spatial index - one gets features which have bbox intersecting the supplied rect.
actually for exact intersection we should allow any geometry rather than just a rect
That can be a simple filterExpression - or we can add a series of shorthand methods to QgsFeatureRequest that transparently create a filterExpression in the background.
I would prefer not to stick intersection geometry into expression: 1. interference with existing filter expression, 2. it won't hurt to add one more member variable, 3. performance, 4. possible loss of precision of geometry (i.e. conversion to text and back)
Yes and also because most of the time the exact intersection is not necessary.
... but it also doesn't hurt ;)
I have just noticed surprised developers more than once in the past and was wondering how to reduce the surprise-potential as much as possible.
I would prefer not to stick intersection geometry into expression: 1. interference with existing filter expression, 2. it won't hurt to add one more member variable, 3. performance, 4. possible loss of precision of geometry (i.e. conversion to text and back)
I'd leave the bbox intersection where it is as shorthand but not extend it with different paramters. Expression functions can also be sent to the server meanwhile (i.e. by using the expression compiler, performance problems are solved). Geometries can also be saved loss-less in variables (QVariant).
I think the main reason for this would be a "nice and tidy" API. The question for me is if we want to put this to QgsFeatureRequest
or into something else (@NathanW2 had something in mind QgsFeatureRequestBuilder
or QgsQuery
or so?)
At the moment, the default behaviour for filtering features in a rect is done based on bounding boxes.
It is a common pitfall, that one is unaware of this.
To alleviate the situation, I see two possibilities:
ExactIntersect
flag by default (or replace it with aBoundingBoxIntersect
flag).setExactFilterRect
The first one would IMO be a bit cleaner, but I guess when updating code it's very easy to miss it. The second one would make updating code easier but it's also "easy" to miss.