Merge pull request #8571 from elpaso/bugfix-20601-rastercalc-duplicat…

…ed-layer-names Fix rastercalc duplicated layer names
qgis · Nov 30, 2018 · 0af1ce4 · 0af1ce4
2 parents e546129 + 50e5414
commit 0af1ce4
Show file tree

Hide file tree

Showing 8 changed files with 170 additions and 37 deletions.
diff --git a/python/analysis/auto_generated/raster/qgsrastercalculator.sip.in b/python/analysis/auto_generated/raster/qgsrastercalculator.sip.in
@@ -23,6 +23,19 @@ Represents an individual raster layer/band number entry within a raster calculat
 %End
   public:
 
+    static QVector<QgsRasterCalculatorEntry> rasterEntries();
+%Docstring
+Creates a list of raster entries from the current project.
+
+If there is more than one layer with the same data source
+only one of them is added to the list, duplicate names are
+also handled by appending an _n integer to the base name.
+
+:return: the list of raster entries form the current project
+
+.. versionadded:: 3.6
+%End
+
     QString ref;
 
     QgsRasterLayer *raster;

diff --git a/python/plugins/processing/algs/qgis/ui/RasterCalculatorWidgets.py b/python/plugins/processing/algs/qgis/ui/RasterCalculatorWidgets.py
@@ -28,10 +28,13 @@
 import re
 import json
 
+from qgis.utils import iface
 from qgis.PyQt import uic
+from qgis.PyQt.QtCore import Qt
 from qgis.PyQt.QtGui import QTextCursor
 from qgis.PyQt.QtWidgets import (QLineEdit, QPushButton, QLabel,
-                                 QComboBox, QSpacerItem, QSizePolicy)
+                                 QComboBox, QSpacerItem, QSizePolicy,
+                                 QListWidgetItem)
 
 from qgis.core import (QgsProcessingUtils,
                        QgsProcessingParameterDefinition,
@@ -44,9 +47,10 @@
 from processing.tools import dataobjects
 from processing.tools.system import userFolder
 
-
 from processing.gui.wrappers import InvalidParameterValue
 
+from qgis.analysis import QgsRasterCalculatorEntry
+
 pluginPath = os.path.dirname(__file__)
 WIDGET_ADD_NEW, BASE_ADD_NEW = uic.loadUiType(
     os.path.join(pluginPath, 'AddNewExpressionDialog.ui'))
@@ -186,8 +190,20 @@ def fillPredefined(self):
     def setList(self, options):
         self.options = options
         self.listWidget.clear()
-        for opt in options.keys():
-            self.listWidget.addItem(opt)
+        entries = QgsRasterCalculatorEntry.rasterEntries()
+
+        def _find_source(name):
+            for entry in entries:
+                if entry.ref == name:
+                    return entry.raster.source()
+            return ''
+
+        for name in options.keys():
+            item = QListWidgetItem(name, self.listWidget)
+            tooltip = _find_source(name)
+            if tooltip:
+                item.setData(Qt.ToolTipRole, tooltip)
+            self.listWidget.addItem(item)
 
     def setValue(self, value):
         self.text.setPlainText(value)
@@ -201,30 +217,28 @@ class ExpressionWidgetWrapper(WidgetWrapper):
     def _panel(self, options):
         return ExpressionWidget(options)
 
+    def _get_options(self):
+        entries = QgsRasterCalculatorEntry.rasterEntries()
+        options = {}
+        for entry in entries:
+            options[entry.ref] = entry.ref
+        return options
+
     def createWidget(self):
         if self.dialogType == DIALOG_STANDARD:
-            layers = QgsProcessingUtils.compatibleRasterLayers(QgsProject.instance(), False)
-            options = {}
-            for lyr in layers:
-                for n in range(lyr.bandCount()):
-                    name = '{:s}@{:d}'.format(lyr.name(), n + 1)
-                    options[name] = name
-            return self._panel(options)
+            if iface is not None and iface.layerTreeView() is not None and iface.layerTreeView().layerTreeModel() is not None:
+                iface.layerTreeView().layerTreeModel().dataChanged.connect(self.refresh)
+            return self._panel(self._get_options())
         elif self.dialogType == DIALOG_BATCH:
             return QLineEdit()
         else:
             layers = self.dialog.getAvailableValuesOfType([QgsProcessingParameterRasterLayer], [QgsProcessingOutputRasterLayer])
             options = {self.dialog.resolveValueDescription(lyr): "{}@1".format(self.dialog.resolveValueDescription(lyr)) for lyr in layers}
-            return self._panel(options)
+            self.widget = self._panel(options)
+            return self.widget
 
-    def refresh(self):
-        # TODO: check if avoid code duplication with self.createWidget
-        layers = QgsProcessingUtils.compatibleRasterLayers(QgsProject.instance())
-        options = {}
-        for lyr in layers:
-            for n in range(lyr.bandCount()):
-                options[lyr.name()] = '{:s}@{:d}'.format(lyr.name(), n + 1)
-        self.widget.setList(options)
+    def refresh(self, *args):
+        self.widget.setList(self._get_options())
 
     def setValue(self, value):
         if self.dialogType == DIALOG_STANDARD:

diff --git a/src/analysis/raster/qgsrastercalculator.cpp b/src/analysis/raster/qgsrastercalculator.cpp
@@ -25,6 +25,7 @@
 #include "qgsrasterprojector.h"
 #include "qgsfeedback.h"
 #include "qgsogrutils.h"
+#include "qgsproject.h"
 
 #include <QFile>
 
@@ -349,3 +350,56 @@ QString QgsRasterCalculator::lastError() const
 {
   return mLastError;
 }
+
+QVector<QgsRasterCalculatorEntry> QgsRasterCalculatorEntry::rasterEntries()
+{
+  QVector<QgsRasterCalculatorEntry> availableEntries;
+  const QMap<QString, QgsMapLayer *> &layers = QgsProject::instance()->mapLayers();
+
+  auto uniqueRasterBandIdentifier = [ & ]( QgsRasterCalculatorEntry & entry ) -> bool
+  {
+    unsigned int i( 1 );
+    entry.ref = QStringLiteral( "%1@%2" ).arg( entry.raster->name() ).arg( entry.bandNumber );
+    while ( true )
+    {
+      bool unique( true );
+      for ( const auto &ref : qgis::as_const( availableEntries ) )
+      {
+        // Safety belt
+        if ( !( entry.raster && ref.raster ) )
+          continue;
+        // Check if a layer with the same data source was already added to the list
+        if ( ref.raster->publicSource() == entry.raster->publicSource() )
+          return false;
+        // If same name but different source
+        if ( ref.ref == entry.ref )
+        {
+          unique = false;
+          entry.ref = QStringLiteral( "%1_%2@%3" ).arg( entry.raster->name() ).arg( i++ ).arg( entry.bandNumber );
+        }
+      }
+      if ( unique )
+        return true;
+    }
+  };
+
+  QMap<QString, QgsMapLayer *>::const_iterator layerIt = layers.constBegin();
+  for ( ; layerIt != layers.constEnd(); ++layerIt )
+  {
+    QgsRasterLayer *rlayer = qobject_cast<QgsRasterLayer *>( layerIt.value() );
+    if ( rlayer && rlayer->dataProvider() && rlayer->dataProvider()->name() == QLatin1String( "gdal" ) )
+    {
+      //get number of bands
+      for ( int i = 0; i < rlayer->bandCount(); ++i )
+      {
+        QgsRasterCalculatorEntry entry;
+        entry.raster = rlayer;
+        entry.bandNumber = i + 1;
+        if ( ! uniqueRasterBandIdentifier( entry ) )
+          continue;
+        availableEntries.push_back( entry );
+      }
+    }
+  }
+  return availableEntries;
+}
diff --git a/src/analysis/raster/qgsrastercalculator.h b/src/analysis/raster/qgsrastercalculator.h
@@ -40,6 +40,18 @@ class ANALYSIS_EXPORT QgsRasterCalculatorEntry
 
   public:
 
+    /**
+     * Creates a list of raster entries from the current project.
+     *
+     * If there is more than one layer with the same data source
+     * only one of them is added to the list, duplicate names are
+     * also handled by appending an _n integer to the base name.
+     *
+     * \return the list of raster entries form the current project
+     * \since QGIS 3.6
+     */
+    static QVector<QgsRasterCalculatorEntry> rasterEntries();
+
     /**
      * Name of entry.
      */

diff --git a/src/app/qgisapp.cpp b/src/app/qgisapp.cpp
@@ -5681,7 +5681,7 @@ void QgisApp::showRasterCalculator()
   if ( d.exec() == QDialog::Accepted )
   {
     //invoke analysis library
-    QgsRasterCalculator rc( d.formulaString(), d.outputFile(), d.outputFormat(), d.outputRectangle(), d.outputCrs(), d.numberOfColumns(), d.numberOfRows(), d.rasterEntries() );
+    QgsRasterCalculator rc( d.formulaString(), d.outputFile(), d.outputFormat(), d.outputRectangle(), d.outputCrs(), d.numberOfColumns(), d.numberOfRows(), QgsRasterCalculatorEntry::rasterEntries() );
 
     QProgressDialog p( tr( "Calculating raster expression…" ), tr( "Abort" ), 0, 0 );
     p.setWindowModality( Qt::WindowModal );

diff --git a/src/app/qgsrastercalcdialog.cpp b/src/app/qgsrastercalcdialog.cpp
@@ -153,6 +153,7 @@ QVector<QgsRasterCalculatorEntry> QgsRasterCalcDialog::rasterEntries() const
   return entries;
 }
 
+
 void QgsRasterCalcDialog::setExtentSize( int width, int height, QgsRectangle bbox )
 {
   mNColumnsSpinBox->setValue( width );
@@ -164,31 +165,24 @@ void QgsRasterCalcDialog::setExtentSize( int width, int height, QgsRectangle bbo
   mExtentSizeSet = true;
 }
 
+
 void QgsRasterCalcDialog::insertAvailableRasterBands()
 {
-  const QMap<QString, QgsMapLayer *> &layers = QgsProject::instance()->mapLayers();
-  QMap<QString, QgsMapLayer *>::const_iterator layerIt = layers.constBegin();
-
-  for ( ; layerIt != layers.constEnd(); ++layerIt )
+  mAvailableRasterBands = QgsRasterCalculatorEntry::rasterEntries().toList();
+  mRasterBandsListWidget->clear();
+  for ( const auto &entry : qgis::as_const( mAvailableRasterBands ) )
   {
-    QgsRasterLayer *rlayer = dynamic_cast<QgsRasterLayer *>( layerIt.value() );
+    QgsRasterLayer *rlayer = entry.raster;
     if ( rlayer && rlayer->dataProvider() && rlayer->dataProvider()->name() == QLatin1String( "gdal" ) )
     {
       if ( !mExtentSizeSet ) //set bounding box / resolution of output to the values of the first possible input layer
       {
         setExtentSize( rlayer->width(), rlayer->height(), rlayer->extent() );
         mCrsSelector->setCrs( rlayer->crs() );
       }
-      //get number of bands
-      for ( int i = 0; i < rlayer->bandCount(); ++i )
-      {
-        QgsRasterCalculatorEntry entry;
-        entry.raster = rlayer;
-        entry.bandNumber = i + 1;
-        entry.ref = rlayer->name() + '@' + QString::number( i + 1 );
-        mAvailableRasterBands.push_back( entry );
-        mRasterBandsListWidget->addItem( entry.ref );
-      }
+      QListWidgetItem *item = new QListWidgetItem( entry.ref, mRasterBandsListWidget );
+      item->setData( Qt::ToolTipRole, rlayer->publicSource() );
+      mRasterBandsListWidget->addItem( item );
     }
   }
 }

diff --git a/src/app/qgsrastercalcdialog.h b/src/app/qgsrastercalcdialog.h
@@ -28,6 +28,13 @@ class APP_EXPORT QgsRasterCalcDialog: public QDialog, private Ui::QgsRasterCalcD
 {
     Q_OBJECT
   public:
+
+    /**
+     * Constructor for raster calculator dialog
+     * \param rasterLayer main raster layer, will be used for default extent and projection
+     * \param parent widget
+     * \param f window flags
+     */
     QgsRasterCalcDialog( QgsRasterLayer *rasterLayer = nullptr, QWidget *parent = nullptr, Qt::WindowFlags f = nullptr );
 
     QString formulaString() const;
@@ -43,7 +50,12 @@ class APP_EXPORT QgsRasterCalcDialog: public QDialog, private Ui::QgsRasterCalcD
     //! Number of pixels in y-direction
     int numberOfRows() const;
 
-    QVector<QgsRasterCalculatorEntry> rasterEntries() const;
+    /**
+     * Extract raster layer information from the current project
+     * \return a vector of raster entries from the current project
+     * \deprecated since QGIS 3.6 use QgsRasterCalculatorEntry::rasterEntries() instead
+     */
+    Q_DECL_DEPRECATED QVector<QgsRasterCalculatorEntry> rasterEntries() const SIP_DEPRECATED;
 
   private slots:
     void mRasterBandsListWidget_itemDoubleClicked( QListWidgetItem *item );
@@ -102,6 +114,8 @@ class APP_EXPORT QgsRasterCalcDialog: public QDialog, private Ui::QgsRasterCalcD
     QList<QgsRasterCalculatorEntry> mAvailableRasterBands;
 
     bool mExtentSizeSet = false;
+
+    friend class TestQgsRasterCalcDialog;
 };
 
 #endif // QGSRASTERCALCDIALOG_H
diff --git a/tests/src/analysis/testqgsrastercalculator.cpp b/tests/src/analysis/testqgsrastercalculator.cpp
@@ -58,6 +58,8 @@ class TestQgsRasterCalculator : public QObject
     void toString();
     void findNodes();
 
+    void testRasterEntries();
+
   private:
 
     QgsRasterLayer *mpLandsatRasterLayer = nullptr;
@@ -554,6 +556,36 @@ void TestQgsRasterCalculator::findNodes()
 
 }
 
+void TestQgsRasterCalculator::testRasterEntries()
+{
+  // Create some test layers
+  QList<QgsMapLayer *> layers;
+  QgsRasterLayer *rlayer = new QgsRasterLayer( QStringLiteral( TEST_DATA_DIR ) + "/analysis/dem.tif",  QStringLiteral( "dem" ) );
+  layers << rlayer;
+  // Duplicate name, same source
+  rlayer = new QgsRasterLayer( QStringLiteral( TEST_DATA_DIR ) + "/analysis/dem.tif",  QStringLiteral( "dem" ) );
+  layers << rlayer;
+  // Duplicated name different source
+  rlayer = new QgsRasterLayer( QStringLiteral( TEST_DATA_DIR ) + "/analysis/dem_int16.tif",  QStringLiteral( "dem" ) );
+  layers << rlayer;
+  // Different name and different source
+  rlayer = new QgsRasterLayer( QStringLiteral( TEST_DATA_DIR ) + "/analysis/slope.tif",  QStringLiteral( "slope" ) );
+  layers << rlayer ;
+  // Different name and same source
+  rlayer = new QgsRasterLayer( QStringLiteral( TEST_DATA_DIR ) + "/analysis/slope.tif",  QStringLiteral( "slope2" ) );
+  layers << rlayer ;
+  QgsProject::instance()->addMapLayers( layers );
+  QVector<QgsRasterCalculatorEntry> availableRasterBands = QgsRasterCalculatorEntry::rasterEntries();
+  QMap<QString, QgsRasterCalculatorEntry> entryMap;
+  for ( const auto &rb : qgis::as_const( availableRasterBands ) )
+  {
+    entryMap[rb.ref] = rb;
+  }
+  QStringList keys( entryMap.keys() );
+  keys.sort();
+  QCOMPARE( keys.join( ',' ), QStringLiteral( "dem@1,dem_1@1,landsat@1,landsat_4326@1,slope2@1" ) );
+}
+
 void TestQgsRasterCalculator::errors( )
 {
   QgsRasterCalculatorEntry entry1;