Use QgsDoubleSpinBoxes for range widget wrapper for longlong field types

We can't use QSpinBox for long long field types, as the range of values allowed by QSpinBox isn't sufficient to store long long values. While a double spin box isn't a perfect fit, it does avoid value truncation in a lot more cases.
qgis · Apr 30, 2021 · 8cd8214 · 8cd8214
1 parent 1df397d
commit 8cd8214
Show file tree

Hide file tree

Showing 3 changed files with 71 additions and 8 deletions.
diff --git a/src/gui/editorwidgets/qgsrangeconfigdlg.cpp b/src/gui/editorwidgets/qgsrangeconfigdlg.cpp
@@ -51,15 +51,23 @@ QgsRangeConfigDlg::QgsRangeConfigDlg( QgsVectorLayer *vl, int fieldIdx, QWidget
 
   QString text;
 
-  QVariant::Type fieldType( vl->fields().at( fieldIdx ).type() );
+  const QVariant::Type fieldType( vl->fields().at( fieldIdx ).type() );
 
   switch ( fieldType )
   {
     case QVariant::Int:
     case QVariant::LongLong:
     case QVariant::Double:
     {
-      rangeStackedWidget->setCurrentIndex( vl->fields().at( fieldIdx ).type() == QVariant::Double ? 1 : 0 );
+      // we use the double spin boxes for double OR long long field types, as QSpinBox does not have sufficient
+      // available range for long long values
+      rangeStackedWidget->setCurrentIndex( fieldType == QVariant::Int ? 0 : 1 );
+      if ( fieldType == QVariant::LongLong )
+      {
+        minimumDoubleSpinBox->setDecimals( 0 );
+        maximumDoubleSpinBox->setDecimals( 0 );
+        stepDoubleSpinBox->setDecimals( 0 );
+      }
 
       rangeWidget->clear();
       rangeWidget->addItem( tr( "Editable" ), QStringLiteral( "SpinBox" ) );
@@ -107,13 +115,15 @@ QVariantMap QgsRangeConfigDlg::config()
   switch ( layer()->fields().at( field() ).type() )
   {
     case QVariant::Int:
-    case QVariant::LongLong:
       cfg.insert( QStringLiteral( "Min" ), minimumSpinBox->value() );
       cfg.insert( QStringLiteral( "Max" ), maximumSpinBox->value() );
       cfg.insert( QStringLiteral( "Step" ), stepSpinBox->value() );
       break;
 
+    // we use the double spin boxes for double OR long long field types, as QSpinBox does not have sufficient
+    // available range for long long values
     case QVariant::Double:
+    case QVariant::LongLong:
       cfg.insert( QStringLiteral( "Min" ), minimumDoubleSpinBox->value() );
       cfg.insert( QStringLiteral( "Max" ), maximumDoubleSpinBox->value() );
       cfg.insert( QStringLiteral( "Step" ), stepDoubleSpinBox->value() );

diff --git a/src/gui/editorwidgets/qgsrangewidgetwrapper.cpp b/src/gui/editorwidgets/qgsrangewidgetwrapper.cpp
@@ -48,13 +48,17 @@ QWidget *QgsRangeWidgetWrapper::createWidget( QWidget *parent )
     switch ( layer()->fields().at( fieldIdx() ).type() )
     {
       case QVariant::Double:
+      // for long long field types we have to use a double spin box with 0 decimal places,
+      // as the min/max value supported by QSpinBox is not large enough
+      case QVariant::LongLong:
       {
+
         editor = new QgsDoubleSpinBox( parent );
         static_cast<QgsDoubleSpinBox *>( editor )->setLineEditAlignment( Qt::AlignRight );
         break;
       }
+
       case QVariant::Int:
-      case QVariant::LongLong:
       default:
         editor = new QgsSpinBox( parent );
         static_cast<QgsSpinBox *>( editor )->setLineEditAlignment( Qt::AlignRight );
@@ -97,7 +101,10 @@ void QgsRangeWidgetWrapper::initWidget( QWidget *editor )
     double stepval = step.isValid() ? step.toDouble() : 1.0;
     double minval = min.isValid() ? min.toDouble() : std::numeric_limits<double>::lowest();
     double maxval  = max.isValid() ? max.toDouble() : std::numeric_limits<double>::max();
-    int precisionval = precision.isValid() ? precision.toInt() : layer()->fields().at( fieldIdx() ).precision();
+
+    const QgsField field = layer()->fields().at( fieldIdx() );
+    // we use the double spin box for long long fields in order to get sufficient range of min/max values
+    const int precisionval = field.type() == QVariant::LongLong ? 0 : ( precision.isValid() ? precision.toInt() : field.precision() );
 
     mDoubleSpinBox->setDecimals( precisionval );
 
@@ -194,7 +201,14 @@ void QgsRangeWidgetWrapper::valueChangedVariant( const QVariant &v )
     Q_NOWARN_DEPRECATED_POP
     emit valuesChanged( v.toInt() );
   }
-  if ( v.type() == QVariant::Double )
+  else if ( v.type() == QVariant::LongLong )
+  {
+    Q_NOWARN_DEPRECATED_PUSH
+    emit valueChanged( v.toLongLong() );
+    Q_NOWARN_DEPRECATED_POP
+    emit valuesChanged( v.toLongLong() );
+  }
+  else if ( v.type() == QVariant::Double )
   {
     Q_NOWARN_DEPRECATED_PUSH
     emit valueChanged( v.toDouble() );
@@ -209,10 +223,24 @@ QVariant QgsRangeWidgetWrapper::value() const
 
   if ( mDoubleSpinBox )
   {
-    value = mDoubleSpinBox->value();
+    const QVariant::Type fieldType = field().type();
+    switch ( fieldType )
+    {
+      case QVariant::Double:
+        value = mDoubleSpinBox->value();
+        break;
+
+      case QVariant::LongLong:
+        value = static_cast< long long >( mDoubleSpinBox->value() );
+        break;
+
+      default:
+        break;
+    }
+
     if ( value == mDoubleSpinBox->minimum() && config( QStringLiteral( "AllowNull" ), true ).toBool() )
     {
-      value = QVariant( field().type() );
+      value = QVariant( fieldType );
     }
   }
   else if ( mIntSpinBox )

diff --git a/tests/src/gui/testqgsrangewidgetwrapper.cpp b/tests/src/gui/testqgsrangewidgetwrapper.cpp
@@ -55,6 +55,7 @@ class TestQgsRangeWidgetWrapper : public QObject
     void test_nulls();
     void test_negativeIntegers(); // see GH issue #32149
     void test_focus();
+    void testLongLong();
 
   private:
     std::unique_ptr<QgsRangeWidgetWrapper> widget0; // For field 0
@@ -98,6 +99,7 @@ void TestQgsRangeWidgetWrapper::init()
   fields.append( dfield2 );
   // simple int
   fields.append( QgsField( "simplenumber", QVariant::Int ) );
+  fields.append( QgsField( "longlong", QVariant::LongLong ) );
   vl->dataProvider()->addAttributes( fields );
   vl->updateFields();
   QVERIFY( vl.get() );
@@ -467,5 +469,28 @@ void TestQgsRangeWidgetWrapper::test_focus()
 
 }
 
+void TestQgsRangeWidgetWrapper::testLongLong()
+{
+  // test range widget with a long long field type
+  std::unique_ptr< QgsRangeWidgetWrapper >wrapper = std::make_unique<QgsRangeWidgetWrapper>( vl.get(), 4, nullptr, nullptr );
+
+  // should use a double spin box, as a integer spin box does not have sufficient range
+  QgsDoubleSpinBox *editor = qobject_cast<QgsDoubleSpinBox *>( wrapper->createWidget( nullptr ) );
+  QVERIFY( editor );
+  wrapper->initWidget( editor );
+  // no decimals, it's for long long value editing!
+  QCOMPARE( editor->decimals(), 0 );
+  QCOMPARE( editor->minimum( ), std::numeric_limits<double>::lowest() );
+  QCOMPARE( editor->maximum( ), std::numeric_limits<double>::max() );
+
+  wrapper->setValue( 1234567890123LL );
+
+  // double spin box value should be lossless
+  QCOMPARE( editor->value(), 1234567890123.0 );
+
+  // wrapper value must be a long long type, not double
+  QCOMPARE( wrapper->value(), 1234567890123LL );
+}
+
 QGSTEST_MAIN( TestQgsRangeWidgetWrapper )
 #include "testqgsrangewidgetwrapper.moc"