[PATCH weston v2 1/3] compositor: remove redundant geometry dirtying

[Kristian Høgsberg]

On Fri, Mar 08, 2013 at 02:56:48PM +0200, Pekka Paalanen wrote:
> Remove redundant geometry dirtying from surface_commit() to simplify
> further changes.
> 
> This code was added in commit 5df8ecac5d31467122a9d8bda6241d5957ae6848
> "compositor: Fix partial repaints"
> 
> as the fix to:
> https://bugs.freedesktop.org/show_bug.cgi?id=56538
> 
> The issue fixed by that commit was making the geometry dirty on every
> attach, which caused full-surface repaints every time. The bug was
> probably introduced by the opaque region changes during implementing
> wl_surface.commit. The mentioned commit fixes the opaque handling by
> comparing the new and old regions.
> 
> However, the commit also introduces additional checks that set
> geometry.dirty. In the current code base, this should be unnecessary.
> 
> If the pending.sx or pending.sy are not zero, or if the surface size
> changes, the configure() hook is responsible for applying the new
> values, and so also marking the geometry dirty.
> 
> The configure() hook is only called, if there has been a new
> wl_surface.attach. Nothing else can change these variables, so this
> should be enough.
> 
> Signed-off-by: Pekka Paalanen <ppaalanen at gmail.com>
> Cc: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira at intel.com>

Yes, this should be fine.  Worst case there's a couple of configure
handlers we need to fix if something breaks.

Kristian

> ---
> 
> Tested only very quickly, I wonder if this breaks anything.
> I can fix the fallout in the various configure() implementations,
> of course, if there is something.
> ---
>  src/compositor.c | 35 +++--------------------------------
>  1 file changed, 3 insertions(+), 32 deletions(-)
> 
> diff --git a/src/compositor.c b/src/compositor.c
> index a2860fd..75ff035 100644
> --- a/src/compositor.c
> +++ b/src/compositor.c
> @@ -1372,31 +1372,6 @@ surface_set_input_region(struct wl_client *client,
>  	}
>  }
>  
> -static int
> -surface_pending_buffer_has_different_size(struct weston_surface *surface)
> -{
> -	int width, height;
> -
> -	switch (surface->pending.buffer_transform) {
> -	case WL_OUTPUT_TRANSFORM_90:
> -	case WL_OUTPUT_TRANSFORM_270:
> -	case WL_OUTPUT_TRANSFORM_FLIPPED_90:
> -	case WL_OUTPUT_TRANSFORM_FLIPPED_270:
> -		height = surface->pending.buffer->width;
> -		width = surface->pending.buffer->height;
> -		break;
> -	default:
> -		width = surface->pending.buffer->width;
> -		height = surface->pending.buffer->height;
> -	}
> -
> -	if (width == surface->geometry.width &&
> -	    height == surface->geometry.height)
> -		return 0;
> -	else
> -		return 1;
> -}
> -
>  static void
>  surface_commit(struct wl_client *client, struct wl_resource *resource)
>  {
> @@ -1405,11 +1380,6 @@ surface_commit(struct wl_client *client, struct wl_resource *resource)
>  	int buffer_width = 0;
>  	int buffer_height = 0;
>  
> -	if (surface->pending.sx || surface->pending.sy ||
> -	    (surface->pending.buffer &&
> -	     surface_pending_buffer_has_different_size(surface)))
> -		surface->geometry.dirty = 1;
> -
>  	/* wl_surface.set_buffer_rotation */
>  	surface->buffer_transform = surface->pending.buffer_transform;
>  
> @@ -1423,8 +1393,9 @@ surface_commit(struct wl_client *client, struct wl_resource *resource)
>  	}
>  
>  	if (surface->configure && surface->pending.newly_attached)
> -		surface->configure(surface, surface->pending.sx,
> -				   surface->pending.sy, buffer_width, buffer_height);
> +		surface->configure(surface,
> +				   surface->pending.sx, surface->pending.sy,
> +				   buffer_width, buffer_height);
>  
>  	if (surface->pending.buffer)
>  		wl_list_remove(&surface->pending.buffer_destroy_listener.link);
> -- 
> 1.7.12.4
>