Re: [Qemu-devel] [PATCH v4] blizzard: Remove support for DEPTH != 32

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 2128 bytes --]

On 03/24/2016 12:18 PM, Pooja Dhannawat wrote:
> Removing support for DEPTH != 32 from blizzard template header
> and file that includes it, as macro DEPTH == 32 only used.
> 
> Signed-off-by: Pooja Dhannawat <dhannawatpooja1@gmail.com>
> ---

>  #define DEPTH 32
>  #include "blizzard_template.h"
>  

> +    assert(surface_bits_per_pixel(surface) == 32)

So this confirms that we only ever cared about DEPTH == 32.

> +
> +    s->line_fn_tab[0] = blizzard_draw_fn_32;
> +    s->line_fn_tab[1] = blizzard_draw_fn_r_32;

But now we are in the situation of having to look in the
blizzard_template.h file...

> +++ b/hw/display/blizzard_template.h
> @@ -19,31 +19,7 @@
>   */
>  
>  #define SKIP_PIXEL(to)         (to += deststep)

> +#if DEPTH == 32
>  # define PIXEL_TYPE            uint32_t
>  # define COPY_PIXEL(to, from)  do { *to = from; SKIP_PIXEL(to); } while (0)
>  # define COPY_PIXEL1(to, from) (*to++ = from)
> @@ -58,9 +34,6 @@
>  static void glue(blizzard_draw_line16_, DEPTH)(PIXEL_TYPE *dest,
>                  const uint16_t *src, unsigned int width)

...and to reconstruct the results of the glue() macros to find the
actual function definitions that we are using.  I think this patch is
okay as a first step, but I think we can go one step further in a
followup patch: remove the glue() magic, delete blizzard_template.h, and
just declare the used functions directly in blizzard.c.

> @@ -74,7 +47,6 @@ static void glue(blizzard_draw_line16_, DEPTH)(PIXEL_TYPE *dest,
>          data >>= 5;
>          COPY_PIXEL1(dest, glue(rgb_to_pixel, DEPTH)(r, g, b));

And even without getting rid of the .h, there are some cleanups you can
do now that DEPTH is a constant.  For example, this line can shorten to:

COPY_PIXEL1(dest, rgb_to_pixel32(r, g, b));

which in turn becomes a bit more legible as:

*dest++ = rgb_to_pixel32(r, g, b);

Since what you have is incrementally better, I'm fine if you add:
Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]