Hi
On Wed, Aug 30, 2023 at 3:21 PM Daniel P. Berrangé <berrange@redhat.com> wrote:
>
> On Wed, Aug 30, 2023 at 03:01:27PM +0400, Marc-André Lureau wrote:
> > Hi
> >
> > On Wed, Aug 30, 2023 at 2:53 PM Thomas Huth <thuth@redhat.com> wrote:
> > >
> > > On 30/08/2023 11.37, marcandre.lureau@redhat.com wrote:
> > > > From: Marc-André Lureau <marcandre.lureau@redhat.com>
> > > >
> > > > Hi,
> > > >
> > > > QEMU system emulators can be made to compile and work without pixman.
> > > >
> > > > Given how pervasively pixman types and API is used in all the code base, it was
> > > > a bit difficult to figure out how to cut the dependency.
> > > >
> > > > I decided that it was important to keep VGA and graphics device working for
> > > > compatibility reasons, although some devices, such as xlnx Display Port, have
> > > > stronger dependency and have to be disabled. The ui/console code also has a lot
> > > > of pixman usage and a bit of a mess to deal with. I made large refactoring to
> > > > allow to compile out the VC code.
> > > >
> > > > The series can be roughly divded as:
> > > > - a few related preliminary cleanups
> > > > - ui/console refactoring to allow ui/vc split
> > > > - add a 'pixman' option, and a minimal pixman-compat.h
> > > > - make some parts depend on 'pixman'
> > > >
> > > > Graphic -display still work, although with some caveats. For ex, -display sdl or
> > > > cocoa don't have VCs, so starting QEMU will print the following warnings when
> > > > pixman is disabled:
> > >
> > > I just had a quick look at the series, but for me it looks like this is
> > > adding a lot of additional complexity to the code (adds lots of #ifdefs, and
> > > adds a subset of the pixman library to the code base), which seems somewhat
> >
> > The #ifdef aren't so bad (at least I can bare it). Just a quick stat:
> >
> > $ git diff origin/master | grep "+#ifdef CONFIG_PIXMAN" | wc -l
> > 11
> >
> > > unfortunate for such a marginal feature request. What's really so bad about
> > > requiring pixman for building QEMU?
> >
> > Not that a good part of the series is cleaning up ui/console.c code
> > that really deserved it. It makes it use QOM, and split VC code. It's
> > still worth it regardless of the outcome for pixman.
>
> I've done a review of the start and like the cleanup patches, and the
> adaption to make sane use of inheritance in QOM rather than the poor
> man's inheritance via the console type field. I agree that's worthwhile
> regardless of what we think about pixman optionality.
>
> > > IMHO, if we really want to go down this path, I think we should rather
> > > disable all graphic related stuff in QEMU instead, i.e. disable VGA cards,
> > > Spice, SDL, etc. completely. I think this is also what has been requested in
> > >
> >
> > The various features and devices can be disabled by other means. I
> > think we should aim at making the different configure options
> > orthogonal, so QEMU without pixman can still provide most gfx/vga/UI
> > experience too, by default.
>
> To me where this series becomes dubious is roughly around the patch
> that adds pixman-compat.h providing a bunch of pixman API as stubs.
Not stubs actually... this shows that you haven't looked at it enough :)
It's really stupid, nothing like pixman! it just provides a few types.
See my reply on the patch.
Completely removing those few pixman types from QEMU will lead to a
very unpleasant and complicated result.
I can give it another try, or you may also play with it and prove me
wrong. At least, I can come up with a better list of arguments.