Georeferencer preview point fixes

src/app/georeferencer/qgsgcpcanvasitem.cpp

@@ -14,6 +14,7 @@
  ***************************************************************************/
 
 #include "qgsgcpcanvasitem.h"
+#include "qgsmapcanvas.h"
 #include "qgsgeorefdatapoint.h"
 #include "qgsproject.h"
 #include "qgsrasterlayer.h"
@@ -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 )
 {
@@ -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();

src/app/georeferencer/qgsgcpcanvasitem.h

@@ -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*/ );

src/app/georeferencer/qgsgeorefdatapoint.cpp

@@ -92,13 +92,18 @@ void QgsGeorefDataPoint::setEnabled( bool enabled )
 
 void QgsGeorefDataPoint::setId( int id )
 {
+  const bool noLongerTemporary = mId < 0 && id >= 0;
   mId = id;
   if ( mGCPSourceItem )
   {
+    if ( noLongerTemporary )
+      mGCPSourceItem->setPointColor( Qt::red );
     mGCPSourceItem->update();
   }
   if ( mGCPDestinationItem )
   {
+    if ( noLongerTemporary )
+      mGCPDestinationItem->setPointColor( Qt::red );
     mGCPDestinationItem->update();
   }
 }

src/app/georeferencer/qgsgeorefmainwindow.cpp

@@ -55,7 +55,6 @@
 #include "qgsgeoreftooladdpoint.h"
 #include "qgsgeoreftooldeletepoint.h"
 #include "qgsgeoreftoolmovepoint.h"
-#include "qgsgcpcanvasitem.h"
 #include "qgscoordinateutils.h"
 
 #include "qgsgcplistwidget.h"
@@ -744,6 +743,7 @@ void QgsGeoreferencerMainWindow::movePoint( QPoint canvasPixels )
   if ( mvPoint )
   {
     mvPoint->moveTo( canvasPixels, pointType );
+    mGCPListWidget->updateResiduals();
   }
 }
 
@@ -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()
@@ -2483,8 +2486,8 @@ void QgsGeoreferencerMainWindow::clearGCPData()
   mPoints.clear();
   mGCPListWidget->setGCPList( &mPoints );
 
-  delete mNewlyAddedPointItem;
-  mNewlyAddedPointItem = nullptr;
+  delete mNewlyAddedPoint;
+  mNewlyAddedPoint = nullptr;
 
   QgisApp::instance()->mapCanvas()->refresh();
 }

src/app/georeferencer/qgsgeorefmainwindow.h

@@ -45,7 +45,6 @@ class QgsGeorefToolAddPoint;
 class QgsGeorefToolDeletePoint;
 class QgsGeorefToolMovePoint;
 class QgsGeorefToolMovePoint;
-class QgsGCPCanvasItem;
 class QgsGcpPoint;
 class QgsMapLayer;
 class QgsScreenHelper;
@@ -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;

src/app/georeferencer/qgsmapcoordsdialog.cpp

@@ -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 );
@@ -73,9 +72,6 @@ QgsMapCoordsDialog::~QgsMapCoordsDialog()
 {
   delete mToolEmitPoint;
 
-  delete mNewlyAddedPointItem;
-  mNewlyAddedPointItem = nullptr;
-
   QgsSettings settings;
   settings.setValue( QStringLiteral( "/Plugin-GeoReferencer/Config/Minimize" ), mMinimizeWindowCheckBox->isChecked() );
 }
@@ -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();
 }
 
@@ -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
@@ -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 )
 {

src/app/georeferencer/qgsmapcoordsdialog.h

@@ -25,7 +25,7 @@
 
 #include "ui_qgsmapcoordsdialogbase.h"
 
-class QgsGCPCanvasItem;
+class QgsGeorefDataPoint;
 
 class QPushButton;
 
@@ -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();
 
@@ -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