[Spice-devel] [PATCH 03/15] worker: move image cache to display
Frediano Ziglio
fziglio at redhat.com
Tue Nov 10 02:58:04 PST 2015
I didn't understand what I did wrong but looks like when I pushed this patch it contained
a new server/stream.h file from another patch...
Should I revert this file change (with a new commit) and merge again on proper patch?
Frediano
> >
> > From: Marc-André Lureau <marcandre.lureau at gmail.com>
> >
> > ---
> > server/display-channel.h | 2 +
> > server/red_worker.c | 119
> > +++++++++++++--------------------------------
> > server/spice_image_cache.c | 60 +++++++++++++++++++++++
> > server/spice_image_cache.h | 19 ++++++--
> > server/stream.h | 6 +--
> > 5 files changed, 110 insertions(+), 96 deletions(-)
> >
> > diff --git a/server/display-channel.h b/server/display-channel.h
> > index 254c3d0..53c5ddc 100644
> > --- a/server/display-channel.h
> > +++ b/server/display-channel.h
> > @@ -298,6 +298,8 @@ struct DisplayChannel {
> > uint32_t next_item_trace;
> > uint64_t streams_size_total;
> >
> > + ImageCache image_cache;
> > +
> > #ifdef RED_STATISTICS
> > uint64_t *cache_hits_counter;
> > uint64_t *add_to_cache_counter;
> > diff --git a/server/red_worker.c b/server/red_worker.c
> > index c460a57..0f405f3 100644
> > --- a/server/red_worker.c
> > +++ b/server/red_worker.c
> > @@ -367,8 +367,6 @@ typedef struct RedWorker {
> >
> > RedMemSlotInfo mem_slots;
> >
> > - ImageCache image_cache;
> > -
> > SpiceImageCompression image_compression;
> > spice_wan_compression_t jpeg_state;
> > spice_wan_compression_t zlib_glz_state;
> > @@ -3133,65 +3131,9 @@ static void image_surface_init(RedWorker *worker)
> > worker->image_surfaces.ops = &image_surfaces_ops;
> > }
> >
> > -static void localize_bitmap(RedWorker *worker, SpiceImage **image_ptr,
> > SpiceImage *image_store,
> > - Drawable *drawable)
> > -{
> > - SpiceImage *image = *image_ptr;
> > -
> > - if (image == NULL) {
> > - spice_assert(drawable != NULL);
> > - spice_assert(drawable->red_drawable->self_bitmap_image != NULL);
> > - *image_ptr = drawable->red_drawable->self_bitmap_image;
> > - return;
> > - }
> > -
> > - if (image_cache_hit(&worker->image_cache, image->descriptor.id)) {
> > - image_store->descriptor = image->descriptor;
> > - image_store->descriptor.type = SPICE_IMAGE_TYPE_FROM_CACHE;
> > - image_store->descriptor.flags = 0;
> > - *image_ptr = image_store;
> > - return;
> > - }
> > -
> > - switch (image->descriptor.type) {
> > - case SPICE_IMAGE_TYPE_QUIC: {
> > - image_store->descriptor = image->descriptor;
> > - image_store->u.quic = image->u.quic;
> > - *image_ptr = image_store;
> > -#ifdef IMAGE_CACHE_AGE
> > - image_store->descriptor.flags |= SPICE_IMAGE_FLAGS_CACHE_ME;
> > -#else
> > - if (image_store->descriptor.width * image->descriptor.height >=
> > 640
> > * 480) {
> > - image_store->descriptor.flags |= SPICE_IMAGE_FLAGS_CACHE_ME;
> > - }
> > -#endif
> > - break;
> > - }
> > - case SPICE_IMAGE_TYPE_BITMAP:
> > - case SPICE_IMAGE_TYPE_SURFACE:
> > - /* nothing */
> > - break;
> > - default:
> > - spice_error("invalid image type");
> > - }
> > -}
> > -
> > -static void localize_brush(RedWorker *worker, SpiceBrush *brush,
> > SpiceImage
> > *image_store)
> > -{
> > - if (brush->type == SPICE_BRUSH_TYPE_PATTERN) {
> > - localize_bitmap(worker, &brush->u.pattern.pat, image_store, NULL);
> > - }
> > -}
> > -
> > -static void localize_mask(RedWorker *worker, SpiceQMask *mask, SpiceImage
> > *image_store)
> > -{
> > - if (mask->bitmap) {
> > - localize_bitmap(worker, &mask->bitmap, image_store, NULL);
> > - }
> > -}
> > -
> > static void red_draw_qxl_drawable(RedWorker *worker, Drawable *drawable)
> > {
> > + DisplayChannel *display = worker->display_channel;
> > RedSurface *surface;
> > SpiceCanvas *canvas;
> > SpiceClip clip = drawable->red_drawable->clip;
> > @@ -3199,7 +3141,7 @@ static void red_draw_qxl_drawable(RedWorker *worker,
> > Drawable *drawable)
> > surface = &worker->surfaces[drawable->surface_id];
> > canvas = surface->context.canvas;
> >
> > - image_cache_aging(&worker->image_cache);
> > + image_cache_aging(&display->image_cache);
> >
> > region_add(&surface->draw_dirty_region,
> > &drawable->red_drawable->bbox);
> >
> > @@ -3207,8 +3149,8 @@ static void red_draw_qxl_drawable(RedWorker *worker,
> > Drawable *drawable)
> > case QXL_DRAW_FILL: {
> > SpiceFill fill = drawable->red_drawable->u.fill;
> > SpiceImage img1, img2;
> > - localize_brush(worker, &fill.brush, &img1);
> > - localize_mask(worker, &fill.mask, &img2);
> > + image_cache_localize_brush(&display->image_cache, &fill.brush,
> > &img1);
> > + image_cache_localize_mask(&display->image_cache, &fill.mask,
> > &img2);
> > canvas->ops->draw_fill(canvas, &drawable->red_drawable->bbox,
> > &clip, &fill);
> > break;
> > @@ -3216,17 +3158,17 @@ static void red_draw_qxl_drawable(RedWorker
> > *worker,
> > Drawable *drawable)
> > case QXL_DRAW_OPAQUE: {
> > SpiceOpaque opaque = drawable->red_drawable->u.opaque;
> > SpiceImage img1, img2, img3;
> > - localize_brush(worker, &opaque.brush, &img1);
> > - localize_bitmap(worker, &opaque.src_bitmap, &img2, drawable);
> > - localize_mask(worker, &opaque.mask, &img3);
> > + image_cache_localize_brush(&display->image_cache, &opaque.brush,
> > &img1);
> > + image_cache_localize(&display->image_cache, &opaque.src_bitmap,
> > &img2, drawable);
> > + image_cache_localize_mask(&display->image_cache, &opaque.mask,
> > &img3);
> > canvas->ops->draw_opaque(canvas, &drawable->red_drawable->bbox,
> > &clip, &opaque);
> > break;
> > }
> > case QXL_DRAW_COPY: {
> > SpiceCopy copy = drawable->red_drawable->u.copy;
> > SpiceImage img1, img2;
> > - localize_bitmap(worker, ©.src_bitmap, &img1, drawable);
> > - localize_mask(worker, ©.mask, &img2);
> > + image_cache_localize(&display->image_cache, ©.src_bitmap,
> > &img1,
> > drawable);
> > + image_cache_localize_mask(&display->image_cache, ©.mask,
> > &img2);
> > canvas->ops->draw_copy(canvas, &drawable->red_drawable->bbox,
> > &clip, ©);
> > break;
> > @@ -3234,7 +3176,7 @@ static void red_draw_qxl_drawable(RedWorker *worker,
> > Drawable *drawable)
> > case QXL_DRAW_TRANSPARENT: {
> > SpiceTransparent transparent =
> > drawable->red_drawable->u.transparent;
> > SpiceImage img1;
> > - localize_bitmap(worker, &transparent.src_bitmap, &img1, drawable);
> > + image_cache_localize(&display->image_cache,
> > &transparent.src_bitmap,
> > &img1, drawable);
> > canvas->ops->draw_transparent(canvas,
> > &drawable->red_drawable->bbox,
> > &clip,
> > &transparent);
> > break;
> > @@ -3242,7 +3184,7 @@ static void red_draw_qxl_drawable(RedWorker *worker,
> > Drawable *drawable)
> > case QXL_DRAW_ALPHA_BLEND: {
> > SpiceAlphaBlend alpha_blend =
> > drawable->red_drawable->u.alpha_blend;
> > SpiceImage img1;
> > - localize_bitmap(worker, &alpha_blend.src_bitmap, &img1, drawable);
> > + image_cache_localize(&display->image_cache,
> > &alpha_blend.src_bitmap,
> > &img1, drawable);
> > canvas->ops->draw_alpha_blend(canvas,
> > &drawable->red_drawable->bbox,
> > &clip,
> > &alpha_blend);
> > break;
> > @@ -3255,8 +3197,8 @@ static void red_draw_qxl_drawable(RedWorker *worker,
> > Drawable *drawable)
> > case QXL_DRAW_BLEND: {
> > SpiceBlend blend = drawable->red_drawable->u.blend;
> > SpiceImage img1, img2;
> > - localize_bitmap(worker, &blend.src_bitmap, &img1, drawable);
> > - localize_mask(worker, &blend.mask, &img2);
> > + image_cache_localize(&display->image_cache, &blend.src_bitmap,
> > &img1, drawable);
> > + image_cache_localize_mask(&display->image_cache, &blend.mask,
> > &img2);
> > canvas->ops->draw_blend(canvas, &drawable->red_drawable->bbox,
> > &clip, &blend);
> > break;
> > @@ -3264,7 +3206,7 @@ static void red_draw_qxl_drawable(RedWorker *worker,
> > Drawable *drawable)
> > case QXL_DRAW_BLACKNESS: {
> > SpiceBlackness blackness = drawable->red_drawable->u.blackness;
> > SpiceImage img1;
> > - localize_mask(worker, &blackness.mask, &img1);
> > + image_cache_localize_mask(&display->image_cache, &blackness.mask,
> > &img1);
> > canvas->ops->draw_blackness(canvas,
> > &drawable->red_drawable->bbox, &clip,
> > &blackness);
> > break;
> > @@ -3272,7 +3214,7 @@ static void red_draw_qxl_drawable(RedWorker *worker,
> > Drawable *drawable)
> > case QXL_DRAW_WHITENESS: {
> > SpiceWhiteness whiteness = drawable->red_drawable->u.whiteness;
> > SpiceImage img1;
> > - localize_mask(worker, &whiteness.mask, &img1);
> > + image_cache_localize_mask(&display->image_cache, &whiteness.mask,
> > &img1);
> > canvas->ops->draw_whiteness(canvas,
> > &drawable->red_drawable->bbox, &clip,
> > &whiteness);
> > break;
> > @@ -3280,7 +3222,7 @@ static void red_draw_qxl_drawable(RedWorker *worker,
> > Drawable *drawable)
> > case QXL_DRAW_INVERS: {
> > SpiceInvers invers = drawable->red_drawable->u.invers;
> > SpiceImage img1;
> > - localize_mask(worker, &invers.mask, &img1);
> > + image_cache_localize_mask(&display->image_cache, &invers.mask,
> > &img1);
> > canvas->ops->draw_invers(canvas,
> > &drawable->red_drawable->bbox, &clip,
> > &invers);
> > break;
> > @@ -3288,9 +3230,9 @@ static void red_draw_qxl_drawable(RedWorker *worker,
> > Drawable *drawable)
> > case QXL_DRAW_ROP3: {
> > SpiceRop3 rop3 = drawable->red_drawable->u.rop3;
> > SpiceImage img1, img2, img3;
> > - localize_brush(worker, &rop3.brush, &img1);
> > - localize_bitmap(worker, &rop3.src_bitmap, &img2, drawable);
> > - localize_mask(worker, &rop3.mask, &img3);
> > + image_cache_localize_brush(&display->image_cache, &rop3.brush,
> > &img1);
> > + image_cache_localize(&display->image_cache, &rop3.src_bitmap,
> > &img2,
> > drawable);
> > + image_cache_localize_mask(&display->image_cache, &rop3.mask,
> > &img3);
> > canvas->ops->draw_rop3(canvas, &drawable->red_drawable->bbox,
> > &clip, &rop3);
> > break;
> > @@ -3298,9 +3240,9 @@ static void red_draw_qxl_drawable(RedWorker *worker,
> > Drawable *drawable)
> > case QXL_DRAW_COMPOSITE: {
> > SpiceComposite composite = drawable->red_drawable->u.composite;
> > SpiceImage src, mask;
> > - localize_bitmap(worker, &composite.src_bitmap, &src, drawable);
> > + image_cache_localize(&display->image_cache, &composite.src_bitmap,
> > &src, drawable);
> > if (composite.mask_bitmap)
> > - localize_bitmap(worker, &composite.mask_bitmap, &mask,
> > drawable);
> > + image_cache_localize(&display->image_cache,
> > &composite.mask_bitmap, &mask, drawable);
> > canvas->ops->draw_composite(canvas, &drawable->red_drawable->bbox,
> > &clip, &composite);
> > break;
> > @@ -3308,7 +3250,7 @@ static void red_draw_qxl_drawable(RedWorker *worker,
> > Drawable *drawable)
> > case QXL_DRAW_STROKE: {
> > SpiceStroke stroke = drawable->red_drawable->u.stroke;
> > SpiceImage img1;
> > - localize_brush(worker, &stroke.brush, &img1);
> > + image_cache_localize_brush(&display->image_cache, &stroke.brush,
> > &img1);
> > canvas->ops->draw_stroke(canvas,
> > &drawable->red_drawable->bbox, &clip,
> > &stroke);
> > break;
> > @@ -3316,8 +3258,8 @@ static void red_draw_qxl_drawable(RedWorker *worker,
> > Drawable *drawable)
> > case QXL_DRAW_TEXT: {
> > SpiceText text = drawable->red_drawable->u.text;
> > SpiceImage img1, img2;
> > - localize_brush(worker, &text.fore_brush, &img1);
> > - localize_brush(worker, &text.back_brush, &img2);
> > + image_cache_localize_brush(&display->image_cache,
> > &text.fore_brush,
> > &img1);
> > + image_cache_localize_brush(&display->image_cache,
> > &text.back_brush,
> > &img2);
> > canvas->ops->draw_text(canvas, &drawable->red_drawable->bbox,
> > &clip, &text);
> > break;
> > @@ -7714,10 +7656,11 @@ static void red_migrate_display(DisplayChannel
> > *display, RedChannelClient *rcc)
> > static SpiceCanvas *create_ogl_context_common(RedWorker *worker, OGLCtx
> > *ctx, uint32_t width,
> > uint32_t height, int32_t
> > stride, uint8_t depth)
> > {
> > + DisplayChannel *display = worker->display_channel;
> > SpiceCanvas *canvas;
> >
> > oglctx_make_current(ctx);
> > - if (!(canvas = gl_canvas_create(width, height, depth,
> > &worker->image_cache.base,
> > + if (!(canvas = gl_canvas_create(width, height, depth,
> > &display->image_cache.base,
> > &worker->image_surfaces, NULL, NULL,
> > NULL))) {
> > return NULL;
> > }
> > @@ -7769,13 +7712,14 @@ static inline void
> > *create_canvas_for_surface(RedWorker *worker, RedSurface *sur
> > uint32_t renderer, uint32_t
> > width, uint32_t height,
> > int32_t stride, uint32_t
> > format, void *line_0)
> > {
> > + DisplayChannel *display = worker->display_channel;
> > SpiceCanvas *canvas;
> >
> > switch (renderer) {
> > case RED_RENDERER_SW:
> > canvas = canvas_create_for_data(width, height, format,
> > line_0, stride,
> > - &worker->image_cache.base,
> > + &display->image_cache.base,
> > &worker->image_surfaces, NULL,
> > NULL,
> > NULL);
> > surface->context.top_down = TRUE;
> > surface->context.canvas_draws_on_surface = TRUE;
> > @@ -8871,6 +8815,7 @@ static void display_channel_create(RedWorker *worker,
> > int migrate)
> > memcpy(display_channel->renderers, renderers,
> > sizeof(display_channel->renderers));
> > display_channel->renderer = RED_RENDERER_INVALID;
> > init_streams(display_channel);
> > + image_cache_init(&display_channel->image_cache);
> > }
> >
> > static void guest_set_client_capabilities(RedWorker *worker)
> > @@ -9487,7 +9432,10 @@ void handle_dev_reset_cursor(void *opaque, void
> > *payload)
> >
> > void handle_dev_reset_image_cache(void *opaque, void *payload)
> > {
> > - image_cache_reset(&((RedWorker *)opaque)->image_cache);
> > + RedWorker *worker = opaque;
> > + DisplayChannel *display = worker->display_channel;
> > +
> > + image_cache_reset(&display->image_cache);
> > }
> >
> > void handle_dev_destroy_surface_wait_async(void *opaque, void *payload)
> > @@ -9999,7 +9947,6 @@ RedWorker* red_worker_new(QXLInstance *qxl,
> > RedDispatcher *red_dispatcher)
> > worker->zlib_glz_state = zlib_glz_state;
> > worker->driver_cap_monitors_config = 0;
> > ring_init(&worker->current_list);
> > - image_cache_init(&worker->image_cache);
> > image_surface_init(worker);
> > drawables_init(worker);
> > stat_init(&worker->add_stat, add_stat_name);
> > diff --git a/server/spice_image_cache.c b/server/spice_image_cache.c
> > index 4f62a47..2221198 100644
> > --- a/server/spice_image_cache.c
> > +++ b/server/spice_image_cache.c
> > @@ -19,6 +19,8 @@
> > #include <config.h>
> > #endif
> > #include "spice_image_cache.h"
> > +#include "red_parse_qxl.h"
> > +#include "display-channel.h"
> >
> > static ImageCacheItem *image_cache_find(ImageCache *cache, uint64_t id)
> > {
> > @@ -153,3 +155,61 @@ void image_cache_aging(ImageCache *cache)
> > }
> > #endif
> > }
> > +
> > +/* TODO: review usage of this fn, simplify */
> > +void image_cache_localize(ImageCache *cache, SpiceImage **image_ptr,
> > + SpiceImage *image_store, Drawable *drawable)
> > +{
> > + SpiceImage *image = *image_ptr;
> > +
> > + if (image == NULL) {
> > + spice_assert(drawable != NULL);
> > + spice_assert(drawable->red_drawable->self_bitmap_image != NULL);
> > + *image_ptr = drawable->red_drawable->self_bitmap_image;
> > + return;
> > + }
> > +
> > + if (image_cache_hit(cache, image->descriptor.id)) {
> > + image_store->descriptor = image->descriptor;
> > + image_store->descriptor.type = SPICE_IMAGE_TYPE_FROM_CACHE;
> > + image_store->descriptor.flags = 0;
> > + *image_ptr = image_store;
> > + return;
> > + }
> > +
> > + switch (image->descriptor.type) {
> > + case SPICE_IMAGE_TYPE_QUIC: {
> > + image_store->descriptor = image->descriptor;
> > + image_store->u.quic = image->u.quic;
> > + *image_ptr = image_store;
> > +#ifdef IMAGE_CACHE_AGE
> > + image_store->descriptor.flags |= SPICE_IMAGE_FLAGS_CACHE_ME;
> > +#else
> > + if (image_store->descriptor.width * image->descriptor.height >=
> > 640
> > * 480) {
> > + image_store->descriptor.flags |= SPICE_IMAGE_FLAGS_CACHE_ME;
> > + }
> > +#endif
> > + break;
> > + }
> > + case SPICE_IMAGE_TYPE_BITMAP:
> > + case SPICE_IMAGE_TYPE_SURFACE:
> > + /* nothing */
> > + break;
> > + default:
> > + spice_error("invalid image type");
> > + }
> > +}
> > +
> > +void image_cache_localize_brush(ImageCache *cache, SpiceBrush *brush,
> > SpiceImage *image_store)
> > +{
> > + if (brush->type == SPICE_BRUSH_TYPE_PATTERN) {
> > + image_cache_localize(cache, &brush->u.pattern.pat, image_store,
> > NULL);
> > + }
> > +}
> > +
> > +void image_cache_localize_mask(ImageCache *cache, SpiceQMask *mask,
> > SpiceImage *image_store)
> > +{
> > + if (mask->bitmap) {
> > + image_cache_localize(cache, &mask->bitmap, image_store, NULL);
> > + }
> > +}
> > diff --git a/server/spice_image_cache.h b/server/spice_image_cache.h
> > index 07ecefb..6d6b32d 100644
> > --- a/server/spice_image_cache.h
> > +++ b/server/spice_image_cache.h
> > @@ -22,9 +22,12 @@
> >
> > #include "common/pixman_utils.h"
> > #include "common/canvas_base.h"
> > -
> > #include "common/ring.h"
> >
> > +/* FIXME: move back to display_channel.h (once structs are private) */
> > +typedef struct Drawable Drawable;
> > +typedef struct DisplayChannelClient DisplayChannelClient;
> > +
> > typedef struct ImageCacheItem {
> > RingItem lru_link;
> > uint64_t id;
> > @@ -48,9 +51,15 @@ typedef struct ImageCache {
> > #endif
> > } ImageCache;
> >
> > -int image_cache_hit(ImageCache *cache, uint64_t id);
> > -void image_cache_init(ImageCache *cache);
> > -void image_cache_reset(ImageCache *cache);
> > -void image_cache_aging(ImageCache *cache);
> > +int image_cache_hit (ImageCache *cache, uint64_t
> > id);
> > +void image_cache_init (ImageCache *cache);
> > +void image_cache_reset (ImageCache *cache);
> > +void image_cache_aging (ImageCache *cache);
> > +void image_cache_localize (ImageCache *cache, SpiceImage
> > **image_ptr,
> > + SpiceImage *image_store,
> > Drawable *drawable);
> > +void image_cache_localize_brush (ImageCache *cache, SpiceBrush
> > *brush,
> > + SpiceImage *image_store);
> > +void image_cache_localize_mask (ImageCache *cache, SpiceQMask
> > *mask,
> > + SpiceImage *image_store);
> >
> > #endif
> > diff --git a/server/stream.h b/server/stream.h
> > index 4b85fb8..5500414 100644
> > --- a/server/stream.h
> > +++ b/server/stream.h
> > @@ -23,11 +23,7 @@
> > #include "mjpeg_encoder.h"
> > #include "common/region.h"
> > #include "red_channel.h"
> > -
> > -/* FIXME: move back to display_channel.h (once structs are private) */
> > -typedef struct Drawable Drawable;
> > -typedef struct DisplayChannelClient DisplayChannelClient;
> > -
> > +#include "spice_image_cache.h"
> >
> > #define RED_STREAM_DETACTION_MAX_DELTA ((1000 * 1000 * 1000) / 5) // 1/5
> > sec
> > #define RED_STREAM_CONTINUS_MAX_DELTA (1000 * 1000 * 1000)
> > --
> > 2.4.3
> >
> Merged
>
> Frediano
More information about the Spice-devel
mailing list