Fullscreen toggle rework#11313
Closed
ronso0 wants to merge 14 commits into
Closed
Conversation
ronso0
force-pushed
the
fullscreen-rework
branch
from
bc163cf to
5114479
Compare
ronso0
force-pushed
the
fullscreen-rework
branch
3 times, most recently
from
31239c8 to
1a4d335
Compare
This was referenced Mar 2, 2023
Merged
ronso0
force-pushed
the
fullscreen-rework
branch
3 times, most recently
from
4cc5a19 to
70dbd1a
Compare
ronso0
force-pushed
the
fullscreen-rework
branch
2 times, most recently
from
aedea6f to
689c085
Compare
daschuer
reviewed
Apr 7, 2023
Member
daschuer
left a comment
daschuer
left a comment
There was a problem hiding this comment.
I have just tested this a bit, with some results:
- "Enter" is still not equal pressing Ok
- The Menu bar disappears when going to fullscreen, which is OK, but it does not com back on window mode, which was my expectation. Maybe we should store the menu bar visible state for both independently. Default: Windowed = menu bar visible; Full-screen = menu bar hidden.
| QString modseq; | ||
| QKeySequence k; | ||
|
|
||
| if (e->key() >= 0x01000020 && e->key() <= 0x01000023) { |
Member
There was a problem hiding this comment.
Suggested change
| if (e->key() >= 0x01000020 && e->key() <= 0x01000023) { | |
| if (e->key() >= Qt::Key_Shift && e->key() <= Qt::Key_Alt) { |
And how about Qt::Key_AltGr?
|
|
||
| pLabelLayout->addStretch(); | ||
|
|
||
| m_pOkayBtn = new QPushButton(tr("&Okay"), this); |
Member
There was a problem hiding this comment.
Suggested change
| m_pOkayBtn = new QPushButton(tr("&Okay"), this); | |
| m_pOkayBtn = new QPushButton(tr("&Ok"), this); |
Member
Author
|
@daschuer Thanks for testing! I'll respond to your comment in #11304 |
Member
|
Ah, I was wondering why my pending review comments where lost. There where not lost. just picked the wrong branch. |
This modal popup is displayed every time when going fullscreen as long as the "Remind me again" checkbox is ticked when Okay is clicked. It can be styled to match the skin.
Now, slotViewFullScreen() just toggles fullscreen and eventFilter() catches ALL fullscreen changes, no matter the origin. Previously, slotViewFullScreen() was also reponsible for recreating and reconnecting the menu bar and for refreshing the Fullscreen checkbox. This is okay if only Mixxx could change the fullscreen state. However, it can happen that the (Linux) window manager consumes the fullscreen hotkey, makes Mixxx go fullscreen and slotViewFullScreen() is never called.
No-op QWindowStateChangeEvent can occur if another window (preferences, About, ...) is shown above MixxxMainWindow. The event's oldState() is QWindowNoState even though the window is and was fullscreen. Ignore these event to not recreate and reconnect the menu bar unnecessarily. Also, that avoids flickering, at least on desktops that feature a global menu (when going fullscreen the menu is moved to the main window and hidden immediately = rapid resizing of window content)
Member
Author
|
Closing in favor of #11566 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR makes
MixxxMainWindow::slotViewFullScreen()just toggle fullscreen.MixxxMainWindow::eventFilter()catches ALL fullscreen changes (QWindowStateChangeEvent), no matter the origin, and also emits a signal to recreate & connect the menu bar (only on Linux desktops that use a global menu) and hide it when going fullscreen (Linux & Windows).This works perfectly fine for me, and I'm curious how it performs on Windows and macOS. Please test!
Motivation
While working on #11304 I noticed that my window manager (xfwm4 on Ubuntu Studio) consumes the fullscreen hotkey F11 and
MixxxMainWindow::slotViewFullScreen()never gets called, except when I click the menu checkbox.This happens because the fullscreen hotkey in Mixxx and in my window manager are identical.
(so most of my fullscreen testing was kinda worthless... : \ )
TODO
Testing
View>Fullscreenas well as all hotkeys printed next to it should toggle fullscreen without any unusual flickering--for--full-screen, regardless the preferences option "Start in fullscreen"This is based on #11304, so you'll also see full keypress/release logging.
Since this is actually a bugfix/workaround I can imagine to rebase onto 2.3 but I need CI builds to test in a Ubuntu 22.04 VM.