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 <dhannawatpoo...@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 <ebl...@redhat.com>
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
