----- Original Message -----
> On Thu, Oct 03, 2013 at 08:51:15AM -0400, Marc-André Lureau wrote:
> >
> >
> > ----- Original Message -----
> > > On Thu, Oct 03, 2013 at 01:53:55PM +0200, Marc-André Lureau wrote:
> > > > palette_get() used to return a ref, and palette_release() used to
> > > > release that ref.
> > > >
> > > > Since ed877341, the palette is no longer refcount'ed, since its usage
> > > > is
> > > > exclusively local in common/canvas code.
> > > >
> > > > palette_release() shouldn't remove the palette from the cache.
> > >
> > > Doesn't this mean we will keep some palettes in the cache forever? If
> > > yes,
> > > some guarantee that the memory used by these palettes is bounded?
> >
> > palette_get() / palette_release() always come in pair
>
> Ah right, looks like we can even go with removal then:
Sure, but you'll have to fix server code too then.
> diff --git a/common/canvas_base.c b/common/canvas_base.c
> index 2753fae..1fb89d2 100644
> --- a/common/canvas_base.c
> +++ b/common/canvas_base.c
> @@ -942,10 +942,6 @@ static pixman_image_t *canvas_get_bits(CanvasBase
> *canvas,
>
> surface = canvas_bitmap_to_surface(canvas, bitmap, palette,
> want_original);
>
> - if (palette && (bitmap->flags & SPICE_BITMAP_FLAGS_PAL_FROM_CACHE)) {
> - canvas->palette_cache->ops->release(canvas->palette_cache,
> palette);
> - }
> -
> return surface;
> }
>
> diff --git a/common/canvas_base.h b/common/canvas_base.h
> index 637cdc1..10b855c 100644
> --- a/common/canvas_base.h
> +++ b/common/canvas_base.h
> @@ -77,8 +77,6 @@ typedef struct {
> SpicePalette *palette);
> SpicePalette *(*get)(SpicePaletteCache *cache,
> uint64_t id);
> - void (*release)(SpicePaletteCache *cache,
> - SpicePalette *palette);
> } SpicePaletteCacheOps;
>
> struct _SpicePaletteCache {
>
> diff --git a/gtk/channel-display.c b/gtk/channel-display.c
> index 794f4eb..0a91a5e 100644
> --- a/gtk/channel-display.c
> +++ b/gtk/channel-display.c
> @@ -559,11 +559,6 @@ static void palette_remove(SpicePaletteCache *cache,
> uint32
> cache_remove(c->palettes, id);
> }
>
> -static void palette_release(SpicePaletteCache *cache, SpicePalette
> *palette)
> -{
> - palette_remove(cache, palette->unique);
> -}
> -
> #ifdef SW_CANVAS_CACHE
> static void image_put_lossy(SpiceImageCache *cache, uint64_t id,
> pixman_image_t *surface)
> @@ -625,7 +620,6 @@ static SpiceImageCacheOps image_cache_ops = {
> static SpicePaletteCacheOps palette_cache_ops = {
> .put = palette_put,
> .get = palette_get,
> - .release = palette_release,
> };
>
>
> > And a palette is remove from cache with
> > SPICE_MSG_DISPLAY_INVAL_{ALL_}PALETTE
>
> Hopefully canvas_get_bits() and handling of
> SPICE_MSG_DISPLAY_INVAL_ALL_PALETTE can't race with each other?
Not in spice-gtk, since canvas operations are not async/reordered or multi-threaded.
> Christophe
>
> _______________________________________________
> Spice-devel mailing list
> Spice-devel@xxxxxxxxxxxxxxxxxxxxx
> http://lists.freedesktop.org/mailman/listinfo/spice-devel
>
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/spice-devel