Fix combining extent of valid rectangle with invalid rectangle results

nyalldawson · nyalldawson · commit 8cf9f8fb6f82 · 2017-11-20T08:29:17.000+11:00
in invalid rectangle
diff --git a/src/core/geometry/qgsrectangle.cpp b/src/core/geometry/qgsrectangle.cpp
@@ -221,15 +221,13 @@ void QgsRectangle::combineExtentWith( const QgsRectangle &rect )
 {
   if ( isNull() )
     *this = rect;
-  else
+  else if ( !rect.isNull() )
   {
-    mXmin = ( ( mXmin < rect.xMinimum() ) ? mXmin : rect.xMinimum() );
-    mXmax = ( ( mXmax > rect.xMaximum() ) ? mXmax : rect.xMaximum() );
-
-    mYmin = ( ( mYmin < rect.yMinimum() ) ? mYmin : rect.yMinimum() );
-    mYmax = ( ( mYmax > rect.yMaximum() ) ? mYmax : rect.yMaximum() );
+    mXmin = std::min( mXmin, rect.xMinimum() );
+    mXmax = std::max( mXmax, rect.xMaximum() );
+    mYmin = std::min( mYmin, rect.yMinimum() );
+    mYmax = std::max( mYmax, rect.yMaximum() );;
   }
-
 }
 
 void QgsRectangle::combineExtentWith( double x, double y )
diff --git a/tests/src/core/testqgsrectangle.cpp b/tests/src/core/testqgsrectangle.cpp
@@ -38,6 +38,7 @@ class TestQgsRectangle: public QObject
     void include();
     void buffered();
     void isFinite();
+    void combine();
     void dataStream();
 };
 
@@ -313,6 +314,25 @@ void TestQgsRectangle::isFinite()
   QVERIFY( !QgsRectangle( 1, 2, 3, std::numeric_limits<double>::quiet_NaN() ).isFinite() );
 }
 
+void TestQgsRectangle::combine()
+{
+  QgsRectangle rect;
+  // combine extent of null rectangle with valid rectangle
+  rect.combineExtentWith( QgsRectangle( 1, 2, 3, 4 ) );
+  QCOMPARE( rect.xMinimum(), 1.0 );
+  QCOMPARE( rect.yMinimum(), 2.0 );
+  QCOMPARE( rect.xMaximum(), 3.0 );
+  QCOMPARE( rect.yMaximum(), 4.0 );
+
+  // combine extent of valid rectangle with null rectangle
+  rect = QgsRectangle( 1, 2, 3, 4 );
+  rect.combineExtentWith( QgsRectangle() );
+  QCOMPARE( rect.xMinimum(), 1.0 );
+  QCOMPARE( rect.yMinimum(), 2.0 );
+  QCOMPARE( rect.xMaximum(), 3.0 );
+  QCOMPARE( rect.yMaximum(), 4.0 );
+}
+
 void TestQgsRectangle::dataStream()
 {
   QgsRectangle original( 10.1, 20.2, 110.3, 220.4 );

Original file line number	Diff line number	Diff line change
`@@ -221,15 +221,13 @@ void QgsRectangle::combineExtentWith( const QgsRectangle &rect )`
`221`	`221`	`{`
`222`	`222`	`if ( isNull() )`
`223`	`223`	`*this = rect;`
`224`		`- else`
	`224`	`+ else if ( !rect.isNull() )`
`225`	`225`	`{`
`226`		`- mXmin = ( ( mXmin < rect.xMinimum() ) ? mXmin : rect.xMinimum() );`
`227`		`- mXmax = ( ( mXmax > rect.xMaximum() ) ? mXmax : rect.xMaximum() );`
`228`		`-`
`229`		`- mYmin = ( ( mYmin < rect.yMinimum() ) ? mYmin : rect.yMinimum() );`
`230`		`- mYmax = ( ( mYmax > rect.yMaximum() ) ? mYmax : rect.yMaximum() );`
	`226`	`+ mXmin = std::min( mXmin, rect.xMinimum() );`
	`227`	`+ mXmax = std::max( mXmax, rect.xMaximum() );`
	`228`	`+ mYmin = std::min( mYmin, rect.yMinimum() );`
	`229`	`+ mYmax = std::max( mYmax, rect.yMaximum() );;`
`231`	`230`	`}`
`232`		`-`
`233`	`231`	`}`
`234`	`232`
`235`	`233`	`void QgsRectangle::combineExtentWith( double x, double y )`