Re: [Qemu-devel] [PATCH 2/6] ui: add basic GTK gui

[Anthony Liguori]

On 02/20/2012 02:45 PM, Stefan Weil wrote:
> Am 20.02.2012 00:45, schrieb Anthony Liguori:
> > This is minimalistic and just contains the basic widget infrastructure. The GUI
> > consists of a menu and a GtkNotebook. To start with, the notebook has its tabs
> > hidden which provides a UI that looks very similar to SDL with the exception of
> > the menu bar.
> >
> > The menu bar allows a user to toggle the visibility of the tabs. Cairo is used
> > for rendering.
> >
> > I used gtk-vnc as a reference. gtk-vnc solves the same basic problems as QEMU
> > since it was originally written as a remote display for QEMU. So for the most
> > part, the approach to rendering and keyboard handling should be pretty solid for
> > GTK.
> >
> > Signed-off-by: Anthony Liguori <address@hidden>
> > ---
> > Makefile | 2 +
> > Makefile.objs | 1 +
> > configure | 25 +++-
> > console.h | 4 +
> > sysemu.h | 1 +
> > ui/gtk.c | 551 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> > 6 files changed, 583 insertions(+), 1 deletions(-)
> > create mode 100644 ui/gtk.c
>
> This patch compiles, but does not provide the minimalistic interface
> because the hooks in vl.c are missing. Parts of patch 6 might be
> moved to patch 2.

Yes, that's intentional in order to avoid bad bisecting.  I could make a 
-display gtk flag though and just not make it the default until the last patch. 
 I'll update accordingly.

> Patch 2 could also be improved by removing the dependency on VTE.
> For some platforms (w32), providing VTE is really a big challenge.
> I did not find a precompiled version and still did not succeed
> in compiling it because of a large and still incomplete dependency
> chain...

I'm not a big fan of this.  Would hosting dependencies on qemu.org help address 
this problem?

In order to do this, we would have to have mirror all of the dependencies onto 
git.qemu.org and create a super repository with submodules and a build script to 
cross compile all of the dependencies.  I don't think it's a huge effort.

> What about offering compilation without VTE also in the "final"
> version of the GTK gui (--enable-vte, --disable-vte)?

That's a pretty significant reduction in functionality.  I'd rather try to 
provide the dependencies directly.

> These includes in ui/gtk.c were not needed:
>
> +#include <vte/vte.h>
> +#include <sys/types.h>
> +#include <sys/socket.h>
> +#include <sys/un.h>
> +#include <sys/wait.h>
> +#include <pty.h>
>
> If the version using VTE needs them, qemu_socket.h should be
> used (needed for w32 which does neither have sys/socket.h
> nor pty.h). Maybe most of these includes are not needed at all
> (relict from VNC?).

They are needed but not in this patch.  I missed this bit as I was extracting 
patches.

> With a patched vl.c and ui/gtk.c, the new GUI starts on w32
> but does not work stable.
> After some minutes, the GUI freezes (no more updates, menu no
> longer working).

My guess is that this has to do with the way the Win32 main loops work.   I'm 
not sure how to fix this since IIUC, the problem is that we basically have two 
loops for Win32, one that calls select() on fds and another that uses 
WaitForEvent or whatever the win32 function is.

> The configuration support still does not work as expected:
>
> configure should support --enable-gtk, --disable-gtk with the
> usual semantic (no GTK UI if --disable-gtk was given, fail if
> --enable-gtk was given and GTK is not found, provide help message).

Oh, I'll look into that.  I expect that it should work.

> The GTK user interface is a good starting point for a more
> powerful and user friendly interface.
>
> Nevertheless SDL should not be removed for the next few years.
> I would not mind if buggy features like zooming were removed
> from SDL again, but having a GUI which also works with framebuffers
> (without X) and which has much less requirements than GTK+-2.0
> (and perhaps also a reduced risk of security problems)
> is a good feature.

As I mentioned in another thread, I'd be very happy to keep the SDL interface if 
we removed the ugly bits (like the virtual console emulation).

> Cheers,
> Stefan Weil
>
> PS: git complains about whitespace at eol in some of the patches,
> and checkpatch.pl is also very noisy :-)

Yes, I'm aware.  I'll address it in the next round.

Thanks for the feedback.

Regards,

Anthony Liguori

>