From 91cce1732b73c4457e474c557aaa7f343c0dc8a2 Mon Sep 17 00:00:00 2001 From: "Wladimir J. van der Laan" Date: Fri, 18 Jul 2014 16:31:13 +0200 Subject: [PATCH 1/3] qt: Use fixed-point arithmetic in amount spinbox Fixes various issues and cleans up code - Fixes issue #4500: Amount widget +/- has floating point rounding artifacts - Amount box can now be emptied again, without clearing to 0 Also aligns the amount to the right, as in other places. --- src/Makefile.qt.include | 1 + src/qt/bitcoinamountfield.cpp | 272 +++++++++++++++++++++------------- src/qt/bitcoinamountfield.h | 14 +- src/qt/bitcoinunits.cpp | 7 + src/qt/bitcoinunits.h | 3 + src/qt/sendcoinsentry.cpp | 9 +- 6 files changed, 190 insertions(+), 116 deletions(-) diff --git a/src/Makefile.qt.include b/src/Makefile.qt.include index 75b7b683d..2772bc753 100644 --- a/src/Makefile.qt.include +++ b/src/Makefile.qt.include @@ -145,6 +145,7 @@ BITCOIN_MM = \ QT_MOC = \ qt/bitcoin.moc \ + qt/bitcoinamountfield.moc \ qt/intro.moc \ qt/overviewpage.moc \ qt/rpcconsole.moc diff --git a/src/qt/bitcoinamountfield.cpp b/src/qt/bitcoinamountfield.cpp index e047c278b..646603901 100644 --- a/src/qt/bitcoinamountfield.cpp +++ b/src/qt/bitcoinamountfield.cpp @@ -9,63 +9,185 @@ #include "qvaluecombobox.h" #include -#include +#include #include #include -#include // for qPow() +#include -// QDoubleSpinBox that shows SI-style thin space thousands separators -class AmountSpinBox: public QDoubleSpinBox +/** QSpinBox that uses fixed-point numbers internally and uses our own + * formatting/parsing functions. + */ +class AmountSpinBox: public QAbstractSpinBox { + Q_OBJECT public: explicit AmountSpinBox(QWidget *parent): - QDoubleSpinBox(parent) + QAbstractSpinBox(parent), + currentUnit(BitcoinUnits::BTC), + singleStep(100000) // satoshis { + setAlignment(Qt::AlignRight); + + connect(lineEdit(), SIGNAL(textEdited(QString)), this, SIGNAL(valueChanged())); + } + + QValidator::State validate(QString &text, int &pos) const + { + if(text.isEmpty()) + return QValidator::Intermediate; + bool valid = false; + parse(text, &valid); + /* Make sure we return Intermediate so that fixup() is called on defocus */ + return valid ? QValidator::Intermediate : QValidator::Invalid; + } + + void fixup(QString &input) const + { + bool valid = false; + qint64 val = parse(input, &valid); + if(valid) + { + input = BitcoinUnits::format(currentUnit, val, false, BitcoinUnits::separatorAlways); + lineEdit()->setText(input); + } } - QString textFromValue(double value) const + + qint64 value(bool *valid_out=0) const { - QStringList parts = QDoubleSpinBox::textFromValue(value).split("."); - QString quotient_str = parts[0]; - QString remainder_str; - if(parts.size() > 1) - remainder_str = parts[1]; - - // Code duplication between here and BitcoinUnits::format - // TODO: Figure out how to share this code - QChar thin_sp(THIN_SP_CP); - int q_size = quotient_str.size(); - if (q_size > 4) - for (int i = 3; i < q_size; i += 3) - quotient_str.insert(q_size - i, thin_sp); - - int r_size = remainder_str.size(); - if (r_size > 4) - for (int i = 3, adj = 0; i < r_size; i += 3, adj++) - remainder_str.insert(i + adj, thin_sp); - - if(remainder_str.isEmpty()) - return quotient_str; + return parse(text(), valid_out); + } + + void setValue(qint64 value) + { + lineEdit()->setText(BitcoinUnits::format(currentUnit, value, false, BitcoinUnits::separatorAlways)); + emit valueChanged(); + } + + void stepBy(int steps) + { + bool valid = false; + qint64 val = value(&valid); + val = val + steps * singleStep; + val = qMin(qMax(val, Q_INT64_C(0)), BitcoinUnits::maxMoney()); + setValue(val); + } + + StepEnabled stepEnabled() const + { + StepEnabled rv = 0; + if(text().isEmpty()) // Allow step-up with empty field + return StepUpEnabled; + bool valid = false; + qint64 val = value(&valid); + if(valid) + { + if(val > 0) + rv |= StepDownEnabled; + if(val < BitcoinUnits::maxMoney()) + rv |= StepUpEnabled; + } + return rv; + } + + void setDisplayUnit(int unit) + { + bool valid = false; + qint64 val = value(&valid); + + currentUnit = unit; + + if(valid) + setValue(val); else - return quotient_str + QString(".") + remainder_str; + clear(); } - QValidator::State validate (QString &text, int &pos) const + + void setSingleStep(qint64 step) { - QString s(BitcoinUnits::removeSpaces(text)); - return QDoubleSpinBox::validate(s, pos); + singleStep = step; + } + + QSize minimumSizeHint() const + { + if(cachedMinimumSizeHint.isEmpty()) + { + ensurePolished(); + + const QFontMetrics fm(fontMetrics()); + int h = lineEdit()->minimumSizeHint().height(); + int w = fm.width(BitcoinUnits::format(BitcoinUnits::BTC, BitcoinUnits::maxMoney(), false, BitcoinUnits::separatorAlways)); + w += 2; // cursor blinking space + + QStyleOptionSpinBox opt; + initStyleOption(&opt); + QSize hint(w, h); + QSize extra(35, 6); + opt.rect.setSize(hint + extra); + extra += hint - style()->subControlRect(QStyle::CC_SpinBox, &opt, + QStyle::SC_SpinBoxEditField, this).size(); + // get closer to final result by repeating the calculation + opt.rect.setSize(hint + extra); + extra += hint - style()->subControlRect(QStyle::CC_SpinBox, &opt, + QStyle::SC_SpinBoxEditField, this).size(); + hint += extra; + + opt.rect = rect(); + + cachedMinimumSizeHint = style()->sizeFromContents(QStyle::CT_SpinBox, &opt, hint, this) + .expandedTo(QApplication::globalStrut()); + } + return cachedMinimumSizeHint; + } +private: + int currentUnit; + qint64 singleStep; + mutable QSize cachedMinimumSizeHint; + + /** + * Parse a string into a number of base monetary units and + * return validity. + * @note Must return 0 if !valid. + */ + qint64 parse(const QString &text, bool *valid_out=0) const + { + qint64 val = 0; + bool valid = BitcoinUnits::parse(currentUnit, text, &val); + if(valid) + { + if(val < 0 || val > BitcoinUnits::maxMoney()) + valid = false; + } + if(valid_out) + *valid_out = valid; + return valid ? val : 0; } - double valueFromText(const QString& text) const + +protected: + bool event(QEvent *event) { - return QDoubleSpinBox::valueFromText(BitcoinUnits::removeSpaces(text)); + if (event->type() == QEvent::KeyPress || event->type() == QEvent::KeyRelease) + { + QKeyEvent *keyEvent = static_cast(event); + if (keyEvent->key() == Qt::Key_Comma) + { + // Translate a comma into a period + QKeyEvent periodKeyEvent(event->type(), Qt::Key_Period, keyEvent->modifiers(), ".", keyEvent->isAutoRepeat(), keyEvent->count()); + return QAbstractSpinBox::event(&periodKeyEvent); + } + } + return QAbstractSpinBox::event(event); } + +signals: + void valueChanged(); }; +#include "bitcoinamountfield.moc" + BitcoinAmountField::BitcoinAmountField(QWidget *parent) : QWidget(parent), - amount(0), - currentUnit(-1) + amount(0) { - nSingleStep = 100000; // satoshis - amount = new AmountSpinBox(this); amount->setLocale(QLocale::c()); amount->installEventFilter(this); @@ -85,21 +207,13 @@ BitcoinAmountField::BitcoinAmountField(QWidget *parent) : setFocusProxy(amount); // If one if the widgets changes, the combined content changes as well - connect(amount, SIGNAL(valueChanged(QString)), this, SIGNAL(textChanged())); + connect(amount, SIGNAL(valueChanged()), this, SIGNAL(valueChanged())); connect(unit, SIGNAL(currentIndexChanged(int)), this, SLOT(unitChanged(int))); // Set default based on configuration unitChanged(unit->currentIndex()); } -void BitcoinAmountField::setText(const QString &text) -{ - if (text.isEmpty()) - amount->clear(); - else - amount->setValue(BitcoinUnits::removeSpaces(text).toDouble()); -} - void BitcoinAmountField::clear() { amount->clear(); @@ -108,16 +222,9 @@ void BitcoinAmountField::clear() bool BitcoinAmountField::validate() { - bool valid = true; - if (amount->value() == 0.0) - valid = false; - else if (!BitcoinUnits::parse(currentUnit, text(), 0)) - valid = false; - else if (amount->value() > BitcoinUnits::maxAmount(currentUnit)) - valid = false; - + bool valid = false; + value(&valid); setValid(valid); - return valid; } @@ -129,14 +236,6 @@ void BitcoinAmountField::setValid(bool valid) amount->setStyleSheet(STYLE_INVALID); } -QString BitcoinAmountField::text() const -{ - if (amount->text().isEmpty()) - return QString(); - else - return amount->text(); -} - bool BitcoinAmountField::eventFilter(QObject *object, QEvent *event) { if (event->type() == QEvent::FocusIn) @@ -144,17 +243,6 @@ bool BitcoinAmountField::eventFilter(QObject *object, QEvent *event) // Clear invalid flag on focus setValid(true); } - else if (event->type() == QEvent::KeyPress || event->type() == QEvent::KeyRelease) - { - QKeyEvent *keyEvent = static_cast(event); - if (keyEvent->key() == Qt::Key_Comma) - { - // Translate a comma into a period - QKeyEvent periodKeyEvent(event->type(), Qt::Key_Period, keyEvent->modifiers(), ".", keyEvent->isAutoRepeat(), keyEvent->count()); - QApplication::sendEvent(object, &periodKeyEvent); - return true; - } - } return QWidget::eventFilter(object, event); } @@ -167,18 +255,12 @@ QWidget *BitcoinAmountField::setupTabChain(QWidget *prev) qint64 BitcoinAmountField::value(bool *valid_out) const { - qint64 val_out = 0; - bool valid = BitcoinUnits::parse(currentUnit, text(), &val_out); - if (valid_out) - { - *valid_out = valid; - } - return val_out; + return amount->value(valid_out); } void BitcoinAmountField::setValue(qint64 value) { - setText(BitcoinUnits::format(currentUnit, value)); + amount->setValue(value); } void BitcoinAmountField::setReadOnly(bool fReadOnly) @@ -195,28 +277,7 @@ void BitcoinAmountField::unitChanged(int idx) // Determine new unit ID int newUnit = unit->itemData(idx, BitcoinUnits::UnitRole).toInt(); - // Parse current value and convert to new unit - bool valid = false; - qint64 currentValue = value(&valid); - - currentUnit = newUnit; - - // Set max length after retrieving the value, to prevent truncation - amount->setDecimals(BitcoinUnits::decimals(currentUnit)); - amount->setMaximum(qPow(10, BitcoinUnits::amountDigits(currentUnit)) - qPow(10, -amount->decimals())); - amount->setSingleStep((double)nSingleStep / (double)BitcoinUnits::factor(currentUnit)); - - if (valid) - { - // If value was valid, re-place it in the widget with the new unit - setValue(currentValue); - } - else - { - // If current value is invalid, just clear field - setText(""); - } - setValid(true); + amount->setDisplayUnit(newUnit); } void BitcoinAmountField::setDisplayUnit(int newUnit) @@ -226,6 +287,5 @@ void BitcoinAmountField::setDisplayUnit(int newUnit) void BitcoinAmountField::setSingleStep(qint64 step) { - nSingleStep = step; - unitChanged(unit->currentIndex()); + amount->setSingleStep(step); } diff --git a/src/qt/bitcoinamountfield.h b/src/qt/bitcoinamountfield.h index 521a9ed56..c713f5d68 100644 --- a/src/qt/bitcoinamountfield.h +++ b/src/qt/bitcoinamountfield.h @@ -8,17 +8,18 @@ #include QT_BEGIN_NAMESPACE -class QDoubleSpinBox; class QValueComboBox; QT_END_NAMESPACE +class AmountSpinBox; + /** Widget for entering bitcoin amounts. */ class BitcoinAmountField: public QWidget { Q_OBJECT - Q_PROPERTY(qint64 value READ value WRITE setValue NOTIFY textChanged USER true) + Q_PROPERTY(qint64 value READ value WRITE setValue NOTIFY valueChanged USER true) public: explicit BitcoinAmountField(QWidget *parent = 0); @@ -49,20 +50,15 @@ public: QWidget *setupTabChain(QWidget *prev); signals: - void textChanged(); + void valueChanged(); protected: /** Intercept focus-in event and ',' key presses */ bool eventFilter(QObject *object, QEvent *event); private: - QDoubleSpinBox *amount; + AmountSpinBox *amount; QValueComboBox *unit; - int currentUnit; - qint64 nSingleStep; - - void setText(const QString &text); - QString text() const; private slots: void unitChanged(int idx); diff --git a/src/qt/bitcoinunits.cpp b/src/qt/bitcoinunits.cpp index 21aed235c..0435ebc5d 100644 --- a/src/qt/bitcoinunits.cpp +++ b/src/qt/bitcoinunits.cpp @@ -4,6 +4,8 @@ #include "bitcoinunits.h" +#include "core.h" + #include BitcoinUnits::BitcoinUnits(QObject *parent): @@ -250,3 +252,8 @@ QVariant BitcoinUnits::data(const QModelIndex &index, int role) const } return QVariant(); } + +qint64 BitcoinUnits::maxMoney() +{ + return MAX_MONEY; +} diff --git a/src/qt/bitcoinunits.h b/src/qt/bitcoinunits.h index 7fa24c854..944b4ec53 100644 --- a/src/qt/bitcoinunits.h +++ b/src/qt/bitcoinunits.h @@ -120,6 +120,9 @@ public: return text; } + //! Return maximum number of base units (Satoshis) + static qint64 maxMoney(); + private: QList unitlist; }; diff --git a/src/qt/sendcoinsentry.cpp b/src/qt/sendcoinsentry.cpp index e0f56f8cd..3c0b8881f 100644 --- a/src/qt/sendcoinsentry.cpp +++ b/src/qt/sendcoinsentry.cpp @@ -72,7 +72,7 @@ void SendCoinsEntry::setModel(WalletModel *model) if (model && model->getOptionsModel()) connect(model->getOptionsModel(), SIGNAL(displayUnitChanged(int)), this, SLOT(updateDisplayUnit())); - connect(ui->payAmount, SIGNAL(textChanged()), this, SIGNAL(payAmountChanged())); + connect(ui->payAmount, SIGNAL(valueChanged()), this, SIGNAL(payAmountChanged())); connect(ui->deleteButton, SIGNAL(clicked()), this, SLOT(deleteClicked())); connect(ui->deleteButton_is, SIGNAL(clicked()), this, SLOT(deleteClicked())); connect(ui->deleteButton_s, SIGNAL(clicked()), this, SLOT(deleteClicked())); @@ -130,6 +130,13 @@ bool SendCoinsEntry::validate() retval = false; } + // Sending a zero amount is invalid + if (ui->payAmount->value(0) <= 0) + { + ui->payAmount->setValid(false); + retval = false; + } + // Reject dust outputs: if (retval && GUIUtil::isDust(ui->payTo->text(), ui->payAmount->value())) { ui->payAmount->setValid(false); From 2a05101efd41f4e86a0323f5e00dd040574bc170 Mon Sep 17 00:00:00 2001 From: "Wladimir J. van der Laan" Date: Fri, 18 Jul 2014 16:36:29 +0200 Subject: [PATCH 2/3] qt: Remove unused functions from BitcoinUnits Remove two functions that are now unused. --- src/qt/bitcoinunits.cpp | 22 ---------------------- src/qt/bitcoinunits.h | 4 ---- 2 files changed, 26 deletions(-) diff --git a/src/qt/bitcoinunits.cpp b/src/qt/bitcoinunits.cpp index 0435ebc5d..6f506d3f2 100644 --- a/src/qt/bitcoinunits.cpp +++ b/src/qt/bitcoinunits.cpp @@ -80,28 +80,6 @@ qint64 BitcoinUnits::factor(int unit) } } -qint64 BitcoinUnits::maxAmount(int unit) -{ - switch(unit) - { - case BTC: return Q_INT64_C(21000000); - case mBTC: return Q_INT64_C(21000000000); - case uBTC: return Q_INT64_C(21000000000000); - default: return 0; - } -} - -int BitcoinUnits::amountDigits(int unit) -{ - switch(unit) - { - case BTC: return 8; // 21,000,000 (# digits, without commas) - case mBTC: return 11; // 21,000,000,000 - case uBTC: return 14; // 21,000,000,000,000 - default: return 0; - } -} - int BitcoinUnits::decimals(int unit) { switch(unit) diff --git a/src/qt/bitcoinunits.h b/src/qt/bitcoinunits.h index 944b4ec53..be9dca601 100644 --- a/src/qt/bitcoinunits.h +++ b/src/qt/bitcoinunits.h @@ -82,10 +82,6 @@ public: static QString description(int unit); //! Number of Satoshis (1e-8) per unit static qint64 factor(int unit); - //! Max amount per unit - static qint64 maxAmount(int unit); - //! Number of amount digits (to represent max number of coins) - static int amountDigits(int unit); //! Number of decimals left static int decimals(int unit); //! Format as string From 29eaa316944477af538b75b439a49ee1a9fc2f2a Mon Sep 17 00:00:00 2001 From: "Wladimir J. van der Laan" Date: Tue, 22 Jul 2014 09:30:04 +0200 Subject: [PATCH 3/3] ui: Make sure sendcoinsentry signals only connected once Move signal connections to constructor where possible. --- src/qt/sendcoinsentry.cpp | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/qt/sendcoinsentry.cpp b/src/qt/sendcoinsentry.cpp index 3c0b8881f..52545c385 100644 --- a/src/qt/sendcoinsentry.cpp +++ b/src/qt/sendcoinsentry.cpp @@ -34,6 +34,12 @@ SendCoinsEntry::SendCoinsEntry(QWidget *parent) : GUIUtil::setupAddressWidget(ui->payTo, this); // just a label for displaying bitcoin address(es) ui->payTo_is->setFont(GUIUtil::bitcoinAddressFont()); + + // Connect signals + connect(ui->payAmount, SIGNAL(valueChanged()), this, SIGNAL(payAmountChanged())); + connect(ui->deleteButton, SIGNAL(clicked()), this, SLOT(deleteClicked())); + connect(ui->deleteButton_is, SIGNAL(clicked()), this, SLOT(deleteClicked())); + connect(ui->deleteButton_s, SIGNAL(clicked()), this, SLOT(deleteClicked())); } SendCoinsEntry::~SendCoinsEntry() @@ -72,11 +78,6 @@ void SendCoinsEntry::setModel(WalletModel *model) if (model && model->getOptionsModel()) connect(model->getOptionsModel(), SIGNAL(displayUnitChanged(int)), this, SLOT(updateDisplayUnit())); - connect(ui->payAmount, SIGNAL(valueChanged()), this, SIGNAL(payAmountChanged())); - connect(ui->deleteButton, SIGNAL(clicked()), this, SLOT(deleteClicked())); - connect(ui->deleteButton_is, SIGNAL(clicked()), this, SLOT(deleteClicked())); - connect(ui->deleteButton_s, SIGNAL(clicked()), this, SLOT(deleteClicked())); - clear(); }