[PATCH 1/2] matrix: track transform type

[Pekka Paalanen]

On Mon, 28 Jan 2013 22:40:28 +0300
Vasily Khoruzhick <anarsoul at gmail.com> wrote:

> Introduce several matrix transform types and track type for matrix.
> Could be usefull for activating some fastpath that depends on some
> transform type.
> 
> Signed-off-by: Vasily Khoruzhick <anarsoul at gmail.com>
> ---
>  shared/matrix.c  | 23 ++++++++++++++++++++---
>  shared/matrix.h  | 10 ++++++++++
>  src/compositor.c |  3 +++
>  src/shell.c      | 17 +++++------------
>  4 files changed, 38 insertions(+), 15 deletions(-)
> 
> diff --git a/shared/matrix.c b/shared/matrix.c
> index 11b5b95..3ff4089 100644
> --- a/shared/matrix.c
> +++ b/shared/matrix.c
...
> +WL_EXPORT void
> +weston_matrix_rotate_xy(struct weston_matrix *matrix, float cos, float sin)
> +{
> +	struct weston_matrix translate = {
> +		.d = { cos, sin, 0, 0,  -sin, cos, 0, 0,  0, 0, 1, 0,  0, 0, 0, 1 },
> +		.type = WESTON_MATRIX_TRANSFORM_ROTATE,
> +	};
> +
> +	weston_matrix_multiply(matrix, &translate);
> +}

Can 'sin' and 'cos' be macros in the C library? Personally I'd avoid
re-using well-known libc names.

> +
>  /* v <- m * v */
>  WL_EXPORT void
>  weston_matrix_transform(struct weston_matrix *matrix, struct weston_vector *v)
> @@ -249,6 +265,7 @@ weston_matrix_invert(struct weston_matrix *inverse,
>  	weston_matrix_init(inverse);
>  	for (c = 0; c < 4; ++c)
>  		inverse_transform(LU, perm, &inverse->d[c * 4]);
> +	inverse->type = matrix->type;
>  
>  	return 0;
>  }
> diff --git a/shared/matrix.h b/shared/matrix.h
> index bacb7bf..47354f6 100644
> --- a/shared/matrix.h
> +++ b/shared/matrix.h
> @@ -24,8 +24,16 @@
>  #ifndef WESTON_MATRIX_H
>  #define WESTON_MATRIX_H
>  
> +enum weston_matrix_transform_type {
> +	WESTON_MATRIX_TRANSFORM_TRANSLATE	= (1 << 0),
> +	WESTON_MATRIX_TRANSFORM_SCALE		= (1 << 1),
> +	WESTON_MATRIX_TRANSFORM_ROTATE		= (1 << 2),
> +	WESTON_MATRIX_TRANSFORM_OTHER		= (1 << 3),
> +};

Couldn't we do with shorter names? Like:
enum weston_matrix_type_bits {
	WESTON_MATRIX_TRANSLATE
	WESTON_MATRIX_SCALE
	WESTON_MATRIX_SKEW
	WESTON_MATRIX_PROJECT
}

These would also be the mathematically more descriptive names. Skew
refers specifically to the non-diagonal elements in the upper-left 3x3
sub-matrix, and rotation in that respect is a combination of scale and
skew.

Having WESTON_MATRIX_PROJECT set would mean that you have to do the
projective division, when converting point coordinates back to
Cartesian. Otherwise, the bottom row is implied as {0, 0, 0, 1} and no
division is necessary (assuming a point is given as {x, y, z, 1}).

Of course, the current use literally is just ROTATE, and OTHER I don't
think can even happen.

Just a suggestion.

> +
>  struct weston_matrix {
>  	float d[16];
> +	unsigned int type;
>  };
>  
>  struct weston_vector {
> @@ -42,6 +50,8 @@ void
>  weston_matrix_translate(struct weston_matrix *matrix,
>  			float x, float y, float z);
>  void
> +weston_matrix_rotate_xy(struct weston_matrix *matrix, float cos, float sin);
> +void
>  weston_matrix_transform(struct weston_matrix *matrix, struct weston_vector *v);
>  
>  int
> diff --git a/src/compositor.c b/src/compositor.c
> index a2e95c9..5ac43f2 100644
> --- a/src/compositor.c
> +++ b/src/compositor.c
> @@ -539,6 +539,7 @@ weston_surface_update_transform_enable(struct weston_surface *surface)
>  	surface->transform.enabled = 1;
>  
>  	/* Otherwise identity matrix, but with x and y translation. */
> +	surface->transform.position.matrix.type = WESTON_MATRIX_TRANSFORM_TRANSLATE;
>  	surface->transform.position.matrix.d[12] = surface->geometry.x;
>  	surface->transform.position.matrix.d[13] = surface->geometry.y;
>  
> @@ -2754,12 +2755,14 @@ weston_output_compute_transform(struct weston_output *output)
>  	int flip;
>  
>  	weston_matrix_init(&transform);
> +	transform.type = WESTON_MATRIX_TRANSFORM_ROTATE;
>  
>  	switch(output->transform) {
>  	case WL_OUTPUT_TRANSFORM_FLIPPED:
>  	case WL_OUTPUT_TRANSFORM_FLIPPED_90:
>  	case WL_OUTPUT_TRANSFORM_FLIPPED_180:
>  	case WL_OUTPUT_TRANSFORM_FLIPPED_270:
> +		transform.type |= WESTON_MATRIX_TRANSFORM_OTHER;
>  		flip = -1;
>  		break;
>  	default:
> diff --git a/src/shell.c b/src/shell.c
> index dcbabf3..a99786b 100644
> --- a/src/shell.c
> +++ b/src/shell.c
> @@ -1876,6 +1876,8 @@ shell_map_popup(struct shell_surface *shsurf)
>  	} else {
>  		/* construct x, y translation matrix */
>  		weston_matrix_init(&shsurf->popup.parent_transform.matrix);
> +		shsurf->popup.parent_transform.matrix.type =
> +			WESTON_MATRIX_TRANSFORM_TRANSLATE;
>  		shsurf->popup.parent_transform.matrix.d[12] =
>  			parent->geometry.x;
>  		shsurf->popup.parent_transform.matrix.d[13] =
> @@ -2502,10 +2504,7 @@ rotate_grab_motion(struct wl_pointer_grab *grab,
>  			&shsurf->rotation.transform.matrix;
>  
>  		weston_matrix_init(&rotate->rotation);
> -		rotate->rotation.d[0] = dx / r;
> -		rotate->rotation.d[4] = -dy / r;
> -		rotate->rotation.d[1] = -rotate->rotation.d[4];
> -		rotate->rotation.d[5] = rotate->rotation.d[0];
> +		weston_matrix_rotate_xy(&rotate->rotation, dx / r, dy / r);
>  
>  		weston_matrix_init(matrix);
>  		weston_matrix_translate(matrix, -cx, -cy, 0.0f);
> @@ -2600,17 +2599,11 @@ rotate_binding(struct wl_seat *seat, uint32_t time, uint32_t button,
>  		struct weston_matrix inverse;
>  
>  		weston_matrix_init(&inverse);
> -		inverse.d[0] = dx / r;
> -		inverse.d[4] = dy / r;
> -		inverse.d[1] = -inverse.d[4];
> -		inverse.d[5] = inverse.d[0];
> +		weston_matrix_rotate_xy(&inverse, dx / r, -dy / r);
>  		weston_matrix_multiply(&surface->rotation.rotation, &inverse);
>  
>  		weston_matrix_init(&rotate->rotation);
> -		rotate->rotation.d[0] = dx / r;
> -		rotate->rotation.d[4] = -dy / r;
> -		rotate->rotation.d[1] = -rotate->rotation.d[4];
> -		rotate->rotation.d[5] = rotate->rotation.d[0];
> +		weston_matrix_rotate_xy(&rotate->rotation, dx / r, dy / r);

Could we replace init && rotate with init_rotation_xy? Avoids useless
multiplication.

>  	} else {
>  		weston_matrix_init(&surface->rotation.rotation);
>  		weston_matrix_init(&rotate->rotation);


Looking good, thanks,
pq