Skip to content
Permalink
Browse files

Avoid all use of QgsCoordinateTransform pointers, replace with

copies or references

Makes the code more robust, fixes leaks and avoids potential
null pointer dereferencing
  • Loading branch information
nyalldawson committed Jul 16, 2016
1 parent ffa9b9b commit adafeda82adf87258e24b28579ee71485630584b
Showing with 453 additions and 421 deletions.
  1. +68 −0 doc/api_break.dox
  2. +4 −1 python/core/qgscrscache.sip
  3. +4 −2 python/core/qgsdatumtransformstore.sip
  4. +4 −13 python/core/qgsdiagramrendererv2.sip
  5. +4 −1 python/core/qgsmaprenderer.sip
  6. +1 −1 python/core/qgsmaprendererjob.sip
  7. +2 −2 python/core/qgsmapsettings.sip
  8. +5 −5 python/core/qgspallabeling.sip
  9. +7 −3 python/core/qgsrendercontext.sip
  10. +3 −2 python/core/qgsvectorfilewriter.sip
  11. +1 −1 python/core/raster/qgsrasterprojector.sip
  12. +6 −6 src/app/nodetool/qgsmaptoolnodetool.cpp
  13. +5 −7 src/app/qgisapp.cpp
  14. +1 −1 src/core/composer/qgsatlascomposition.cpp
  15. +12 −18 src/core/qgscrscache.cpp
  16. +5 −2 src/core/qgscrscache.h
  17. +3 −3 src/core/qgsdatumtransformstore.cpp
  18. +3 −2 src/core/qgsdatumtransformstore.h
  19. +4 −7 src/core/qgsdiagramrendererv2.cpp
  20. +8 −13 src/core/qgsdiagramrendererv2.h
  21. +19 −23 src/core/qgsdistancearea.cpp
  22. +4 −4 src/core/qgsdistancearea.h
  23. +2 −2 src/core/qgslabel.cpp
  24. +2 −2 src/core/qgsmaphittest.cpp
  25. +38 −38 src/core/qgsmaprenderer.cpp
  26. +4 −1 src/core/qgsmaprenderer.h
  27. +4 −5 src/core/qgsmaprenderercustompainterjob.cpp
  28. +13 −13 src/core/qgsmaprendererjob.cpp
  29. +1 −1 src/core/qgsmaprendererjob.h
  30. +23 −17 src/core/qgsmapsettings.cpp
  31. +2 −2 src/core/qgsmapsettings.h
  32. +9 −12 src/core/qgspallabeling.cpp
  33. +5 −5 src/core/qgspallabeling.h
  34. +8 −10 src/core/qgspointlocator.cpp
  35. +2 −3 src/core/qgspointlocator.h
  36. +2 −3 src/core/qgsrendercontext.cpp
  37. +8 −5 src/core/qgsrendercontext.h
  38. +16 −17 src/core/qgsvectorfilewriter.cpp
  39. +5 −4 src/core/qgsvectorfilewriter.h
  40. +9 −9 src/core/qgsvectorlayerdiagramprovider.cpp
  41. +4 −10 src/core/qgsvectorlayerimport.cpp
  42. +10 −10 src/core/qgsvectorlayerlabelprovider.cpp
  43. +4 −4 src/core/qgsvectorlayerrenderer.cpp
  44. +7 −7 src/core/raster/qgsrasterlayerrenderer.cpp
  45. +24 −24 src/core/raster/qgsrasterprojector.cpp
  46. +10 −10 src/core/raster/qgsrasterprojector.h
  47. +3 −3 src/core/symbology-ng/qgsheatmaprenderer.cpp
  48. +5 −5 src/core/symbology-ng/qgsinvertedpolygonrenderer.cpp
  49. +3 −3 src/core/symbology-ng/qgslinesymbollayerv2.cpp
  50. +11 −11 src/core/symbology-ng/qgssymbolv2.cpp
  51. +2 −2 src/core/symbology-ng/qgssymbolv2.h
  52. +4 −4 src/gui/qgshighlight.cpp
  53. +3 −3 src/gui/qgsmaptoolcapture.cpp
  54. +1 −1 src/plugins/grass/qgsgrassplugin.cpp
  55. +6 −6 src/plugins/grass/qgsgrassregion.cpp
  56. +2 −2 src/plugins/grass/qgsgrassregion.h
  57. +3 −6 src/plugins/spatialquery/qgsgeometrycoordinatetransform.cpp
  58. +1 −1 src/plugins/spatialquery/qgsgeometrycoordinatetransform.h
  59. +2 −3 src/plugins/spatialquery/qgsspatialquerydialog.cpp
  60. +4 −16 src/providers/wcs/qgswcsprovider.cpp
  61. +2 −1 src/providers/wcs/qgswcsprovider.h
  62. +8 −12 src/providers/wms/qgswmsprovider.cpp
  63. +0 −3 src/providers/wms/qgswmsprovider.h
  64. +8 −8 src/server/qgswmsserver.cpp
@@ -37,6 +37,30 @@ plugins calling these methods will need to be updated.</li>
<li>readXML() and writeXML() have been renamed to readXml() and writeXml() for consistency</li>
</ul>

\subsection qgis_api_break_3_0_QgsCoordinateTransformCache QgsCoordinateTransformCache

<ul>
<li>transform() now returns a QgsCoordinateTransform object, not a pointer. An invalid QgsCoordinateTransform will
be returned in place of a null pointer.</li>
</ul>

\subsection qgis_api_break_3_0_QgsDiagramLayerSettings QgsDiagramLayerSettings

<ul>
<li>coordinateTransform() now returns a QgsCoordinateTransform object, not a pointer. An invalid QgsCoordinateTransform will
be returned in place of a null pointer.</li>
<li>setCoordinateTransform() now takes a QgsCoordinateTransform object, not a pointer. Use an invalid QgsCoordinateTransform in
place of a null pointer.</li>
<li>The ct member has been removed. Use coordinateTransform() and setCoordinateTransform() instead.
</ul>

\subsection qgis_api_break_3_0_QgsDatumTransformStore QgsDatumTransformStore

<ul>
<li>transformation() now returns a QgsCoordinateTransform object, not a pointer. An invalid QgsCoordinateTransform will
be returned in place of a null pointer.</li>
</ul>

\subsection qgis_api_break_3_0_DataProviders Data Providers

<ul>
@@ -79,6 +103,43 @@ screenUpdateRequested() were removed. These members have had no effect for a num
<li>theError parameter in appendError() and setError() were renamed to 'error'.</li>
</ul>

\subsection qgis_api_break_3_0_QgsMapRenderer QgsMapRenderer

<ul>
<li>transformation() now returns a QgsCoordinateTransform object, not a pointer. An invalid QgsCoordinateTransform will
be returned instead of a null pointer if no transformation is required.</li>
</ul>

\subsection qgis_api_break_3_0_QgsMapRendererJob QgsMapRendererJob

<ul>
<li>reprojectToLayerExtent() now takes a QgsCoordinateTransform reference, not a pointer. An invalid QgsCoordinateTransform should
be used instead of a null pointer if no transformation is required.</li>
</ul>

\subsection qgis_api_break_3_0_QgsMapSettings QgsMapSettings

<ul>
<li>layerTransform() now returns a QgsCoordinateTransform object, not a pointer. An invalid QgsCoordinateTransform will
be returned instead of a null pointer if no transformation is required.</li>
</ul>

\subsection qgis_api_break_3_0_QgsRenderContext QgsRenderContext

<ul>
<li>coordinateTransform() now returns a QgsCoordinateTransform object, not a pointer. An invalid QgsCoordinateTransform will
be returned instead of a null pointer if no transformation is required.</li>
<li>setCoordinateTransform() now takes a QgsCoordinateTransform reference, not a pointer. An invalid QgsCoordinateTransform should be used instead of a null pointer if no transformation is required.</li>
</ul>

\subsection qgis_api_break_3_0_QgsPalLayerSettings QgsPalLayerSettings

<ul>
<li>ct is now a QgsCoordinateTransform object, not a pointer. An invalid QgsCoordinateTransform will
be used instead of a null pointer if no transformation is required.</li>
<li>prepareGeometry() and geometryRequiresPreparation() now take a QgsCoordinateTransform reference, not a pointer. An invalid QgsCoordinateTransform should be used instead of a null pointer if no transformation is required.</li>
</ul>

\subsection qgis_api_break_3_0_QgsJSONExporter QgsJSONExporter

<ul>
@@ -125,13 +186,20 @@ plugins calling this method will need to be updated.</li>
<li>subsetString() was made const. This has no effect on PyQGIS code, but c++ code implementing derived layer classes will need to update the signature of this method to match.</li>
</ul>

\subsection qgis_api_break_3_0_QgsRasterProjector QgsRasterProjector

<ul>
<li>extentSize() now takes a QgsCoordinateTransform reference, not a pointer. An invalid QgsCoordinateTransform should be used instead of a null pointer if no transformation is required.</li>
</ul>

\subsection qgis_api_break_3_0_QgsVectorFileWriter QgsVectorFileWriter

<ul>
<li>QgsVectorFileWriter now takes references instead of pointers to QgsCoordinateReferenceSystem objects. Since
QgsCoordinateReferenceSystem is now implicitly shared, using references to QgsCoordinateReferenceSystem rather than
pointers makes for more robust, safer code. Use an invalid (default constructed) QgsCoordinateReferenceSystem
in code which previously passed a null pointer to QgsVectorFileWriter.</li>
<li>writeAsVectorFormat() now takes a QgsCoordinateTransform reference, not a pointer. An invalid QgsCoordinateTransform should be used instead of a null pointer if no transformation is required.</li>
</ul>

\subsection qgis_api_break_3_0_QgsVectorLayerEditBuffer QgsVectorLayerEditBuffer
@@ -7,13 +7,16 @@ class QgsCoordinateTransformCache
static QgsCoordinateTransformCache* instance();

~QgsCoordinateTransformCache();

/** Returns coordinate transformation. Cache keeps ownership
@param srcAuthId auth id string of source crs
@param destAuthId auth id string of dest crs
@param srcDatumTransform id of source's datum transform
@param destDatumTransform id of destinations's datum transform
@returns matching transform, or an invalid transform if none could be created
*/
const QgsCoordinateTransform* transform( const QString& srcAuthId, const QString& destAuthId, int srcDatumTransform = -1, int destDatumTransform = -1 );
QgsCoordinateTransform transform( const QString& srcAuthId, const QString& destAuthId, int srcDatumTransform = -1, int destDatumTransform = -1 );

/** Removes transformations where a changed crs is involved from the cache*/
void invalidateCrs( const QString& crsAuthId );

@@ -23,9 +23,11 @@ class QgsDatumTransformStore

/** Will return transform from layer's CRS to current destination CRS.
* Will emit datumTransformInfoRequested signal if the layer has no entry.
* Returns an instance from QgsCoordinateTransformCache
* @returns transformation associated with layer, or an invalid QgsCoordinateTransform
* if no transform is associated with the layer
*/
const QgsCoordinateTransform* transformation( QgsMapLayer* layer ) const;
QgsCoordinateTransform transformation( QgsMapLayer* layer ) const;


void readXML( const QDomNode& parentNode );

@@ -181,28 +181,19 @@ class QgsDiagramLayerSettings
// TODO QGIS 3.0 - make private, rename to mRenderer
QgsDiagramRendererV2* renderer;

/** Returns the coordinate transform associated with the layer.
/** Returns the coordinate transform associated with the layer, or an
* invalid transform if no transformation is required.
* @see setCoordinateTransform()
* @note added in QGIS 2.16
*/
QgsCoordinateTransform* coordinateTransform();

/** Returns the coordinate transform associated with the layer.
* @see setCoordinateTransform()
* @note added in QGIS 2.16
*/
//const QgsCoordinateTransform* coordinateTransform() const;
QgsCoordinateTransform coordinateTransform() const;

/** Sets the coordinate transform associated with the layer.
* @param transform coordinate transform. Ownership is transferred to the object.
* @see coordinateTransform()
* @note added in QGIS 2.16
*/
void setCoordinateTransform( QgsCoordinateTransform* transform /Transfer/ );

//! Associated coordinate transform. Owned by this object.
// TODO QGIS 3.0 - make private, rename to mCt
QgsCoordinateTransform* ct;
void setCoordinateTransform( const QgsCoordinateTransform& transform );

//! @deprecated will be removed in QGIS 3.0
const QgsMapToPixel* xform;
@@ -305,7 +305,10 @@ class QgsMapRenderer : QObject
void addLayerCoordinateTransform( const QString& layerId, const QString& srcAuthId, const QString& destAuthId, int srcDatumTransform = -1, int destDatumTransform = -1 );
void clearLayerCoordinateTransforms();

const QgsCoordinateTransform* transformation( const QgsMapLayer *layer ) const;
/** Returns the coordinate transform associated with a renderered layer,
* or an invalid transform is no transform is required for the layer.
*/
QgsCoordinateTransform transformation( const QgsMapLayer *layer ) const;

//! bridge to QgsMapSettings
//! @note added in 2.4
@@ -110,7 +110,7 @@ class QgsMapRendererJob : QObject
* source CRS coordinates, and if it was split, returns true, and
* also sets the contents of the r2 parameter
*/
static bool reprojectToLayerExtent( const QgsMapLayer *ml, const QgsCoordinateTransform *ct, QgsRectangle &extent, QgsRectangle &r2 );
static bool reprojectToLayerExtent( const QgsMapLayer *ml, const QgsCoordinateTransform& ct, QgsRectangle &extent, QgsRectangle &r2 );

//! @note not available in python bindings
// LayerRenderJobs prepareJobs( QPainter* painter, QgsPalLabeling* labelingEngine );
@@ -218,9 +218,9 @@ class QgsMapSettings
/**
* @brief Return coordinate transform from layer's CRS to destination CRS
* @param layer
* @return transform - may be null if the transform is not needed
* @return transform - may be invalid if the transform is not needed
*/
const QgsCoordinateTransform* layerTransform( QgsMapLayer *layer ) const;
QgsCoordinateTransform layerTransform( QgsMapLayer *layer ) const;

//! returns current extent of layer set
QgsRectangle fullExtent() const;
@@ -662,7 +662,7 @@ class QgsPalLayerSettings
QgsFields mCurFields;
int fieldIndex;
const QgsMapToPixel* xform;
const QgsCoordinateTransform* ct;
QgsCoordinateTransform ct;

QgsPoint ptZero;
QgsPoint ptOne;
@@ -916,22 +916,22 @@ class QgsPalLabeling : QgsLabelingEngineInterface
/** Prepares a geometry for registration with PAL. Handles reprojection, rotation, clipping, etc.
* @param geometry geometry to prepare
* @param context render context
* @param ct coordinate transform
* @param ct coordinate transform, or invalid transform if no transformation required
* @param clipGeometry geometry to clip features to, if applicable
* @returns prepared geometry, the caller takes ownership
* @note added in QGIS 2.9
*/
static QgsGeometry* prepareGeometry( const QgsGeometry *geometry, QgsRenderContext &context, const QgsCoordinateTransform *ct, QgsGeometry *clipGeometry = 0 ) /Factory/;
static QgsGeometry* prepareGeometry( const QgsGeometry *geometry, QgsRenderContext &context, const QgsCoordinateTransform& ct, QgsGeometry *clipGeometry = 0 ) /Factory/;

/** Checks whether a geometry requires preparation before registration with PAL
* @param geometry geometry to prepare
* @param context render context
* @param ct coordinate transform
* @param @param ct coordinate transform, or invalid transform if no transformation required
* @param clipGeometry geometry to clip features to, if applicable
* @returns true if geometry requires preparation
* @note added in QGIS 2.9
*/
static bool geometryRequiresPreparation( const QgsGeometry *geometry, QgsRenderContext &context, const QgsCoordinateTransform *ct, QgsGeometry *clipGeometry = 0 );
static bool geometryRequiresPreparation( const QgsGeometry *geometry, QgsRenderContext &context, const QgsCoordinateTransform& ct, QgsGeometry *clipGeometry = 0 );

/** Splits a text string to a list of separate lines, using a specified wrap character.
* The text string will be split on either newline characters or the wrap character.
@@ -55,7 +55,10 @@ class QgsRenderContext
QPainter* painter();
const QPainter* constPainter() const;

const QgsCoordinateTransform* coordinateTransform() const;
/** Returns the current coordinate transform for the context, or an invalid
* transform is no coordinate transformation is required.
*/
QgsCoordinateTransform coordinateTransform() const;

const QgsRectangle& extent() const;

@@ -99,8 +102,9 @@ class QgsRenderContext

//setters

/** Sets coordinate transformation. QgsRenderContext does not take ownership*/
void setCoordinateTransform( const QgsCoordinateTransform* t );
/** Sets coordinate transformation.*/
void setCoordinateTransform( const QgsCoordinateTransform& t );

void setMapToPixel( const QgsMapToPixel& mtp );
void setExtent( const QgsRectangle& extent );

@@ -179,7 +179,8 @@ class QgsVectorFileWriter
* @param layer layer to write
* @param fileName file name to write to
* @param fileEncoding encoding to use
* @param ct pointer to coordinate transform to reproject exported geometries with
* @param ct coordinate transform to reproject exported geometries with, or invalid transform
* for no transformation
* @param driverName OGR driver to use
* @param onlySelected write only selected features of layer
* @param errorMessage pointer to buffer fo error message
@@ -201,7 +202,7 @@ class QgsVectorFileWriter
static WriterError writeAsVectorFormat( QgsVectorLayer* layer,
const QString& fileName,
const QString& fileEncoding,
const QgsCoordinateTransform* ct,
const QgsCoordinateTransform& ct,
const QString& driverName = "ESRI Shapefile",
bool onlySelected = false,
QString *errorMessage = 0,
@@ -81,7 +81,7 @@ class QgsRasterProjector : QgsRasterInterface
QgsRectangle& theDestExtent, int& theDestXSize, int& theDestYSize );

/** Calculate destination extent and size from source extent and size */
static bool extentSize( const QgsCoordinateTransform* ct,
static bool extentSize( const QgsCoordinateTransform& ct,
const QgsRectangle& theSrcExtent, int theSrcXSize, int theSrcYSize,
QgsRectangle& theDestExtent, int& theDestXSize, int& theDestYSize );
};
@@ -97,8 +97,8 @@ void QgsMapToolNodeTool::createTopologyRubberBands()
rb->setBrushStyle( Qt::NoBrush );
rb->setOutlineWidth( settings.value( "/qgis/digitizing/line_width", 1 ).toInt() );
QgsAbstractGeometryV2* rbGeom = feature.constGeometry()->geometry()->clone();
if ( mCanvas->mapSettings().layerTransform( vlayer ) )
rbGeom->transform( *mCanvas->mapSettings().layerTransform( vlayer ) );
if ( mCanvas->mapSettings().layerTransform( vlayer ).isValid() )
rbGeom->transform( mCanvas->mapSettings().layerTransform( vlayer ) );
rb->setGeometry( rbGeom );
mMoveRubberBands.insert( snapFeatureId, rb );
}
@@ -140,8 +140,8 @@ void QgsMapToolNodeTool::canvasMoveEvent( QgsMapMouseEvent* e )
rb->setBrushStyle( Qt::NoBrush );
rb->setOutlineWidth( settings.value( "/qgis/digitizing/line_width", 1 ).toInt() );
QgsAbstractGeometryV2* rbGeom = mSelectedFeature->geometry()->geometry()->clone();
if ( mCanvas->mapSettings().layerTransform( vlayer ) )
rbGeom->transform( *mCanvas->mapSettings().layerTransform( vlayer ) );
if ( mCanvas->mapSettings().layerTransform( vlayer ).isValid() )
rbGeom->transform( mCanvas->mapSettings().layerTransform( vlayer ) );
rb->setGeometry( rbGeom );
mMoveRubberBands.insert( mSelectedFeature->featureId(), rb );
Q_FOREACH ( const QgsVertexEntry* vertexEntry, mSelectedFeature->vertexMap() )
@@ -402,8 +402,8 @@ void QgsMapToolNodeTool::updateSelectFeature( QgsGeometry &geom )

QgsAbstractGeometryV2* rbGeom = geom.geometry()->clone();
QgsVectorLayer *vlayer = mSelectedFeature->vlayer();
if ( mCanvas->mapSettings().layerTransform( vlayer ) )
rbGeom->transform( *mCanvas->mapSettings().layerTransform( vlayer ) );
if ( mCanvas->mapSettings().layerTransform( vlayer ).isValid() )
rbGeom->transform( mCanvas->mapSettings().layerTransform( vlayer ) );
mSelectRubberBand->setGeometry( rbGeom );
}
else
@@ -6177,12 +6177,12 @@ void QgisApp::saveAsVectorFileGeneral( QgsVectorLayer* vlayer, bool symbologyOpt
bool autoGeometryType = dialog->automaticGeometryType();
QgsWKBTypes::Type forcedGeometryType = dialog->geometryType();

QgsCoordinateTransform* ct = nullptr;
QgsCoordinateTransform ct;
destCRS = QgsCRSCache::instance()->crsBySrsId( dialog->crs() );

if ( destCRS.isValid() && destCRS != vlayer->crs() )
{
ct = new QgsCoordinateTransform( vlayer->crs(), destCRS );
ct = QgsCoordinateTransform( vlayer->crs(), destCRS );

//ask user about datum transformation
QSettings settings;
@@ -6195,13 +6195,13 @@ void QgisApp::saveAsVectorFileGeneral( QgsVectorLayer* vlayer, bool symbologyOpt
QList< int > sdt = d.selectedDatumTransform();
if ( !sdt.isEmpty() )
{
ct->setSourceDatumTransform( sdt.at( 0 ) );
ct.setSourceDatumTransform( sdt.at( 0 ) );
}
if ( sdt.size() > 1 )
{
ct->setDestinationDatumTransform( sdt.at( 1 ) );
ct.setDestinationDatumTransform( sdt.at( 1 ) );
}
ct->initialise();
ct.initialise();
}
}
}
@@ -6235,8 +6235,6 @@ void QgisApp::saveAsVectorFileGeneral( QgsVectorLayer* vlayer, bool symbologyOpt
converterPtr
);

delete ct;

QApplication::restoreOverrideCursor();

if ( error == QgsVectorFileWriter::NoError )
@@ -925,7 +925,7 @@ QgsGeometry QgsAtlasComposition::currentGeometry( const QgsCoordinateReferenceSy
}

QgsGeometry transformed = *mCurrentFeature.constGeometry();
transformed.transform( *QgsCoordinateTransformCache::instance()->transform( mCoverageLayer->crs().authid(), crs.authid() ) );
transformed.transform( QgsCoordinateTransformCache::instance()->transform( mCoverageLayer->crs().authid(), crs.authid() ) );
mGeometryCache[crs.srsid()] = transformed;
return transformed;
}

0 comments on commit adafeda

Please sign in to comment.
You can’t perform that action at this time.