-
Notifications
You must be signed in to change notification settings - Fork 127
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
Remove all cairo usage from RunLoop and fix #334 (open/close editor c… #337
base: develop
Are you sure you want to change the base?
Conversation
…close editor crash)
This does not look right. The DrawHandler is not the owner of the device and thus should not remove it. |
OK, forget my last comment. Currently the DrawHandler adds the device in its constructor. So this looks ok for now. |
Just for explanation: When I do not call VSTGUI::exit is not called when closing the editor but only when the plug-in is unloaded entirely. |
The patch works fine here. Thank you. |
I tested with this commit in Reaper, but I could still get a crash when clicking open/close UI for two plugin instances:
|
Valgrind shows this:
|
Sorry, my bad! 🤦♂️ Should be fixed, please try again! Btw, how do you test two plug-in editors? I do:
Any other way of testing it? |
After applying the latest commit, AGain crashed when opening 2 windows from 2 different plugin instances. This might be a separate issue as the backtrace suggests something is wrong on timer. I used debug build of AGain on Reaper on Ubuntu. The same crash happened on release build, however it was hard to reproduce. It seems that the crash happens when closing the window. A video is attached to the bottom of this comment. Below are the error messages right after the crash.
Below is backtrace from gdb:
2025-02-02.20-23-48.mp4 |
Nice work! With the latest commit, 27ec53a (Check for end iterator before using it), I don't get a crash anymore. I only tested my own plugin, not AGain.vst3. Though sometimes I get flooded with these printouts:
Adding two instances of the same plugin to the same track in Reaper is basically how I tested this also. Then I quickly bash the UI button on both of them randomly, which switches between Reaper's internal UI (parameter list) and the plugin's own UI. A small thing; The commit message should be updated to say "fix #334", since #335 was my pull request that didn't work out. If you put "Fixes #334" on its own line, github will automatically close the issue when the pull request is merged. |
Yes, I know this one, it is a different issue. The animated "Open UI Editor" button on the top left corner is causing this (though the reason is something else). If you close the editor while the animation of this button is running the assert is triggered. I already investigated into this but could not fix this "quickly". It needs some more effort. But first, let's get this one ready ;) |
Should be fine I assume. I did not see this message but this is what we do basically.
Yes, quite similar to my stress test, good!
Done! |
@rehans That's okay, at least the plugins are usable now. Thanks again for this fix. |
Like discussed here: #335 (comment)
I stress tested on reaper without any crash.
@ryukau @madah81pnz1 Probably you can do another testing as well