Fix empty editor windows

[firewall1110]

(1) Bug affects not only Song Editor window, but also Piano Roll, Pattern Editor, Automation Editor windows (all are extending Editor class). Using MainWindow::refocus() makes closing event handling close to closing by shortcut (from MainWindow code)
(2) This is BugFix PR so I do not change coding convention violation ( variable _ce ).
[Edited: after some refactoring commits by LMMS member I decide to change _ce to event]

Reworks #7035
Fixes #7412

[firewall1110]

@michaelgregorius wrote

... with an explanation of why you think this fixes the bug. ...
ensure that the solution does not break anything else what worked before

(1) I think that it fix the bug because I test my code.

(2) Bug is result of not proper another bug solution done by PR #7035

So I add another line
getGUI()->mainWindow()->refocus();

but restore

_ce->ignore();

as it was before.

I tested it against PR #7035 fixed bug steps:

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.

so line
getGUI()->mainWindow()->refocus();
is proper solution (and should be added in PR #7035 instead to change _ce->ignore();).

And this code is called, than windows are closed by Ctrl+1 , Ctrl+2, Ctrl+3, Ctrl+4 .
So calling it from here can not harm more than calling in Ctrl+1 , Ctrl+2, Ctrl+3, Ctrl+4 context.

(3)

Using _ce->ignore(); in such situation is also in Qt 5.12 documentation:

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

[irrenhaus3]

Fixes the bug. Might prefer a signal/slot channel instead of making the private refocus() public. Not a blocker in my eyes though.

[michaelgregorius]

Can someone explain why and how it fixes the bug? What caused the bug in the first place?

[michaelgregorius]

I have checked the implementation of MainWindow::refocus and found that it could use some refactoring. Commit 4e78fb5 introduces modern C++ and slightly simplifies the logic. It's less cluttered now.

[firewall1110]

how it fixes the bug?

Empty editors window bug is kind of bugs I call "mystical bugs":
empty not functional window should not be fixed simply by hiding + un - hiding , but it happens , so all inner structures are ok.
My guess is
that after accepting close event Qt library make some kind of optimization , resource release, ... assuming, that this widget will not be used. So if we do not follow Qt documentation we get some undefined thing.
But it is only my guess.

Might prefer a signal/slot channel instead of making the private refocus() public.

My opinion is, that signal/slot channel is "overkill".
Variant is use friend line in Editor.h but:

• if once it needed, may be will be needed once more;
• from my point off view friend is more complex and more "exotic".

Commit 4e78fb5 introduces modern C++ and slightly simplifies the logic.

It is better and is in LMMS style, so I accept this.

[ But I prefer old good (only for my taste illustration):

void MainWindow::refocus()
{
	QWidget* editor = getGUI()->songEditor()->parentWidget();
	if (!editor->isHidden()) { editor->setFocus(); return; }
	editor = getGUI()->patternEditor()->parentWidget();
	if (!editor->isHidden()) { editor->setFocus(); return; }
	editor = getGUI()->pianoRoll()->parentWidget();
	if (!editor->isHidden()) { editor->setFocus(); return; }
	editor = getGUI()->automationEditor()->parentWidget();
	if (!editor->isHidden()) { editor->setFocus(); return; }
	this->setFocus();
}

Modern compilers will make editor register variable, so it will be faster and more compact in result assembler code.

But such things not allowed here ...
]

[messmerd]

Suggested change

	if(!editorParent->isHidden())
	if (!editorParent->isHidden())

style

[firewall1110]

Now in refocus() refactoring context I have one question:

is MainWindow class is right place for refocus()?

What about GuiApplication class?

P.S.
But my opinion : not here (in this bug fix PR) such changes ...

[firewall1110]

Now MainWindow::refocus compiled with clang++ (ver. 14) with exported CXXFLAGS="-Oz" is little bit more compact, than my "old style variant", but assembler listing is near the same ...

[michaelgregorius]

Approving this PR as it seems to fix the problem.

[firewall1110]

Thank You!
Not so long ... ~ 15 days
P.S.
Should I open new issue about:

is MainWindow class is right place for refocus()?
What about GuiApplication class?

May be I find more things in MainWindow and Editor context to refactor.
Or it is not actual?

[michaelgregorius]

@firewall1110, I am currently not sure where to put it. I would have assumed that the other editors would be children of the MainWindow and in that case having refocus in MainWindow would be fitting as it would act on its children. However, in GuiApplication the editors seem to be parallel to MainWindow with regards to the hierarchy. It would be interesting to know if the windows are reparented at some point or what this all means.

So for now I'd rather keep the refocus method in MainWindow as it seems to make sense from an intuitive model.