[Spice-devel] [spice-common PATCH 4/7 v2] ppc: Fix colors on ppc when using QUIC

[Lukas Venhoda]

Hi,

> Any reason why there is not a patch fixing canvas_get_jpeg{_alpha} ?
I didn't know how to test it, or when it is used (I was just trying to fix image compressions), so I left it as is.
If someone provides a test case, I can test it and change it later.

> I suspect we are not going to properly handle 16bpp canvas ?

I was just following ifdefs inside pixman_utils, and for some reason, there is no BE alternative for 16bpp.

> Did you try using a guest with a 16bpp colordepth ? I'd at least add some warning in
> the 16bpp case in these various patches when we are running on a big
> endian machine.

Yes, that will probebly be better, then just leaving it as is.

I'll test the 16bpp, and maybe even try to fix it, if it's fixable
(there doesn't seem to be BE alternative).

Lukas

----- Original Message -----
> From: "Christophe Fergeau" <cfergeau at redhat.com>
> To: "Lukas Venhoda" <lvenhoda at redhat.com>
> Cc: spice-devel at lists.freedesktop.org
> Sent: Wednesday, July 8, 2015 5:33:59 PM
> Subject: Re: [Spice-devel] [spice-common PATCH 4/7 v2] ppc: Fix colors on ppc when using QUIC
> 
> Any reason why there is not a patch fixing canvas_get_jpeg{_alpha} ?
> 
> On Thu, Jul 02, 2015 at 04:01:41PM +0200, Lukas Venhoda wrote:
> > Fixes color order on PowerPC when using QUIC image compression.
> > ---
> > Changes since v1:
> >  - No changes
> > ---
> >  common/canvas_base.c | 6 +++---
> >  1 file changed, 3 insertions(+), 3 deletions(-)
> > 
> > diff --git a/common/canvas_base.c b/common/canvas_base.c
> > index 6f48340..31f3cef 100644
> > --- a/common/canvas_base.c
> > +++ b/common/canvas_base.c
> > @@ -406,19 +406,19 @@ static pixman_image_t *canvas_get_quic(CanvasBase
> > *canvas, SpiceImage *image,
> >      switch (type) {
> >      case QUIC_IMAGE_TYPE_RGBA:
> >          as_type = QUIC_IMAGE_TYPE_RGBA;
> > -        pixman_format = PIXMAN_a8r8g8b8;
> > +        pixman_format = PIXMAN_LE_a8r8g8b8;
> >          break;
> >      case QUIC_IMAGE_TYPE_RGB32:
> >      case QUIC_IMAGE_TYPE_RGB24:
> >          as_type = QUIC_IMAGE_TYPE_RGB32;
> > -        pixman_format = PIXMAN_x8r8g8b8;
> > +        pixman_format = PIXMAN_LE_x8r8g8b8;
> >          break;
> >      case QUIC_IMAGE_TYPE_RGB16:
> >          if (!want_original &&
> >              (canvas->format == SPICE_SURFACE_FMT_32_xRGB ||
> >               canvas->format == SPICE_SURFACE_FMT_32_ARGB)) {
> >              as_type = QUIC_IMAGE_TYPE_RGB32;
> > -            pixman_format = PIXMAN_x8r8g8b8;
> > +            pixman_format = PIXMAN_LE_x8r8g8b8;
> >          } else {
> >              as_type = QUIC_IMAGE_TYPE_RGB16;
> >              pixman_format = PIXMAN_x1r5g5b5;
> 
> I suspect we are not going to properly handle 16bpp canvas ? Did you try
> using a guest with a 16bpp colordepth ? I'd at least add some warning in
> the 16bpp case in these various patches when we are running on a big
> endian machine.
> 
> Christophe
>