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

Add the custom UI host #111

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from
Open

Add the custom UI host #111

wants to merge 1 commit into from

Conversation

jpcima
Copy link
Contributor

@jpcima jpcima commented May 22, 2020

HI, this permits to implements another custom UI host than the 3 choices offered.

This is in particular for Linux audio plugins, in which case Gtk cannot work.
Instead I implement a custom host over the window library pugl.

However this needs some small changes in Elements, which are brought by this PR.

For reference my host is located here.
https://github.com/jpcima/sfizz/tree/editor%2Belements/editor/src/host

@Xeverous
Copy link
Contributor

I like the idea and I'm also surprised you need only such small diff to build custom host implementation. I think there could be added some checks like static_assert or (later) concepts to verify interface correctness.

@jpcima
Copy link
Contributor Author

jpcima commented May 22, 2020

It works, but note that at this moment I don't build with Elements's cmake, I build the source myself.
That is not only to exclude host source, but there exist a small caveat.

When defining window and base_view methods outside of Elements's static lib, this can end in link errors. That is because both libraries, my editor and Element, have become co-dependent.

Cmake suggests this solution which works, link A to B and link B to A.
https://cmake.org/cmake/help/v3.4/command/target_link_libraries.html#cyclic-dependencies-of-static-libraries
In case of using add_subdirectory though, I'm not sure of how this problem can be avoided.

@Xeverous
Copy link
Contributor

In case of using add_subdirectory though, I'm not sure of how this problem can be avoided.

You should be able to do the same. Just link target in both directions.

Optionally, as the documentation says, use an object library.

@djowel
Copy link
Member

djowel commented May 23, 2020

One of the important decisions I had was to make it very easy to port Elements to just about any target, by implementing only a handful of files in the "host" directory. There was a time when I seriously considered Pugl as a backend. I would very much like to directly support pugl instead of a generic "custom" target.

That would mean, bringing in the whole port. Oh, and while you are at it, a bonus would be actually documenting how you did the port that we can add to the documentation currently being written.

Thoughts?

@djowel
Copy link
Member

djowel commented May 23, 2020

Another thought is to go back to its roots: originally, elements' base_window (nonexistent now) and base_view were abstract classes, which means that you can actually have multiple 'backends' working at the same time, by instantiating each. It was "cool" but I thought was not really practical at the time: I haven't really come across a case where a platform supports multiple 'backends',... until now. OSes (mac, windows, linux), will have their own unique backends, and so I thought... incorrectly.

@redtide
Copy link
Contributor

redtide commented May 24, 2020

As a side note both GTK and Qt can be used under Windows and macOS, and looking at wxWidgets you have also more than a GTK version (2 and 3 AFAIK).

@djowel
Copy link
Member

djowel commented May 25, 2020

GTK version (2 and 3 AFAIK).

GTK2 will not be supported because it does not support HiDPI.

@redtide
Copy link
Contributor

redtide commented May 25, 2020

sorry, it was just an example to point out that other than more than one backend per platform there could be also a matter of versions of the same one, and this could lead to some choice where there should be applied some limit to avoid too many versions, if taken into account.

@djowel
Copy link
Member

djowel commented May 25, 2020

sorry, it was just an example to point out that other than more than one backend per platform there could be also a matter of versions of the same one, and this could lead to some choice where there should be applied some limit to avoid too many versions, if taken into account.

Very good point! I think we should also be checking versions then.

@jpcima
Copy link
Contributor Author

jpcima commented May 26, 2020

That would mean, bringing in the whole port. Oh, and while you are at it, a bonus would be actually documenting how you did the port that we can add to the documentation currently being written.

To bring the whole port, sure, when having some free time to dedicate for this task.
There is some things to note on the current state.

  • hi-DPI support not made yet, no doubt it's just a matter to replicate what is in other hosts; I don't have a display that lets me test it in real conditions
  • there is something not feeling right in Pugl itself, that one needs a view handle to access the clipboard, so I have to examine and discuss with David Robillard
  • also Pugl no mouse cursor support yet

@djowel
Copy link
Member

djowel commented May 27, 2020

There is some things to note on the current state.

  • hi-DPI support not made yet, no doubt it's just a matter to replicate what is in other hosts; I don't have a display that lets me test it in real conditions
  • there is something not feeling right in Pugl itself, that one needs a view handle to access the clipboard, so I have to examine and discuss with David Robillard
  • also Pugl no mouse cursor support yet

I see. I guess we have to take this one step at a time then. What is the view handle type that Pugl uses? Is it really void* ? If possible, I'd like to have a bit more type-safety, by possibly also specifying the actual handle type exposed as a macro that I can typedef as well.

@jpcima
Copy link
Contributor Author

jpcima commented May 27, 2020

What is the view handle type that Pugl uses?

The host_window_handle is the native window id HWND or what's appropriate for platform.

My implementation keeps Pugl window data separately, which exists in a global map whose keys are window ID.

At the time of implementing, I determined that I needed host_window_handle to be HWND, as the parent ID arrives as such from the VST plugin and there isn't any way to wrap this into a Pugl handle.

By no means it's a fantastic solution, but I could not find a way to do without the global.

@djowel
Copy link
Member

djowel commented May 27, 2020

What is the view handle type that Pugl uses?

The host_window_handle is the native window id HWND or what's appropriate for platform.

My implementation keeps Pugl window data separately, which exists in a global map whose keys are window ID.

At the time of implementing, I determined that I needed host_window_handle to be HWND, as the parent ID arrives as such from the VST plugin and there isn't any way to wrap this into a Pugl handle.

By no means it's a fantastic solution, but I could not find a way to do without the global.

Alright, then we can still have a two cmake options, one for ELEMENTS_HOST_UI_LIBRARY and one for, perhaps ELEMENTS_HOST_VIEW_TYPE, which you can set to void* or anything as appropriate.

TBH, I am slowly being tempted to just make base_view and friends abstract classes. It shouldn't be to much work for me... But I am not sure if that will hide the handle type. Let me think about this some more...

@djowel djowel mentioned this pull request Jul 12, 2020
@djowel
Copy link
Member

djowel commented Aug 31, 2020

As suggested, I'm going to place this one in an experimental branch. Before I do, I just want to make sure. Is this PR still updated with the latest cmake changes and stuff?

redtide added a commit to redtide/elements that referenced this pull request Feb 15, 2021
@riri riri mentioned this pull request Apr 24, 2021
@djowel djowel force-pushed the develop branch 3 times, most recently from 3c3b6d8 to 2d6f517 Compare December 5, 2023 02:28
@djowel djowel deleted the branch cycfi:develop March 29, 2024 13:03
@djowel djowel closed this Mar 29, 2024
@djowel djowel reopened this Jun 2, 2024
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.

4 participants