[QGIS-Developer] Empty vs. Null rectangle, bogus ?

[Sandro Santilli]

I've filed a PR to change QgsRectangle NULL to NOT include the 0,0,0,0
case. I suspect it will open a can of warms but let me know what do
you think about it:
https://github.com/qgis/QGIS/pull/45607

--strk;

On Thu, Oct 21, 2021 at 10:20:04AM +0200, Sandro Santilli wrote:
> I've added - in my PR https://github.com/qgis/QGIS/pull/45384 -
> a test for QgsRectangle doing this:
> 
>   QgsRectangle r1( 0, 0, 0, 0 );
>   QVERIFY( ! r1.isNull() );
>   QVERIFY( r1.isEmpty() );
> 
>   QgsRectangle r2( 1, 1, 1, 1 );
>   QVERIFY( ! r2.isNull() );
>   QVERIFY( r2.isEmpty() );
> 
> The first test fails, the second succeeds.
> See
> https://cdash.orfeo-toolbox.org/testDetails.php?test=32862378&build=87747
> 
> This confirms the confusion in the meaning of "null"
> 
> --strk;
> 
> On Mon, Oct 18, 2021 at 08:05:22PM +0200, Sandro Santilli wrote:
> > Still chasing bug
> > QgsVectorLayerFeatureIterator::QgsVectorLayerFeatureIterator
> > I reached QgsRectangle, whose "Null" or "Empty" nature changes
> > behavior of QgsFeatureVectorFeatureIterator in a way that prevents
> > proper WithinDistance filter from working.
> > 
> > Details are written in
> > https://github.com/qgis/QGIS/issues/45352
> > but what I wanted to ask here is:
> > 
> > According to the comments Empty means that some information is still
> > available (bounding box of a point) but the unit test for QgsRectangle
> > (and the isEmpty method implementation) seem to check for Max < Min,
> > which would NOT be the case for the bounding box of a point,
> > so is puzzling.
> > 
> > I'm not sure how to proceed for fixing the bug I'm seeing because
> > one simple fix would be avoiding to call setRectangle on a QgsRequest
> > if the iterator's mFilterRect is Empty, but a more complex solution
> > could be to *allow* those QgsRequests to have multiple filters, rather
> > than one excluding the other.
> > 
> > Also I've yet to understand WHY at construction time a
> > QgsAbstractFeatureIterator shoul dhave an Empty rather than Null
> > rectangle, which brings back to the first question (is the distinction
> > really defined?).
> > 
> > --strk;