[Spice-devel] [PATCH 4/5] DisplayChannel: use proper function name conventions

[Frediano Ziglio]

> 
> On Fri, 2017-02-10 at 06:57 -0500, Frediano Ziglio wrote:
> > > 
> > > Change current_add_drawable() to display_channel_add_drawable() and
> > > current_remove_drawable() to display_channel_remove_drawable().
> > > ---
> > >  server/display-channel.c | 28 ++++++++++++++--------------
> > >  1 file changed, 14 insertions(+), 14 deletions(-)
> > > 
> > > diff --git a/server/display-channel.c b/server/display-channel.c
> > > index e12751b..ed00740 100644
> > > --- a/server/display-channel.c
> > > +++ b/server/display-channel.c
> > > @@ -379,8 +379,8 @@ static void
> > > pipes_add_drawable_after(DisplayChannel
> > > *display,
> > >      }
> > >  }
> > >  
> > > -static void current_add_drawable(DisplayChannel *display,
> > > -                                 Drawable *drawable, RingItem
> > > *pos)
> > > +static void display_channel_add_drawable(DisplayChannel *display,
> > > +                                         Drawable *drawable,
> > > RingItem *pos)
> > >  {
> > >      RedSurface *surface;
> > >      uint32_t surface_id = drawable->surface_id;
> > > @@ -394,7 +394,7 @@ static void current_add_drawable(DisplayChannel
> > > *display,
> > >  
> > >  /* Unrefs the drawable and removes it from any rings that it's in,
> > > as well
> > >  as
> > >   * removing any associated shadow item */
> > > -static void current_remove_drawable(DisplayChannel *display,
> > > Drawable *item)
> > > +static void display_channel_remove_drawable(DisplayChannel
> > > *display,
> > > Drawable *item)
> > >  {
> > >      /* todo: move all to unref? */
> > >      stream_trace_add_drawable(display, item);
> > > @@ -436,7 +436,7 @@ static void current_remove(DisplayChannel
> > > *display,
> > > TreeItem *item)
> > >              Drawable *drawable = SPICE_CONTAINEROF(now, Drawable,
> > >              tree_item.base);
> > >              ring_item = now->siblings_link.prev;
> > >              drawable_remove_from_pipes(drawable);
> > > -            current_remove_drawable(display, drawable);
> > > +            display_channel_remove_drawable(display, drawable);
> > >          } else {
> > >              Container *now_as_container = CONTAINER(now);
> > >  
> > > @@ -523,9 +523,9 @@ static int current_add_equal(DisplayChannel
> > > *display,
> > > DrawItem *item, TreeItem *
> > >          int add_after = !!other_drawable->stream &&
> > >                          is_drawable_independent_from_surfaces(draw
> > > able);
> > >          stream_maintenance(display, drawable, other_drawable);
> > > -        current_add_drawable(display, drawable, &other-
> > > >siblings_link);
> > > +        display_channel_add_drawable(display, drawable,
> > > &other->siblings_link);
> > >          other_drawable->refs++;
> > > -        current_remove_drawable(display, other_drawable);
> > > +        display_channel_remove_drawable(display, other_drawable);
> > >          if (add_after) {
> > >              pipes_add_drawable_after(display, drawable,
> > > other_drawable);
> > >          } else {
> > > @@ -545,7 +545,7 @@ static int current_add_equal(DisplayChannel
> > > *display,
> > > DrawItem *item, TreeItem *
> > >              GListIter iter;
> > >  
> > >              other_drawable->refs++;
> > > -            current_remove_drawable(display, other_drawable);
> > > +            display_channel_remove_drawable(display,
> > > other_drawable);
> > >  
> > >              /* sending the drawable to clients that already
> > > received
> > >               * (or will receive) other_drawable */
> > > @@ -567,9 +567,9 @@ static int current_add_equal(DisplayChannel
> > > *display,
> > > DrawItem *item, TreeItem *
> > >          break;
> > >      case QXL_EFFECT_OPAQUE_BRUSH:
> > >          if (is_same_geometry(drawable, other_drawable)) {
> > > -            current_add_drawable(display, drawable, &other-
> > > >siblings_link);
> > > +            display_channel_add_drawable(display, drawable,
> > > &other->siblings_link);
> > >              drawable_remove_from_pipes(other_drawable);
> > > -            current_remove_drawable(display, other_drawable);
> > > +            display_channel_remove_drawable(display,
> > > other_drawable);
> > >              pipes_add_drawable(display, drawable);
> > >              return TRUE;
> > >          }
> > > @@ -744,7 +744,7 @@ static int
> > > current_add_with_shadow(DisplayChannel
> > > *display, Ring *ring, Drawable
> > >      /* Prepend the draw item to the beginning of the current ring.
> > > NOTE:
> > >      this
> > >       * means that the drawable is placed *before* its associated
> > > shadow in
> > >       the
> > >       * tree. Changing this order will violate several unstated
> > > assumptions
> > >       */
> > > -    current_add_drawable(display, item, ring);
> > > +    display_channel_add_drawable(display, item, ring);
> > >      if (item->tree_item.effect == QXL_EFFECT_OPAQUE) {
> > >          QRegion exclude_rgn;
> > >          region_clone(&exclude_rgn, &item->tree_item.base.rgn);
> > > @@ -895,7 +895,7 @@ static int current_add(DisplayChannel *display,
> > > Ring
> > > *ring, Drawable *drawable)
> > >           * safety (todo: Not sure if exclude_region can affect the
> > > drawable
> > >           * if it is added to the tree before calling
> > > exclude_region).
> > >           */
> > > -        current_add_drawable(display, drawable, ring);
> > > +        display_channel_add_drawable(display, drawable, ring);
> > >      } else {
> > >          /*
> > >           * stream_detach_behind can affect the current tree since
> > > @@ -903,7 +903,7 @@ static int current_add(DisplayChannel *display,
> > > Ring
> > > *ring, Drawable *drawable)
> > >           * drawable should be added to the tree before calling
> > >           * stream_detach_behind
> > >           */
> > > -        current_add_drawable(display, drawable, ring);
> > > +        display_channel_add_drawable(display, drawable, ring);
> > >          if (is_primary_surface(display, drawable->surface_id)) {
> > >              stream_detach_behind(display, &drawable-
> > > >tree_item.base.rgn,
> > >              drawable);
> > >          }
> > > @@ -1351,7 +1351,7 @@ static bool free_one_drawable(DisplayChannel
> > > *display,
> > > int force_glz_free)
> > >      drawable_draw(display, drawable);
> > >      container = drawable->tree_item.base.container;
> > >  
> > > -    current_remove_drawable(display, drawable);
> > > +    display_channel_remove_drawable(display, drawable);
> > >      container_cleanup(container);
> > >      return TRUE;
> > >  }
> > > @@ -1707,7 +1707,7 @@ static void draw_until(DisplayChannel
> > > *display,
> > > RedSurface *surface, Drawable *l
> > >          now = SPICE_CONTAINEROF(ring_item, Drawable,
> > > surface_list_link);
> > >          now->refs++;
> > >          container = now->tree_item.base.container;
> > > -        current_remove_drawable(display, now);
> > > +        display_channel_remove_drawable(display, now);
> > >          container_cleanup(container);
> > >          /* drawable_draw may call display_channel_draw for the
> > > surfaces
> > >          'now' depends on. Notice,
> > >             that it is valid to call display_channel_draw in this
> > > case and
> > >             not display_channel_draw_till:
> > 
> > Not really sure about this change.
> > What about if after understanding the code we manage to encapsulate
> > this "current" ?
> > 
> 
> Fair point. We can drop this patch if you don't think it's necessary.
> 
> Jonathon
> 

I would just move the patch at the end of the queue or put into a
"todo" branch if loooks too expensive to rebase every time.

About branch, maybe would be easier if with the patchset we could
have a link (web git interface) of a published branch.

Frediano