<div class="gmail_quote">On Tue, Dec 18, 2012 at 7:55 AM, Pekka Paalanen <<a href="mailto:ppaalanen@gmail.com" target="_blank">ppaalanen@gmail.com</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> <div class="im">On Tue, 18 Dec 2012 07:24:17 -0700 Scott Moreau <<a href="mailto:oreaus@gmail.com">oreaus@gmail.com</a>> wrote: > On Tue, Dec 18, 2012 at 4:58 AM, Pekka Paalanen <<a href="mailto:ppaalanen@gmail.com">ppaalanen@gmail.com</a>> 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 <<a href="mailto:ppaalanen@gmail.com">ppaalanen@gmail.com</a>> > > --- > > 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(-) > > </div>... <div><div class="h5">> > 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. </div></div>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. </blockquote><div> 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 </div></div>