[PATCH weston 2/4] compositor: introduce weston_surface_geometry_dirty()

[Scott Moreau]

On Tue, Dec 18, 2012 at 7:55 AM, Pekka Paalanen <ppaalanen at gmail.com> wrote:

> On Tue, 18 Dec 2012 07:24:17 -0700
> Scott Moreau <oreaus at gmail.com> wrote:
>
> > On Tue, Dec 18, 2012 at 4:58 AM, Pekka Paalanen <ppaalanen at gmail.com>
> wrote:
> >
> > > Instead of directly setting the dirty flag on weston_surface geometry,
> > > use a function for that.
> > >
> > > This allows us to hook into geometry dirtying in following patches.
> > >
> > > Also add comments to weston_surface fields, whose modification causes
> > > transform state to become outdated.
> > >
> > > Signed-off-by: Pekka Paalanen <ppaalanen at gmail.com>
> > > ---
> > >  src/compositor.c              |   20 +++++++++++++-------
> > >  src/compositor.h              |   17 ++++++++++-------
> > >  src/shell.c                   |   24 ++++++++++--------------
> > >  src/util.c                    |    4 ++--
> > >  src/xwayland/window-manager.c |    4 ++--
> > >  tests/weston-test.c           |    2 +-
> > >  6 files changed, 38 insertions(+), 33 deletions(-)
> > >
> ...
> > > diff --git a/src/compositor.h b/src/compositor.h
> > > index 1d790d3..3a3580a 100644
> > > --- a/src/compositor.h
> > > +++ b/src/compositor.h
> > > @@ -360,8 +360,8 @@ struct weston_region {
> > >   * To add a transformation to a surface, create a struct
> > > weston_transform, and
> > >   * add it to the list surface->geometry.transformation_list. Whenever
> you
> > >   * change the list, anything under surface->geometry, or anything in
> the
> > > - * weston_transforms linked into the list, you must set
> > > - * surface->geometry.dirty = 1.
> > > + * weston_transforms linked into the list, you must call
> > > + * weston_surface_geometry_dirty().
> > >   *
> > >   * The order in the list defines the order of transformations. Let the
> > > list
> > >   * contain the transformation matrices M1, ..., Mn as head to tail.
> The
> > > @@ -385,17 +385,17 @@ struct weston_surface {
> > >         struct weston_compositor *compositor;
> > >         pixman_region32_t clip;
> > >         pixman_region32_t damage;
> > > -       pixman_region32_t opaque;
> > > +       pixman_region32_t opaque;        /* geometry dirty */
> > >
> >
> > I'm not sure what this comment is supposed to convey.
> >
> >
> > >         pixman_region32_t input;
> > >         struct wl_list link;
> > >         struct wl_list layer_link;
> > > -       float alpha;
> > > +       float alpha;                     /* geometry dirty */
> > >
> >
> > This one as well.
>
> Hi Scott,
>
> like written in the commit message, they are just a reminder of the
> fields' unusual nature. We might want to do a couple of additional
> patches, moving the fields under the 'geometry' anonymous struct. That
> would be also an easy way to review that all modifications actually set
> the dirty flag. However, it's a fair amount of churn without any other
> benefits.
>
>
Hi Pekka,

Thanks for clarifying. If the comment is meant to be a reminder to set
geometry dirty flag when on of these variables are changed, I think it
should be more verbose and say just that. Alone, /* geometry dirty */ next
to members that are not directly related may cause more confusion than no
comment at all.

- Scott
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/wayland-devel/attachments/20121218/5e100c50/attachment.html>