refactor: Post Qt 6 cleanup #863

hebasto commented at 7:54 PM on April 2, 2025: member

This PR:

Removes outdated Qt version-specific code.
Inlines two functions from GUIUtil.

qt, refactor: Remove outdated Qt version-specific code

Since bitcoin/bitcoin#30997, the minimum supported Qt version is 6.2.

4b36ab3a6a

qt, refactor: Inline `GUIUtil::GetImage` function d1ec6db249

qt, refactor: Inline `GUIUtil::SplitSkipEmptyParts` function 3aa58bea8e

hebasto added the label Refactoring on Apr 2, 2025

DrahtBot commented at 7:54 PM on April 2, 2025: contributor

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Reviews

See the guideline for information on the review process.

Type	Reviewers
ACK	davidgumberg

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

in src/qt/qrimagewidget.cpp:99 in d1ec6db249 outdated

  91 | @@ -92,7 +92,11 @@ bool QRImageWidget::setQR(const QString& data, const QString& text)
  92 |  
  93 |  QImage QRImageWidget::exportImage()
  94 |  {
  95 | -    return GUIUtil::GetImage(this);
  96 | +    if (!GUIUtil::HasPixmap(this)) {
  97 | +        return QImage();
  98 | +    }
  99 | +
 100 | +    return this->pixmap(Qt::ReturnByValue).toImage();

davidgumberg commented at 8:50 PM on April 2, 2025:

Micro-Nit:

As far as I understand this is deprecated in favor of QLabel::pixmap() at least as early as QT 6.2 (https://doc.qt.io/archives/qt-6.2/qlabel-obsolete.html#pixmap) and was marked obsolete in 6.0 (https://doc.qt.io/archives/qt-6.0/qlabel-obsolete.html#pixmap)

Previously, it was recommended to use QLabel::pixmap(Qt::ReturnByValue) to maintain compatibility with old code, since previously QLabel::pixmap() returned a pixmap by pointer. (https://doc.qt.io/qt-5/qlabel.html#pixmap-prop)

    return this->pixmap().toImage();

davidgumberg commented at 9:34 PM on April 2, 2025: contributor

crACK https://github.com/bitcoin-core/gui/pull/863/commits/3aa58bea8e703e33a1eb8e2f9fdf887aca8d03c1

Nice cleanup, doesn't change any behavior, mostly drops workaround code for versions of QT < 6.2.

Left an insignificant nit about use of deprecated QLabel::pixmap(Qt::ReturnByValue()), but this can be addressed in a follow up (or here, if the author prefers) with the other place it appears.

(https://github.com/davidgumberg/gui/commit/ca5390f9b0af348e8f4ea0cf2149716c0b77bd8f)

</summary>

diff --git a/src/qt/guiutil.cpp b/src/qt/guiutil.cpp
@@ -959,7 +959,7 @@ QDateTime StartOfDay(const QDate& date)
 bool HasPixmap(const QLabel* label)
 {
-    return !label->pixmap(Qt::ReturnByValue).isNull();
+    return !label->pixmap().isNull();
 }

diff --git a/src/qt/qrimagewidget.cpp b/src/qt/qrimagewidget.cpp
@@ -96,7 +96,7 @@ QImage QRImageWidget::exportImage()
         return QImage();
     }
-    return this->pixmap(Qt::ReturnByValue).toImage();
+    return this->pixmap().toImage();
 }

hebasto commented at 7:43 AM on April 3, 2025: member

@davidgumberg

crACK 3aa58be

Nice cleanup, doesn't change any behavior, mostly drops workaround code for versions of QT < 6.2.

Thank you for your review!

Left an insignificant nit about use of deprecated QLabel::pixmap(Qt::ReturnByValue()), but this can be addressed in a follow up (or here, if the author prefers) with the other place it appears.

(davidgumberg@ca5390f)

diff --git a/src/qt/guiutil.cpp b/src/qt/guiutil.cpp
@@ -959,7 +959,7 @@ QDateTime StartOfDay(const QDate& date)
 bool HasPixmap(const QLabel* label)
 {
-    return !label->pixmap(Qt::ReturnByValue).isNull();
+    return !label->pixmap().isNull();
 }

diff --git a/src/qt/qrimagewidget.cpp b/src/qt/qrimagewidget.cpp
@@ -96,7 +96,7 @@ QImage QRImageWidget::exportImage()
         return QImage();
     }
-    return this->pixmap(Qt::ReturnByValue).toImage();
+    return this->pixmap().toImage();
 }

I agree with your comment and suggested changes. I also agree that this can be addressed in a follow-up, along with other closely related changes, such as inlining the QRImageWidget::exportImage() function.

hebasto merged this on Apr 3, 2025

hebasto closed this on Apr 3, 2025

hebasto deleted the branch on Apr 3, 2025

sedited referenced this in commit a9c46ce3c3 on Apr 24, 2025

stickies-v referenced this in commit 772a33e052 on May 23, 2025

yuvicc referenced this in commit 069643f094 on Jul 6, 2025

bug-castercv502 referenced this in commit 5b0af4b944 on Sep 28, 2025

bitcoin-core locked this on Apr 3, 2026