Re: [PATCH 03/15] worker: move image cache to display

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 

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@xxxxxxxxx>

---
  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, &copy.src_bitmap, &img1, drawable);
-        localize_mask(worker, &copy.mask, &img2);
+        image_cache_localize(&display->image_cache, &copy.src_bitmap,
&img1,
drawable);
+        image_cache_localize_mask(&display->image_cache, &copy.mask,
&img2);
          canvas->ops->draw_copy(canvas, &drawable->red_drawable->bbox,
                                 &clip, &copy);
          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@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/spice-devel



_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/spice-devel
[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]     [Monitors]