[Spice-devel] [PATCH 10/15] worker s/surfaces_dest/surface_deps

Frediano Ziglio fziglio at redhat.com
Tue Nov 10 01:51:29 PST 2015


> 
> On Mon, 2015-11-09 at 10:36 -0500, Frediano Ziglio wrote:
> > > 
> > > From: Marc-André Lureau <marcandre.lureau at gmail.com>
> > > 
> > > ---
> > >  server/display-channel.h |  2 +-
> > >  server/red_parse_qxl.c   | 10 +++++-----
> > >  server/red_parse_qxl.h   |  2 +-
> > >  server/red_worker.c      | 32 ++++++++++++++++----------------
> > >  4 files changed, 23 insertions(+), 23 deletions(-)
> > > 
> > > diff --git a/server/display-channel.h b/server/display-channel.h
> > > index 5202a2f..c7709ad 100644
> > > --- a/server/display-channel.h
> > > +++ b/server/display-channel.h
> > > @@ -154,7 +154,7 @@ struct Drawable {
> > >      DependItem depend_items[3];
> > >  
> > >      int surface_id;
> > > -    int surfaces_dest[3];
> > > +    int surface_deps[3];
> > >  
> > >      uint32_t process_commands_generation;
> > >  };
> > > diff --git a/server/red_parse_qxl.c b/server/red_parse_qxl.c
> > > index 5fc1a13..2cfd5ea 100644
> > > --- a/server/red_parse_qxl.c
> > > +++ b/server/red_parse_qxl.c
> > > @@ -1028,7 +1028,7 @@ static int
> > > red_get_native_drawable(RedMemSlotInfo
> > > *slots, int group_id,
> > >      red->surface_id       = qxl->surface_id;
> > >  
> > >      for (i = 0; i < 3; i++) {
> > > -        red->surfaces_dest[i] = qxl->surfaces_dest[i];
> > > +        red->surface_deps[i] = qxl->surfaces_dest[i];
> > >          red_get_rect_ptr(&red->surfaces_rects[i], &qxl
> > > ->surfaces_rects[i]);
> > >      }
> > >  
> > > @@ -1110,9 +1110,9 @@ static int
> > > red_get_compat_drawable(RedMemSlotInfo
> > > *slots, int group_id,
> > >      red->self_bitmap = (qxl->bitmap_offset != 0);
> > >      red_get_rect_ptr(&red->self_bitmap_area, &qxl->bitmap_area);
> > >  
> > > -    red->surfaces_dest[0] = -1;
> > > -    red->surfaces_dest[1] = -1;
> > > -    red->surfaces_dest[2] = -1;
> > > +    red->surface_deps[0] = -1;
> > > +    red->surface_deps[1] = -1;
> > > +    red->surface_deps[2] = -1;
> > >  
> > >      red->type = qxl->type;
> > >      switch (red->type) {
> > > @@ -1132,7 +1132,7 @@ static int
> > > red_get_compat_drawable(RedMemSlotInfo
> > > *slots, int group_id,
> > >          break;
> > >      case QXL_COPY_BITS:
> > >          red_get_point_ptr(&red->u.copy_bits.src_pos,
> > >          &qxl->u.copy_bits.src_pos);
> > > -        red->surfaces_dest[0] = 0;
> > > +        red->surface_deps[0] = 0;
> > >          red->surfaces_rects[0].left   = red
> > > ->u.copy_bits.src_pos.x;
> > >          red->surfaces_rects[0].right  = red->u.copy_bits.src_pos.x
> > > +
> > >              (red->bbox.right - red->bbox.left);
> > > diff --git a/server/red_parse_qxl.h b/server/red_parse_qxl.h
> > > index 3adc9fa..87862fa 100644
> > > --- a/server/red_parse_qxl.h
> > > +++ b/server/red_parse_qxl.h
> > > @@ -35,7 +35,7 @@ typedef struct RedDrawable {
> > >      SpiceRect bbox;
> > >      SpiceClip clip;
> > >      uint32_t mm_time;
> > > -    int32_t surfaces_dest[3];
> > > +    int32_t surface_deps[3];
> > >      SpiceRect surfaces_rects[3];
> > >      union {
> > >          SpiceFill fill;
> > > diff --git a/server/red_worker.c b/server/red_worker.c
> > > index 10dfd8b..a8d9aa6 100644
> > > --- a/server/red_worker.c
> > > +++ b/server/red_worker.c
> > > @@ -592,7 +592,7 @@ static inline void
> > > red_handle_drawable_surfaces_client_synced(
> > >      for (x = 0; x < 3; ++x) {
> > >          int surface_id;
> > >  
> > > -        surface_id = drawable->surfaces_dest[x];
> > > +        surface_id = drawable->surface_deps[x];
> > >          if (surface_id != -1) {
> > >              if (dcc->surface_client_created[surface_id] == TRUE) {
> > >                  continue;
> > > @@ -855,13 +855,13 @@ static void remove_depended_item(DependItem
> > > *item)
> > >      ring_remove(&item->ring_item);
> > >  }
> > >  
> > > -static void drawable_unref_surfaces_dest(DisplayChannel *display,
> > > Drawable
> > > *drawable)
> > > +static void drawable_unref_surface_deps(DisplayChannel *display,
> > > Drawable
> > > *drawable)
> > >  {
> > >      int x;
> > >      int surface_id;
> > >  
> > >      for (x = 0; x < 3; ++x) {
> > > -        surface_id = drawable->surfaces_dest[x];
> > > +        surface_id = drawable->surface_deps[x];
> > >          if (surface_id == -1) {
> > >              continue;
> > >          }
> > > @@ -875,7 +875,7 @@ static void
> > > drawable_remove_dependencies(DisplayChannel
> > > *display, Drawable *draw
> > >      int surface_id;
> > >  
> > >      for (x = 0; x < 3; ++x) {
> > > -        surface_id = drawable->surfaces_dest[x];
> > > +        surface_id = drawable->surface_deps[x];
> > >          if (surface_id != -1 && drawable
> > > ->depend_items[x].drawable) {
> > >              remove_depended_item(&drawable->depend_items[x]);
> > >          }
> > > @@ -901,7 +901,7 @@ void
> > > display_channel_drawable_unref(DisplayChannel
> > > *display, Drawable *drawable)
> > >      region_destroy(&drawable->tree_item.base.rgn);
> > >  
> > >      drawable_remove_dependencies(display, drawable);
> > > -    drawable_unref_surfaces_dest(display, drawable);
> > > +    drawable_unref_surface_deps(display, drawable);
> > >      display_channel_surface_unref(display, drawable->surface_id);
> > >  
> > >      RING_FOREACH_SAFE(item, next, &drawable->glz_ring) {
> > > @@ -982,7 +982,7 @@ static void
> > > red_flush_source_surfaces(DisplayChannel
> > > *display, Drawable *drawabl
> > >      int surface_id;
> > >  
> > >      for (x = 0; x < 3; ++x) {
> > > -        surface_id = drawable->surfaces_dest[x];
> > > +        surface_id = drawable->surface_deps[x];
> > >          if (surface_id != -1 && drawable
> > > ->depend_items[x].drawable) {
> > >              remove_depended_item(&drawable->depend_items[x]);
> > >              surface_flush(display, surface_id,
> > >              &drawable->red_drawable->surfaces_rects[x]);
> > > @@ -1101,7 +1101,7 @@ static int
> > > red_clear_surface_drawables_from_pipe(DisplayChannelClient *dcc,
> > > int
> > >          }
> > >  
> > >          for (x = 0; x < 3; ++x) {
> > > -            if (drawable->surfaces_dest[x] == surface_id) {
> > > +            if (drawable->surface_deps[x] == surface_id) {
> > >                  depend_found = TRUE;
> > >                  break;
> > >              }
> > > @@ -2065,7 +2065,7 @@ static inline int
> > > is_drawable_independent_from_surfaces(Drawable *drawable)
> > >      int x;
> > >  
> > >      for (x = 0; x < 3; ++x) {
> > > -        if (drawable->surfaces_dest[x] != -1) {
> > > +        if (drawable->surface_deps[x] != -1) {
> > >              return FALSE;
> > >          }
> > >      }
> > > @@ -2625,8 +2625,8 @@ static Drawable *get_drawable(RedWorker
> > > *worker,
> > > uint8_t effect, RedDrawable *re
> > >          return NULL;
> > >      }
> > >      for (x = 0; x < 3; ++x) {
> > > -        if (red_drawable->surfaces_dest[x] != -1) {
> > > -            VALIDATE_SURFACE_RETVAL(display, red_drawable
> > > ->surfaces_dest[x],
> > > NULL)
> > > +        if (red_drawable->surface_deps[x] != -1) {
> > > +            VALIDATE_SURFACE_RETVAL(display, red_drawable
> > > ->surface_deps[x],
> > > NULL)
> > >          }
> > >      }
> > >  
> > > @@ -2648,7 +2648,7 @@ static Drawable *get_drawable(RedWorker
> > > *worker,
> > > uint8_t effect, RedDrawable *re
> > >      drawable->group_id = group_id;
> > >  
> > >      drawable->surface_id = red_drawable->surface_id;
> > > -    memcpy(drawable->surfaces_dest, red_drawable->surfaces_dest,
> > > sizeof(drawable->surfaces_dest));
> > > +    memcpy(drawable->surface_deps, red_drawable->surface_deps,
> > > sizeof(drawable->surface_deps));
> > >      ring_init(&drawable->pipes);
> > >      ring_init(&drawable->glz_ring);
> > >  
> > > @@ -2696,11 +2696,11 @@ static inline int
> > > red_handle_surfaces_dependencies(DisplayChannel *display, Draw
> > >      for (x = 0; x < 3; ++x) {
> > >          // surface self dependency is handled by shadows in
> > > "current", or by
> > >          // handle_self_bitmap
> > > -        if (drawable->surfaces_dest[x] != drawable->surface_id) {
> > > -            add_to_surface_dependency(display, drawable
> > > ->surfaces_dest[x],
> > > +        if (drawable->surface_deps[x] != drawable->surface_id) {
> > > +            add_to_surface_dependency(display, drawable
> > > ->surface_deps[x],
> > >                                        &drawable->depend_items[x],
> > > drawable);
> > >  
> > > -            if (drawable->surfaces_dest[x] == 0) {
> > > +            if (drawable->surface_deps[x] == 0) {
> > >                  QRegion depend_region;
> > >                  region_init(&depend_region);
> > >                  region_add(&depend_region,
> > >                  &drawable->red_drawable->surfaces_rects[x]);
> > > @@ -2719,7 +2719,7 @@ static inline void
> > > red_inc_surfaces_drawable_dependencies(DisplayChannel *displa
> > >      RedSurface *surface;
> > >  
> > >      for (x = 0; x < 3; ++x) {
> > > -        surface_id = drawable->surfaces_dest[x];
> > > +        surface_id = drawable->surface_deps[x];
> > >          if (surface_id == -1) {
> > >              continue;
> > >          }
> > > @@ -5238,7 +5238,7 @@ static inline int
> > > drawable_depends_on_areas(Drawable
> > > *drawable,
> > >          int dep_surface_id;
> > >  
> > >           for (x = 0; x < 3; ++x) {
> > > -            dep_surface_id = drawable->surfaces_dest[x];
> > > +            dep_surface_id = drawable->surface_deps[x];
> > >              if (dep_surface_id == surface_ids[i]) {
> > >                  if (rect_intersects(&surface_areas[i],
> > >                  &red_drawable->surfaces_rects[x])) {
> > >                      return TRUE;
> > > --
> > > 2.4.3
> > 
> > This patch does exactly as it said.
> > Personally I never understood the dest as it usually means
> > destination but
> > in this case are usually source. dep suggests dependency which is
> > appropriate
> > and also usually array names are plural like surface_deps.
> > 
> > So this patch has my ack.
> > 
> > I'll wait tomorrow for another ack/nack (and I'll assume an ack
> > tomorrow).
> > 
> > Frediano
> 
> Looks fine to me. ACK #2.
> 

Merged

Frediano


More information about the Spice-devel mailing list