On Tue, 2015-11-03 at 11:25 -0500, Frediano Ziglio wrote:
> >
> > On Tue, 2015-11-03 at 10:20 +0000, Frediano Ziglio wrote:
> > > From: Marc-André Lureau <marcandre.lureau@xxxxxxxxx>
> > >
> > > ---
> > > server/Makefile.am | 2 +
> > > server/red_common.h | 13 ----
> > > server/red_parse_qxl.c | 1 +
> > > server/red_worker.c | 158 ++++--------------------------
> > > ----
> > > ----------
> > > server/spice-bitmap-utils.c | 119
> > > +++++++++++++++++++++++++++++++++
> > > server/spice-bitmap-utils.h | 91 +++++++++++++++++++++++++
> > > server/tree.h | 9 +--
> > > 7 files changed, 226 insertions(+), 167 deletions(-)
> > > create mode 100644 server/spice-bitmap-utils.c
> > > create mode 100644 server/spice-bitmap-utils.h
> > >
> > > diff --git a/server/Makefile.am b/server/Makefile.am
> > > index d2a7343..7216ab0 100644
> > > --- a/server/Makefile.am
> > > +++ b/server/Makefile.am
> > > @@ -133,6 +133,8 @@ libspice_server_la_SOURCES =
> > > \
> > > pixmap-cache.c \
> > > tree.h \
> > > tree.c \
> > > + spice-bitmap-utils.h \
> > > + spice-bitmap-utils.c \
> > > utils.h \
> > > $(NULL)
> > >
> > > diff --git a/server/red_common.h b/server/red_common.h
> > > index 47e591d..04d4c02 100644
> > > --- a/server/red_common.h
> > > +++ b/server/red_common.h
> > > @@ -44,17 +44,4 @@ static const LzImageType
> > > MAP_BITMAP_FMT_TO_LZ_IMAGE_TYPE[] = {
> > > LZ_IMAGE_TYPE_A8
> > > };
> > >
> > > -static inline int bitmap_fmt_is_rgb(uint8_t fmt)
> > > -{
> > > - static const int
> > > BITMAP_FMT_IS_RGB[SPICE_BITMAP_FMT_ENUM_END] =
> > > - {0, 0, 0, 0, 0, 0, 1, 1,
> > > 1,
> > > 1, 1};
> > > -
> > > - if (fmt >= SPICE_BITMAP_FMT_ENUM_END) {
> > > - spice_warning("fmt >= SPICE_BITMAP_FMT_ENUM_END; %d >=
> > > %d",
> > > - fmt, SPICE_BITMAP_FMT_ENUM_END);
> > > - return 0;
> > > - }
> > > - return BITMAP_FMT_IS_RGB[fmt];
> > > -}
> > > -
> > > #endif
> > > diff --git a/server/red_parse_qxl.c b/server/red_parse_qxl.c
> > > index dd52602..5fc1a13 100644
> > > --- a/server/red_parse_qxl.c
> > > +++ b/server/red_parse_qxl.c
> > > @@ -23,6 +23,7 @@
> > > #include <inttypes.h>
> > > #include <glib.h>
> > > #include "common/lz_common.h"
> > > +#include "spice-bitmap-utils.h"
> > > #include "red_common.h"
> > > #include "red_memslots.h"
> > > #include "red_parse_qxl.h"
> > > diff --git a/server/red_worker.c b/server/red_worker.c
> > > index 0d0c603..bad8e47 100644
> > > --- a/server/red_worker.c
> > > +++ b/server/red_worker.c
> > > @@ -310,13 +310,6 @@ typedef struct StreamClipItem {
> > > SpiceClipRects *rects;
> > > } StreamClipItem;
> > >
> > > -static const int BITMAP_FMT_IS_PLT[] = {0, 1, 1, 1, 1, 1, 0, 0,
> > > 0,
> > > 0, 0};
> > > -static const int BITMAP_FMP_BYTES_PER_PIXEL[] = {0, 0, 0, 0, 0,
> > > 1,
> > > 2, 3, 4, 4, 1};
> > > -
> > > -#define BITMAP_FMT_HAS_GRADUALITY(f)
> > > \
> > > - (bitmap_fmt_is_rgb(f) &&
> > > \
> > > - ((f) != SPICE_BITMAP_FMT_8BIT_A))
> > > -
> > > typedef struct {
> > > QuicUsrContext usr;
> > > EncoderData data;
> > > @@ -613,10 +606,6 @@ static int
> > > red_display_free_some_independent_glz_drawables(DisplayChannelCli
> > > ent
> > > static void red_display_free_glz_drawable(DisplayChannelClient
> > > *dcc,
> > > RedGlzDrawable *drawable);
> > > static ImageItem
> > > *red_add_surface_area_image(DisplayChannelClient
> > > *dcc, int surface_id,
> > > SpiceRect *area,
> > > PipeItem *pos, int can_lossy);
> > > -static BitmapGradualType _get_bitmap_graduality_level(RedWorker
> > > *worker, SpiceBitmap *bitmap,
> > > - uint32_t
> > > group_id);
> > > -static inline int _stride_is_extra(SpiceBitmap *bitmap);
> > > -
> > > static void
> > > display_channel_client_release_item_before_push(DisplayChannelCli
> > > ent
> > > *dcc,
> > >
> > > PipeItem
> > > *item);
> > > static void
> > > display_channel_client_release_item_after_push(DisplayChannelClie
> > > nt
> > > *dcc,
> > > @@ -2650,12 +2639,11 @@ static inline void
> > > red_update_copy_graduality(RedWorker* worker, Drawable *drawa
> > >
> > > bitmap = &drawable->red_drawable->u.copy.src_bitmap
> > > ->u.bitmap;
> > >
> > > - if (!BITMAP_FMT_HAS_GRADUALITY(bitmap->format) ||
> > > _stride_is_extra(bitmap) ||
> > > + if (!bitmap_fmt_has_graduality(bitmap->format) ||
> > > bitmap_has_extra_stride(bitmap) ||
> > > (bitmap->data->flags & SPICE_CHUNKS_FLAGS_UNSTABLE)) {
> > > drawable->copy_bitmap_graduality =
> > > BITMAP_GRADUAL_NOT_AVAIL;
> > > } else {
> > > - drawable->copy_bitmap_graduality =
> > > - _get_bitmap_graduality_level(worker, bitmap,drawable
> > > ->group_id);
> > > + drawable->copy_bitmap_graduality =
> > > bitmap_get_graduality_level(bitmap);
> > > }
> > > }
> > >
> > > @@ -4902,124 +4890,6 @@ static inline void
> > > red_init_zlib(RedWorker
> > > *worker)
> > > }
> > > }
> > >
> > > -typedef struct {
> > > - uint8_t b;
> > > - uint8_t g;
> > > - uint8_t r;
> > > - uint8_t pad;
> > > -} rgb32_pixel_t;
> > > -
> > > -G_STATIC_ASSERT(sizeof(rgb32_pixel_t) == 4);
> > > -
> > > -typedef struct {
> > > - uint8_t b;
> > > - uint8_t g;
> > > - uint8_t r;
> > > -} rgb24_pixel_t;
> > > -
> > > -G_STATIC_ASSERT(sizeof(rgb24_pixel_t) == 3);
> > > -
> > > -typedef uint16_t rgb16_pixel_t;
> > > -
> > > -#define RED_BITMAP_UTILS_RGB16
> > > -#include "red_bitmap_utils_tmpl.c"
> > > -#define RED_BITMAP_UTILS_RGB24
> > > -#include "red_bitmap_utils_tmpl.c"
> > > -#define RED_BITMAP_UTILS_RGB32
> > > -#include "red_bitmap_utils_tmpl.c"
> > > -
> > > -#define GRADUAL_HIGH_RGB24_TH -0.03
> > > -#define GRADUAL_HIGH_RGB16_TH 0
> > > -
> > > -// setting a more permissive threshold for stream identification
> > > in
> > > order
> > > -// not to miss streams that were artificially scaled on the
> > > guest
> > > (e.g., full screen view
> > > -// in window media player 12). see red_stream_add_frame
> > > -#define GRADUAL_MEDIUM_SCORE_TH 0.002
> > > -
> > > -// assumes that stride doesn't overflow
> > > -static BitmapGradualType _get_bitmap_graduality_level(RedWorker
> > > *worker, SpiceBitmap *bitmap,
> > > - uint32_t
> > > group_id)
> > > -{
> > > - double score = 0.0;
> > > - int num_samples = 0;
> > > - int num_lines;
> > > - double chunk_score = 0.0;
> > > - int chunk_num_samples = 0;
> > > - uint32_t x, i;
> > > - SpiceChunk *chunk;
> > > -
> > > - chunk = bitmap->data->chunk;
> > > - for (i = 0; i < bitmap->data->num_chunks; i++) {
> > > - num_lines = chunk[i].len / bitmap->stride;
> > > - x = bitmap->x;
> > > - switch (bitmap->format) {
> > > - case SPICE_BITMAP_FMT_16BIT:
> > > - compute_lines_gradual_score_rgb16((rgb16_pixel_t
> > > *)chunk[i].data, x, num_lines,
> > > - &chunk_score,
> > > &chunk_num_samples);
> > > - break;
> > > - case SPICE_BITMAP_FMT_24BIT:
> > > - compute_lines_gradual_score_rgb24((rgb24_pixel_t
> > > *)chunk[i].data, x, num_lines,
> > > - &chunk_score,
> > > &chunk_num_samples);
> > > - break;
> > > - case SPICE_BITMAP_FMT_32BIT:
> > > - case SPICE_BITMAP_FMT_RGBA:
> > > - compute_lines_gradual_score_rgb32((rgb32_pixel_t
> > > *)chunk[i].data, x, num_lines,
> > > - &chunk_score,
> > > &chunk_num_samples);
> > > - break;
> > > - default:
> > > - spice_error("invalid bitmap format (not RGB) %u",
> > > bitmap
> > > ->format);
> > > - }
> > > - score += chunk_score;
> > > - num_samples += chunk_num_samples;
> > > - }
> > > -
> > > - spice_assert(num_samples);
> > > - score /= num_samples;
> > > -
> > > - if (bitmap->format == SPICE_BITMAP_FMT_16BIT) {
> > > - if (score < GRADUAL_HIGH_RGB16_TH) {
> > > - return BITMAP_GRADUAL_HIGH;
> > > - }
> > > - } else {
> > > - if (score < GRADUAL_HIGH_RGB24_TH) {
> > > - return BITMAP_GRADUAL_HIGH;
> > > - }
> > > - }
> > > -
> > > - if (score < GRADUAL_MEDIUM_SCORE_TH) {
> > > - return BITMAP_GRADUAL_MEDIUM;
> > > - } else {
> > > - return BITMAP_GRADUAL_LOW;
> > > - }
> > > -}
> > > -
> > > -static inline int _stride_is_extra(SpiceBitmap *bitmap)
> > > -{
> > > - spice_assert(bitmap);
> > > - if (bitmap_fmt_is_rgb(bitmap->format)) {
> > > - return ((bitmap->x * BITMAP_FMP_BYTES_PER_PIXEL[bitmap
> > > ->format]) < bitmap->stride);
> > > - } else {
> > > - switch (bitmap->format) {
> > > - case SPICE_BITMAP_FMT_8BIT:
> > > - return (bitmap->x < bitmap->stride);
> > > - case SPICE_BITMAP_FMT_4BIT_BE:
> > > - case SPICE_BITMAP_FMT_4BIT_LE: {
> > > - int bytes_width = SPICE_ALIGN(bitmap->x, 2) >> 1;
> > > - return bytes_width < bitmap->stride;
> > > - }
> > > - case SPICE_BITMAP_FMT_1BIT_BE:
> > > - case SPICE_BITMAP_FMT_1BIT_LE: {
> > > - int bytes_width = SPICE_ALIGN(bitmap->x, 8) >> 3;
> > > - return bytes_width < bitmap->stride;
> > > - }
> > > - default:
> > > - spice_error("invalid image type %u", bitmap
> > > ->format);
> > > - return 0;
> > > - }
> > > - }
> > > - return 0;
> > > -}
> > > -
> > > typedef struct compress_send_data_t {
> > > void* comp_buf;
> > > uint32_t comp_buf_size;
> > > @@ -5517,7 +5387,7 @@ static inline int
> > > red_compress_image(DisplayChannelClient *dcc,
> > > ((src->y * src->stride) < MIN_SIZE_TO_COMPRESS)) { //
> > > TODO:
> > > change the size cond
> > > return FALSE;
> > > } else if (image_compression ==
> > > SPICE_IMAGE_COMPRESSION_QUIC) {
> > > - if (BITMAP_FMT_IS_PLT[src->format]) {
> > > + if (bitmap_fmt_is_plt(src->format)) {
> > > return FALSE;
> > > } else {
> > > quic_compress = TRUE;
> > > @@ -5527,11 +5397,11 @@ static inline int
> > > red_compress_image(DisplayChannelClient *dcc,
> > > lz doesn't handle (1) bitmaps with strides that are
> > > larger than the width
> > > of the image in bytes (2) unstable bitmaps
> > > */
> > > - if (_stride_is_extra(src) || (src->data->flags &
> > > SPICE_CHUNKS_FLAGS_UNSTABLE)) {
> > > + if (bitmap_has_extra_stride(src) || (src->data->flags &
> > > SPICE_CHUNKS_FLAGS_UNSTABLE)) {
> > > if ((image_compression ==
> > > SPICE_IMAGE_COMPRESSION_LZ) ||
> > > (image_compression ==
> > > SPICE_IMAGE_COMPRESSION_GLZ)
> > > > >
> > > (image_compression ==
> > > SPICE_IMAGE_COMPRESSION_LZ4)
> > > > >
> > > - BITMAP_FMT_IS_PLT[src->format]) {
> > > + bitmap_fmt_is_plt(src->format)) {
> > > return FALSE;
> > > } else {
> > > quic_compress = TRUE;
> > > @@ -5543,10 +5413,8 @@ static inline int
> > > red_compress_image(DisplayChannelClient *dcc,
> > > quic_compress = FALSE;
> > > } else {
> > > if (drawable->copy_bitmap_graduality ==
> > > BITMAP_GRADUAL_INVALID) {
> > > - quic_compress =
> > > BITMAP_FMT_HAS_GRADUALITY(src->format) &&
> > > -
> > > (_get_bitmap_graduality_level(display_channel->common.worker,
> > > src,
> > > -
> > > drawable
> > > ->group_id) ==
> > > - BITMAP_GRADUAL_HIGH);
> > > + quic_compress =
> > > bitmap_fmt_has_graduality(src->format) &&
> > > + bitmap_get_graduality_level(src) ==
> > > BITMAP_GRADUAL_HIGH;
> > > } else {
> > > quic_compress = (drawable
> > > ->copy_bitmap_graduality == BITMAP_GRADUAL_HIGH);
> > > }
> > > @@ -5566,7 +5434,7 @@ static inline int
> > > red_compress_image(DisplayChannelClient *dcc,
> > > ((image_compression ==
> > > SPICE_IMAGE_COMPRESSION_AUTO_LZ)
> > > > >
> > > (image_compression ==
> > > SPICE_IMAGE_COMPRESSION_AUTO_GLZ))) {
> > > // if we use lz for alpha, the stride can't be extra
> > > - if (src->format != SPICE_BITMAP_FMT_RGBA ||
> > > !_stride_is_extra(src)) {
> > > + if (src->format != SPICE_BITMAP_FMT_RGBA ||
> > > !bitmap_has_extra_stride(src)) {
> > > return red_jpeg_compress_image(dcc, dest,
> > > src, o_comp_data,
> > > drawable->group_id);
> > > }
> > > @@ -5578,7 +5446,7 @@ static inline int
> > > red_compress_image(DisplayChannelClient *dcc,
> > > int ret;
> > > if ((image_compression ==
> > > SPICE_IMAGE_COMPRESSION_AUTO_GLZ)
> > > > >
> > > (image_compression == SPICE_IMAGE_COMPRESSION_GLZ))
> > > {
> > > - glz = BITMAP_FMT_HAS_GRADUALITY(src->format) && (
> > > + glz = bitmap_fmt_has_graduality(src->format) && (
> > > (src->x * src->y) <
> > > glz_enc_dictionary_get_size(
> > > dcc->glz_dict->dict));
> > > } else if ((image_compression ==
> > > SPICE_IMAGE_COMPRESSION_AUTO_LZ) ||
> > > @@ -7847,14 +7715,12 @@ static void
> > > red_marshall_image(RedChannelClient *rcc, SpiceMarshaller *m,
> > > ImageI
> > > comp_mode = display_channel->common.worker
> > > ->image_compression;
> > >
> > > if (((comp_mode == SPICE_IMAGE_COMPRESSION_AUTO_LZ) ||
> > > - (comp_mode == SPICE_IMAGE_COMPRESSION_AUTO_GLZ)) &&
> > > !_stride_is_extra(&bitmap)) {
> > > + (comp_mode == SPICE_IMAGE_COMPRESSION_AUTO_GLZ)) &&
> > > !bitmap_has_extra_stride(&bitmap)) {
> > >
> > > - if (BITMAP_FMT_HAS_GRADUALITY(item->image_format)) {
> > > + if (bitmap_fmt_has_graduality(item->image_format)) {
> > > BitmapGradualType grad_level;
> > >
> > > - grad_level =
> > > _get_bitmap_graduality_level(display_channel->common.worker,
> > > - &bitmap,
> > > - worker
> > > ->mem_slots.internal_groupslot_id);
> > > + grad_level = bitmap_get_graduality_level(&bitmap);
> > > if (grad_level == BITMAP_GRADUAL_HIGH) {
> > > // if we use lz for alpha, the stride can't be
> > > extra
> > > lossy_comp = display_channel->enable_jpeg &&
> > > item
> > > ->can_lossy;
> > > diff --git a/server/spice-bitmap-utils.c b/server/spice-bitmap
> > > -utils.c
> > > new file mode 100644
> > > index 0000000..c4ec625
> > > --- /dev/null
> > > +++ b/server/spice-bitmap-utils.c
> > > @@ -0,0 +1,119 @@
> > > +/* -*- Mode: C; c-basic-offset: 4; indent-tabs-mode: nil -*- */
> > > +/*
> > > + Copyright (C) 2009-2015 Red Hat, Inc.
> > > +
> > > + This library is free software; you can redistribute it and/or
> > > + modify it under the terms of the GNU Lesser General Public
> > > + License as published by the Free Software Foundation; either
> > > + version 2.1 of the License, or (at your option) any later
> > > version.
> > > +
> > > + This library is distributed in the hope that it will be
> > > useful,
> > > + but WITHOUT ANY WARRANTY; without even the implied warranty
> > > of
> > > + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> > > GNU
> > > + Lesser General Public License for more details.
> > > +
> > > + You should have received a copy of the GNU Lesser General
> > > Public
> > > + License along with this library; if not, see <
> > > http://www.gnu.org/licenses/>;.
> > > +*/
> > > +#ifdef HAVE_CONFIG_H
> > > +#include <config.h>
> > > +#endif
> > > +#include "spice-bitmap-utils.h"
> > > +
> > > +#define RED_BITMAP_UTILS_RGB16
> > > +#include "red_bitmap_utils_tmpl.c"
> > > +#define RED_BITMAP_UTILS_RGB24
> > > +#include "red_bitmap_utils_tmpl.c"
> > > +#define RED_BITMAP_UTILS_RGB32
> > > +#include "red_bitmap_utils_tmpl.c"
> > > +
> > > +#define GRADUAL_HIGH_RGB24_TH -0.03
> > > +#define GRADUAL_HIGH_RGB16_TH 0
> > > +
> > > +// setting a more permissive threshold for stream identification
> > > in
> > > order
> > > +// not to miss streams that were artificially scaled on the
> > > guest
> > > (e.g., full screen view
> > > +// in window media player 12). see red_stream_add_frame
> > > +#define GRADUAL_MEDIUM_SCORE_TH 0.002
> > > +
> > > +// assumes that stride doesn't overflow
> > > +BitmapGradualType bitmap_get_graduality_level(SpiceBitmap
> > > *bitmap)
> > > +{
> > > + double score = 0.0;
> > > + int num_samples = 0;
> > > + int num_lines;
> > > + double chunk_score = 0.0;
> > > + int chunk_num_samples = 0;
> > > + uint32_t x, i;
> > > + SpiceChunk *chunk;
> > > +
> > > + chunk = bitmap->data->chunk;
> > > + for (i = 0; i < bitmap->data->num_chunks; i++) {
> > > + num_lines = chunk[i].len / bitmap->stride;
> > > + x = bitmap->x;
> > > + switch (bitmap->format) {
> > > + case SPICE_BITMAP_FMT_16BIT:
> > > + compute_lines_gradual_score_rgb16((rgb16_pixel_t
> > > *)chunk[i].data, x, num_lines,
> > > + &chunk_score,
> > > &chunk_num_samples);
> > > + break;
> > > + case SPICE_BITMAP_FMT_24BIT:
> > > + compute_lines_gradual_score_rgb24((rgb24_pixel_t
> > > *)chunk[i].data, x, num_lines,
> > > + &chunk_score,
> > > &chunk_num_samples);
> > > + break;
> > > + case SPICE_BITMAP_FMT_32BIT:
> > > + case SPICE_BITMAP_FMT_RGBA:
> > > + compute_lines_gradual_score_rgb32((rgb32_pixel_t
> > > *)chunk[i].data, x, num_lines,
> > > + &chunk_score,
> > > &chunk_num_samples);
> > > + break;
> > > + default:
> > > + spice_error("invalid bitmap format (not RGB) %u",
> > > bitmap
> > > ->format);
> > > + }
> > > + score += chunk_score;
> > > + num_samples += chunk_num_samples;
> > > + }
> > > +
> > > + spice_assert(num_samples);
> > > + score /= num_samples;
> > > +
> > > + if (bitmap->format == SPICE_BITMAP_FMT_16BIT) {
> > > + if (score < GRADUAL_HIGH_RGB16_TH) {
> > > + return BITMAP_GRADUAL_HIGH;
> > > + }
> > > + } else {
> > > + if (score < GRADUAL_HIGH_RGB24_TH) {
> > > + return BITMAP_GRADUAL_HIGH;
> > > + }
> > > + }
> > > +
> > > + if (score < GRADUAL_MEDIUM_SCORE_TH) {
> > > + return BITMAP_GRADUAL_MEDIUM;
> > > + } else {
> > > + return BITMAP_GRADUAL_LOW;
> > > + }
> > > +}
> > > +
> > > +int bitmap_has_extra_stride(SpiceBitmap *bitmap)
> > > +{
> > > + spice_assert(bitmap);
> > > + if (bitmap_fmt_is_rgb(bitmap->format)) {
> > > + return ((bitmap->x *
> > > bitmap_fmt_get_bytes_per_pixel(bitmap
> > > ->format)) < bitmap->stride);
> > > + } else {
> > > + switch (bitmap->format) {
> > > + case SPICE_BITMAP_FMT_8BIT:
> > > + return (bitmap->x < bitmap->stride);
> > > + case SPICE_BITMAP_FMT_4BIT_BE:
> > > + case SPICE_BITMAP_FMT_4BIT_LE: {
> > > + int bytes_width = SPICE_ALIGN(bitmap->x, 2) >> 1;
> > > + return bytes_width < bitmap->stride;
> > > + }
> > > + case SPICE_BITMAP_FMT_1BIT_BE:
> > > + case SPICE_BITMAP_FMT_1BIT_LE: {
> > > + int bytes_width = SPICE_ALIGN(bitmap->x, 8) >> 3;
> > > + return bytes_width < bitmap->stride;
> > > + }
> > > + default:
> > > + spice_error("invalid image type %u", bitmap
> > > ->format);
> > > + return 0;
> > > + }
> > > + }
> > > + return 0;
> > > +}
> > > diff --git a/server/spice-bitmap-utils.h b/server/spice-bitmap
> > > -utils.h
> > > new file mode 100644
> > > index 0000000..38cb88a
> > > --- /dev/null
> > > +++ b/server/spice-bitmap-utils.h
> > > @@ -0,0 +1,91 @@
> > > +/* -*- Mode: C; c-basic-offset: 4; indent-tabs-mode: nil -*- */
> > > +/*
> > > + Copyright (C) 2009-2015 Red Hat, Inc.
> > > +
> > > + This library is free software; you can redistribute it and/or
> > > + modify it under the terms of the GNU Lesser General Public
> > > + License as published by the Free Software Foundation; either
> > > + version 2.1 of the License, or (at your option) any later
> > > version.
> > > +
> > > + This library is distributed in the hope that it will be
> > > useful,
> > > + but WITHOUT ANY WARRANTY; without even the implied warranty
> > > of
> > > + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> > > GNU
> > > + Lesser General Public License for more details.
> > > +
> > > + You should have received a copy of the GNU Lesser General
> > > Public
> > > + License along with this library; if not, see <
> > > http://www.gnu.org/licenses/>;.
> > > +*/
> > > +#ifndef RED_BITMAP_UTILS_H_
> > > +# define RED_BITMAP_UTILS_H_
> > > +
> > > +#include <glib.h>
> > > +#include <stdint.h>
> > > +#include "common/draw.h"
> > > +#include "common/log.h"
> > > +
> > > +typedef enum {
> > > + BITMAP_GRADUAL_INVALID,
> > > + BITMAP_GRADUAL_NOT_AVAIL,
> > > + BITMAP_GRADUAL_LOW,
> > > + BITMAP_GRADUAL_MEDIUM,
> > > + BITMAP_GRADUAL_HIGH,
> > > +} BitmapGradualType;
> > > +
> > > +typedef struct {
> > > + uint8_t b;
> > > + uint8_t g;
> > > + uint8_t r;
> > > + uint8_t pad;
> > > +} rgb32_pixel_t;
> > > +
> > > +G_STATIC_ASSERT(sizeof(rgb32_pixel_t) == 4);
> > > +
> > > +typedef struct {
> > > + uint8_t b;
> > > + uint8_t g;
> > > + uint8_t r;
> > > +} rgb24_pixel_t;
> > > +
> > > +G_STATIC_ASSERT(sizeof(rgb24_pixel_t) == 3);
> > > +
> > > +typedef uint16_t rgb16_pixel_t;
> > > +
> > > +
> > > +static inline int bitmap_fmt_get_bytes_per_pixel(uint8_t fmt)
> > > +{
> > > + static const int bytes_per_pixel[] = {0, 0, 0, 0, 0, 1, 2,
> > > 3, 4,
> > > 4, 1};
> > > +
> > > + spice_return_val_if_fail(fmt <
> > > SPICE_N_ELEMENTS(bytes_per_pixel), 0);
> > > +
> > > + return bytes_per_pixel[fmt];
> > > +}
> > > +
> > > +
> > > +static inline int bitmap_fmt_is_plt(uint8_t fmt)
> > > +{
> > > + static const int fmt_is_plt[] = {0, 1, 1, 1, 1, 1, 0, 0, 0,
> > > 0,
> > > 0};
> > > +
> > > + spice_return_val_if_fail(fmt < SPICE_N_ELEMENTS(fmt_is_plt),
> > > 0);
> > > +
> > > + return fmt_is_plt[fmt];
> > > +}
> > > +
> > > +static inline int bitmap_fmt_is_rgb(uint8_t fmt)
> > > +{
> > > + static const int fmt_is_rgb[] = {0, 0, 0, 0, 0, 0, 1, 1, 1,
> > > 1,
> > > 1};
> > > +
> > > + spice_return_val_if_fail(fmt < SPICE_N_ELEMENTS(fmt_is_rgb),
> > > 0);
> > > +
> > > + return fmt_is_rgb[fmt];
> > > +}
> > > +
> > > +static inline int bitmap_fmt_has_graduality(uint8_t fmt)
> > > +{
> > > + return bitmap_fmt_is_rgb(fmt) && fmt !=
> > > SPICE_BITMAP_FMT_8BIT_A;
> > > +}
> > > +
> > > +
> > > +BitmapGradualType bitmap_get_graduality_level (SpiceBitmap
> > > *bitmap);
> > > +int bitmap_has_extra_stride (SpiceBitmap
> > > *bitmap);
> > > +
> > > +#endif /* RED_BITMAP_UTILS_H_ */
> > > diff --git a/server/tree.h b/server/tree.h
> > > index 8cd7b05..77f1fbf 100644
> > > --- a/server/tree.h
> > > +++ b/server/tree.h
> > > @@ -21,6 +21,7 @@
> > > #include <stdint.h>
> > > #include "common/region.h"
> > > #include "common/ring.h"
> > > +#include "spice-bitmap-utils.h"
> > >
> > > enum {
> > > TREE_ITEM_TYPE_NONE,
> > > @@ -64,14 +65,6 @@ struct DrawItem {
> > >
> > > #define IS_DRAW_ITEM(item) ((item)->type ==
> > > TREE_ITEM_TYPE_DRAWABLE)
> > >
> > > -typedef enum {
> > > - BITMAP_GRADUAL_INVALID,
> > > - BITMAP_GRADUAL_NOT_AVAIL,
> > > - BITMAP_GRADUAL_LOW,
> > > - BITMAP_GRADUAL_MEDIUM,
> > > - BITMAP_GRADUAL_HIGH,
> > > -} BitmapGradualType;
> > > -
> > > typedef struct DependItem {
> > > Drawable *drawable;
> > > RingItem ring_item;
> >
> >
> >
> > ACK from me. I almost want to suggest changing some of these
> > functions
> > from an int return type to gboolean while we're moving them, but I
> > don't know if it's really important enough to (potentially)
> > introduce
> > rebasing conflicts.
> >
> > Also, it's probably worth mentioning in the commit log that we're
> > removing some unused parameters from bitmap_get_graduality_level()
> >
> > Jonathon
> >
>
> I won't mix move and changes beside small ones like renaming
> following
> the reasoning about move. But another patch could go.
> About rebasing and complexity I think is a balance between effort and
> gain.
> We are improving code so I would spend a bit more if result is much
> better.
> About gboolean I would say that C99 have also a stdbool.h (and
> actually some
> code in spice-server is using it).
>
> Can you suggest something to add to the commit message?
>
> I agree to the ack too.
>
> Frediano
I would just add something like this to the commit log body:
"Also remove some unused function parameters from
bitmap_get_graduality_level()"
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/spice-devel