Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Georeferencer preview point fixes #57984

Merged
merged 4 commits into from
Oct 2, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion src/app/georeferencer/qgsgcpcanvasitem.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
***************************************************************************/

#include "qgsgcpcanvasitem.h"
#include "qgsmapcanvas.h"
#include "qgsgeorefdatapoint.h"
#include "qgsproject.h"
#include "qgsrasterlayer.h"
Expand All @@ -25,7 +26,7 @@
QgsGCPCanvasItem::QgsGCPCanvasItem( QgsMapCanvas *mapCanvas, QgsGeorefDataPoint *dataPoint, bool isGCPSource )
: QgsMapCanvasItem( mapCanvas )
, mDataPoint( dataPoint )
, mPointBrush( Qt::red )
, mPointBrush( mDataPoint && mDataPoint->id() < 0 ? QColor( 0, 200, 0 ) : Qt::red )
, mLabelBrush( Qt::yellow )
, mIsGCPSource( isGCPSource )
{
Expand Down Expand Up @@ -64,6 +65,10 @@ void QgsGCPCanvasItem::paint( QPainter *p )
p->setBrush( mPointBrush );
p->drawEllipse( -2, -2, 5, 5 );

// Don't draw point tip for temporary points
if ( id < 0 )
return;

const QgsSettings s;
const bool showIDs = s.value( QStringLiteral( "/Plugin-GeoReferencer/Config/ShowId" ) ).toBool();
const bool showCoords = s.value( QStringLiteral( "/Plugin-GeoReferencer/Config/ShowCoords" ) ).toBool();
Expand Down
7 changes: 5 additions & 2 deletions src/app/georeferencer/qgsgcpcanvasitem.h
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,15 @@
#ifndef QGSGCPCANVASITEM_H
#define QGSGCPCANVASITEM_H

#include "qgsmapcanvas.h"
#include <QBrush>
#include <QPen>

#include "qgsmapcanvasitem.h"

class QgsMapCanvas;
class QgsGeorefDataPoint;

class QgsGCPCanvasItem : public QgsMapCanvasItem
class QgsGCPCanvasItem final: public QgsMapCanvasItem
{
public:
QgsGCPCanvasItem( QgsMapCanvas *mapCanvas, QgsGeorefDataPoint *dataPoint, bool isGCPSource/* = true*/ );
Expand Down
5 changes: 5 additions & 0 deletions src/app/georeferencer/qgsgeorefdatapoint.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -92,13 +92,18 @@ void QgsGeorefDataPoint::setEnabled( bool enabled )

void QgsGeorefDataPoint::setId( int id )
{
const bool noLongerTemporary = mId < 0 && id >= 0;
Copy link
Contributor

Choose a reason for hiding this comment

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

Variable name doesn't seem optimal, maybe "isValid" or "isPermanent"?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We only want to change the color if this call changes the point form a temporary ( < 0 ) to a permanent, hence the noLonger in the name

mId = id;
if ( mGCPSourceItem )
{
if ( noLongerTemporary )
mGCPSourceItem->setPointColor( Qt::red );
mGCPSourceItem->update();
}
if ( mGCPDestinationItem )
{
if ( noLongerTemporary )
mGCPDestinationItem->setPointColor( Qt::red );
mGCPDestinationItem->update();
}
}
Expand Down
29 changes: 16 additions & 13 deletions src/app/georeferencer/qgsgeorefmainwindow.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,6 @@
#include "qgsgeoreftooladdpoint.h"
#include "qgsgeoreftooldeletepoint.h"
#include "qgsgeoreftoolmovepoint.h"
#include "qgsgcpcanvasitem.h"
#include "qgscoordinateutils.h"

#include "qgsgcplistwidget.h"
Expand Down Expand Up @@ -744,6 +743,7 @@ void QgsGeoreferencerMainWindow::movePoint( QPoint canvasPixels )
if ( mvPoint )
{
mvPoint->moveTo( canvasPixels, pointType );
mGCPListWidget->updateResiduals();
}
}

Expand All @@ -764,29 +764,32 @@ void QgsGeoreferencerMainWindow::releasePoint( QPoint p )

void QgsGeoreferencerMainWindow::showCoordDialog( const QgsPointXY &sourceCoordinates )
{
delete mNewlyAddedPointItem;
mNewlyAddedPointItem = nullptr;
if ( !mLayer )
return;

// show a temporary marker at the clicked source point on the raster while we show the coordinate dialog.
mNewlyAddedPointItem = new QgsGCPCanvasItem( mCanvas, nullptr, true );
mNewlyAddedPointItem->setPointColor( QColor( 0, 200, 0 ) );
mNewlyAddedPointItem->setPos( mNewlyAddedPointItem->toCanvasCoordinates( sourceCoordinates ) );


QgsCoordinateReferenceSystem lastProjection = mLastGCPProjection.isValid() ? mLastGCPProjection : mTargetCrs;
if ( mLayer && !mMapCoordsDialog )
if ( !mMapCoordsDialog )
{
mMapCoordsDialog = new QgsMapCoordsDialog( QgisApp::instance()->mapCanvas(), sourceCoordinates, lastProjection, this );
mNewlyAddedPoint = new QgsGeorefDataPoint( mCanvas, QgisApp::instance()->mapCanvas(), sourceCoordinates, QgsPointXY(), QgsCoordinateReferenceSystem(), true );
mMapCoordsDialog = new QgsMapCoordsDialog( QgisApp::instance()->mapCanvas(), mNewlyAddedPoint, lastProjection, this );
connect( mMapCoordsDialog, &QgsMapCoordsDialog::pointAdded, this, [ = ]( const QgsPointXY & sourceLayerCoordinate, const QgsPointXY & destinationCoordinate, const QgsCoordinateReferenceSystem & destinationCrs )
{
addPoint( sourceLayerCoordinate, destinationCoordinate, destinationCrs );
} );
connect( mMapCoordsDialog, &QObject::destroyed, this, [ = ]
{
delete mNewlyAddedPointItem;
mNewlyAddedPointItem = nullptr;
delete mNewlyAddedPoint;
mNewlyAddedPoint = nullptr;
} );
mMapCoordsDialog->show();
}
else
{
mMapCoordsDialog->updateSourceCoordinates( sourceCoordinates );
}
mMapCoordsDialog->show();
}

void QgsGeoreferencerMainWindow::loadGCPsDialog()
Expand Down Expand Up @@ -2483,8 +2486,8 @@ void QgsGeoreferencerMainWindow::clearGCPData()
mPoints.clear();
mGCPListWidget->setGCPList( &mPoints );

delete mNewlyAddedPointItem;
mNewlyAddedPointItem = nullptr;
delete mNewlyAddedPoint;
mNewlyAddedPoint = nullptr;

QgisApp::instance()->mapCanvas()->refresh();
}
Expand Down
4 changes: 1 addition & 3 deletions src/app/georeferencer/qgsgeorefmainwindow.h
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@ class QgsGeorefToolAddPoint;
class QgsGeorefToolDeletePoint;
class QgsGeorefToolMovePoint;
class QgsGeorefToolMovePoint;
class QgsGCPCanvasItem;
class QgsGcpPoint;
class QgsMapLayer;
class QgsScreenHelper;
Expand Down Expand Up @@ -285,10 +284,9 @@ class APP_EXPORT QgsGeoreferencerMainWindow : public QMainWindow, private Ui::Qg
QgsGeorefToolMovePoint *mToolMovePoint = nullptr;
QgsGeorefToolMovePoint *mToolMovePointQgis = nullptr;

QgsGCPCanvasItem *mNewlyAddedPointItem = nullptr;

QgsGeorefDataPoint *mMovingPoint = nullptr;
QgsGeorefDataPoint *mMovingPointQgis = nullptr;
QgsGeorefDataPoint *mNewlyAddedPoint = nullptr;
QPointer<QgsMapCoordsDialog> mMapCoordsDialog;

bool mUseZeroForTrans = false;
Expand Down
26 changes: 11 additions & 15 deletions src/app/georeferencer/qgsmapcoordsdialog.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -23,14 +23,13 @@
#include "qgsgui.h"
#include "qgsapplication.h"
#include "qgsprojectionselectionwidget.h"
#include "qgsproject.h"
#include "qgsgcpcanvasitem.h"
#include "qgsgeorefdatapoint.h"

QgsMapCoordsDialog::QgsMapCoordsDialog( QgsMapCanvas *qgisCanvas, const QgsPointXY &sourceLayerCoordinates, QgsCoordinateReferenceSystem &rasterCrs, QWidget *parent )
QgsMapCoordsDialog::QgsMapCoordsDialog( QgsMapCanvas *qgisCanvas, QgsGeorefDataPoint *georefDataPoint, QgsCoordinateReferenceSystem &rasterCrs, QWidget *parent )
: QDialog( parent, Qt::Dialog )
, mQgisCanvas( qgisCanvas )
, mRasterCrs( rasterCrs )
, mSourceLayerCoordinates( sourceLayerCoordinates )
, mNewlyAddedPoint( georefDataPoint )
{
setupUi( this );
QgsGui::enableAutoGeometryRestore( this );
Expand Down Expand Up @@ -73,9 +72,6 @@ QgsMapCoordsDialog::~QgsMapCoordsDialog()
{
delete mToolEmitPoint;

delete mNewlyAddedPointItem;
mNewlyAddedPointItem = nullptr;

QgsSettings settings;
settings.setValue( QStringLiteral( "/Plugin-GeoReferencer/Config/Minimize" ), mMinimizeWindowCheckBox->isChecked() );
}
Expand Down Expand Up @@ -103,7 +99,7 @@ void QgsMapCoordsDialog::buttonBox_accepted()
if ( !ok )
y = dmsToDD( leYCoord->text() );

emit pointAdded( mSourceLayerCoordinates, QgsPointXY( x, y ), mProjectionSelector->crs().isValid() ? mProjectionSelector->crs() : mRasterCrs );
emit pointAdded( mNewlyAddedPoint->sourcePoint(), QgsPointXY( x, y ), mProjectionSelector->crs().isValid() ? mProjectionSelector->crs() : mRasterCrs );
close();
}

Expand All @@ -119,13 +115,7 @@ void QgsMapCoordsDialog::maybeSetXY( const QgsPointXY &xy, Qt::MouseButton butto
leXCoord->setText( qgsDoubleToString( mapCoordPoint.x() ) );
leYCoord->setText( qgsDoubleToString( mapCoordPoint.y() ) );

delete mNewlyAddedPointItem;
mNewlyAddedPointItem = nullptr;

// show a temporary marker at the clicked source point
mNewlyAddedPointItem = new QgsGCPCanvasItem( mQgisCanvas, nullptr, true );
mNewlyAddedPointItem->setPointColor( QColor( 0, 200, 0 ) );
mNewlyAddedPointItem->setPos( mNewlyAddedPointItem->toCanvasCoordinates( mapCoordPoint ) );
mNewlyAddedPoint->setDestinationPoint( mapCoordPoint );
}

// only restore window if it was minimized
Expand Down Expand Up @@ -185,6 +175,12 @@ double QgsMapCoordsDialog::dmsToDD( const QString &dms )
return res;
}

void QgsMapCoordsDialog::updateSourceCoordinates( const QgsPointXY &sourceCoordinates )
{
mNewlyAddedPoint->setSourcePoint( sourceCoordinates );
}


QgsGeorefMapToolEmitPoint::QgsGeorefMapToolEmitPoint( QgsMapCanvas *canvas )
: QgsMapTool( canvas )
{
Expand Down
15 changes: 8 additions & 7 deletions src/app/georeferencer/qgsmapcoordsdialog.h
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@

#include "ui_qgsmapcoordsdialogbase.h"

class QgsGCPCanvasItem;
class QgsGeorefDataPoint;

class QPushButton;

Expand Down Expand Up @@ -64,13 +64,16 @@ class QgsMapCoordsDialog : public QDialog, private Ui::QgsMapCoordsDialogBase
/**
* Constructor for QgsMapCoordsDialog.
* \param qgisCanvas
* \param sourceCoordinates must be in source layer coordinates, NOT pixels (unless source image is completely non-referenced)!
* \param georefDataPoint Temporary data point used for preview on source and destination canvases while dialog is visible
* \param rasterCrs
* \param parent
*/
QgsMapCoordsDialog( QgsMapCanvas *qgisCanvas, const QgsPointXY &sourceCoordinates, QgsCoordinateReferenceSystem &rasterCrs, QWidget *parent = nullptr );
QgsMapCoordsDialog( QgsMapCanvas *qgisCanvas, QgsGeorefDataPoint *georefDataPoint, QgsCoordinateReferenceSystem &rasterCrs, QWidget *parent = nullptr );
~QgsMapCoordsDialog() override;

//! Update the source coordinates of the newly added
void updateSourceCoordinates( const QgsPointXY &sourceCoordinates );

private slots:
void buttonBox_accepted();

Expand Down Expand Up @@ -101,12 +104,10 @@ class QgsMapCoordsDialog : public QDialog, private Ui::QgsMapCoordsDialogBase
QgsMapTool *mPrevMapTool = nullptr;
QgsMapCanvas *mQgisCanvas = nullptr;

QgsGCPCanvasItem *mNewlyAddedPointItem = nullptr;

QgsCoordinateReferenceSystem mRasterCrs;

//! Source layer coordinates -- must be in source layer coordinates, not pixels (unless source image is completely non-referenced)
QgsPointXY mSourceLayerCoordinates;
//! Used for point preview. Holds the source layer coordinates -- must be in source layer coordinates, not pixels (unless source image is completely non-referenced)
QgsGeorefDataPoint *mNewlyAddedPoint = nullptr;
};

#endif
Loading