-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
[detach-window] Use eventFilter to process closeEvent #8096
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
Merged
messmerd
merged 70 commits into
LMMS:feature/detach-window
from
sqrvrt:detachwindow/event-filter
Nov 21, 2025
Merged
Changes from all commits
Commits
Show all changes
70 commits
Select commit
Hold shift + click to select a range
0e7048e
Use eventFilter to process closeEvent
sqrvrt 76da997
revert MixerView height cap
sqrvrt 6fb9f9a
Remove obsolete part of ITW::closeEvent
sqrvrt 0fe643f
refactor SampleTrackWindow, sync detached icon
sqrvrt c1e9c1e
Set window icon directly on InstrumenTrackView
sqrvrt b128009
Preserve detached state in ITW prev/next buttons
sqrvrt 806d964
remove leftover comment
sqrvrt 7656b1c
Account for size of decorations
sqrvrt a7f6c60
Fix ITW movement again (lazy version)
sqrvrt e3c854b
Move the close hook into SubWindow eventFilter
sqrvrt fac6160
Keep track of detached window while it's hidden
sqrvrt bad5f04
Code cleanup
sqrvrt 6e45b96
re-fix ITW because apparently it still breaks
sqrvrt 34681b2
fix codestyle
sqrvrt 5529ed3
make another `if` inline
sqrvrt 50f70ac
more codestyle fixes
sqrvrt e62b039
Expand SubWindow::setWidget() docs
sqrvrt 608fe14
Constrain attached window positions
sqrvrt ddcadc9
Hide windows when attaching on closeEvent
sqrvrt fedfef8
Clarify wayland move workaround
sqrvrt c21f641
Remove redundant includes
sqrvrt c8cfe67
make detached closeEvent transparent
sqrvrt 21beffd
fix editor windows minimizing their height on close
sqrvrt 9b596b0
Enable minimize button on detached windows
sqrvrt 5b1b493
Change detach button SVG
sqrvrt deeb802
rename detach icon
sqrvrt 8968652
clean up detach.svg
sqrvrt f6e2cdd
Disable broken detach for MacOS builds
sqrvrt 36cbf35
fix typo
sqrvrt d6129cc
revert SimpleTextFloat to master branch
sqrvrt 08fbfed
clean up diff against master
sqrvrt 58ab680
Fix effect control views bing killed on close
sqrvrt 6e35157
Use, init and update childGeomm only when detached
sqrvrt 50d527c
Use std::clamp
sqrvrt c60029e
Make hide-on-attach-on-close configurable
sqrvrt 1d0634c
Fix window size resetting for real this time
sqrvrt ca14048
Restore EffectView functionality actually
sqrvrt 696e2e3
Fix SubWindow label overlapping detach button
sqrvrt 12e1509
Add SubWindow::eventFilter docs
sqrvrt 69e4ed0
Get rid of childGeom
sqrvrt a9654fc
Keep attached widget position
sqrvrt d144abb
Initial attempt at always-detach
sqrvrt 34888a7
close autodetached windows properly
sqrvrt 93f4dc6
Improve window positioning on attach
sqrvrt c1ed85d
Fix editor windows having minimal height on first show
sqrvrt 49b23d5
Remove redundant subwindow-related code from MainWindow
sqrvrt 53c23c2
fix typo in setup menu
sqrvrt 0255db6
Fix ITW icon always showing up as piano
sqrvrt 80a9ffd
Update ControllerView
sqrvrt 485c8c1
minor formatting fixes
sqrvrt 4a8d6f9
even more minor formatting fixes
sqrvrt ff29cf5
Don't hard-set size of EffectView
sqrvrt 666094a
Change label text
sqrvrt f0282aa
Destroy childless subwindows
sqrvrt 6acbae9
Get rid of isResizable in EffectView
sqrvrt f0d57d1
Fix EffectControlDialog not hiding the title bar
sqrvrt 50a73e4
Remove redundant widget->show() on showEvent
sqrvrt e50247e
fix drag&drop not updating ITW SubWindow
sqrvrt 98e6078
Disable FixedSizeDialogHint on ITW
sqrvrt 1a7f6be
Disable SubWindow flag when detaching a window
sqrvrt 49817e4
Try disabling MacOS soft-fail
sqrvrt 0ca0474
Enable Undo/Redo globally
sqrvrt 84e83e0
Save detached window visibility properly; refactor
sqrvrt 944328f
remove obsolete qt version workarounds
sqrvrt 029a0e4
remove setFixedSize from LadspaControlDialog
sqrvrt 40d3420
Fix resizing LadspaMatrixControlDialog
sqrvrt 6aa4b13
Allow detached windows in topLevelITW; refactor
sqrvrt 17157a0
allow detach to be disabled
sqrvrt fcb8fdb
disable detach on embedded VST subwindows
sqrvrt d74cd8d
add attach/detach everything actions
sqrvrt File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
This file was deleted.
Oops, something went wrong.
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
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
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
This file was deleted.
Oops, something went wrong.
This file was deleted.
Oops, something went wrong.
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
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -73,7 +73,6 @@ public slots: | |
| void moveUp(); | ||
| void moveDown(); | ||
| void deletePlugin(); | ||
| void closeEffects(); | ||
|
|
||
|
|
||
| signals: | ||
|
|
||
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
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
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
Oops, something went wrong.
Oops, something went wrong.
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.
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.
I don't like that this has been removed. It feels like a step backwards since now plugin devs need to remember to call
setSizeConstraint.I don't really understand the problem that led to you removing
isResizable, but is there any other solution?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.
The problem is that like in other cases, the way this was done without the patch was applying size constraints to the SubWindow. Because detached widgets are no longer contained within a subwindow, they can be resized freely.
The reason
isResizable()can't be applied to the child widget is because the child can basically be whatever it wants as long as that is a widget, including but not limited to:setFixedSize(), even before this commitsetFixedSize()with a different size compared to what layout wants (StereoMatrix)As a result of the above, removing isResizable is mostly just a standardization of sorts - of 23 native effects (not counting LV2 and VST since I can't test them), only 7 were dependent on
isResizable()! All others were either resizable by themselves or fixed their size explicitly already.I'd argue for the reverse, most LMMS-native plugins nowadays aren't just solely knobs and have something doing GUI stuff already. And being resizable unless told otherwise is the default behavior of Qt, so I don't see any reason to stray from it.
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.
As an alternative, we could just lie and set
isResizable(true)on StereoMatrix since it's the only widget I've seen to break but that's even worse IMO.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.
Pinging @rubiefawn since she's using
isResizable()over at #7805 (6cf9d41), and an alternative would be needed if we go through with removing that method as discussed here.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.
@rubiefawn actually could you test if the issues #7805 fixes are even present here? As far as I'm concerned, the issue of replacing instruments keeping their size looks to be gone (possibly associated with layout changes to
SubWindow), and I wasn't able to reproduce the other issue.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.
The testing above was performed on the msvc build, but I installed the mingw build and encountered the same behavior there.
While I don't have a msvc or mingw dev environment set up (I do all that on the Linux partition or WSL if I'm desperate) I do still have the installer for the msvc build of 50a73e4 from my initial tests. Reinstalling and testing that shows that actually, ITWs were the only ones that had the controls to begin with.
Compare this to the mingw build of 98e6078:

Other things I noticed:
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.
What in the actual... alright, cool. I'm kinda wondering if rebasing this to qt6 would fix this since it's extremely wrong.
...Maybe if I enable
FixedSizeDialogHintit starts working?.. I don't know anymore...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.
So, a couple of observations I have after testing myself.
Qt::SubWindowisn't mutually exclusive withQt::Window, that could be the source of some problems.MSWindowsFixedSizeDialogHintacts as it should, and takes priority over whatever causes no flags to work. If disablingSubWindowflag doesn't work, perhapsCustomizeWindowHintwill be the one to go on windows specifically.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.
That did it!!!
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.
I wonder if that also fixes the MacOS bug