-
-
Notifications
You must be signed in to change notification settings - Fork 100
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
Progress on UI_TYPE=external #313
Comments
this is normal, the vst generation is part of the main makefile, not the individual plugins. until the embed-external-ui is more complete, I wont add it as default part of the build
no idea sorry, but this is all still very new. I need to have a working example plugin so I can begin check on the issues with the implementation
Sure, will add this now, pretty easy. |
All makes sense, thanks. Will test external on the three platforms as it evolves. |
Reorganized and added documentation at e59b5a5 |
Just found out something for the X11 case, if the UI is embed a call to |
I have questions how to use the methods of As far as I understood the plugin UI implementation has to call
So when should the plugin UI implemenation call BTW: I'm calling |
Placing the size on the UI constructor call like Plugins wanting to change window size is a next step to do and verify, I dont think this works as-is yet. |
FYI you should use XMapWindow instead, as if non-embed the XMapRaised will make the window visible without the host ever asking for such a thing. Maybe not an issue atm, but it will be if there is ever any lv2 host without direct raw x11 ui support. |
I'm already setting the size, minSize and maxSize in the native window construction. So it's strange to also set the size in the UI constructor. So my question is: when should the plugin UI implementation call |
I'm sorry, but I don't understand "will make the window visible without the host ever asking for such a thing". |
your constructor should have the correct size matching the native x11 window size, I thought this was obvious. what the calls that end up being made in dpf, that is irrelevant and not for plugins to care. |
So I should never call |
I juist want to understand this: if I receive a configure event I should not call setSize, right? |
I mean that the host might not give you a parent window to embed into, on that case calling XMapRaised will place that window on the desktop before the host has asked for it to become visible.
err whatever you prefer? as long as the size at the end is correct.
You can but this will do nothing to what the host is concerned. |
Curious to know if Windows support is planned? just saw this:
Btw already ported my project to external UI on Linux and it is working great on REAPER and Carla. It works on Bitwig as well but there is an issue with the keyboard input. This also happened with the previous SubWidget-based approach, and the workaround was relying on Wnere do the keystrokes for |
I think I found it: DistrhoPluginVST2.cpp
DistrhoUIInternal.hpp
Still puzzled why the webview on REAPER/Carla catches keystrokes by itself, and why on Bitwig they need to be read from the host, despite forcing keyboard focus to the webview. I am using code stolen from |
The PR still does not fix my issue but thought it could be useful to have. |
Yes of course. It is just the code to start the external tool that is missing at the moment. One thing we need to try is to do external process embed ui, like the linux mpv test. afaik windows also allows to embed into windows of another process, it is just macOS that blocks this. |
Yes unfortunately there is no solution for Mac at least for what I've researched during 2021. |
I have been updating all my stuff that runs on top of DPF and
|
for the resize on vst2, we need to put in place something to report that change to the host. |
Makes sense, that perfectly explains what I'm seeing on REAPER when resizing, it works but takes the host a little while to catch up. |
I tried something like this, inspired on the DGL version: DistrhoUI.cpp
Here UIWidget is typedef'd to ExternalWindow in DistrhoUI.hpp, and uiData->setSizeCallback() results in the host calling effEditGetRect. Inside effEditGetRect:
fVstUI->getWidth() and fVstUI->getHeight() both return 0. Setting fVstRect.right and fVstRect.bottom to a constant also does not work, the plugin view just disappears. I feel this is totally wrong and the fix is actually deeper lol. This issue and the plugin not showing on Live (maybe both things are related?) is the only thing that is preventing me from completely ditching DGL. Any hint? thanks. |
That seems like the correct approach to me. Do you override this method as well? |
Yes, I think. Since Then adapted DGL
Yes, my UI subclass looks like this:
|
Yeah that seems correct. I added that in d3bd0ec just now. |
Great. The only resizable example I have is this (branch nodgl). I am not sure it is the best way to debug the issue because of the complexity, maybe I'd suggest adding a thread to EmbedExternalUI that polls for user entered width,height from a file or something like that. Also that would rule out any mistake in my own code lol. |
I have an idea. I can make the example UI already in dpf use width/height as parameters. so changing them from the generic host editor should change the native/custom-ui side. |
Right, it is expected fWindow to be null when running embedded, because the host window/view is provided by getParentWindowHandle() instead. All initialization code for the embedded case looks good. Isn't polling events in |
This is wrong... immediately after opening the UI parameterChanged() is called for width and height, making REAPER suitable for testing. Still seems that setSize() does not reach that host. |
incorrect. I mean on x11 for example if we want to repaint, handle keyboard, etc we need to fetch the events for our respective window. on linux/x11 we always need to drive the events ourselves, and there is no global event loop to hook into. |
not that it matters too much, but I added some initial win32 code on the example plugin now. |
Thanks for your time having a look at this issue, and the clarification regarding X11. I agree it is great to have this example native code because it is likely what a plugin will end up doing. Moreover I propose having a generic version of EmbedExternalUI that could be used as the base for non-DGL plugins, ie. abstract the hosted and standalone cases. Likely you want a main window for the standalone like JUCE does. For the Linux the default would be X11, then it is up to the user to create GTK or whatever versions at their own risk. Not that I dislike DGL, I only think the framework would be more flexible if it was an option at the same abstraction level as 3rd party UIs. This is starting to manifest with ExternalWindow replicating some interfaces, if I correctly understand. But this is really a design choice... things work pretty good the way they are now. So I retried EmbedExternalUI on the three platforms, this is the current state:
|
I am heavily against this. this would basically replicate what pugl already does, I have no intentions to provide such a thing.
Well you typically would not even be allowed to use 3rd party GUIs on frameworks like DPF. This external ui/window access is an extra we are trying to make work as if native, but obviously it wont be the default choice.
This is expected, I have more experience with X11 than the other 2 :)
I just pushed the code, so I guess I forgot something. PRs welcome.
That will be because |
All points valid
What I don't understand is, shouldn't all these things expected to be empty for the embedded case? I mean, handling a custom window dimensions is only needed for standalone. The issue is not the content view itself not being resized ( yes, that is the plugin dev responsibility ), but the host not correctly being notified of a size change, for example REAPER never gets a chance to tightly wrap the contents in Mac. This is backed by my observation that on Windows, there are no calls at all to Win32 in the EmbedExternalUI example, but still REAPER is still able to resize its container after tweaking w/h parameters. When running as a plugin, shouldn't this little new code be enough for all platforms? DistrhoUI.cpp
EmbedExternalExampleUI.cpp
I am happy to work on these on my own but I frankly hit a wall. I feel either I am missing something, or the issue is deeper than we think, hidden in some layer. |
well the code is the same for all platforms for this case, so it should just work. seems more likely that the host expects the plugin view to be resized too.
yes...ish, except those that try to be a bit smart and look into the child window to see if it is doing things incorrectly. for LV2 we are even trying to get this to work without calling any host features, so that the host detects the child window change all on its own. |
So far only visual inspection. What conclusive check to use? checking effEditGetRect is called?
Oh my... considered something like that at first then thought, it would be a very dirty thing to do. But it is definitely worth looking, if the Mac/Win children are never resized (or don't exist?) that could explain why the hosts are always getting the same dimensions.
Will write some code and get back. |
effEditGetRect is for host->plugin call, the setSizeCallback on the VST2 DPF code is the important part. the plugin->host call is at https://github.com/DISTRHO/DPF/blob/main/distrho/src/DistrhoPluginVST2.cpp#L400-L408 |
Well I found the cause why EmbedExternalUI is not showing on Live/Mac: DPF/distrho/src/DistrhoUIInternal.hpp Line 127 in dbbfef6
That always returns 0, I am surprised the host does not crash after dividing w/h by 0 later on. Changing that function to return 1 fixes the issue. Could you point me to a proper solution? I got lost in the layers, not sure where that scale factor should be set for the ExternalWindow. Thanks. |
huh very interesting. just switch on the WindowPrivateData to have 1 as fallback instead of 0, that should do the trick. |
actually I can do this here hang on... |
I was wrong, the 0 value is fine. issue is that we never replace it with something else for the external-ui case, but always do for the regular dgl stuff. |
I left a comment in 37e6922#diff-98d4a2bee9607ee4512f666bf07dd845a0a4abc56588b67c3f15d4a35b4f727fR57 , a stopper and also a picky one that I can address on my own. |
To comment about the progress on this, everything seems to work now. |
Do you mean the EmbedExternalUI example for Windows? which is lacking standalone support. Another issue is |
This is what I mean for the windows case, when non-embed we do not get a parent window. so we need to grab whatever scale factor is the default on the desktop (in case host does not provide this info) The parent window being 0 in non-embed cases is normal and expected. That is why there is a check for this on the |
How about a virtual I first thought about making |
you mean the transient window parent? I think that is already in place there. |
DistrhoUI.cpp does this:
That |
Yes correct. On LV2 we can still get a transient window id, but that won't be a much used scenario.
I don't think you can do that, since it would required creating the window first. |
I agree, initialization order would make all this complicated. |
I plan now to close this ticket. the parent-less situation with win32 was handled in b2e683e (turns out 0,0 always refers to the default screen/monitor) Is there anything still to do, besides perhaps documentation and examples? |
On my side there’s nothing I can think of. External has been working great on all platform/host combinations tried so far, including resizing.
… On 8. Sep 2021, at 20:13, Filipe Coelho ***@***.***> wrote:
I plan now to close this ticket. the parent-less situation with win32 was handled in b2e683e (turns out 0,0 always refers to the default screen/monitor)
Is there anything still to do, besides perhaps documentation and examples?
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications on the go with GitHub Mobile for iOS or Android.
|
Nothing added or to be said from my side in 10 days, so I am closing this ticket. |
Great progress there, not sure these are issues or pending features. Thought it is still useful for tracking:
(All macOS for now)
The text was updated successfully, but these errors were encountered: