[PATCH] cosmetic: update comments that refer to weston_surface_update_transform()

[Derek Foreman]

On 11/09/14 08:22 AM, Pekka Paalanen wrote:
> On Thu, 11 Sep 2014 08:10:25 -0500
> Derek Foreman <derekf at osg.samsung.com> wrote:
> 
>> On 11/09/14 04:00 AM, Pekka Paalanen wrote:
>>> On Wed, 10 Sep 2014 15:37:33 -0500
>>> Derek Foreman <derekf at osg.samsung.com> 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.

I'll get over it. :)