Skip to content
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

Fix Lv2 help window related issues (#6753) #6957

Merged
merged 6 commits into from
Nov 12, 2023

Conversation

JohannesLorenz
Copy link
Contributor

@JohannesLorenz JohannesLorenz commented Nov 2, 2023

This fixes at least 3 issues:

  • Help window is not synched with window manager's X-button
  • Help window is not being closed on track destruction or on closing the plugin window
  • Trims help window strings and force-adds a newline, because QLabel::sizeHint sometimes computes too small values. Now, together with SubWindow: Increase to respect child's sizeHint #6956, all help windows fit their strings. Some help windows are too large by one line, but I think this is better than forcing the user to resize them if they are too small by one line.

Fixes #6753

This fixes at least two issues:

* Help window is not synched with window manager's `X`-button
* Help window is not being closed on track destruction



bool HelpWindowEventFilter::eventFilter(QObject* , QEvent* event)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was not sure whether I should install the eventFilter on a separate object (as here, on HelpWindowEventFilter), or directly in Lv2ViewBase.

@JohannesLorenz JohannesLorenz marked this pull request as ready for review November 3, 2023 22:37
... because Qt still sometimes computes the `sizeHint` incorrectly. This
commit sometimes adds a blank line, but I think this is better than
showing the help window to small (and forcing the user to resize it).
@zonkmachine
Copy link
Member

I get a crash when removing tracks with an LV2 synth that does implement getDescription().

  • Open amsynth or any calf synth
  • remove the track while the window is still open
Thread 1 "lmms" received signal SIGSEGV, Segmentation fault.
0x00007ffff7a84f54 in QWidget::close() () from /lib/x86_64-linux-gnu/libQt5Widgets.so.5
(gdb) bt full
#0  0x00007ffff7a84f54 in QWidget::close() () at /lib/x86_64-linux-gnu/libQt5Widgets.so.5
#1  0x00005555557f7e4f in lmms::gui::Lv2ViewBase::~Lv2ViewBase() ()
#2  0x00007fff7833ed8f in lmms::gui::Lv2InsView::~Lv2InsView() () at /home/zonkmachine/builds/lmms/build/plugins/liblv2instrument.so
#3  0x000055555588a7b8 in lmms::gui::InstrumentTrackWindow::~InstrumentTrackWindow() ()
#4  0x000055555588a81d in lmms::gui::InstrumentTrackWindow::~InstrumentTrackWindow() ()
#5  0x00005555558b794f in lmms::gui::InstrumentTrackView::~InstrumentTrackView() ()
#6  0x00005555558b7c7d in lmms::gui::InstrumentTrackView::~InstrumentTrackView() ()
#7  0x000055555587a07d in lmms::gui::TrackContainerView::deleteTrackView(lmms::gui::TrackView*) ()
#8  0x00007ffff66f741e in QObject::event(QEvent*) () at /lib/x86_64-linux-gnu/libQt5Core.so.5
#9  0x00007ffff7a46713 in QApplicationPrivate::notify_helper(QObject*, QEvent*) () at /lib/x86_64-linux-gnu/libQt5Widgets.so.5
#10 0x00007ffff66c9e3a in QCoreApplication::notifyInternal2(QObject*, QEvent*) () at /lib/x86_64-linux-gnu/libQt5Core.so.5
#11 0x00007ffff66ccf27 in QCoreApplicationPrivate::sendPostedEvents(QObject*, int, QThreadData*) () at /lib/x86_64-linux-gnu/libQt5Core.so.5
#12 0x00007ffff6723a67 in  () at /lib/x86_64-linux-gnu/libQt5Core.so.5
#13 0x00007ffff50bbd3b in g_main_context_dispatch () at /lib/x86_64-linux-gnu/libglib-2.0.so.0
#14 0x00007ffff5111258 in  () at /lib/x86_64-linux-gnu/libglib-2.0.so.0
#15 0x00007ffff50b93e3 in g_main_context_iteration () at /lib/x86_64-linux-gnu/libglib-2.0.so.0
#16 0x00007ffff67230b8 in QEventDispatcherGlib::processEvents(QFlags<QEventLoop::ProcessEventsFlag>) () at /lib/x86_64-linux-gnu/libQt5Core.so.5
#17 0x00007ffff66c875b in QEventLoop::exec(QFlags<QEventLoop::ProcessEventsFlag>) () at /lib/x86_64-linux-gnu/libQt5Core.so.5
#18 0x00007ffff66d0cf4 in QCoreApplication::exec() () at /lib/x86_64-linux-gnu/libQt5Core.so.5
#19 0x00005555556a8aa5 in main ()
(gdb) 

@JohannesLorenz
Copy link
Contributor Author

I get a crash when removing tracks with an LV2 synth that does implement getDescription().

Great finding! You mean "that doesn't", right? Fixed it.

@zonkmachine
Copy link
Member

zonkmachine commented Nov 4, 2023

You mean "that doesn't", right?

That I did. Fix tested. 👍

Copy link
Member

@zonkmachine zonkmachine left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested, works well.

@zonkmachine
Copy link
Member

I wonder if the help window should also close when it's parent window is closed. Is there a case for when you want it to stay open after the instrument plugin has been closed?
(this behavior isn't new by the way)

@JohannesLorenz
Copy link
Contributor Author

I wonder if the help window should also close when it's parent window is closed. Is there a case for when you want it to stay open after the instrument plugin has been closed?

Good point. I cannot see of any use case to leave it open - I pushed a fix.

Also, note that the whole thing should work not only for instruments, but also for effects.

@zonkmachine
Copy link
Member

I cannot see of any use case to leave it open - I pushed a fix.

Yes, this is better. It's what you expect really.

Also, note that the whole thing should work not only for instruments, but also for effects.

I'm testing this together with #6956 and haven't faced any issues apart from what has been fixed already.

@zonkmachine
Copy link
Member

I tested it some more with FPE turned on and we're good. I think this can be merged.

@JohannesLorenz
Copy link
Contributor Author

Small impact, minor changes. Will merge on Sunday if no one will complain.

@JohannesLorenz JohannesLorenz merged commit ecc5ff8 into LMMS:master Nov 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

LV2 help window - Issue on closing with the 'x' button
2 participants