[Spice-devel] [common PATCH 3/8 v3] ppc: Added supprt for bigendian color byte order

[Lukas Venhoda]

Hi

On Mon, Oct 26, 2015 at 2:17 PM, Christophe Fergeau <cfergeau at redhat.com>
wrote:

> I'm still a bit worried whether we are missing a more generic fix here
> :-/ the non-LE surfaces come from canvas_get_as_surface /
> canvas_put_image I guess?
>

So I was looking into this, and it turns out, that with my PPC fixes, I
actually call the
surface_create function twice. Once from the canvas_get_(quic|lz|lz4...)
wrapper functions
for the decoders, and once again in the canvas_get_image_internal()
function, which handles
what type of compression to use, and whether to use the alpha channel.

What hapenns is basically this:
1. canvas_get_image_internal() decides which compression to use.
2. One of the wrapper functions decides what kind of image to use (ARGB,
XRGB, 16bit etc...).
3. Inside the wrapper function, we create a BE order surface (FE
PIXMAN_b8g8r8a8).
4. Just before returning from canvas_get_image_internal(), we check if the
new surfaces format
    matches the format we actually want.
5. The "wanted_format" is in LE, so it obviously doesn't match our BE
"surface_format".
6. Because of this, a new LE order surface is created.
7. Then we copy the data from BE surface to LE surface using
pixman_image_composite32(),
    which byteswaps the data automatically using the surface's format.

The code that handles the conversion looks like this:
(spice-common, canvas_base.c, canvas_get_image_internal())

    if (!saved_want_original) {
        wanted_format = canvas_get_target_format(canvas,
                                                 surface_format ==
PIXMAN_a8r8g8b8);

        if (surface_format != wanted_format) {
            converted = surface_create(
                                       wanted_format,
                                       pixman_image_get_width(surface),
                                       pixman_image_get_height(surface),
                                       TRUE);
            pixman_image_composite32 (PIXMAN_OP_SRC,
                                      surface, NULL, converted,
                                      0, 0,
                                      0, 0,
                                      0, 0,
                                      pixman_image_get_width(surface),
                                      pixman_image_get_height(surface));
            pixman_image_unref (surface);
            surface = converted;
        }
    }

Proper way to fix this would be to rewrite the decoders for every image
compression, but I think
this is fine for the few cases someone actually needs to use a BE machine.

We have 2 possible ways to fix this:
1. Reuse this part of code, and just convert EVERY time on BE machine
    (instead of checking for saved_want_original)
2. Create a new function, that converts the surface before this part of
code.
    (This could end up with surface_create being called 3 times)
-- 
Lukas Venhoda
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/spice-devel/attachments/20151119/3c42931f/attachment.html>