[LV2] Qt5 plugin UIs and libsuil

[Stefan Westerfeld]

   Hi!

On Thu, Mar 02, 2017 at 08:06:03PM +0100, David Robillard wrote:
> On Mon, 2017-02-27 at 13:55 +0100, Stefan Westerfeld wrote:
> [...]
> > Ok, so you merged the falkTX Qt5 embedding implementation (not mine).
> > Now that
> > it is in upstream libsuil I rebuilt Ardour5 with that suil. For
> > synthv1
> > everything indeed works as expected. For the SpectMorph UI, it
> > somewhat works.
> 
> No, I merged neither.
> 
> > However there is the annoying problem that if you open the SpectMorph
> > UI in
> > Arodur, resize it to a larger window size, close it and reopen it,
> > the
> > SpectMorph UI widget will not cover the entire UI space Ardour
> > provides, but a
> > smaller subarea.
> [...]
> > I also believe that falkTK himself said that his approach would be
> > somewhat
> > of a hack. It for instance uses raw Xlib functions like XMoveWindow.
> > My
> > implementation on the other hand tells the Qt5 toolkit to do the
> > embedding
> > basically like this:
> > 
> >   wrap->qembed->windowHandle()->setParent (QWindow::fromWinId
> > (gtk_socket_get_id (s)));
> > 
> > which is why I believe it handles all cases properly, because Qt5 is
> > doing
> > the actual embedding, whereas the falkTX implementation is broken at
> > least
> > in one case (triggered by SpectMorph).
> 
> They both crash 99% of the time in jalv.gtk, the closest thing to a
> reference LV2 host there is at the moment, so frankly they're all
> garbage anyway as far as I can tell, and would need to be disabled by
> default in the next release.

Ok, this is of course an issue that needs to be adressed. I've tried
to come up with a solution here:

https://github.com/swesterfeld/suil/commit/a5fb6c228739a200fe8c2b0ddc0a518fa8397921

Unfortunately, when putting multiple toolkits in the same process under
linux, I believe things will probably never be elegant. So this is
an attempt to solve the problem pragmatically: I've introduced a new
function suil_init(). On a system that has X11, it will call XInitThreads();

I used a module to do that (so -lX11 is not a suil dependancy), but it is a
matter of taste whether to add X11 to the suil depedencies on platforms
that use X11.

Since XInitThreads() needs to be called before any other Xlib function,
this is now also true for suil_init(). In any case if you add suil_init()
to jalv, Qt5 stuff no longer crashes at initialization. It crashes at exit
though, not sure if this is related or unrelated.

If you think the basic idea is sound, a few more things can be done to
make it a bit more elegant, like

 - avoiding code duplication for module loading
 - enforcing that suil_init was called before any other suil function
   can be used

Also when it comes to the suil_init() API itself, we could

 - pass the host toolkit as argument suil_init (SUIL_QT5)
 - pass argc/argv

and other stuff. However, I tried to intentionally keep the code as small
as possible to fix the problem we have now, without overdesigning.

> > So after having tested both, I strongly recommend merging my code.
> 
> I strongly recommend all you Qt people agree on a solution that works
> in whatever cases you care about, submit a patch for it, and I will
> happily merge that.  I'm not oscillating between patches in some he-
> said she-said game.

Ok, I've also put my suggestion on github:

https://github.com/swesterfeld/suil/commit/ea400f7e70cd6ca81a03a919371f46779b737f71

to make it a bit easier to test. Please, especially falkTX, could
you test this? Would that work for everyone?

   Cu... Stefan
-- 
Stefan Westerfeld, http://space.twc.de/~stefan