surge-synthesizer / surge

Synthesizer plug-in (previously released as Vember Audio Surge)
https://surge-synthesizer.github.io/
GNU General Public License v3.0
3.15k stars 400 forks source link

Space key is ignored in Store window and triggers Reaper-project playback instead #984

Closed Marat-Tanalin closed 4 years ago

Marat-Tanalin commented 5 years ago

Summary

In the Reaper DAW, when trying to enter a space character by pressing the Space key in the “Store” (save preset) dialog of Surge, pressing the Space key is ignored and instead triggers project playback in DAW.

Other VST instruments running under the same DAW do not have this issue.

Surge version

Steps to reproduce

  1. Open Surge in Reaper. Make sure you run Surge in the “Native” (nonbridged) mode: Right click on the left project panel → Insert virtual instrument on new track → Right click on Surge → Native only [the native mode is implicitly used by default, so it’s typically unneeded to choose it explicitly, but just in case].
  2. Press the “Store” button in Surge.
  3. Press space in a text field (Name, Category, Creator, Comment).
  4. See that space character is not entered into the text field, and Reaper-project playback is started instead.

Expected behavior

The Space key should work for entering space characters into text fields in the Store dialog of Surge.

Desktop

Additional context

The bug does not reproduce under other DAWs and even in Reaper when running in a separate bridged process (but it’s important to run Surge in the native mode because this is the only way to run DPI-aware [native] and non-DPI-aware [bridged] plugins side by side in the same project in Reaper).

Tested non-Reaper DAWs are:

Reaper has a fully-functional trial version, so you can easily test on your own. Thanks.

Workarounds

baconpaul commented 5 years ago

Wow thanks for such a clear bug report.

I wonder what VSTGUI and Surge are doing to not consume the space key. I'll take a look when I'm next debugging windows DAW issues. Appreciated.

baconpaul commented 5 years ago

Just as a note to myself:

These controls are just CTextEdit items in immediate mode; they have the SurgeGUIEditor as their IControlListener but those are almost all void methods. So my first suspicion is somewhere in vstgui the event is not being suppressed properly somehow (this is also likely since in bridged mode where there is a process space break the event doesn't percolate).

mkruselj commented 5 years ago

If you right-click the plugin title bar, you have this option which works around the issue:

image

Marat-Tanalin commented 5 years ago

@mkruselj Thanks for another workaround option. Unfortunately, it has a downside: pressing space does not start project playback even when a Store-like dialog is not open.

baconpaul commented 5 years ago

Yeah @mkruselj and I were just discussing that on slack.

I will need to set up a debug environment that lets me see if reaper is even giving surge the option on the event. If so I can probably suppress it when and only when the save dialog is up but not sure. I'm going to tag this for the 1.6.2 release and take a look when I go and look at the span of other windows DAW issues we have open too.

mkruselj commented 5 years ago

There is another Reaper gem here.

You don't have to enable that option at all. Just use Shift+Space when you need a whitespace in this situation. It'll work.

baconpaul commented 5 years ago

Yeah the original report includes that fact. I wonder if that is Reaper ignoring the space.

If Reaper gets the first look it may be tricky to fix.

I just tried it on reaper/mac with the VST3 and the transport doesn't start there when I use space. So I also tagged this windows.

mkruselj commented 5 years ago

No the Shift+Space thing is a Reaper feature for those misbehaving plugins that don't steal space properly, at least to my knowledge.

baconpaul commented 5 years ago

Right.

I wonder what the 'properly' configuration is. Clearly 'use vstgui out of the box' is not it ;)

Marat-Tanalin commented 5 years ago

// Using VSTGUI is probably not a proper way to go at all. // Just kidding. :D

baconpaul commented 5 years ago

Yeah you won't find me disagreeing with that...

Marat-Tanalin commented 5 years ago

Forgot to mention that Bitwig Studio 3.0.1 (demo mode) is another DAW the issue does not reproduce in, but looks like it uses bridging always (BitwigPluginHost64.exe) while [offtopic] being more flexible than Reaper thanks to ability to choose whether to scale plugin GUI on per-plugin basis with no need to choose between native/bridged modes just for the purpose of scaling [/offtopic]. Added it to the starting message.

baconpaul commented 5 years ago

Thanks.

Hey and this scaling thing. This is because some plugins are retina aware and some are not?

On macOS we have proper support for the HDPI APIs but surge windows doesn't yet. I am assuming that you use the separate process mode because you end up "too small" in reaper on windows on a high res display?

I could (optionally) change that so we could support HDPI but that would be a separate issue of course!

Marat-Tanalin commented 5 years ago

Scaling is not a Surge fault. Surge’s own scaling works well, except for that it’s limited to 300% which would be too small in 8K@400% mode in future. It would probably make sense to make it possible to use unlimited scaling ratios which should be possible, especially given that Surge now uses vector graphics.

The scaling issue I mentioned in the previous comment is about scaling older plugins that don’t have a GUI-scaling option.

Fwiw, yes, I use a HiDPI mode (via OS-level zoom of 200% with a 4K monitor), and (trying to use) Reaper in HiDPI-mode with the official (still incomplete) HiDPI theme.

The only way to run scaling-capable (HiDPI-compatible like Surge) plugins side by side with scaling-uncapable ones (like NI Battery) in Reaper is the following:

The separate-process plugin-hosting mode uses a separate executable not declared as DPI-aware and is therefore scaled automatically by Windows with a ratio equal to OS-level zoom (2x=200% in my case).

Such OS-driven scaling scales application GUI as a bitmap (raster) image and causes blur under Windows 7 at any ratio, and pixelation at integer ratios (e.g. 2x=200%, 3x=300%) and blur at fractional ratios (e.g. 1.5x=150%) under Windows 10. So running HiDPI-compatible plugins in such blurry/pixelated mode is undesirable because they would then look worse than they are capable of.

baconpaul commented 5 years ago

Gotcha. Very useful.

BTW if you want to go above 300% you can use the + key to expand further. I can add a 400 option trivially easily too.

Marat-Tanalin commented 5 years ago

Oh, thanks, zooming with +/- is a nice future-proof trick.

baconpaul commented 5 years ago

So I just tried this and can reproduce it. Not sure why it is happening though. Just adding that note in case i forget that in the future.

baconpaul commented 5 years ago

I’m sorry, I can’t see a clean fix for this as we approach 1.6.2 and since there is a (admittedly somewhat gross) pair of workarounds I’m going to bump this to the 1.6.3 milestone.

mkruselj commented 4 years ago

This issue (Reaper not responding to Space when a text field is focused in the plugin) is a Reaper-specific issue with VSTGUI-based plugins. Same thing happens in Waldorf Largo or Steinberg Halion 6, for example (both VSTGUI based). It doesn't happen with JUCE-based plugins (Valhalla, Pianoteq...).

I think we can close this one, since Reaper needs to fix this (eventually, unless their devs consider Shift+Space as "the fix").

baconpaul commented 4 years ago

OK I’ll close it then!

Marat-Tanalin commented 4 years ago

If the issue is specific to VSTGUI, does that mean that it’s impossible to fix it on the Surge side?

Fixing the general Reaper issue would be a perfect universal solution, but is it the only solution specifically for Surge?

baconpaul commented 4 years ago

we could patch vstgui - we have for linux - if we knew roughly what the problem was and what had to happen.

when i tried that i could make it so space never worked in either setting but i didn't try that hard.

but we have our own fork of vstgui because we have found plenty of bugs and needed to fix them at a schedule different than steinberg in order to make linux work. lemme reopen this if you think the same could work

mkruselj commented 4 years ago

The issue is really isolated to VSTGUI in solely Reaper. I tried the very same plugins I mentioned (Halion 6, Waldorf Largo) in all other DAWs that I have (Bitwig 3, Live 10, FL20, Maschine 2, Reason 11, Studio One 4) and the issue is NOT there. You can simply press space when a text input field is focused and the DAW does NOT start transport but indeed adds space to text.

So yeah. Reaper issue in handling keystrokes between itself and VSTGUI plugins, not Surge issue. I don't think patching VSTGUI is the way to go.

Marat-Tanalin commented 4 years ago

If the issue does not affect JUCE-based plugins in Reaper (so it is possible to have proper Space-key handling in Reaper), the same should be possible with a patched VSTGUI. It’s probably up to the Surge developer to decide whether to do that.

baconpaul commented 4 years ago

yeah! The 'surge developer' is "anyone who wants to develop to fix that bug" though remember :)

Lets leave this open and if one of us gets inspired to fix it, or if another person comes along and he/she/they are inspired to fix it, then we can close this.

baconpaul commented 4 years ago

Oh if someone does figure it out, by the way, probably want to push the info back to steinberg.

baconpaul commented 4 years ago

Here's a response I chose to not put in https://forum.cockos.com/showthread.php?t=236843

but gives me an idea about why JUCE works and why VSTGUI does not.


Hi Justin yeah it does indeed. I think folks get tired toggling it on and off though. And I'm presuming this happens because the Reaper window gets the first look at the winproc and handles keys before it passes it to the subordinate window?

But that makes me think: any thought for some mechanism by which a plugin could advertise that it wants this to be a default setting? Like if there was a feature we could stick in our vst3 that reaper checked and turned this on by default we would implement that in a hot second. Or does your HostContext implement some sort of message receiver we could call to activate this in reaper?

As to JUCE working, I just scanned the JUCE6 code and found some interesting things

  1. They don't use the win32 text field but roll their own completely (with draw, insert, caret and cursor management)

  2. They handle keypresses through their ComponentPeer architecture. So each component has a peer (which is the native component it looks like).

  3. That gets called (basically) by something set with SetWindowsHookEx to a callback which has this gem (from juce_WindowsHooks.h in juce_audio_plugin_client

        static LRESULT CALLBACK keyboardHookCallback (int nCode, WPARAM wParam, LPARAM lParam)
        {   
            MSG& msg = *(MSG*) lParam;

            if (nCode == HC_ACTION && wParam == PM_REMOVE
                 && offerKeyMessageToJUCEWindow (msg))
            {   
                zerostruct (msg);
                msg.message = WM_USER;
                return 1;
            }

            return CallNextHookEx (keyboardHook, nCode, wParam, lParam);
        }

which it looks like is changing the message type from a KEYPRESS to a USER and zeroing it out. Which would, I suppose, explain why you don't process it as a space maybe?

mkruselj commented 4 years ago

So this issue is then squarely on Reaper side of things - it's the only host that has this problem. I am closing this one, Cockos gotta fix this somehow...