After closing Song Editor new project loaded ends with empty song editor window (must be closed and opened)

[firewall1110]

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.

• I have searched all existing issues and confirmed that this is not a duplicate.

[firewall1110]

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.

[michaelgregorius]

Interestingly the whole window acts strange:

1. Start LMMS.
2. Close the Song Editor.
3. Open a project.
4. Resize the Song Editor. The window will stay empty and the title of the window will not stay in the middle of the window title.
5. Close and reopen the Song Editor. The content will now be shown.
6. Resize the Song Editor. The window title will now stay in the middle of the window title.

[firewall1110]

@michaelgregorius : Thank You for bug confirmation!

Man plan is (Edited 2024.08.13) : somehow compare what happens when Song Editor :

• closed by mouse;
• closed by Ctrl + W;
• closed by Ctrl + 1;

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 ...

[michaelgregorius]

@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?

[firewall1110]

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 ...

[michaelgregorius]

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 ...

👍

[firewall1110]

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

• there are + refocus()
• and // Workaround for Qt Bug #260116 with code.

[firewall1110]

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 ...

[michaelgregorius]

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.

[firewall1110]

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 ...

[michaelgregorius]

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.

[firewall1110]

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.

[firewall1110]

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.

[firewall1110]

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" .

[Rossmaxx]

@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

[firewall1110]

@Rossmaxx ! Thank You for hint!

[firewall1110]

Now my plan is:
(1) see other 11 (from 14) Qt::WA_DeleteOnClose usage places to prove that

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.

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.

[firewall1110]

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 ...

[firewall1110]

Must be

getGUI()->mainWindow()->refocus();
_ce->ignore();

• add headers for getGUI() , MainWindow
• makerefocus();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" .