On Wed, 2016-06-15 at 10:37 +0100, Frediano Ziglio wrote:
> Remove some coupling, we mainly need to store a list of RedGlzDrawables.
> Signed-off-by: Frediano Ziglio <fziglio@xxxxxxxxxx>
> ---
> server/dcc-encoders.c | 8 ++++----
> server/dcc-encoders.h | 4 ++--
> server/display-channel.c | 4 ++--
> 3 files changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/server/dcc-encoders.c b/server/dcc-encoders.c
> index 9916d41..6fccb17 100644
> --- a/server/dcc-encoders.c
> +++ b/server/dcc-encoders.c
> @@ -612,20 +612,20 @@ void image_encoders_free_glz_drawables(ImageEncoders
> *enc)
> pthread_rwlock_unlock(&glz_dict->encode_lock);
> }
>
> -void drawable_free_glz_drawables(struct Drawable *drawable)
> +void drawable_ring_free_glz_drawables(Ring *drawable_ring)
> {
> RingItem *glz_item, *next_item;
> RedGlzDrawable *glz;
> - DRAWABLE_FOREACH_GLZ_SAFE(drawable, glz_item, next_item, glz) {
> + SAFE_FOREACH(glz_item, next_item, TRUE, drawable_ring, glz,
> LINK_TO_GLZ(glz_item)) {
> red_glz_drawable_free(glz);
> }
> }
Hmm, after this change, there's really nothing related to Drawable here. The
function can free any ring of RedGlzDrawables. So I might drop the drawable_
from the function name: ring_free_glz_drawables() or free_glz_drawables() or
even red_glz_drawable_free_many() or something similar.
>
> -void drawable_detach_glz_drawables(struct Drawable *drawable)
> +void drawable_ring_detach_glz_drawables(Ring *drawable_ring)
> {
> RingItem *item, *next;
>
> - RING_FOREACH_SAFE(item, next, &drawable->glz_ring) {
> + RING_FOREACH_SAFE(item, next, drawable_ring) {
> SPICE_CONTAINEROF(item, RedGlzDrawable, drawable_link)->has_drawable
> = FALSE;
> ring_remove(item);
> }
Same naming comment here.
> diff --git a/server/dcc-encoders.h b/server/dcc-encoders.h
> index e307383..a38de41 100644
> --- a/server/dcc-encoders.h
> +++ b/server/dcc-encoders.h
> @@ -48,8 +48,8 @@ void
> image_encoders_free_glz_drawables_to_free(ImageEncoders* enc);
> gboolean image_encoders_glz_create(ImageEncoders *enc, uint8_t id);
> void image_encoders_glz_get_restore_data(ImageEncoders *enc,
> uint8_t *out_id,
> GlzEncDictRestoreData *out_data);
> -void drawable_free_glz_drawables(struct Drawable *drawable);
> -void drawable_detach_glz_drawables(struct Drawable *drawable);
> +void drawable_ring_free_glz_drawables(Ring *drawable_ring);
> +void drawable_ring_detach_glz_drawables(Ring *drawable_ring);
>
> #define RED_COMPRESS_BUF_SIZE (1024 * 64)
> struct RedCompressBuf {
> diff --git a/server/display-channel.c b/server/display-channel.c
> index fae2e25..1b5a03a 100644
> --- a/server/display-channel.c
> +++ b/server/display-channel.c
> @@ -1179,7 +1179,7 @@ static bool free_one_drawable(DisplayChannel *display,
> int force_glz_free)
>
> drawable = SPICE_CONTAINEROF(ring_item, Drawable, list_link);
> if (force_glz_free) {
> - drawable_free_glz_drawables(drawable);
> + drawable_ring_free_glz_drawables(&drawable->glz_ring);
> }
> drawable_draw(display, drawable);
> container = drawable->tree_item.base.container;
> @@ -1348,7 +1348,7 @@ void drawable_unref(Drawable *drawable)
> drawable_unref_surface_deps(display, drawable);
> display_channel_surface_unref(display, drawable->surface_id);
>
> - drawable_detach_glz_drawables(drawable);
> + drawable_ring_detach_glz_drawables(&drawable->glz_ring);
>
> if (drawable->red_drawable) {
> red_drawable_unref(drawable->red_drawable);
Other than the naming issue, looks fine
Acked-by: Jonathon Jongsma <jjongsma@xxxxxxxxxx>
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel