[mutter/gbsneto/graphene2: 45/45] clutter/actor: Revert transform order

[Georges Basile Stavracas Neto <gbsneto src gnome org>]

commit 3004c5423b5a37dadc45886517f469280749c3b8
Author: Georges Basile Stavracas Neto <georges stavracas gmail com>
Date:   Tue Mar 12 14:38:11 2019 -0300

    clutter/actor: Revert transform order
    
    When doing affine transforms on 2D and 3D spaces, operations are
    done relative to the origin. That means that, when applying
    rotations and scales, we must:
    
     * translate (-anchor_x, -anchor_y, -anchor_z)
     * apply the operation
     * translate (anchor_x, anchor_y, anchor_z)
    
    Since OpenGL has its matrices applied in the reverse order, the
    usual way to do it is, then:
    
     * translate (anchor_x, anchor_y, anchor_z)
     * apply the operation
     * translate (-anchor_x, anchor_y, anchor_z)
    
    However, graphene matrices do not follow the GL format, so matrix
    operations are done as the first example. Now that we are using
    graphene_matrix_t for every matrix operation, the transform order
    is wrong.
    
    Apply the transform operations in the opposite order.

 clutter/clutter/clutter-actor.c | 122 ++++++++++++++++------------------------
 1 file changed, 50 insertions(+), 72 deletions(-)
---
diff --git a/clutter/clutter/clutter-actor.c b/clutter/clutter/clutter-actor.c
index 13d8399aa..496539a6a 100644
--- a/clutter/clutter/clutter-actor.c
+++ b/clutter/clutter/clutter-actor.c
@@ -36,15 +36,15 @@
  * or clutter_actor_set_rotation(). The order in which the transformations are
  * applied is decided by Clutter and it is the following:
  *
- *  1. translation by the origin of the #ClutterActor:allocation property
- *  2. translation by the actor's #ClutterActor:z-position property
- *  3. translation by the actor's #ClutterActor:pivot-point property
- *  4. scaling by the #ClutterActor:scale-x and #ClutterActor:scale-y factors
+ *  1. negative translation by the actor's #ClutterActor:pivot-point
+ *  2. negative translation by the #ClutterActor:anchor-x and #ClutterActor:anchor-y point.
+ *  3. rotation around the #ClutterActor:rotation-angle-z and #ClutterActor:rotation-center-z
+ *  4. rotation around the #ClutterActor:rotation-angle-y and #ClutterActor:rotation-center-y
  *  5. rotation around the #ClutterActor:rotation-angle-x and #ClutterActor:rotation-center-x
- *  6. rotation around the #ClutterActor:rotation-angle-y and #ClutterActor:rotation-center-y
- *  7. rotation around the #ClutterActor:rotation-angle-z and #ClutterActor:rotation-center-z
- *  8. negative translation by the #ClutterActor:anchor-x and #ClutterActor:anchor-y point.
- *  9. negative translation by the actor's #ClutterActor:pivot-point
+ *  6. scaling by the #ClutterActor:scale-x and #ClutterActor:scale-y factors
+ *  7. translation by the actor's #ClutterActor:pivot-point property
+ *  8. translation by the actor's #ClutterActor:z-position property
+ *  9. translation by the origin of the #ClutterActor:allocation property
  *
  * ## Modifying an actor's geometry ## {#clutter-actor-geometry}
  *
@@ -1069,8 +1069,6 @@ static ClutterGravity clutter_anchor_coord_get_gravity (const AnchorCoord *coord
 static void           clutter_anchor_coord_set_gravity (AnchorCoord       *coord,
                                                         ClutterGravity     gravity);
 
-static gboolean clutter_anchor_coord_is_zero (const AnchorCoord *coord);
-
 static void _clutter_actor_get_relative_transformation_matrix (ClutterActor *self,
                                                                ClutterActor *ancestor,
                                                                CoglMatrix *matrix);
@@ -1111,9 +1109,9 @@ static void clutter_actor_pop_in_cloned_branch (ClutterActor *self,
 #define TRANSFORM_ABOUT_ANCHOR_COORD(a,m,c,_transform)  G_STMT_START { \
   gfloat _tx, _ty, _tz;                                                \
   clutter_anchor_coord_get_units ((a), (c), &_tx, &_ty, &_tz);         \
-  cogl_matrix_translate ((m), _tx, _ty, _tz);                          \
+  cogl_matrix_translate ((m), -_tx, -_ty, -_tz);                       \
   { _transform; }                                                      \
-  cogl_matrix_translate ((m), -_tx, -_ty, -_tz);        } G_STMT_END
+  cogl_matrix_translate ((m), _tx, _ty, _tz);        } G_STMT_END
 
 static GQuark quark_actor_layout_info = 0;
 static GQuark quark_actor_transform_info = 0;
@@ -3150,6 +3148,7 @@ clutter_actor_real_apply_transform (ClutterActor  *self,
   CoglMatrix *transform = &priv->transform;
   const ClutterTransformInfo *info;
   float pivot_x = 0.f, pivot_y = 0.f;
+  float tx, ty, tz;
 
   /* we already have a cached transformation */
   if (priv->transform_valid)
@@ -3193,54 +3192,37 @@ clutter_actor_real_apply_transform (ClutterActor  *self,
        * space, and to the pivot point
        */
       cogl_matrix_translate (transform,
-                             priv->allocation.x1 + pivot_x,
-                             priv->allocation.y1 + pivot_y,
-                             info->pivot_z);
-      cogl_matrix_multiply (transform, transform, &info->transform);
+                             -pivot_x,
+                             -pivot_y,
+                             -info->pivot_z);
+      cogl_matrix_multiply (transform, &info->transform, transform);
+      cogl_matrix_translate (transform,
+                             priv->allocation.x1,
+                             priv->allocation.y1,
+                             0.0f);
       goto roll_back_pivot;
     }
 
-  /* basic translation: :allocation's origin and :z-position; instead
-   * of decomposing the pivot and translation info separate operations,
-   * we just compose everything into a single translation
-   */
-  cogl_matrix_translate (transform,
-                         priv->allocation.x1 + pivot_x + info->translation.x,
-                         priv->allocation.y1 + pivot_y + info->translation.y,
-                         info->z_position + info->pivot_z + info->translation.z);
-
-  /* because the rotation involves translations, we must scale
-   * before applying the rotations (if we apply the scale after
-   * the rotations, the translations included in the rotation are
-   * not scaled and so the entire object will move on the screen
-   * as a result of rotating it).
-   *
-   * XXX:2.0 the comment has to be reworded once we remove the
-   * per-transformation centers; we also may want to apply rotation
-   * first and scaling after, to match the matrix decomposition
-   * code we use when interpolating transformations
-   */
-  if (info->scale_x != 1.0 || info->scale_y != 1.0 || info->scale_z != 1.0)
-    {
-      /* XXX:2.0 remove anchor coord */
-      TRANSFORM_ABOUT_ANCHOR_COORD (self, transform,
-                                    &info->scale_center,
-                                    cogl_matrix_scale (transform,
-                                                       info->scale_x,
-                                                       info->scale_y,
-                                                       info->scale_z));
-    }
+  /* XXX:2.0 remove anchor point translation */
+  clutter_anchor_coord_get_units (self, &info->anchor, &tx, &ty, &tz);
+  tx += pivot_x;
+  ty += pivot_y;
+  tz += info->pivot_z;
 
-  if (info->rz_angle)
+  if (tx != 0.f || ty != 0.f || tz != 0.f)
+    cogl_matrix_translate (transform, -tx, -ty, -tz);
+
+  if (info->rx_angle)
     {
       /* XXX:2.0 remove anchor coord */
       TRANSFORM_ABOUT_ANCHOR_COORD (self, transform,
-                                    &info->rz_center,
+                                    &info->rx_center,
                                     cogl_matrix_rotate (transform,
-                                                        info->rz_angle,
-                                                        0, 0, 1.0));
+                                                        info->rx_angle,
+                                                        1.0, 0, 0));
     }
 
+
   if (info->ry_angle)
     {
       /* XXX:2.0 remove anchor coord */
@@ -3251,29 +3233,36 @@ clutter_actor_real_apply_transform (ClutterActor  *self,
                                                         0, 1.0, 0));
     }
 
-  if (info->rx_angle)
+  if (info->rz_angle)
     {
       /* XXX:2.0 remove anchor coord */
       TRANSFORM_ABOUT_ANCHOR_COORD (self, transform,
-                                    &info->rx_center,
+                                    &info->rz_center,
                                     cogl_matrix_rotate (transform,
-                                                        info->rx_angle,
-                                                        1.0, 0, 0));
+                                                        info->rz_angle,
+                                                        0, 0, 1.0));
     }
 
-  /* XXX:2.0 remove anchor point translation */
-  if (!clutter_anchor_coord_is_zero (&info->anchor))
+  if (info->scale_x != 1.0 || info->scale_y != 1.0 || info->scale_z != 1.0)
     {
-      gfloat x, y, z;
-
-      clutter_anchor_coord_get_units (self, &info->anchor, &x, &y, &z);
-      cogl_matrix_translate (transform, -x, -y, -z);
+      /* XXX:2.0 remove anchor coord */
+      TRANSFORM_ABOUT_ANCHOR_COORD (self, transform,
+                                    &info->scale_center,
+                                    cogl_matrix_scale (transform,
+                                                       info->scale_x,
+                                                       info->scale_y,
+                                                       info->scale_z));
     }
 
+  cogl_matrix_translate (transform,
+                         priv->allocation.x1 + info->translation.x,
+                         priv->allocation.y1 + info->translation.y,
+                         info->z_position + info->translation.z);
+
 roll_back_pivot:
   /* roll back the pivot translation */
   if (pivot_x != 0.f || pivot_y != 0.f || info->pivot_z != 0.f)
-    cogl_matrix_translate (transform, -pivot_x, -pivot_y, -info->pivot_z);
+    cogl_matrix_translate (transform, pivot_x, pivot_y, info->pivot_z);
 
   /* we have a valid modelview */
   priv->transform_valid = TRUE;
@@ -16167,17 +16156,6 @@ clutter_anchor_coord_set_gravity (AnchorCoord    *coord,
   coord->is_fractional = TRUE;
 }
 
-static gboolean
-clutter_anchor_coord_is_zero (const AnchorCoord *coord)
-{
-  if (coord->is_fractional)
-    return coord->v.fraction.x == 0.0 && coord->v.fraction.y == 0.0;
-  else
-    return (coord->v.units.x == 0.0
-            && coord->v.units.y == 0.0
-            && coord->v.units.z == 0.0);
-}
-
 /**
  * clutter_actor_get_flags:
  * @self: a #ClutterActor