Skip to content

Commit

Permalink
consider datum transformation when pasting features (fixes #16846)
Browse files Browse the repository at this point in the history
  • Loading branch information
jef-n committed Jul 17, 2017
1 parent bcc8e90 commit bae6d56
Show file tree
Hide file tree
Showing 7 changed files with 42 additions and 8 deletions.
1 change: 1 addition & 0 deletions doc/api_break.dox
Expand Up @@ -960,6 +960,7 @@ QgsDatumTransformStore {#qgis_api_break_3_0_QgsDatumTransformStore}

- transformation() now returns a QgsCoordinateTransform object, not a pointer. An invalid QgsCoordinateTransform will
be returned in place of a null pointer.
- transformation() also optional source and destination authid parameters


QgsDiagram {#qgis_api_break_3_0_QgsDiagram}
Expand Down
5 changes: 3 additions & 2 deletions python/core/qgsdatumtransformstore.sip
Expand Up @@ -36,12 +36,13 @@ class QgsDatumTransformStore
:rtype: bool
%End

QgsCoordinateTransform transformation( const QgsMapLayer *layer ) const;
QgsCoordinateTransform transformation( const QgsMapLayer *layer, QString srcAuthId = QString(), QString dstAuthId = QString() ) const;
%Docstring
Will return transform from layer's CRS to current destination CRS.
Will emit datumTransformInfoRequested signal if the layer has no entry.
:return: transformation associated with layer, or an invalid QgsCoordinateTransform
if no transform is associated with the layer
\param srcAuthId source CRS (defaults to layer crs)
\param dstAuthId destination CRS (defaults to store's crs)
:rtype: QgsCoordinateTransform
%End

Expand Down
1 change: 1 addition & 0 deletions python/gui/qgsmapcanvas.sip
Expand Up @@ -853,6 +853,7 @@ called when panning is in action, reset indicates end of panning




void updateDatumTransformEntries();
%Docstring
Make sure the datum transform store is properly populated
Expand Down
25 changes: 24 additions & 1 deletion src/app/qgsclipboard.cpp
Expand Up @@ -36,6 +36,8 @@
#include "qgsogrutils.h"
#include "qgsjsonutils.h"
#include "qgssettings.h"
#include "qgisapp.h"
#include "qgsmapcanvas.h"

QgsClipboard::QgsClipboard()
: QObject()
Expand All @@ -59,6 +61,9 @@ void QgsClipboard::replaceWithCopyOf( QgsVectorLayer *src )
mFeatureFields = src->fields();
mFeatureClipboard = src->selectedFeatures();
mCRS = src->crs();
layerDestroyed();
mSrcLayer = src;
connect( mSrcLayer, &QObject::destroyed, this, &QgsClipboard::layerDestroyed );

QgsDebugMsg( "replaced QGis clipboard." );

Expand All @@ -73,11 +78,19 @@ void QgsClipboard::replaceWithCopyOf( QgsFeatureStore &featureStore )
mFeatureFields = featureStore.fields();
mFeatureClipboard = featureStore.features();
mCRS = featureStore.crs();
disconnect( mSrcLayer, &QObject::destroyed, this, &QgsClipboard::layerDestroyed );
mSrcLayer = nullptr;
setSystemClipboard();
mUseSystemClipboard = false;
emit changed();
}

void QgsClipboard::layerDestroyed()

This comment has been minimized.

Copy link
@nyalldawson

nyalldawson Jul 17, 2017

Collaborator

If you make mSrcLayer a QPointer < QgsVectorLayer> then all this is taken care of for you, and mSrcLayer will be automatically cleared if the layer is deleted.

{
disconnect( mSrcLayer, &QObject::destroyed, this, &QgsClipboard::layerDestroyed );
mSrcLayer = nullptr;
}

QString QgsClipboard::generateClipboardText() const
{
QgsSettings settings;
Expand Down Expand Up @@ -264,7 +277,17 @@ bool QgsClipboard::isEmpty() const
QgsFeatureList QgsClipboard::transformedCopyOf( const QgsCoordinateReferenceSystem &destCRS, const QgsFields &fields ) const
{
QgsFeatureList featureList = copyOf( fields );
QgsCoordinateTransform ct( crs(), destCRS );

QgsCoordinateTransform ct;
if ( mSrcLayer )
{
QgisApp::instance()->mapCanvas()->getDatumTransformInfo( mSrcLayer, crs().authid(), destCRS.authid() );
ct = QgisApp::instance()->mapCanvas()->mapSettings().datumTransformStore().transformation( mSrcLayer, crs().authid(), destCRS.authid() );
}
else
{
ct = QgsCoordinateTransform( crs(), destCRS );
}

QgsDebugMsg( "transforming clipboard." );
for ( QgsFeatureList::iterator iter = featureList.begin(); iter != featureList.end(); ++iter )
Expand Down
5 changes: 5 additions & 0 deletions src/app/qgsclipboard.h
Expand Up @@ -148,6 +148,10 @@ class APP_EXPORT QgsClipboard : public QObject
//! Emitted when content changed
void changed();

private slots:
//! source layer destroyed
void layerDestroyed();

private:

/**
Expand Down Expand Up @@ -180,6 +184,7 @@ class APP_EXPORT QgsClipboard : public QObject
QgsFeatureList mFeatureClipboard;
QgsFields mFeatureFields;
QgsCoordinateReferenceSystem mCRS;
QgsVectorLayer *mSrcLayer = nullptr;

//! True when the data from the system clipboard should be read
bool mUseSystemClipboard;
Expand Down
8 changes: 5 additions & 3 deletions src/core/qgsdatumtransformstore.cpp
Expand Up @@ -50,13 +50,15 @@ bool QgsDatumTransformStore::hasEntryForLayer( QgsMapLayer *layer ) const
return mEntries.contains( layer->id() );
}

QgsCoordinateTransform QgsDatumTransformStore::transformation( const QgsMapLayer *layer ) const
QgsCoordinateTransform QgsDatumTransformStore::transformation( const QgsMapLayer *layer, QString srcAuthId, QString dstAuthId ) const
{
if ( !layer )
return QgsCoordinateTransform();

QString srcAuthId = layer->crs().authid();
QString dstAuthId = mDestCRS.authid();
if ( srcAuthId.isEmpty() )
srcAuthId = layer->crs().authid();
if ( dstAuthId.isEmpty() )
dstAuthId = mDestCRS.authid();

if ( srcAuthId == dstAuthId )
{
Expand Down
5 changes: 3 additions & 2 deletions src/core/qgsdatumtransformstore.h
Expand Up @@ -45,11 +45,12 @@ class CORE_EXPORT QgsDatumTransformStore

/**
* Will return transform from layer's CRS to current destination CRS.
* Will emit datumTransformInfoRequested signal if the layer has no entry.
* \returns transformation associated with layer, or an invalid QgsCoordinateTransform
* if no transform is associated with the layer
* \param srcAuthId source CRS (defaults to layer crs)
* \param dstAuthId destination CRS (defaults to store's crs)
*/
QgsCoordinateTransform transformation( const QgsMapLayer *layer ) const;
QgsCoordinateTransform transformation( const QgsMapLayer *layer, QString srcAuthId = QString(), QString dstAuthId = QString() ) const;

void readXml( const QDomNode &parentNode );

Expand Down

7 comments on commit bae6d56

@3nids
Copy link
Member

@3nids 3nids commented on bae6d56 Jul 18, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jef-n any chance to backport this to 2.18 ?

@3nids
Copy link
Member

@3nids 3nids commented on bae6d56 Jul 18, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this issue is even a bit broader....
taking the example of the issue (canvas in epsg 2056, layer in 21781 with custom datum transform)
if you use the select map tool and try to select features on the 21781 layer, the map tool does not respect the transformation either....

@3nids
Copy link
Member

@3nids 3nids commented on bae6d56 Jul 18, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@3nids
Copy link
Member

@3nids 3nids commented on bae6d56 Jul 19, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

snapping is also affected....

@nyalldawson
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@3nids actually the issue is everywhere. IMO datum transforms need a total rethink and rewrite - the current approach just isn't working.

@3nids
Copy link
Member

@3nids 3nids commented on bae6d56 Jul 20, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nyalldawson do you have something in mind already? I think this is a major concern in Switzerland for the upcoming months/years, and I guess it'll be easier to fix this before 3.0.

@andreasneumann
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree. Datum transformations are currently quite important in Switzerland. Some cantons already migrated everything, others are in transition or will do the migration soon.

Please sign in to comment.