-
Notifications
You must be signed in to change notification settings - Fork 327
Fix bitcoin-qt visual glitches on Wayland #904
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
Fix bitcoin-qt visual glitches on Wayland #904
Conversation
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ReviewsSee the guideline for information on the review process.
If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update. |
| w->setWindowFlags(flags|Qt::WindowStaysOnTopHint); | ||
| w->show(); | ||
| w->setWindowFlags(flags); | ||
| w->show(); |
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.
Is it necessary now to call w->show(); here?
Would it be more clear to simply revert 15aa7d0?
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.
@hebasto Yeah, I think a revert would also do the trick.
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.
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.
0fcab7d to
5535da8
Compare
5535da8 to
76e4b71
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.
Concept ACK
If you can, please check how this behaves on #817.
6542d5f to
949e953
Compare
|
@pablomartin4btc can you please re-review? |
6f71ab2 to
50f4b19
Compare
The main window (BitcoinGUI) does not need to be passed to bringToFront(), doing so sets Qt::WindowStaysOnTopHint on the main window and causes it to flicker. Fixes #903.
50f4b19 to
095f920
Compare
pablomartin4btc
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 095f920 and it fixes the problem.
I still need to check if this is the right approach, need to check against xcb and macOS. Also there are other places where bringToFront is being used, the rpcconsole has the same issue as the main window in wayland, so perhaps the fix has to be within bringToFront, need more time to test. Thanks for finding the issue and taking a look at this!
Great! Thanks for confirming that.
Sure.
I am not able to reproduce the flickering with the Can you please test if removing those help with the
You're welcome, thank you for your work as well. |
hebasto
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 095f920, it breaks UX.
For example, on Fedora 43 with Gnome 49 and Wayland, follow these steps:
- Run
bitcoin-qt. - Hide the main window using the "Hide" command in context menu.
- Click on "Receive" in the system tray icon menu.
On the master branch, the main window reappears.
With this PR, however, the main window remains hidden.
|
From #904 (comment):
Done in #914. |
|
Closing in favor of #904. |
Ah, I haven't tested this on GNOME. I suspect the use of |
…for Wayland" 0672e72 Revert "gui, qt: brintToFront workaround for Wayland" (Hennadii Stepanov) Pull request description: This PR reverts a Wayland-specific workaround introduced in bitcoin-core/gui#831, which appears to break the UI, as reported in: - #33432 - bitcoin-core/gui#903 An alternative to bitcoin-core/gui#904, as suggested in bitcoin-core/gui#904 (comment). Fixes bitcoin-core/gui#903 ACKs for top commit: maflcko: review ACK 0672e72 🎩 Tree-SHA512: 3c2fba4a601de82b8c73553d54e93d133f9f474ee1f55a77320c0fc198735b68559859f7efeb125aa5282b8334bfa09f3927d6d7c984d2f87f54fa1ca45ee60e
The main window (BitcoinGUI) does not need to be passed to bringToFront(), doing so sets Qt::WindowStaysOnTopHint on the main window and causes it to flicker.
Fixes #903.