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

[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 4e78fb53dc9 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 https://github.com/LMMS/lmms/commit/4e78fb53dc945c4dd47b5d95279f7e9eb409df04 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 ... ]

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