Fix geometry simplification for rendering altering feature geometry

nyalldawson · nyalldawson · commit 5d3382a86a2e · 2015-08-04T21:26:24.000+10:00
I noticed this when using memory layers - zooming out and then in would result in the features getting more and more generalised. Possibly it would affect other areas of the codebase too. The geometry simplifier was unhelpfully casting away the const from the pointer to the geometry's wkb, and was happily overwriting the wkb for its own purposes. With QgsGeometry now implicitly sharing this wkb pointer the non-const cast meant that the geometry was not getting correctly detached and the original geometry was being modified. (cherry-picked from f00c52d)
diff --git a/src/core/qgsmaptopixelgeometrysimplifier.cpp b/src/core/qgsmaptopixelgeometrysimplifier.cpp
@@ -41,7 +41,7 @@ float QgsMapToPixelSimplifier::calculateLengthSquared2D( double x1, double y1, d
 }
 
 //! Returns the BBOX of the specified WKB-point stream
-inline static QgsRectangle calculateBoundingBox( QGis::WkbType wkbType, unsigned char* wkb, size_t numPoints )
+inline static QgsRectangle calculateBoundingBox( QGis::WkbType wkbType, const unsigned char* wkb, size_t numPoints )
 {
   double x, y;
   QgsRectangle r;
@@ -63,7 +63,7 @@ inline static QgsRectangle calculateBoundingBox( QGis::WkbType wkbType, unsigned
 //! Generalize the WKB-geometry using the BBOX of the original geometry
 static bool generalizeWkbGeometryByBoundingBox(
   QGis::WkbType wkbType,
-  unsigned char* sourceWkb, size_t sourceWkbSize,
+  const unsigned char* sourceWkb, size_t sourceWkbSize,
   unsigned char* targetWkb, size_t& targetWkbSize,
   const QgsRectangle& envelope, bool writeHeader )
 {
@@ -151,7 +151,7 @@ static bool generalizeWkbGeometryByBoundingBox(
 //! Simplify the WKB-geometry using the specified tolerance
 bool QgsMapToPixelSimplifier::simplifyWkbGeometry(
   int simplifyFlags, QGis::WkbType wkbType,
-  unsigned char* sourceWkb, size_t sourceWkbSize,
+  const unsigned char* sourceWkb, size_t sourceWkbSize,
   unsigned char* targetWkb, size_t& targetWkbSize,
   const QgsRectangle& envelope, double map2pixelTol,
   bool writeHeader, bool isaLinearRing )
@@ -161,7 +161,7 @@ bool QgsMapToPixelSimplifier::simplifyWkbGeometry(
   bool result = false;
 
   // Save initial WKB settings to use when the simplification creates invalid geometries
-  unsigned char* sourcePrevWkb = sourceWkb;
+  const unsigned char* sourcePrevWkb = sourceWkb;
   unsigned char* targetPrevWkb = targetWkb;
   size_t targetWkbPrevSize = targetWkbSize;
 
@@ -194,7 +194,7 @@ bool QgsMapToPixelSimplifier::simplifyWkbGeometry(
     targetWkbSize += 5;
   }
 
-  unsigned char* wkb1 = sourceWkb;
+  const unsigned char* wkb1 = sourceWkb;
   unsigned char* wkb2 = targetWkb;
   unsigned int flatType = QGis::flatType( wkbType );
 
@@ -230,10 +230,10 @@ bool QgsMapToPixelSimplifier::simplifyWkbGeometry(
     {
       double x1, y1, x2, y2;
 
-      unsigned char* startWkbX = sourceWkb;
-      unsigned char* startWkbY = startWkbX + sizeOfDoubleX;
-      unsigned char* finalWkbX = sourceWkb + ( numPoints - 1 ) * ( sizeOfDoubleX + sizeOfDoubleY );
-      unsigned char* finalWkbY = finalWkbX + sizeOfDoubleX;
+      const unsigned char* startWkbX = sourceWkb;
+      const unsigned char* startWkbY = startWkbX + sizeOfDoubleX;
+      const unsigned char* finalWkbX = sourceWkb + ( numPoints - 1 ) * ( sizeOfDoubleX + sizeOfDoubleY );
+      const unsigned char* finalWkbY = finalWkbX + sizeOfDoubleX;
 
       memcpy( &x1, startWkbX, sizeof( double ) );
       memcpy( &y1, startWkbY, sizeof( double ) );
@@ -418,7 +418,7 @@ QgsGeometry* QgsMapToPixelSimplifier::simplify( QgsGeometry* geometry ) const
 //! Simplifies the geometry (Removing duplicated points) when is applied the specified map2pixel context
 bool QgsMapToPixelSimplifier::simplifyGeometry( QgsGeometry* geometry, int simplifyFlags, double tolerance )
 {
-  size_t targetWkbSize = 0;
+  size_t finalWkbSize = 0;
 
   // Check whether the geometry can be simplified using the map2pixel context
   QGis::GeometryType geometryType = geometry->type();
@@ -428,17 +428,21 @@ bool QgsMapToPixelSimplifier::simplifyGeometry( QgsGeometry* geometry, int simpl
   QgsRectangle envelope = geometry->boundingBox();
   QGis::WkbType wkbType = geometry->wkbType();
 
-  unsigned char* wkb = ( unsigned char* )geometry->asWkb();
+  const unsigned char* wkb = geometry->asWkb();
   size_t wkbSize = geometry->wkbSize();
 
-  // Simplify the geometry rewriting temporally its WKB-stream for saving calloc's.
-  if ( simplifyWkbGeometry( simplifyFlags, wkbType, wkb, wkbSize, wkb, targetWkbSize, envelope, tolerance ) )
+  unsigned char* targetWkb = new unsigned char[wkbSize];
+  memcpy( targetWkb, wkb, wkbSize );
+
+  if ( simplifyWkbGeometry( simplifyFlags, wkbType, wkb, wkbSize, targetWkb, finalWkbSize, envelope, tolerance ) )
   {
-    unsigned char* targetWkb = new unsigned char[targetWkbSize];
-    memcpy( targetWkb, wkb, targetWkbSize );
-    geometry->fromWkb( targetWkb, targetWkbSize );
+    unsigned char* finalWkb = new unsigned char[finalWkbSize];
+    memcpy( finalWkb, targetWkb, finalWkbSize );
+    geometry->fromWkb( finalWkb, finalWkbSize );
+    delete [] targetWkb;
     return true;
   }
+  delete [] targetWkb;
   return false;
 }
 
diff --git a/src/core/qgsmaptopixelgeometrysimplifier.h b/src/core/qgsmaptopixelgeometrysimplifier.h
@@ -45,7 +45,7 @@ class CORE_EXPORT QgsMapToPixelSimplifier : public QgsAbstractGeometrySimplifier
 
   private:
     //! Simplify the WKB-geometry using the specified tolerance
-    static bool simplifyWkbGeometry( int simplifyFlags, QGis::WkbType wkbType, unsigned char* sourceWkb, size_t sourceWkbSize, unsigned char* targetWkb, size_t& targetWkbSize, const QgsRectangle& envelope, double map2pixelTol, bool writeHeader = true, bool isaLinearRing = false );
+    static bool simplifyWkbGeometry( int simplifyFlags, QGis::WkbType wkbType, const unsigned char* sourceWkb, size_t sourceWkbSize, unsigned char* targetWkb, size_t& targetWkbSize, const QgsRectangle& envelope, double map2pixelTol, bool writeHeader = true, bool isaLinearRing = false );
 
   protected:
     //! Current simplification flags
