Closed firewall1110 closed 2 weeks ago
Checked after "Remove term "blacklist" (#7365)" (2024-07-09 00:02:49) : the same bug.
P.S. Session really not ended - just close and reopen song editor window (Ctrl+1, Ctrl+1). Interesting thing: if song editor closed using Ctrl+1, then new project will be loaded without this bug.
Interestingly the whole window acts strange:
@michaelgregorius : Thank You for bug confirmation!
Man plan is (Edited 2024.08.13) : somehow compare what happens when Song Editor :
Than change code to make "closed by mouse" and "closed by Ctrl + W" the same as "closed by Ctrl + 1".
But how I see: member and maintainer start work - and Your will make this job better and faster ...
@firewall1110, I am not planning to work on this so please go ahead if you like.
Did you find that the three ways of closing the Song-Editor behave differently?
Quote: "Did you find that the three ways of closing the Song-Editor behave differently?"
Yes - if "closed by Ctrl + 1", than new project loaded without bug. (I have wrote this in previous comment).
P.S. OK - I will continue ...
Quote: "Did you find that the three ways of closing the Song-Editor behave differently?"
Yes - if "closed by Ctrl + 1", than new project loaded without bug. (I have wrote this in previous comment).
Oh, I missed that. But it is interesting indeed. My suspicion is that closing the window by mouse or with "Ctrl + W" triggers a direct interaction with the sub-window whereas closing it via "Ctrl + 1" will trigger something else which then interacts with the Song-Editor sub-window. And as you have already written this will likely enter a different code path which does not exhibit/introduce the problem.
That it's a problem with the subwindow can also be seen with the broken window title of the subwindow.
P.S. OK - I will continue ...
:+1:
New information:
(1) Closing by mouse and Ctrl+W generate "Qt close event" on window (in focus).
For song editor it is going to base class: Editor::closeEvent (src/gui/editors/Editor.cpp line ~141)
(2) Closing by Ctrl+1 generate "Qt shortcut event" for MainWindow and goes to:
(MainWindow::toggleSongEditorWin() -> ) MainWindow::toggleWindow (src/gui/MainWindow.cpp line ~928)
And
refocus()
// Workaround for Qt Bug #260116
with code. Negative result:
To invoke MainWindow::toggleSongEditorWin()
in all 3 variants I have tried to provide
In SongEditror.h after void changeEvent( QEvent ) override; (line ~187) `void closeEvent( QCloseEvent _ce ) override;`
In SongEditor.cpp in ending of file (but in namespace) (line ~1136):
void SongEditorWindow::closeEvent( QCloseEvent * _ce )
{
getGUI()->mainWindow()->toggleSongEditorWin();
_ce->accept(); // Tested without this line too
}
And - nothing changed : the same bug ...
With "debug code insertion" I have checked - MainWindow::toggleSongEditorWin()
is invoked, but ...
P.S. It seems that close event invokes some additional code , that introduce this bug ...
My theory is that all Qt children of the Song-Editor window get removed (perhaps even by the Qt framework code) when the sub window is closed via mouse click or Ctrl+W
and not when it is closed with Ctrl+1
. When a song is loaded in the first scenario the children do not get added and the window stays empty.
Might be worth to check how the window title painting code works. If it uses information about the children to position the title correctly then this would speak for this theory as well.
It seems mate in 1:
Editor.cpp line ~151
In master: _ce->accept();
In pre release: _ce->ignore();
When I change in master this line as it was in pre release - it seems no more bug.
But I have to check this in fresh master code : without my inserted code ...
Some more results. I first thought that the following line in MainWindow::addWindowedWidget
could be a source of the problem:
win->setAttribute(Qt::WA_DeleteOnClose);
This method is called for the following four editors in MainWindow::finalize
: Song-Editor, Piano Roll, Pattern Editor, Automation Editor.
However, I then noticed that in the calling method MainWindow::finalize
there is also this line:
window->setAttribute(Qt::WA_DeleteOnClose, false);
Which is a little bit weird because it means that just a few moments later it is decided that the subwindow is not destroyed when it is closed. The reasons seems to be that MainWindow::addWindowedWidget
is also called in several other places. And I assume that in some of these other places the Qt::WA_DeleteOnClose
is not rolled back.
It might be cleaner to add another parameter of type WidgetAttribute
to MainWindow::addWindowedWidget
so that each client can clearly communicate the behavior it wishes for the added subwindow. This would be much clearer than the confusing back and forth between deleting the windows and not doing it.
Adding a destructor implementation for SongEditor
and setting a break point in it also shows that the destructor is not called when the window is closed with the mouse. So the SongEditor
widget seems to stay intact and might only be hidden in the buggy state.
It is mate in 1:
Editor.cpp line ~151
_ce->accept();
must be changed to
_ce->ignore();
Quote: "The event handler QWidget::closeEvent() receives close events. The default implementation of this event handler accepts the close event. If you do not want your widget to be hidden, or want some special handling, you should reimplement the event handler and ignore() the event." {Qt 5.12 documentation}
Key words are started from "want some special handling" ...
void Editor::closeEvent( QCloseEvent * _ce )
{
if( parentWidget() )
{
parentWidget()->hide();
}
else
{
hide();
}
_ce->accept();
}
is such special handling.
Problem is , that after this mate all fine tuning with Qt::WA_DeleteOnClose will became dead code, which simply do nothing. P.S. This is why I hate such drug like frameworks.
Quote: "This method is called for the following four editors in MainWindow::finalize: Song-Editor, Piano Roll, Pattern Editor, Automation Editor." @michaelgregorius
Piano Roll, Pattern Editor and Automation Editor : all have the same bug, but most projects are not saved with this windows opened (so it is harder to reproduce it and mostly not possible "meet" this bug in LMMS usage).
I just add this to bug summary.
Usage off Qt::WA_DeleteOnClose (grep -i "Qt::WA_DeleteOnClose" -R ./
from src):
Edited: line numbers + some comments
(1) ./gui/MidiCCRackView.cpp: subWin->setAttribute(Qt::WA_DeleteOnClose, false);
(2) ./gui/MainWindow.cpp: setAttribute( Qt::WA_DeleteOnClose );
(3) ./gui/MainWindow.cpp: window->setAttribute(Qt::WA_DeleteOnClose, false);
(4) ./gui/MainWindow.cpp: win->setAttribute(Qt::WA_DeleteOnClose);
(5) ./gui/widgets/TextFloat.cpp: tf->setAttribute(Qt::WA_DeleteOnClose, true);
(6) ./gui/ControllerRackView.cpp: subWin->setAttribute( Qt::WA_DeleteOnClose, false );
(7) ./gui/Lv2ViewBase.cpp: m_helpWindow->setAttribute(Qt::WA_DeleteOnClose, false);
(8) ./gui/tracks/TrackView.cpp: setAttribute( Qt::WA_DeleteOnClose, true );
(9) ./gui/MixerView.cpp: parentWidget()->setAttribute(Qt::WA_DeleteOnClose, false);
(10) ./gui/clips/ClipView.cpp: setAttribute( Qt::WA_DeleteOnClose, true );
(11) ./gui/ToolPluginView.cpp: parentWidget()->setAttribute( Qt::WA_DeleteOnClose, false );
(12) ./gui/MicrotunerConfig.cpp: subWin->setAttribute(Qt::WA_DeleteOnClose, false);
(13) ./gui/ProjectNotes.cpp: parentWidget()->setAttribute( Qt::WA_DeleteOnClose, false );
(14) ./gui/instrument/InstrumentView.cpp: setAttribute( Qt::WA_DeleteOnClose, true );
(2) is for MainWindow itself; (3) and (4) are commented before.
Quote: (@michaelgregorius )
" I first thought that the following line in MainWindow::addWindowedWidget could be a source of the problem:
win->setAttribute(Qt::WA_DeleteOnClose); This method is called for the following four editors in MainWindow::finalize: Song-Editor, Piano Roll, Pattern Editor, >Automation Editor.
However, I then noticed that in the calling method MainWindow::finalize there is also this line:
window->setAttribute(Qt::WA_DeleteOnClose, false); Which is a little bit weird because it means that just a few moments later it is decided that the subwindow is >not destroyed when it is closed. "
How I understand:
MainWindow::addWindowedWidget
- "default" way how add such windows/widgets ;
but
window->setAttribute(Qt::WA_DeleteOnClose, false)
- is tuning for Song-Editor, Piano Roll, Pattern Editor, Automation Editor.
So code is logical , problem is - that setAttribute(Qt::WA_DeleteOnClose, false)
do not work as "is assumed".
Good news: it seems, that only this one line will became "dead code" after my "mate in one" .
@firewall1110 there's a better way of saying "Quote"
Just put > at the beginning of the sentence. Makes it easier for readers. Will wrap the sentence in a quote block.
For eg:-
> hello, it's quotes
How it looks:-
hello, it's quotes
@Rossmaxx ! Thank You for hint!
Now my plan is: (1) see other 11 (from 14) Qt::WA_DeleteOnClose usage places to prove that
that
MainWindow::addWindowedWidge
t is also called in several other places. And I assume that in some of these other places theQt::WA_DeleteOnClose
is not rolled back.
is not true.
(2) find PR, that make change in Editor.cpp
line ~151, to understand why it was changed.
[Edited: found - Accept Editor close event (#7035)
(2024-01-01 05:53:52) and this line was the only change ... - negative result is result too]
[Edited: Title of #7035: "Fix Editor windows keeping focus after close"]
P.S.
But it is mostly for "educational purpose" : mate in one + maybe somehow mark, comment off or even delete "dead code" line.
So there is not "Mate in 1", "focus defends King".:
1: place notes on the grid. 2: reduce the grid until at least one of the notes doesn't line up. 3: play the loop. 4: close the piano roll editor. 5: hit space to pause.
I copy this to keep problems together ...
Must be
getGUI()->mainWindow()->refocus();
_ce->ignore();
getGUI()
, MainWindow
refocus();
public in MainWindow.h
Really mate in 6 .
P.S. Quantization not happens, but focus remain to hidden window if use only "mate in one" .
10 day alert :: I am about to make PR that will fix this bug,
but may be LMMS team member can do this better and faster
You do not need to post any alerts here. Just issue a pull request so that it can be evaluated by some team members.
You do not need to post any alerts here. Just issue a pull request so that it can be evaluated by some team members.
May be somebody started to work - so I make possible to stop me in this case.
@michaelgregorius , You make signal that I can go forward without any wait.
Ah, so it is intended as some way of synchronization. Unfortunately, I don't have an overview of who's working on what. But to my knowledge it is rather seldom a problem that several people are working on the same thing. However, even if several people are working on a problem sometimes you can even get a solution which is a combination of the individual solutions which is a good thing.
However, even if several people are working on a problem sometimes you can even get a solution which is a combination of the individual solutions which is a good thing.
You are right for new feature issue: I made mistake to not make my own PR for audio recording 2 years ago.
getGUI()->mainWindow()->refocus(); _ce->ignore();
- ...
Is my solution without any PR, but I was waiting hoping that somebody else , who is "in LMMS project" (for example You, @michaelgregorius ) makes PR using (and improving) my solution. For bug fix the most difficult part is to find code line, that has bug. Find right solution idea - it is much less difficult. After this (idea is found) - PR is formal routine work and LMMS member (with some assistant, who simply approves PR and make some testing) get it merged to master in 1-2 hours. Such not member but "insider" as You (@michaelgregorius ) makes it merged in master in 2 - 6 days (so, if my solution is not wrong, and You made PR , probably it be merged to master and this issue closed right now).
I hope to got it in master after 2 - 3 months ... [And it is OK - I am not in project. Without such barrier project will be ruined .... ]
P.S. And good variant sometimes happens: #7504 (so this issue is closed now - not after 2 - 3 months: in October- November as it be if I made such kind of PR; and in #7504 is added one detail: so why I write "faster and better")
I wouldn't even know where to put it. Please create a PR for it with an explanation of why you think this fixes the bug. This is usually the much harder part of a bug fix. Finding out what causes a problem and then to ensure that the solution does not break anything else what worked before.
If it is just a few lines and you can explain it well then it's very likely to make its way into the code base rather quickly.
I wouldn't even know where to put it.
O yes ... You need to combine information from Mate in 1: _Editor.cpp line ~151 _ce->accept(); must be changed to ce->ignore();
and from mate in 6: Must be
getGUI()->mainWindow()->refocus();
_ce->ignore();
add headers for getGUI() , MainWindow makerefocus();public in MainWindow.h
Please create a PR for it
It is done 2 days ago ...
... with an explanation of why you think this fixes the bug. ... ensure that the solution does not break anything else what worked before
And for this welcome to PR #7515 ...
P.S. @michaelgregorius , only after Your comment I understand:
System Information
Linux Debian Stable, wine32 on Linux, Windows 10 64-bit
LMMS Version(s)
lmms-1.3.0-alpha.1.667+g1c865843f
Most Recent Working Version
lmms-1.3.0-alpha.1.102+g89fc6c9-linux-x86_64.AppImage
Bug Summary
At start working version (1.102) then compiled master (windows downloads - the same). https://github.com/user-attachments/assets/b6bc879f-ca66-48ad-b8c0-ec69307458ef
The same bug is with Piano Roll, Pattern Editor, Automation Editor windows: if you will open project, saved in state (only) Piano Roll or Pattern Editor or Automation Editor window opened, just after this type window closed by mouse, this new project will be loaded with this type window in buggy state.
Expected Behaviour
If close Song Editor windows (with mouse or with Ctrl+W) after load another project song editor window should be not empty. Should not be needed to close and reopen it.
Steps To Reproduce
Load any project, close Song Editor window (as on video), open another project - project loading, but result - empty Song Editor window.
Logs
Click to expand
Screenshots / Minimum Reproducible Project
No response
Please search the issue tracker for existing bug reports before submitting your own.