-
Notifications
You must be signed in to change notification settings - Fork 141
feat(qt): Port win_taskbar_progress to Qt6 #215
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: 29.x-knots
Are you sure you want to change the base?
feat(qt): Port win_taskbar_progress to Qt6 #215
Conversation
|
I think this would be easier to review if the Qt6 depends changes were split to a separate PR, and the fixups squashed. |
3c0a6ca to
9757d00
Compare
|
I've split this PR as requested:
|
|
My thought was more of having this PR just port win_taskbar_progress to native Windows, and have the Qt6 PR independently change depends builds, neither depending on each other. |
9757d00 to
8986737
Compare
|
Gotcha @luke-jr I've updated both PRs to reflect. |
bdc4742 to
52502ef
Compare
52502ef to
0dc86b2
Compare
0dc86b2 to
b895717
Compare
b895717 to
c527b35
Compare
c527b35 to
c8fe155
Compare
luke-jr
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure if windowHandle() can return null, but it should only ever return itself anyway, so might as well just pass this?
d132c1b to
6f9bf62
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Testing this a little today but ran into some unexpected difficulties.
- built as is, the progress bar never shows/initiates for me. I ended up moving the m_taskbar_progress->setWindow out of the constructor (line 235) back down closer to its original position inside of setNumBlocks. Something like:
Line 1270
#ifdef BITCOIN_QT_WIN_TASKBAR
if (!m_taskbar_progress->window()) {
m_taskbar_progress->setWindow(this);
}
m_taskbar_progress->setValue(qRound(nVerificationProgress * 100.0));
m_taskbar_progress->setVisible(true);
#endif
This fixed the issue but with a caveat (see #2)
- with it now working, the progress bar will still take 1 to 3 minutes to actually show up. (variable)
Not sure what the issue is - I'm wondering if having setWindow in the constructor is problematic (windowhandle still null?)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since this PR replaces Qt5 WinExtras, do you want to remove the old dependencies?
- CMakeLists.txt, line 242 - remove
list(APPEND qt_components WinExtras) - depends/packages/qt.mk - has references to qtwinextras
6f9bf62 to
527b8f9
Compare
527b8f9 to
972972c
Compare
|
@bigshiny90 should be resolved in 972972c |
Running the new code. New Taskbar progress is now loading. But, i'm still having the issue where the actual taskbar progress takes over a minute (1.5 minutes in my last test) before it switches from normal taskbar style to actually showing progress. Screenshots showing INITIAL STATE / POST 1.5 minutes:
Running normal QtWinExtras taskbar progress will show progress indicator immediately. |
972972c to
eee7c82
Compare
|
@bigshiny90 thank you for testing, I pushed up some fixes in eee7c82, let me know if that helped. |
So i don't think these changes are addressing the real issue. i did some quick research and came up with the idea that it is missing the proper QT NativeEventFilter. Made some changes and tested and everything now works correctly - instant taskbar progress. (Note - not sure it matters - but i'm building using the windows msvc method and QT5. Not sure if the same issue exists with QT6). (I hope you don't mind me hacking away at this!) My guess: Windows needs us to wait for a The fix? I followed the same pattern already used in Here are the changes that worked for me, all pretty boilerplate stuff: wintaskbarprogress.h#include <QAbstractNativeEventFilter>
#include <QObject>
#include <windows.h>
// ...
class WinTaskbarProgress : public QObject, public QAbstractNativeEventFilter
{
Q_OBJECT
public:
// ... existing methods ...
/** Implements QAbstractNativeEventFilter interface for processing Windows messages */
bool nativeEventFilter(const QByteArray &eventType, void *pMessage, long *pnResult) override;
private:
// ... existing members ...
UINT m_taskbar_button_created_msg = 0;
bool m_taskbar_ready = false;
};wintaskbarprogress.cppConstructor - register the message: WinTaskbarProgress::WinTaskbarProgress(QObject *parent)
: QObject(parent)
{
m_taskbar_button_created_msg = RegisterWindowMessage(L"TaskbarButtonCreated");
}Add native event filter: bool WinTaskbarProgress::nativeEventFilter(const QByteArray &eventType, void *pMessage, long *pnResult)
{
Q_UNUSED(eventType);
Q_UNUSED(pnResult);
MSG *msg = static_cast<MSG *>(pMessage);
if (msg->message == m_taskbar_button_created_msg && m_window) {
// Taskbar button is now ready - safe to initialize ITaskbarList3
m_taskbar_ready = true;
initTaskbarButton();
updateProgress();
}
return false; // Allow normal processing
}Update void WinTaskbarProgress::initTaskbarButton()
{
if (m_taskbar_button || !m_window || !m_taskbar_ready) {
return;
}
// ... rest unchanged ...
}bitcoingui.cppConstructor - install filter: #ifdef BITCOIN_QT_WIN_TASKBAR
m_taskbar_progress = new WinTaskbarProgress(this);
QApplication::instance()->installNativeEventFilter(m_taskbar_progress);
#endifDestructor - remove filter: #ifdef BITCOIN_QT_WIN_TASKBAR
QApplication::instance()->removeNativeEventFilter(m_taskbar_progress);
#endifTested and confirmed working - progress shows up immediately now |
eee7c82 to
23b4023
Compare
|
@bigshiny90 thank you, that's helpful. Reflected in 23b4023 |
|
Working great. Everything has been addressed as far as what i can see. I will run it through the day during IBD just to confirm end-to-end that everything works. I will let you know if any surprises... |
bigshiny90
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested and working. Ran thru a full IBD to watch progress bar and visibility.
Big ACK from me as this is necessary for moving forward with QT6


Replace QtWinExtras (removed in Qt6) with native Windows ITaskbarList3 COM API, making the Windows taskbar progress feature compatible with both Qt5 and Qt6.
Closes #191
Changes
Implementation
The new
WinTaskbarProgressclass uses WindowsITaskbarList3directly, maintaining full feature parity with the Qt5 implementation while working with both Qt5 and Qt6.