Re: [PATCH weston v2 05/20] pixman-renderer: Add a weston_matrix_to_pixman_transform function and simplify the buffer-to-output matrix computation

2015-01-09 Thread Giulio Camuffo
One comment below, otherwise looks fine

2014-10-16 18:55 GMT+03:00 Derek Foreman der...@osg.samsung.com:
 From: Jason Ekstrand ja...@jlekstrand.net

 Now that we have a buffer-to-surface matrix and the global-to-output matrix
 is in pixels, we can remove a large chunk of confusing code from the pixman
 renderer.  Hopefully, having this stuff in weston core will keep the pixman
 renderer from gettin broken quite as often.
 ---
  src/pixman-renderer.c | 155 
 +++---
  1 file changed, 20 insertions(+), 135 deletions(-)

 diff --git a/src/pixman-renderer.c b/src/pixman-renderer.c
 index 2c26c3a..18b6476 100644
 --- a/src/pixman-renderer.c
 +++ b/src/pixman-renderer.c
 @@ -139,32 +139,20 @@ region_global_to_output(struct weston_output *output, 
 pixman_region32_t *region)
  #define D2F(v) pixman_double_to_fixed((double)v)

  static void
 -transform_apply_viewport(pixman_transform_t *transform,
 -struct weston_surface *surface)
 +weston_matrix_to_pixman_transform(pixman_transform_t *pt,
 + const struct weston_matrix *wm)
  {
 -   struct weston_buffer_viewport *vp = surface-buffer_viewport;
 -   double src_width, src_height;
 -   double src_x, src_y;
 -
 -   if (vp-buffer.src_width == wl_fixed_from_int(-1)) {
 -   if (vp-surface.width == -1)
 -   return;
 -
 -   src_x = 0.0;
 -   src_y = 0.0;
 -   src_width = surface-width_from_buffer;
 -   src_height = surface-height_from_buffer;
 -   } else {
 -   src_x = wl_fixed_to_double(vp-buffer.src_x);
 -   src_y = wl_fixed_to_double(vp-buffer.src_y);
 -   src_width = wl_fixed_to_double(vp-buffer.src_width);
 -   src_height = wl_fixed_to_double(vp-buffer.src_height);
 -   }
 -
 -   pixman_transform_scale(transform, NULL,
 -  D2F(src_width / surface-width),
 -  D2F(src_height / surface-height));
 -   pixman_transform_translate(transform, NULL, D2F(src_x), D2F(src_y));
 +   /* Pixman supports only 2D transform matrix, but Weston uses 3D, *
 +* so we're omitting Z coordinate here. */
 +   pt-matrix[0][0] = pixman_double_to_fixed(wm-d[0]);
 +   pt-matrix[0][1] = pixman_double_to_fixed(wm-d[4]);
 +   pt-matrix[0][2] = pixman_double_to_fixed(wm-d[12]);
 +   pt-matrix[1][0] = pixman_double_to_fixed(wm-d[1]);
 +   pt-matrix[1][1] = pixman_double_to_fixed(wm-d[5]);
 +   pt-matrix[1][2] = pixman_double_to_fixed(wm-d[13]);
 +   pt-matrix[2][0] = pixman_double_to_fixed(wm-d[3]);
 +   pt-matrix[2][1] = pixman_double_to_fixed(wm-d[7]);
 +   pt-matrix[2][2] = pixman_double_to_fixed(wm-d[15]);
  }

  static void
 @@ -180,7 +168,7 @@ repaint_region(struct weston_view *ev, struct 
 weston_output *output,
 pixman_region32_t final_region;
 float view_x, view_y;
 pixman_transform_t transform;
 -   pixman_fixed_t fw, fh;
 +   struct weston_matrix matrix;

This new matrix isn't being initialized anywhere, or am i blind?

 pixman_image_t *mask_image;
 pixman_color_t mask = { 0, };

 @@ -217,121 +205,18 @@ repaint_region(struct weston_view *ev, struct 
 weston_output *output,
 /* Set up the source transformation based on the surface
position, the output position/transform/scale and the client
specified buffer transform/scale */
 -   pixman_transform_init_identity(transform);
 -   pixman_transform_scale(transform, NULL,
 -  pixman_double_to_fixed 
 ((double)1.0/output-current_scale),
 -  pixman_double_to_fixed 
 ((double)1.0/output-current_scale));
 -
 -   fw = pixman_int_to_fixed(output-width);
 -   fh = pixman_int_to_fixed(output-height);
 -   switch (output-transform) {
 -   default:
 -   case WL_OUTPUT_TRANSFORM_NORMAL:
 -   case WL_OUTPUT_TRANSFORM_FLIPPED:
 -   break;
 -   case WL_OUTPUT_TRANSFORM_90:
 -   case WL_OUTPUT_TRANSFORM_FLIPPED_90:
 -   pixman_transform_rotate(transform, NULL, 0, -pixman_fixed_1);
 -   pixman_transform_translate(transform, NULL, 0, fh);
 -   break;
 -   case WL_OUTPUT_TRANSFORM_180:
 -   case WL_OUTPUT_TRANSFORM_FLIPPED_180:
 -   pixman_transform_rotate(transform, NULL, -pixman_fixed_1, 0);
 -   pixman_transform_translate(transform, NULL, fw, fh);
 -   break;
 -   case WL_OUTPUT_TRANSFORM_270:
 -   case WL_OUTPUT_TRANSFORM_FLIPPED_270:
 -   pixman_transform_rotate(transform, NULL, 0, pixman_fixed_1);
 -   pixman_transform_translate(transform, NULL, fw, 0);
 -   break;
 -   }
 -
 -   switch (output-transform) {
 -   case WL_OUTPUT_TRANSFORM_FLIPPED:
 -   case 

Re: [PATCH weston v2 05/20] pixman-renderer: Add a weston_matrix_to_pixman_transform function and simplify the buffer-to-output matrix computation

2015-01-09 Thread Derek Foreman
Thanks for looking at this!

On 09/01/15 03:15 PM, Giulio Camuffo wrote:
 One comment below, otherwise looks fine
 
 2014-10-16 18:55 GMT+03:00 Derek Foreman der...@osg.samsung.com:
 From: Jason Ekstrand ja...@jlekstrand.net

 Now that we have a buffer-to-surface matrix and the global-to-output matrix
 is in pixels, we can remove a large chunk of confusing code from the pixman
 renderer.  Hopefully, having this stuff in weston core will keep the pixman
 renderer from gettin broken quite as often.
 ---
  src/pixman-renderer.c | 155 
 +++---
  1 file changed, 20 insertions(+), 135 deletions(-)

 diff --git a/src/pixman-renderer.c b/src/pixman-renderer.c
 index 2c26c3a..18b6476 100644

snip

  static void
 @@ -180,7 +168,7 @@ repaint_region(struct weston_view *ev, struct 
 weston_output *output,
 pixman_region32_t final_region;
 float view_x, view_y;
 pixman_transform_t transform;
 -   pixman_fixed_t fw, fh;
 +   struct weston_matrix matrix;
 
 This new matrix isn't being initialized anywhere, or am i blind?

It's used as a target further down...

 pixman_image_t *mask_image;
 pixman_color_t mask = { 0, };

 @@ -217,121 +205,18 @@ repaint_region(struct weston_view *ev, struct 
 weston_output *output,
 /* Set up the source transformation based on the surface
position, the output position/transform/scale and the client
specified buffer transform/scale */
 -   pixman_transform_init_identity(transform);
 -   pixman_transform_scale(transform, NULL,
 -  pixman_double_to_fixed 
 ((double)1.0/output-current_scale),
 -  pixman_double_to_fixed 
 ((double)1.0/output-current_scale));
 -
 -   fw = pixman_int_to_fixed(output-width);
 -   fh = pixman_int_to_fixed(output-height);
 -   switch (output-transform) {
 -   default:
 -   case WL_OUTPUT_TRANSFORM_NORMAL:
 -   case WL_OUTPUT_TRANSFORM_FLIPPED:
 -   break;
 -   case WL_OUTPUT_TRANSFORM_90:
 -   case WL_OUTPUT_TRANSFORM_FLIPPED_90:
 -   pixman_transform_rotate(transform, NULL, 0, 
 -pixman_fixed_1);
 -   pixman_transform_translate(transform, NULL, 0, fh);
 -   break;
 -   case WL_OUTPUT_TRANSFORM_180:
 -   case WL_OUTPUT_TRANSFORM_FLIPPED_180:
 -   pixman_transform_rotate(transform, NULL, -pixman_fixed_1, 
 0);
 -   pixman_transform_translate(transform, NULL, fw, fh);
 -   break;
 -   case WL_OUTPUT_TRANSFORM_270:
 -   case WL_OUTPUT_TRANSFORM_FLIPPED_270:
 -   pixman_transform_rotate(transform, NULL, 0, pixman_fixed_1);
 -   pixman_transform_translate(transform, NULL, fw, 0);
 -   break;
 -   }
 -
 -   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:
 -   pixman_transform_scale(transform, NULL,
 -  pixman_int_to_fixed (-1),
 -  pixman_int_to_fixed (1));
 -   pixman_transform_translate(transform, NULL, fw, 0);
 -   break;
 -   }
 -
 -pixman_transform_translate(transform, NULL,
 -  pixman_double_to_fixed (output-x),
 -  pixman_double_to_fixed (output-y));
 +   weston_matrix_invert(matrix, output-matrix);

Right here - the inverse of output-matrix is placed in matrix... though
if the output-matrix is singular matrix will be left uninitialized.  I
don't think this can happen...

 if (ev-transform.enabled) {
 -   /* Pixman supports only 2D transform matrix, but Weston uses 
 3D,
 -* so we're omitting Z coordinate here
 -*/
 -   pixman_transform_t surface_transform = {{
 -   { D2F(ev-transform.matrix.d[0]),

snip
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel


[PATCH weston v2 05/20] pixman-renderer: Add a weston_matrix_to_pixman_transform function and simplify the buffer-to-output matrix computation

2014-10-16 Thread Derek Foreman
From: Jason Ekstrand ja...@jlekstrand.net

Now that we have a buffer-to-surface matrix and the global-to-output matrix
is in pixels, we can remove a large chunk of confusing code from the pixman
renderer.  Hopefully, having this stuff in weston core will keep the pixman
renderer from gettin broken quite as often.
---
 src/pixman-renderer.c | 155 +++---
 1 file changed, 20 insertions(+), 135 deletions(-)

diff --git a/src/pixman-renderer.c b/src/pixman-renderer.c
index 2c26c3a..18b6476 100644
--- a/src/pixman-renderer.c
+++ b/src/pixman-renderer.c
@@ -139,32 +139,20 @@ region_global_to_output(struct weston_output *output, 
pixman_region32_t *region)
 #define D2F(v) pixman_double_to_fixed((double)v)
 
 static void
-transform_apply_viewport(pixman_transform_t *transform,
-struct weston_surface *surface)
+weston_matrix_to_pixman_transform(pixman_transform_t *pt,
+ const struct weston_matrix *wm)
 {
-   struct weston_buffer_viewport *vp = surface-buffer_viewport;
-   double src_width, src_height;
-   double src_x, src_y;
-
-   if (vp-buffer.src_width == wl_fixed_from_int(-1)) {
-   if (vp-surface.width == -1)
-   return;
-
-   src_x = 0.0;
-   src_y = 0.0;
-   src_width = surface-width_from_buffer;
-   src_height = surface-height_from_buffer;
-   } else {
-   src_x = wl_fixed_to_double(vp-buffer.src_x);
-   src_y = wl_fixed_to_double(vp-buffer.src_y);
-   src_width = wl_fixed_to_double(vp-buffer.src_width);
-   src_height = wl_fixed_to_double(vp-buffer.src_height);
-   }
-
-   pixman_transform_scale(transform, NULL,
-  D2F(src_width / surface-width),
-  D2F(src_height / surface-height));
-   pixman_transform_translate(transform, NULL, D2F(src_x), D2F(src_y));
+   /* Pixman supports only 2D transform matrix, but Weston uses 3D, *
+* so we're omitting Z coordinate here. */
+   pt-matrix[0][0] = pixman_double_to_fixed(wm-d[0]);
+   pt-matrix[0][1] = pixman_double_to_fixed(wm-d[4]);
+   pt-matrix[0][2] = pixman_double_to_fixed(wm-d[12]);
+   pt-matrix[1][0] = pixman_double_to_fixed(wm-d[1]);
+   pt-matrix[1][1] = pixman_double_to_fixed(wm-d[5]);
+   pt-matrix[1][2] = pixman_double_to_fixed(wm-d[13]);
+   pt-matrix[2][0] = pixman_double_to_fixed(wm-d[3]);
+   pt-matrix[2][1] = pixman_double_to_fixed(wm-d[7]);
+   pt-matrix[2][2] = pixman_double_to_fixed(wm-d[15]);
 }
 
 static void
@@ -180,7 +168,7 @@ repaint_region(struct weston_view *ev, struct weston_output 
*output,
pixman_region32_t final_region;
float view_x, view_y;
pixman_transform_t transform;
-   pixman_fixed_t fw, fh;
+   struct weston_matrix matrix;
pixman_image_t *mask_image;
pixman_color_t mask = { 0, };
 
@@ -217,121 +205,18 @@ repaint_region(struct weston_view *ev, struct 
weston_output *output,
/* Set up the source transformation based on the surface
   position, the output position/transform/scale and the client
   specified buffer transform/scale */
-   pixman_transform_init_identity(transform);
-   pixman_transform_scale(transform, NULL,
-  pixman_double_to_fixed 
((double)1.0/output-current_scale),
-  pixman_double_to_fixed 
((double)1.0/output-current_scale));
-
-   fw = pixman_int_to_fixed(output-width);
-   fh = pixman_int_to_fixed(output-height);
-   switch (output-transform) {
-   default:
-   case WL_OUTPUT_TRANSFORM_NORMAL:
-   case WL_OUTPUT_TRANSFORM_FLIPPED:
-   break;
-   case WL_OUTPUT_TRANSFORM_90:
-   case WL_OUTPUT_TRANSFORM_FLIPPED_90:
-   pixman_transform_rotate(transform, NULL, 0, -pixman_fixed_1);
-   pixman_transform_translate(transform, NULL, 0, fh);
-   break;
-   case WL_OUTPUT_TRANSFORM_180:
-   case WL_OUTPUT_TRANSFORM_FLIPPED_180:
-   pixman_transform_rotate(transform, NULL, -pixman_fixed_1, 0);
-   pixman_transform_translate(transform, NULL, fw, fh);
-   break;
-   case WL_OUTPUT_TRANSFORM_270:
-   case WL_OUTPUT_TRANSFORM_FLIPPED_270:
-   pixman_transform_rotate(transform, NULL, 0, pixman_fixed_1);
-   pixman_transform_translate(transform, NULL, fw, 0);
-   break;
-   }
-
-   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:
-   pixman_transform_scale(transform, NULL,
-  pixman_int_to_fixed (-1),
-