[Spice-devel] [PATCH 03/15] worker: move image cache to display
Uri Lublin
uril at redhat.com
Tue Nov 10 05:23:02 PST 2015
On 11/10/2015 12:58 PM, Frediano Ziglio wrote:
> 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...
Hi Frediano,
I was looking into that patch too.
I think it has to come after the first patch
"[PATCH 01/15] worker: move stream to display channel"
that created the server/stream.h file.
I guess what happened was that you 'git add' all files changed by this
patch, and the whole file got added.
>
> Should I revert this file change (with a new commit) and merge again on proper patch?
Or just revert the whole patch, and re-add it after PATCH 1 is accepted.
Uri.
>
>>>
>>> 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
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/spice-devel
>
More information about the Spice-devel
mailing list