Re: [PATCH weston v2 08/20] compositor: add weston_matrix_transform_rect() and use it where appropriate

2015-01-19 Thread Bill Spitzak 



On 01/09/2015 01:42 PM, Giulio Camuffo wrote:

Just a nitpick below:



+WL_EXPORT pixman_box32_t
+weston_matrix_transform_rect(struct weston_matrix *matrix,
+pixman_box32_t rect)


The function name doesn't imply it returns an axis aligned bounding
box. Maybe something like weston_matrix_rect_to_transformed_aabb
would be better?


I think that is implied pretty well by the return type, since that 
return type cannot possibly contain anything other than an axis-aligned box.


But it may be useful to indicate this is returning a bounding box and 
not some other rectangle. Another useful transform is an area-preserving 
one (the resulting rectangle has the same area and center as transformed 
quadrilateral).


That said, it has been very common for people to transform to a bounding 
box first and name this function transform rectangle. When they 
realize they need the other function it is then often called transform 
area. So this naming fits what many other libraries do.

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

Re: [PATCH weston v2 08/20] compositor: add weston_matrix_transform_rect() and use it where appropriate

2015-01-09 Thread Giulio Camuffo 
Just a nitpick below:

2014-10-16 18:55 GMT+03:00 Derek Foreman der...@osg.samsung.com:
 New function that transforms a pixman_box32_t rectangle by a matrix.

 Since pixman rectangles are represented by 2 corners, non-90 degree
 rotations can't be properly represented.  This function gives the
 axis aligned rectangle that encloses the rotated rectangle.

 We use this for matrix_transform_region() and weston_matrix_transform_rect(),
 simplifying them and allowing them to work for non 90 degree rotations.
 ---
  src/compositor.c | 96 
 
  src/compositor.h |  4 +++
  2 files changed, 52 insertions(+), 48 deletions(-)

 diff --git a/src/compositor.c b/src/compositor.c
 index e45dd62..7c0f050 100644
 --- a/src/compositor.c
 +++ b/src/compositor.c
 @@ -654,6 +654,50 @@ weston_view_to_global_float(struct weston_view *view,
 }
  }

 +WL_EXPORT pixman_box32_t
 +weston_matrix_transform_rect(struct weston_matrix *matrix,
 +pixman_box32_t rect)

The function name doesn't imply it returns an axis aligned bounding
box. Maybe something like weston_matrix_rect_to_transformed_aabb
would be better?

 +{
 +   int i;
 +   pixman_box32_t out;
 +
 +   /* since pixman regions are defined by two corners we have
 +* to be careful with rotations that aren't multiples of 90.
 +* We need to take all four corners of the region and rotate
 +* them, then construct the largest possible two corner
 +* rectangle from the result.
 +*/
 +   struct weston_vector corners[4] = {
 +   {{rect.x1, rect.y1, 0, 1}},
 +   {{rect.x2, rect.y1, 0, 1}},
 +   {{rect.x1, rect.y2, 0, 1}},
 +   {{rect.x2, rect.y2, 0, 1}},
 +   };
 +
 +   for (i = 0; i  4; i++) {
 +   weston_matrix_transform(matrix, corners[i]);
 +   corners[i].f[0] /= corners[i].f[3];
 +   corners[i].f[1] /= corners[i].f[3];
 +   }
 +
 +   out.x1 = floor(corners[0].f[0]);
 +   out.y1 = floor(corners[0].f[1]);
 +   out.x2 = ceil(corners[0].f[0]);
 +   out.y2 = ceil(corners[0].f[1]);
 +
 +   for (i = 1; i  4; i++) {
 +   if (floor(corners[i].f[0])  out.x1)
 +   out.x1 = floor(corners[i].f[0]);
 +   if (floor(corners[i].f[1])  out.y1)
 +   out.y1 = floor(corners[i].f[1]);
 +   if (ceil(corners[i].f[0])  out.x2)
 +   out.x2 = ceil(corners[i].f[0]);
 +   if (ceil(corners[i].f[1])  out.y2)
 +   out.y2 = ceil(corners[i].f[1]);
 +   }
 +   return out;
 +}
 +
  WL_EXPORT void
  weston_transformed_coord(int width, int height,
  enum wl_output_transform transform,
 @@ -747,38 +791,8 @@ weston_matrix_transform_region(pixman_region32_t *dest,
 if (!dest_rects)
 return;

 -   for (i = 0; i  nrects; i++) {
 -   struct weston_vector vec1 = {{
 -   src_rects[i].x1, src_rects[i].y1, 0, 1
 -   }};
 -   weston_matrix_transform(matrix, vec1);
 -   vec1.f[0] /= vec1.f[3];
 -   vec1.f[1] /= vec1.f[3];
 -
 -   struct weston_vector vec2 = {{
 -   src_rects[i].x2, src_rects[i].y2, 0, 1
 -   }};
 -   weston_matrix_transform(matrix, vec2);
 -   vec2.f[0] /= vec2.f[3];
 -   vec2.f[1] /= vec2.f[3];
 -
 -   if (vec1.f[0]  vec2.f[0]) {
 -   dest_rects[i].x1 = floor(vec1.f[0]);
 -   dest_rects[i].x2 = ceil(vec2.f[0]);
 -   } else {
 -   dest_rects[i].x1 = floor(vec2.f[0]);
 -   dest_rects[i].x2 = ceil(vec1.f[0]);
 -   }
 -
 -
 -   if (vec1.f[1]  vec2.f[1]) {
 -   dest_rects[i].y1 = floor(vec1.f[1]);
 -   dest_rects[i].y2 = ceil(vec2.f[1]);
 -   } else {
 -   dest_rects[i].y1 = floor(vec2.f[1]);
 -   dest_rects[i].y2 = ceil(vec1.f[1]);
 -   }
 -   }
 +   for (i = 0; i  nrects; i++)
 +   dest_rects[i] = weston_matrix_transform_rect(matrix, 
 src_rects[i]);

 pixman_region32_clear(dest);
 pixman_region32_init_rects(dest, dest_rects, nrects);
 @@ -939,22 +953,8 @@ WL_EXPORT pixman_box32_t
  weston_surface_to_buffer_rect(struct weston_surface *surface,
   pixman_box32_t rect)
  {
 -   struct weston_buffer_viewport *vp = surface-buffer_viewport;
 -   float xf, yf;
 -
 -   /* first transform box coordinates if the scaler is set */
 -   scaler_surface_to_buffer(surface, rect.x1, rect.y1, xf, yf);
 -   rect.x1 = floorf(xf);
 -   rect.y1 = floorf(yf);
 -
 -   scaler_surface_to_buffer(surface, rect.x2, rect.y2, xf,

[PATCH weston v2 08/20] compositor: add weston_matrix_transform_rect() and use it where appropriate

2014-10-16 Thread Derek Foreman 
New function that transforms a pixman_box32_t rectangle by a matrix.

Since pixman rectangles are represented by 2 corners, non-90 degree
rotations can't be properly represented.  This function gives the
axis aligned rectangle that encloses the rotated rectangle.

We use this for matrix_transform_region() and weston_matrix_transform_rect(),
simplifying them and allowing them to work for non 90 degree rotations.
---
 src/compositor.c | 96 
 src/compositor.h |  4 +++
 2 files changed, 52 insertions(+), 48 deletions(-)

diff --git a/src/compositor.c b/src/compositor.c
index e45dd62..7c0f050 100644
--- a/src/compositor.c
+++ b/src/compositor.c
@@ -654,6 +654,50 @@ weston_view_to_global_float(struct weston_view *view,
}
 }
 
+WL_EXPORT pixman_box32_t
+weston_matrix_transform_rect(struct weston_matrix *matrix,
+pixman_box32_t rect)
+{
+   int i;
+   pixman_box32_t out;
+
+   /* since pixman regions are defined by two corners we have
+* to be careful with rotations that aren't multiples of 90.
+* We need to take all four corners of the region and rotate
+* them, then construct the largest possible two corner
+* rectangle from the result.
+*/
+   struct weston_vector corners[4] = {
+   {{rect.x1, rect.y1, 0, 1}},
+   {{rect.x2, rect.y1, 0, 1}},
+   {{rect.x1, rect.y2, 0, 1}},
+   {{rect.x2, rect.y2, 0, 1}},
+   };
+
+   for (i = 0; i  4; i++) {
+   weston_matrix_transform(matrix, corners[i]);
+   corners[i].f[0] /= corners[i].f[3];
+   corners[i].f[1] /= corners[i].f[3];
+   }
+
+   out.x1 = floor(corners[0].f[0]);
+   out.y1 = floor(corners[0].f[1]);
+   out.x2 = ceil(corners[0].f[0]);
+   out.y2 = ceil(corners[0].f[1]);
+
+   for (i = 1; i  4; i++) {
+   if (floor(corners[i].f[0])  out.x1)
+   out.x1 = floor(corners[i].f[0]);
+   if (floor(corners[i].f[1])  out.y1)
+   out.y1 = floor(corners[i].f[1]);
+   if (ceil(corners[i].f[0])  out.x2)
+   out.x2 = ceil(corners[i].f[0]);
+   if (ceil(corners[i].f[1])  out.y2)
+   out.y2 = ceil(corners[i].f[1]);
+   }
+   return out;
+}
+
 WL_EXPORT void
 weston_transformed_coord(int width, int height,
 enum wl_output_transform transform,
@@ -747,38 +791,8 @@ weston_matrix_transform_region(pixman_region32_t *dest,
if (!dest_rects)
return;
 
-   for (i = 0; i  nrects; i++) {
-   struct weston_vector vec1 = {{
-   src_rects[i].x1, src_rects[i].y1, 0, 1
-   }};
-   weston_matrix_transform(matrix, vec1);
-   vec1.f[0] /= vec1.f[3];
-   vec1.f[1] /= vec1.f[3];
-
-   struct weston_vector vec2 = {{
-   src_rects[i].x2, src_rects[i].y2, 0, 1
-   }};
-   weston_matrix_transform(matrix, vec2);
-   vec2.f[0] /= vec2.f[3];
-   vec2.f[1] /= vec2.f[3];
-
-   if (vec1.f[0]  vec2.f[0]) {
-   dest_rects[i].x1 = floor(vec1.f[0]);
-   dest_rects[i].x2 = ceil(vec2.f[0]);
-   } else {
-   dest_rects[i].x1 = floor(vec2.f[0]);
-   dest_rects[i].x2 = ceil(vec1.f[0]);
-   }
-
-
-   if (vec1.f[1]  vec2.f[1]) {
-   dest_rects[i].y1 = floor(vec1.f[1]);
-   dest_rects[i].y2 = ceil(vec2.f[1]);
-   } else {
-   dest_rects[i].y1 = floor(vec2.f[1]);
-   dest_rects[i].y2 = ceil(vec1.f[1]);
-   }
-   }
+   for (i = 0; i  nrects; i++)
+   dest_rects[i] = weston_matrix_transform_rect(matrix, 
src_rects[i]);
 
pixman_region32_clear(dest);
pixman_region32_init_rects(dest, dest_rects, nrects);
@@ -939,22 +953,8 @@ WL_EXPORT pixman_box32_t
 weston_surface_to_buffer_rect(struct weston_surface *surface,
  pixman_box32_t rect)
 {
-   struct weston_buffer_viewport *vp = surface-buffer_viewport;
-   float xf, yf;
-
-   /* first transform box coordinates if the scaler is set */
-   scaler_surface_to_buffer(surface, rect.x1, rect.y1, xf, yf);
-   rect.x1 = floorf(xf);
-   rect.y1 = floorf(yf);
-
-   scaler_surface_to_buffer(surface, rect.x2, rect.y2, xf, yf);
-   rect.x2 = floorf(xf);
-   rect.y2 = floorf(yf);
-
-   return weston_transformed_rect(surface-width_from_buffer,
-  surface-height_from_buffer,
-  vp-buffer.transform, vp-buffer.scale,
-  rect);
+   return