On Thu, 11 Sep 2014 09:30:30 -0500
Derek Foreman <[email protected]> wrote:

> On 11/09/14 08:22 AM, Pekka Paalanen wrote:
> > On Thu, 11 Sep 2014 08:10:25 -0500
> > Derek Foreman <[email protected]> wrote:
> > 
> >> On 11/09/14 04:00 AM, Pekka Paalanen wrote:
> >>> On Wed, 10 Sep 2014 15:37:33 -0500
> >>> Derek Foreman <[email protected]> wrote:
> >>>
> >>>> weston_surface_update_transform() no longer exists, except in comments.
> >>>>
> >>>> Fix that.
> >>>> ---
> >>>>  desktop-shell/shell.c | 2 +-
> >>>>  src/compositor-drm.c  | 3 +--
> >>>>  src/compositor.c      | 4 ++--
> >>>>  3 files changed, 4 insertions(+), 5 deletions(-)
> >>>>
> >>>> diff --git a/desktop-shell/shell.c b/desktop-shell/shell.c
> >>>> index 3cc5733..3a5a702 100644
> >>>> --- a/desktop-shell/shell.c
> >>>> +++ b/desktop-shell/shell.c
> >>>> @@ -4706,7 +4706,7 @@ rotate_grab_motion(struct weston_pointer_grab 
> >>>> *grab, uint32_t time,
> >>>>                                           shsurf->view->geometry.y + 
> >>>> dposy);
> >>>>          }
> >>>>  
> >>>> -        /* Repaint implies weston_surface_update_transform(), which
> >>>> +        /* Repaint implies weston_view_update_transform(), which
> >>>>           * lazily applies the damage due to rotation update.
> >>>>           */
> >>>>          weston_compositor_schedule_repaint(shsurf->surface->compositor);
> >>>> diff --git a/src/compositor-drm.c b/src/compositor-drm.c
> >>>> index 9c83b1a..e8720c7 100644
> >>>> --- a/src/compositor-drm.c
> >>>> +++ b/src/compositor-drm.c
> >>>> @@ -900,8 +900,7 @@ drm_output_prepare_overlay_view(struct weston_output 
> >>>> *output_base,
> >>>>  
> >>>>          /*
> >>>>           * Calculate the source & dest rects properly based on actual
> >>>> -         * position (note the caller has called 
> >>>> weston_surface_update_transform()
> >>>> -         * for us already).
> >>>> +         * position.
> >>>>           */
> >>>>          pixman_region32_init(&dest_rect);
> >>>>          pixman_region32_intersect(&dest_rect, 
> >>>> &ev->transform.boundingbox,
> >>>> diff --git a/src/compositor.c b/src/compositor.c
> >>>> index 18975bf..b0bc86c 100644
> >>>> --- a/src/compositor.c
> >>>> +++ b/src/compositor.c
> >>>> @@ -2621,13 +2621,13 @@ subsurface_configure(struct weston_surface 
> >>>> *surface, int32_t dx, int32_t dy)
> >>>>          if (!weston_surface_is_mapped(surface)) {
> >>>>                  struct weston_output *output;
> >>>>  
> >>>> -                /* Cannot call weston_surface_update_transform(),
> >>>> +                /* Cannot call weston_view_update_transform(),
> >>>>                   * because that would call it also for the parent 
> >>>> surface,
> >>>>                   * which might not be mapped yet. That would lead to
> >>>>                   * inconsistent state, where the window could never be
> >>>>                   * mapped.
> >>>>                   *
> >>>> -                 * Instead just assing any output, to make
> >>>> +                 * Instead just assign any output, to make
> >>>>                   * weston_surface_is_mapped() return true, so that when 
> >>>> the
> >>>>                   * parent surface does get mapped, this one will get
> >>>>                   * included, too. See view_list_add().
> >>>
> >>> Hi,
> >>>
> >>> I edited this a bit to not lose the comment in compositor-drm.c.
> >>> Pushed!
> >>
> >>
> >> Hmm, ok, but the comment in compositor-drm hasn't really been 100%
> >> accurate since ca14ef049 :)
> > 
> > Hmm, it should still be accurate. What was removed in ca14ef049 was
> > really an *extra* call.
> > 
> > The proper call chain is:
> > weston_output_repaint
> >   weston_compositor_build_view_list
> >     view_list_add
> >       weston_view_update_transform
> >       ...
> >   output->assign_planes
> > 
> > The call to assign_planes comes after, and view_list_add should also
> > guarantee that all sub-surfaces get update_transform called.
> > 
> > Do you see any case where this fails?
> 
> No, the logic is sound.
> 
> I just found the comment slightly confusing when I actually went looking
> for which "caller" was responsible for the update.  At the time of the
> comment's writing it happened (extraneously) in the direct caller, now
> it's much more indirect, and not actually a function that will be on the
> same call stack.

Yeah, which is all the reason to have that comment there, as the call
not really obvious. ;-)

Sure it could be written more clearly...

> I'll get over it. :)

Thanks,
pq
_______________________________________________
wayland-devel mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

Reply via email to