PopupMenu broken on embedded, bridged plugins

[DomClark]

A number of DAWs these days feature bridge processes to enable loading 32-bit plugins into a 64-bit host, or vice-versa. The plugin is loaded into the bridge process, with which the host communicates to process/generate audio. A number of DAWs also embed VST GUIs into their main window, usually as an MDI child. However, when a VST from a bridge process is embedded as a child window of the DAW, JUCE's PopupMenus can break, where the popup menu does not appear, or only appears very briefly.

This was originally observed with the Helm VST loaded into LMMS, but I have reproduced it with Helm in Hermann Seib's VSTHost too. (I have heard others have had this problem with other JUCE-based VSTs such as Dexed, but I have not been able to reproduce this particular case.)

To reproduce: download Helm and VSTHost (both 32- and 64-bit). Load an instance of Helm with a matching architecture to that of VSTHost, and observe that opening the "PATTERN" menu under "ARP" and the "TYPE" menu under "DISTORTION" behaves exactly as expected. Now load an instance with a non-matching architecture. VSTHost will load this in a bridge process, and attempts to open the same menus will not succeed, the menus either not appearing, or only appearing very briefly.

This behaviour occurs for both 32- and 64-bit VST2 builds under Windows. It may well exist in other versions but I don't own a host with the capabilities to test this. I have built Helm with the latest version of JUCE's develop branch (f80df37 at time of writing; a few simple patches to Helm were required) and the issue still persists.

The issue seems to stem from the Win32 version of Process::isForegroundProcess https://github.com/WeAreROLI/JUCE/blob/f80df37183382a3194023129f3c2badf23176b74/modules/juce_gui_basics/native/juce_win32_Windowing.cpp#L3699-L3710 which returns false in the case that the plugin is bridged and embedded, since the foreground window belongs to the DAW, which is in a different process to the plugin. Then MenuWindow::doesAnyJuceCompHaveFocus returns false, causing MouseSourceState::checkButtonState to immediately dismiss the menu.

FL Studio, while it can have embedded, bridged plugins, has a workaround; they don't set the WS_CHILD style on their VST wrapper window when embedding. Thus, while the plugin is embedded as a child of FL, Windows recognises it as a separate top-level window for the purposes of GetForegroundWindow and so the above code works as intended. This, however, is not standard/correct usage of the Win32 API as far as I am aware - child windows should always have WS_CHILD set - and so ideally should not be required.

[zonkmachine]

Plugins built with Cabbage see this too:
http://forum.cabbageaudio.com/t/listbox-no-longer-appears-to-exist/1157/4

[sagamusix]

I can confirm this issue in OpenMPT (The host I'm developing), both using jBridge and using my own bridge (to be precise, popup menus currently work using my own bridge but I will use a similar approach as jBridge in an upcoming version, so it will break the same way). I found it happening with the recently released Korg Triton as well as the open-source Dexed synth.

When I dug a bit into the JUCE code I arrived at similar conclusions as the initial post; The problem seems to appear when starting a modal UI and the container window of the bridge process is a child window embedded in the host's plugin window; It does not happen if the bridge process window is actually a popup window (without WS_CHILD).

As a host developer I'm willing to assist JUCE developers in diagnosing this issue further if required. OpenMPT is open-source as the name implies; the plugin bridge code can be found here: https://github.com/OpenMPT/openmpt/tree/master/pluginBridge

[sagamusix]

Possibly the least invasive fix for JUCE would be to check if the window returned by GetForegroundWindow is a parent of the JUCE plugin GUI in addition to checking the foreground process.

Not setting WS_CHILD for bridged plugins in jBridge and OpenMPT's new plugin bridge is a conscious choice as it will keep the host's plugin window to be shown as the currently active window when clicking into the plugin GUI, among other things. Hence I wouldn't really want to remove that flag from my plugin bridge (and the issue would remain in jBridge anyway).

[sagamusix]

Ping. Is there anything that could be done about this broken implementation?

[JoaCHIP]

Using Rhizomatic Plasmonic (which appears to be made with Juce) in Jeskola Buzz using Polac's VST wrapper, I see the exact same thing: Drop-downs appearing for a fraction of second, and the disappearing again.

Having watched the bug reports video on youtube and read the descriptions leading to this bug report, I am fairly certain this is the same problem happening.

[reuk]

I've added a fix for this issue here: 1ee106d

Please try it out and let me know if you run into any issues.

[sagamusix]

I tried upgrading Helm (linked above) to use the latest JUCE, and while it was a bit of a battle, I think the plugin (or at least the parts I care about for testing) should still work. However, rightclicking on components in Helm that should open a context menu still doesn't work in OpenMPT's plugin bridge:

• isEmbeddedInForegroundProcess is called with a nullptr (the parameter is componentAttachedTo) parameter so it returns false early.
• isForegroundOrEmbeddedProcess and doesAnyJuceCompHaveFocus return false as consequence.
• Ultimately this leads to the menu destroying itself again.

Unless there are any changes that would have to be made to Helm's GUI code (which wouldn't be too surprising since this was last updated in 2018), I think something is still not quite correct here.

[reuk]

It sounds like Helm is not setting the popup's target component. For PopupMenus to work as expected in a bridged plugin, they must be created with a 'target component'. The change I linked above checks whether the target component is embedded in an HWND owned by the foreground process. An alternative implementation strategy would have been to check all the parents of all the active desktop components, but this felt a bit wasteful and potentially error-prone.

Embedding aside, setting the target component is useful for other reasons. The target component is used to determine the scale to use when drawing the menu, which is essential on HiDPI displays. If Helm is not currently setting a target component when creating menus, I'd highly recommend updating/patching it to include this change.

[sagamusix]

The popup is invoked like this:

m.showMenuAsync(PopupMenu::Options(), ModalCallbackFunction::forComponent(sliderPopupCallback, this));

That looks like it does have a target component? (assuming that this is what forComponent does, I'm not using JUCE myself)

[reuk]

Yes. To set a target component, you can pass PopupMenu::Options().withTargetComponent (clickedComponent) as the first argument to showMenuAsync.

[sagamusix]

I can confirm that this now works through the plugin bridge. However, the behaviour when using withTargetComponent is slightly different: The popup menu is now aligned with the component, rather than with the mouse cursor. I'm concerned that 1) plugin authors need to actually be aware that they have to make this change for their plugins to work in bridged environments, and 2) the alignment change might be a reason for plugin authors to not go with using withTargetComponent because they want to align the popup menu with the mouse cursor instead.

I wonder if, as a compromise, you could keep a pointer to the last opened desktop component and only check its parent in case the componentAttachedTo is null?

[reuk]

For 1), plugin authors need to set the target component anyway in order for menus to render correctly on HiDPI displays.
For 2), to align to the mouse cursor, a TargetScreenArea can also be set using the current mouse position.

[reuk]

I think this issue is resolved, so I'm going to close it. If you run into any issues or problems with the fix, please let us know and I can reopen this issue.