Raster classification: show number of digits according to data type

Not really a bug fix but related to #39058: - depending on data type, show a different number of digits when classifying the raster - code cleaning
qgis · Sep 28, 2020 · e19239b · e19239b
1 parent 8d2a0d1
commit e19239b
Show file tree

Hide file tree

Showing 4 changed files with 103 additions and 53 deletions.
diff --git a/src/core/raster/qgscolorrampshader.cpp b/src/core/raster/qgscolorrampshader.cpp
@@ -404,66 +404,69 @@ bool QgsColorRampShader::shade( double value, int *returnRedValue, int *returnGr
 
   const QgsColorRampShader::ColorRampItem &currentColorRampItem = colorRampItems[idx];
 
-  if ( colorRampType() == Interpolated )
+  switch ( colorRampType() )
   {
-    // Interpolate the color between two class breaks linearly.
-    if ( idx < 1 || overflow || currentColorRampItem.value - DOUBLE_DIFF_THRESHOLD <= value )
+    case Interpolated:
     {
-      if ( mClip && ( overflow
-                      || currentColorRampItem.value - DOUBLE_DIFF_THRESHOLD > value ) )
+      // Interpolate the color between two class breaks linearly.
+      if ( idx < 1 || overflow || currentColorRampItem.value - DOUBLE_DIFF_THRESHOLD <= value )
       {
-        return false;
+        if ( mClip && ( overflow
+                        || currentColorRampItem.value - DOUBLE_DIFF_THRESHOLD > value ) )
+        {
+          return false;
+        }
+        *returnRedValue   = currentColorRampItem.color.red();
+        *returnGreenValue = currentColorRampItem.color.green();
+        *returnBlueValue  = currentColorRampItem.color.blue();
+        *returnAlphaValue = currentColorRampItem.color.alpha();
+        return true;
       }
-      *returnRedValue   = currentColorRampItem.color.red();
-      *returnGreenValue = currentColorRampItem.color.green();
-      *returnBlueValue  = currentColorRampItem.color.blue();
-      *returnAlphaValue = currentColorRampItem.color.alpha();
-      return true;
-    }
 
-    const QgsColorRampShader::ColorRampItem &previousColorRampItem = colorRampItems[idx - 1];
+      const QgsColorRampShader::ColorRampItem &previousColorRampItem = colorRampItems[idx - 1];
 
-    float currentRampRange = currentColorRampItem.value - previousColorRampItem.value;
-    float offsetInRange = value - previousColorRampItem.value;
-    float scale = offsetInRange / currentRampRange;
-    const QRgb c1 = previousColorRampItem.color.rgba();
-    const QRgb c2 = currentColorRampItem.color.rgba();
+      float currentRampRange = currentColorRampItem.value - previousColorRampItem.value;
+      float offsetInRange = value - previousColorRampItem.value;
+      float scale = offsetInRange / currentRampRange;
+      const QRgb c1 = previousColorRampItem.color.rgba();
+      const QRgb c2 = currentColorRampItem.color.rgba();
 
-    *returnRedValue   = qRed( c1 )   + static_cast< int >( ( qRed( c2 )   - qRed( c1 ) )   * scale );
-    *returnGreenValue = qGreen( c1 ) + static_cast< int >( ( qGreen( c2 ) - qGreen( c1 ) ) * scale );
-    *returnBlueValue  = qBlue( c1 )  + static_cast< int >( ( qBlue( c2 )  - qBlue( c1 ) )  * scale );
-    *returnAlphaValue = qAlpha( c1 ) + static_cast< int >( ( qAlpha( c2 ) - qAlpha( c1 ) ) * scale );
-    return true;
-  }
-  else if ( colorRampType() == Discrete )
-  {
-    // Assign the color of the higher class for every pixel between two class breaks.
-    // NOTE: The implementation has always been different than the documentation,
-    //       which said lower class before, see https://github.com/qgis/QGIS/issues/22009
-    if ( overflow )
-    {
-      return false;
-    }
-    *returnRedValue   = currentColorRampItem.color.red();
-    *returnGreenValue = currentColorRampItem.color.green();
-    *returnBlueValue  = currentColorRampItem.color.blue();
-    *returnAlphaValue = currentColorRampItem.color.alpha();
-    return true;
-  }
-  else // EXACT
-  {
-    // Assign the color of the exact matching value in the color ramp item list
-    if ( !overflow && currentColorRampItem.value - DOUBLE_DIFF_THRESHOLD <= value )
+      *returnRedValue   = qRed( c1 )   + static_cast< int >( ( qRed( c2 )   - qRed( c1 ) )   * scale );
+      *returnGreenValue = qGreen( c1 ) + static_cast< int >( ( qGreen( c2 ) - qGreen( c1 ) ) * scale );
+      *returnBlueValue  = qBlue( c1 )  + static_cast< int >( ( qBlue( c2 )  - qBlue( c1 ) )  * scale );
+      *returnAlphaValue = qAlpha( c1 ) + static_cast< int >( ( qAlpha( c2 ) - qAlpha( c1 ) ) * scale );
+      return true;
+    };
+    case Discrete:
     {
+      // Assign the color of the higher class for every pixel between two class breaks.
+      // NOTE: The implementation has always been different than the documentation,
+      //       which said lower class before, see https://github.com/qgis/QGIS/issues/22009
+      if ( overflow )
+      {
+        return false;
+      }
       *returnRedValue   = currentColorRampItem.color.red();
       *returnGreenValue = currentColorRampItem.color.green();
       *returnBlueValue  = currentColorRampItem.color.blue();
       *returnAlphaValue = currentColorRampItem.color.alpha();
       return true;
-    }
-    else
+    };
+    case Exact:
     {
-      return false;
+      // Assign the color of the exact matching value in the color ramp item list
+      if ( !overflow && currentColorRampItem.value - DOUBLE_DIFF_THRESHOLD <= value )
+      {
+        *returnRedValue   = currentColorRampItem.color.red();
+        *returnGreenValue = currentColorRampItem.color.green();
+        *returnBlueValue  = currentColorRampItem.color.blue();
+        *returnAlphaValue = currentColorRampItem.color.alpha();
+        return true;
+      }
+      else
+      {
+        return false;
+      }
     }
   }
 }

diff --git a/src/core/raster/qgspalettedrasterrenderer.cpp b/src/core/raster/qgspalettedrasterrenderer.cpp
@@ -70,10 +70,10 @@ QgsRasterRenderer *QgsPalettedRasterRenderer::create( const QDomElement &elem, Q
       QString label;
       entryElem = paletteEntries.at( i ).toElement();
       value = static_cast<int>( entryElem.attribute( QStringLiteral( "value" ), QStringLiteral( "0" ) ).toDouble() );
-      QgsDebugMsgLevel( entryElem.attribute( "color", "#000000" ), 4 );
       color = QColor( entryElem.attribute( QStringLiteral( "color" ), QStringLiteral( "#000000" ) ) );
       color.setAlpha( entryElem.attribute( QStringLiteral( "alpha" ), QStringLiteral( "255" ) ).toInt() );
       label = entryElem.attribute( QStringLiteral( "label" ) );
+      QgsDebugMsgLevel( QStringLiteral( "Value: %1, label: %2, color: %3" ).arg( value ).arg( label, entryElem.attribute( QStringLiteral( "color" ) ) ), 4 );
       classData << Class( value, color, label );
     }
   }

diff --git a/src/gui/raster/qgssinglebandpseudocolorrendererwidget.cpp b/src/gui/raster/qgssinglebandpseudocolorrendererwidget.cpp
@@ -195,7 +195,7 @@ void QgsSingleBandPseudoColorRendererWidget::loadMinMax( int bandNo, double min,
   }
   else
   {
-    whileBlocking( mMinLineEdit )->setText( QLocale().toString( min ) );
+    whileBlocking( mMinLineEdit )->setText( displayValue( min ) );
   }
 
   if ( std::isnan( max ) )
@@ -204,13 +204,13 @@ void QgsSingleBandPseudoColorRendererWidget::loadMinMax( int bandNo, double min,
   }
   else
   {
-    whileBlocking( mMaxLineEdit )->setText( QLocale().toString( max ) );
+    whileBlocking( mMaxLineEdit )->setText( displayValue( max ) );
   }
 
   // We compare old min and new min as text because QString::number keeps a fixed number of significant
   // digits (default 6) and so loaded min/max will always differ from current one, which triggers a
   // classification, and wipe out every user modification (see https://github.com/qgis/QGIS/issues/36172)
-  if ( mMinLineEdit->text() != QLocale().toString( min ) || mMaxLineEdit->text() != QLocale().toString( max ) )
+  if ( mMinLineEdit->text() != displayValue( min ) || mMaxLineEdit->text() != displayValue( max ) )
   {
     whileBlocking( mColorRampShaderWidget )->setRasterBand( bandNo );
     whileBlocking( mColorRampShaderWidget )->setMinimumMaximumAndClassify( min, max );
@@ -220,8 +220,8 @@ void QgsSingleBandPseudoColorRendererWidget::loadMinMax( int bandNo, double min,
 
 void QgsSingleBandPseudoColorRendererWidget::loadMinMaxFromTree( double min, double max )
 {
-  whileBlocking( mMinLineEdit )->setText( QLocale().toString( min ) );
-  whileBlocking( mMaxLineEdit )->setText( QLocale().toString( max ) );
+  whileBlocking( mMinLineEdit )->setText( displayValue( min ) );
+  whileBlocking( mMaxLineEdit )->setText( displayValue( max ) );
   minMaxModified();
 }
 
@@ -231,7 +231,7 @@ void QgsSingleBandPseudoColorRendererWidget::setLineEditValue( QLineEdit *lineEd
   QString s;
   if ( !std::isnan( value ) )
   {
-    s = QLocale().toString( value );
+    s = displayValue( value );
   }
   lineEdit->setText( s );
 }
@@ -277,3 +277,46 @@ void QgsSingleBandPseudoColorRendererWidget::minMaxModified()
 {
   mMinMaxWidget->userHasSetManualMinMaxValues();
 }
+
+QString QgsSingleBandPseudoColorRendererWidget::displayValue( const double value )
+{
+  int precision { 9 };
+  if ( mRasterLayer->dataProvider() )
+  {
+    switch ( mRasterLayer->dataProvider()->dataType( mBandComboBox->currentBand() ) )
+    {
+      case Qgis::DataType::Int16:
+      case Qgis::DataType::UInt16:
+      {
+        precision = 5;
+        break;
+      }
+      case Qgis::DataType::Int32:
+      case Qgis::DataType::UInt32:
+      {
+        precision = 10;
+        break;
+      }
+      case Qgis::DataType::Byte:
+      {
+        precision = 3;
+        break;
+      }
+      case Qgis::DataType::Float32:
+      {
+        precision = 9;
+        break;
+      }
+      case Qgis::DataType::Float64:
+      {
+        precision = 17;
+        break;
+      }
+      default:
+      {
+        precision = 9;
+      }
+    }
+  }
+  return QLocale().toString( value, 'g', precision );
+}
diff --git a/src/gui/raster/qgssinglebandpseudocolorrendererwidget.h b/src/gui/raster/qgssinglebandpseudocolorrendererwidget.h
@@ -80,6 +80,10 @@ class GUI_EXPORT QgsSingleBandPseudoColorRendererWidget: public QgsRasterRendere
     int mMinMaxOrigin;
 
     void minMaxModified();
+
+    // Convert min/max to localized display value with correct precision
+    QString displayValue( const double value );
+
 };
 
 #endif // QGSSINGLEBANDCOLORRENDERERWIDGET_H