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

VST3 prevent resize loops #599

Closed
wants to merge 1 commit into from

Conversation

ctsexton
Copy link
Contributor

Hey Ardour team, this PR should fix some major performance issues I've noticed with resizable VST3 plugins on Linux. I didn't entirely understand why this worked or how the resizing logic all fits together, but based on my testing it seems to fix most of the problems.

To summarize the problem: plugin UIs were getting stuck in loops with view_size_allocate getting called repeatedly even though resize_callback and onSize had already successfully run. After much messing around adding print statements everywhere I found that, since resizable plugins will call resizeView (which forwards that to resize_callback), there is no need for them to be connected to the _gui_widget resize signals. These signals seem only necessary for plugin UIs that are not resizable in the drag-the-corner sense. I might have misunderstood something though.

Here are my observations from testing various plugins:

TAL U-NO-LX v4.4.1

Completely unusable before this fix, but stable and runs well after. Earlier plugin versions still have issues, so it looks like a recent update fixed some resize logic on the plugin developer's end as well. The only remaining issue for this plugin is the initial plugin window size is a bit wider than the plugin UI, but as soon as the user resizes the window it matches up. Might be easily fixable in a follow-up.

U-he Diva

This plugin is not by-default resizable, but it DOES have a menu to change the UI sizing to one of several fixed percentages. I noticed it does call resizeView when selecting a new size in that menu, but it returns canResize as false. So with this change we do still hook it up to the _gui_widget resize signals and that seems to work well for its UI.

My own JUCE-based plugins

Similar to the TAL plugin, the UIs were not usable at all before this fix but work great with the fix. Resizable plugins now work as expected, non-resizable plugin behavior seems unchanged.

@x42
Copy link
Member

x42 commented Feb 27, 2021

This does not seem right. requisition has to be set unconditionally for embedded child windows.

I expect the problem is deeper. Ardour does not yet call checkSizeConstraint. There is a conceptual disconnect with how X11/gdk handles size-requests and allocation vs how Steinberg specified it.

PS. u-he plugins work fine here.

@x42
Copy link
Member

x42 commented Feb 27, 2021

PPS. There is identical code for Windows (VST3HWNDPluginUI) and MacOS (VST3NSViewPluginUI).

In both cases there are unconditional subscriptions top to _gui_widget.signal_size_request() and _gui_widget.signal_size_allocate(), working fine with a wide variety of plugins.

@ctsexton
Copy link
Contributor Author

Thanks for the tips! I'll keep digging then.

Also, should have clarified - yes the U-he Diva plugin works fine for me too before and after applying this change. But it is not resizable by dragging the bottom right corner. JUCE plugins as well - if you make them non-resizable they are fine. Only once plugins are resizable by dragging the window corner does this issue become apparent.

@ctsexton
Copy link
Contributor Author

Ok, yes this is definitely a deeper issue. Here's what I found today:

When we call onSize on a JUCE plugin in view_size_allocate, the plugin then makes a call directly to X11 to resize the window here:

XWindowSystem::getInstance()->setBounds (windowH, physicalBounds, isNowFullScreen);

I'm guessing that this ends up causing signal_size_request and signal_size_allocate to fire again, in turn calling view_size_allocate and hence causing the loop. That would explain why the code on this PR did the trick to prevent the loop. Commenting out that XWindowSystem setBounds call in the plugin also severs the loop and the plugins work fine.

I tried adding code to check if the window size had changed before calling onSize but that didn't work reliably.

So if my above reasoning is correct then I agree @x42 you're right there is a conceptual disconnect here. Is Ardour or the plugin responsible for resizing the window? I am pretty new to VST development in general so my opinion isn't worth much here but I'd say the plugin should leave the window resizing to the host, in which case the existing Ardour code is fine. Why is JUCE trying to mess with the XWindow?

If we agree it's JUCE's problem and not Ardour then I'll close this PR and mark the bug resolved on the bug tracker.

@x42
Copy link
Member

x42 commented Mar 1, 2021

the plugin then makes a call directly to X11 to resize the window

Indeed. That seems indeed wrong. According to https://steinbergmedia.github.io/vst3_doc/base/classSteinberg_1_1IPlugView.html#a3e741e55c2c047a4cc10f102661f5654
the plugin should request a resize (IPlugFrame::resizeView ()), and the host then resizes the window and acknowledges it by calling onSize afterward.

PS. don't close this PR too soon. The diff may come in handy to discuss things with JUCE devs.

@x42
Copy link
Member

x42 commented Mar 19, 2021

Fixed by juce-framework/JUCE@d4e8020

@x42 x42 closed this Mar 19, 2021
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.

2 participants