[Spice-devel] [common PATCH 1/9 v4] ppc: Always convert surface on BE machine

[Christophe Fergeau]

Hey,

On Wed, Nov 25, 2015 at 05:14:27PM +0100, Lukas Venhoda wrote:
> The newly created surface can be converted, if saved_want_original
> equals TRUE.
> 
> On BE machine we want to always convert, in order to properly byteswap
> colors from LE order to BE order.
> 
> This is done by first creating BGRA/X surface with LE order data, and
> then copying the data into a new A/XRGB surface. The copy process will
> automatically byteswap the colors into A/XRGB format.
> ---
> Changes since v3:
>  - New file
> ---
>  common/canvas_base.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/common/canvas_base.c b/common/canvas_base.c
> index 6747465..30baa22 100644
> --- a/common/canvas_base.c
> +++ b/common/canvas_base.c
> @@ -1116,7 +1116,9 @@ static pixman_image_t *canvas_get_image_internal(CanvasBase *canvas, SpiceImage
>      SpiceImageDescriptor *descriptor = &image->descriptor;
>      pixman_image_t *surface, *converted;
>      pixman_format_code_t wanted_format, surface_format;
> +#ifndef WORDS_BIGENDIAN
>      int saved_want_original;
> +#endif
> 
>      /* When touching, only really allocate if we need to cache, or
>       * if we're loading a GLZ stream (since those need inter-thread communication
> @@ -1132,7 +1134,9 @@ static pixman_image_t *canvas_get_image_internal(CanvasBase *canvas, SpiceImage
>          return NULL;
>      }
> 
> +#ifndef WORDS_BIGENDIAN
>      saved_want_original = want_original;
> +#endif

Rather than having #ifndef everywhere, I think this bit could be changed
to
+#ifndef WORDS_BIGENDIAN
      saved_want_original = want_original;
+#else
+        /* On BE machines, we want to explicitly convert the surface all the time */
+     saved_want_original = TRUE;
+#endif

I'm wondering if this could cause issues when blitting transparent
surfaces to a 16bpp canvas, as the canvas_get_image_internal documentation
mentions this as a usecase (and the only caller with want_original =
TRUE is canvas_draw_alpha_blend)
(NB: my understanding is that "want_original" really means
"keep_original_image_format")

Christophe

>      if (descriptor->flags & SPICE_IMAGE_FLAGS_CACHE_ME
>  #ifdef SW_CANVAS_CACHE
>          || descriptor->flags & SPICE_IMAGE_FLAGS_CACHE_REPLACE_ME
> @@ -1257,12 +1261,15 @@ static pixman_image_t *canvas_get_image_internal(CanvasBase *canvas, SpiceImage
>          return NULL;
>      }
> 
> +#ifndef WORDS_BIGENDIAN
>      if (!saved_want_original) {
>          /* Conversion to canvas format was requested, but maybe it didn't
>             happen above (due to save/load to cache for instance, or
>             maybe the reader didn't support conversion).
>             If so we convert here. */
> +#endif
> 
> +        /* On BE machines, we want to explicitly convert the surface all the time */
>          wanted_format = canvas_get_target_format(canvas,
>                                                   surface_format == PIXMAN_a8r8g8b8);
> 
> @@ -1285,7 +1292,9 @@ static pixman_image_t *canvas_get_image_internal(CanvasBase *canvas, SpiceImage
>              pixman_image_unref (surface);
>              surface = converted;
>          }
> +#ifndef WORDS_BIGENDIAN
>      }
> +#endif
> 
>      return surface;
>  }
> --
> 2.5.0
> 
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/spice-devel
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/spice-devel/attachments/20151211/aeb852f6/attachment.sig>