Re: [QGIS-Developer] Empty vs. Null rectangle, bogus ?
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=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; ___ QGIS-Developer mailing list QGIS-Developer@lists.osgeo.org List info: https://lists.osgeo.org/mailman/listinfo/qgis-developer Unsubscribe: https://lists.osgeo.org/mailman/listinfo/qgis-developer
Re: [QGIS-Developer] Empty vs. Null rectangle, bogus ?
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=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; > > Libre GIS consultant/developer > https://strk.kbt.io/services.html > ___ > QGIS-Developer mailing list > QGIS-Developer@lists.osgeo.org > List info: https://lists.osgeo.org/mailman/listinfo/qgis-developer > Unsubscribe: https://lists.osgeo.org/mailman/listinfo/qgis-developer -- Libre GIS consultant/developer https://strk.kbt.io/services.html ___ QGIS-Developer mailing list QGIS-Developer@lists.osgeo.org List info: https://lists.osgeo.org/mailman/listinfo/qgis-developer Unsubscribe: https://lists.osgeo.org/mailman/listinfo/qgis-developer
[QGIS-Developer] Empty vs. Null rectangle, bogus ?
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; Libre GIS consultant/developer https://strk.kbt.io/services.html ___ QGIS-Developer mailing list QGIS-Developer@lists.osgeo.org List info: https://lists.osgeo.org/mailman/listinfo/qgis-developer Unsubscribe: https://lists.osgeo.org/mailman/listinfo/qgis-developer
