Stop considering Rectangle(0,0,0,0) null - while Rectangle(1,1,1,1) is not null

python/core/auto_generated/geometry/qgsrectangle.sip.in

@@ -59,7 +59,6 @@ Copy constructor
 
     ~QgsRectangle();
 
-
     static QgsRectangle fromWkt( const QString &wkt );
 %Docstring
 Creates a new rectangle from a ``wkt`` string.

src/core/geometry/qgsgeometry.cpp

@@ -1204,7 +1204,7 @@ QgsRectangle QgsGeometry::boundingBox() const
   {
     return d->geometry->boundingBox();
   }
-  return QgsRectangle::createNull();
+  return QgsRectangle();
 }
 
 QgsBox3D QgsGeometry::boundingBox3D() const

src/core/geometry/qgsrectangle.cpp

@@ -146,8 +146,8 @@ QString QgsRectangle::toString( int precision ) const
     }
   }
 
-  if ( isEmpty() )
-    rep = QStringLiteral( "Empty" );
+  if ( isNull() )
+    rep = QStringLiteral( "Null" );
   else
     rep = QStringLiteral( "%1,%2 : %3,%4" )
           .arg( mXmin, 0, 'f', precision )

src/core/geometry/qgsrectangle.h

@@ -99,22 +99,6 @@ class CORE_EXPORT QgsRectangle
     // see https://github.com/qgis/QGIS/pull/4720#issuecomment-308652392
     ~QgsRectangle() = default;
 
-    /**
-    * Creates a null rectangle.
-    *
-    * This is a temporary method to use while the default
-    * constructor is still not constructing a null rectangle.
-    *
-    * \since QGIS 3.34
-    */
-    static QgsRectangle createNull() SIP_SKIP
-    {
-      // TODO: remove this method once the default constructor gives a null
-      QgsRectangle rectNull;
-      rectNull.setNull(); // TODO: have setNull() return *this ?
-      return rectNull;
-    }
-
     /**
     * Creates a new rectangle from a \a wkt string.
     * The WKT must contain only 5 vertices, representing a rectangle aligned with X and Y axes.
@@ -516,7 +500,7 @@ class CORE_EXPORT QgsRectangle
      */
     bool isEmpty() const
     {
-      return mXmax < mXmin || mYmax < mYmin || qgsDoubleNear( mXmax, mXmin ) || qgsDoubleNear( mYmax, mYmin );
+      return isNull() || mXmax <= mXmin || mYmax <= mYmin || qgsDoubleNear( mXmax, mXmin ) || qgsDoubleNear( mYmax, mYmin );
     }
 
     /**
@@ -533,7 +517,6 @@ class CORE_EXPORT QgsRectangle
       // rectangle created QgsRectangle() or with rect.setNull() or
       // otherwise having NaN ordinates
       return ( std::isnan( mXmin )  && std::isnan( mXmax ) && std::isnan( mYmin ) && std::isnan( mYmax ) ) ||
-             ( qgsDoubleNear( mXmin, 0.0 ) && qgsDoubleNear( mXmax, 0.0 ) && qgsDoubleNear( mYmin, 0.0 ) && qgsDoubleNear( mYmax, 0.0 ) ) ||
              ( qgsDoubleNear( mXmin, std::numeric_limits<double>::max() ) && qgsDoubleNear( mYmin, std::numeric_limits<double>::max() ) &&
                qgsDoubleNear( mXmax, -std::numeric_limits<double>::max() ) && qgsDoubleNear( mYmax, -std::numeric_limits<double>::max() ) );
     }
@@ -665,10 +648,10 @@ class CORE_EXPORT QgsRectangle
 
   private:
 
-    double mXmin = 0.0;
-    double mYmin = 0.0;
-    double mXmax = 0.0;
-    double mYmax = 0.0;
+    double mXmin = std::numeric_limits<double>::max();
+    double mYmin = std::numeric_limits<double>::max();
+    double mXmax = -std::numeric_limits<double>::max();
+    double mYmax = -std::numeric_limits<double>::max();
 
 };
 

tests/src/core/geometry/testqgsgeometry.cpp

@@ -837,7 +837,7 @@ void TestQgsGeometry::fromPolyline()
 
 void TestQgsGeometry::fromRect()
 {
-  QgsRectangle rectNull = QgsRectangle::createNull();
+  QgsRectangle rectNull;
 
   QgsGeometry fromRect = QgsGeometry::fromRect( rectNull );
   QVERIFY( fromRect.isNull() );
@@ -2183,7 +2183,7 @@ void TestQgsGeometry::orientedMinimumBoundingBox()
 void TestQgsGeometry::boundingBox()
 {
   QgsGeometry geomTest;
-  QgsRectangle nullRect = QgsRectangle::createNull();
+  QgsRectangle nullRect;
   QCOMPARE( geomTest.boundingBox(), nullRect );
 
   geomTest = QgsGeometry::fromWkt( QStringLiteral( "LINESTRING(-1 -2, 4 5)" ) );

tests/src/core/geometry/testqgsrectangle.cpp

@@ -72,11 +72,13 @@ void TestQgsRectangle::isNull()
   QVERIFY( QgsRectangle().isNull() );
   QVERIFY( QgsRectangle( std::numeric_limits<double>::quiet_NaN(), std::numeric_limits<double>::quiet_NaN(),
                          std::numeric_limits<double>::quiet_NaN(), std::numeric_limits<double>::quiet_NaN() ).isNull() );
-  QVERIFY( QgsRectangle( 0.0, 0.0, 0.0, 0.0 ).isNull() );
+  QVERIFY( !QgsRectangle( 0.0, 0.0, 0.0, 0.0 ).isNull() );
+  QVERIFY( !QgsRectangle( 1.0, 1.0, 1.0, 1.0 ).isNull() );
   QVERIFY( !QgsRectangle( std::numeric_limits<double>::max(), -std::numeric_limits<double>::max(), std::numeric_limits<double>::max(), -std::numeric_limits<double>::max() ).isNull() );
   QVERIFY( !QgsRectangle( 1, 2, 2, 1 ).isNull() );
 }
 
+
 void TestQgsRectangle::fromWkt()
 {
   QgsRectangle rect = QgsRectangle::fromWkt( QStringLiteral( "POLYGON((0 0,1 0,1 1,0 1,0 0))" ) );

tests/src/providers/testqgsvirtuallayerprovider.cpp

@@ -83,7 +83,7 @@ void TestQgsVirtualLayerProvider::testConstructor()
   QString uri;
   std::unique_ptr<QgsVectorDataProvider> prov( new QgsVirtualLayerProvider( uri, opts ) );
 
-  const QgsRectangle rectNull = QgsRectangle::createNull();
+  const QgsRectangle rectNull;
   QCOMPARE( prov->sourceExtent(), rectNull );
 }
 

tests/src/python/providertestbase.py

@@ -249,6 +249,10 @@ def providerCompatibleOfSubsetStringWithStableFID(self):
             The WFS provider might not always be able to have that guarantee. """
         return True
 
+    def referenceExtent(self):
+        """ Extent of the reference dataset """
+        return QgsRectangle(-71.123, 66.33, -65.32, 78.3)
+
     def getSubsetString(self):
         """Individual providers may need to override this depending on their subset string formats"""
         return '"cnt" > 100 and "cnt" < 410'
@@ -257,6 +261,10 @@ def getSubsetString2(self):
         """Individual providers may need to override this depending on their subset string formats"""
         return '"cnt" > 100 and "cnt" < 400'
 
+    def referenceSubsetString3Extent(self):
+        """ Extent of the data selected by subset string 3 """
+        return QgsRectangle(-68.2, 70.8, -68.2, 70.8)
+
     def getSubsetString3(self):
         """Individual providers may need to override this depending on their subset string formats"""
         return '"name"=\'Apple\''
@@ -422,13 +430,12 @@ def testMaxValue(self):
             self.assertEqual(max_value, 300)
 
     def testExtent(self):
-        reference = QgsGeometry.fromRect(
-            QgsRectangle(-71.123, 66.33, -65.32, 78.3))
+        reference_extent = self.referenceExtent()
         provider_extent = self.source.extent()
-        self.assertAlmostEqual(provider_extent.xMinimum(), -71.123, 3)
-        self.assertAlmostEqual(provider_extent.xMaximum(), -65.32, 3)
-        self.assertAlmostEqual(provider_extent.yMinimum(), 66.33, 3)
-        self.assertAlmostEqual(provider_extent.yMaximum(), 78.3, 3)
+        self.assertAlmostEqual(provider_extent.xMinimum(), reference_extent.xMinimum())
+        self.assertAlmostEqual(provider_extent.xMaximum(), reference_extent.xMaximum())
+        self.assertAlmostEqual(provider_extent.yMinimum(), reference_extent.yMinimum())
+        self.assertAlmostEqual(provider_extent.yMaximum(), reference_extent.yMaximum())
 
     def testExtentSubsetString(self):
         if self.source.supportsSubsetString():
@@ -437,12 +444,13 @@ def testExtentSubsetString(self):
             self.source.setSubsetString(subset)
             count = self.source.featureCount()
             provider_extent = self.source.extent()
+            subset_extent = self.referenceSubsetString3Extent()
             self.source.setSubsetString(None)
             self.assertEqual(count, 1)
-            self.assertAlmostEqual(provider_extent.xMinimum(), -68.2, 3)
-            self.assertAlmostEqual(provider_extent.xMaximum(), -68.2, 3)
-            self.assertAlmostEqual(provider_extent.yMinimum(), 70.8, 3)
-            self.assertAlmostEqual(provider_extent.yMaximum(), 70.8, 3)
+            self.assertAlmostEqual(provider_extent.xMinimum(), subset_extent.xMinimum(), 3)
+            self.assertAlmostEqual(provider_extent.xMaximum(), subset_extent.xMaximum(), 3)
+            self.assertAlmostEqual(provider_extent.yMinimum(), subset_extent.yMinimum(), 3)
+            self.assertAlmostEqual(provider_extent.yMaximum(), subset_extent.yMaximum(), 3)
 
             # with no points
             subset = self.getSubsetStringNoMatching()
@@ -451,7 +459,7 @@ def testExtentSubsetString(self):
             provider_extent = self.source.extent()
             self.source.setSubsetString(None)
             self.assertEqual(count, 0)
-            self.assertTrue(provider_extent.isNull())
+            self.assertEqual(provider_extent, QgsRectangle())
             self.assertEqual(self.source.featureCount(), 5)
 
     def testUnique(self):

tests/src/python/test_qgsrectangle.py

@@ -125,7 +125,7 @@ def testAsWktPolygon(self):
 
     def testToString(self):
         """Test the different string representations"""
-        self.assertEqual(QgsRectangle().toString(), 'Empty')
+        self.assertEqual(QgsRectangle().toString(), 'Null')
         rect = QgsRectangle(0, 0.1, 0.2, 0.3)
         self.assertEqual(rect.toString(), '0.0000000000000000,0.1000000000000000 : 0.2000000000000000,0.3000000000000000')