Created attachment 231577 [details] [review]
actor: rollback pivot translation even on explicit transforms

When setting an explicit transform with clutter_actor_set_transform()
and a non (0,0) pivot-point, clutter_actor_apply_transform() will fail
to roll back the pivot-point translation done before multiplying the
transformation matrix due to the "out:" label being slightly misplaced
in clutter_actor_real_apply_transform().

This works properly:
  clutter_actor_set_pivot_point (actor, 0.5, 0.5);
  clutter_actor_set_rotation_angle (actor, CLUTTER_Z_AXIS, 30);

This results in the actor being moved to the pivot-point position:
  clutter_actor_set_pivot_point (actor, 0.5, 0.5);
  clutter_matrix_init_identity(&matrix);
  cogl_matrix_rotate (&matrix, 30, 0, 0, 1.0);
  clutter_actor_set_transform (actor, &matrix);

This also add a conformance test checking that even when using a
pivot-point, no matter how a rotation is set the resulting
transformation matrix will be the same.

Review of attachment 231577 [details] [review]:

looks okay, a couple of issues in the unit test.

::: tests/conform/actor-invariants.c
@@ +450,3 @@
+  ClutterActor *stage, *actor_implicit, *actor_explicit;
+  ClutterMatrix transform, result_implicit, result_explicit;
+  ClutterActorBox allocation = { 0, 0, 90, 30 };

you can use CLUTTER_ACTOR_BOX_INIT here.

@@ +453,3 @@
+  gfloat angle = 30;
+
+  stage = clutter_stage_new ();

you need to destroy the stage you created here, at the end of the test, otherwise you'll leak it and two actors; though I'm not entirely sure a stage is needed at all - in which case you want to create actors, ref_sink() them, and destroy them at the end of the test.

@@ +458,3 @@
+  actor_explicit = clutter_actor_new ();
+
+  clutter_actor_add_child(stage, actor_implicit);

missing space between function and arguments.

@@ +459,3 @@
+
+  clutter_actor_add_child(stage, actor_implicit);
+  clutter_actor_add_child(stage, actor_explicit);

same as above.

Review of attachment 231577 [details] [review]:

::: tests/conform/actor-invariants.c
@@ +453,3 @@
+  gfloat angle = 30;
+
+  stage = clutter_stage_new ();

_clutter_actor_get_transform_info_or_defaults(priv->parent) complained (rightly so, called by way of clutter_actor_get_transform()→clutter_actor_real_apply_transform()) that the object was NULL.

Created attachment 231581 [details] [review]
actor: rollback pivot translation even on explicit transforms

When setting an explicit transform with clutter_actor_set_transform()
and a non (0,0) pivot-point, clutter_actor_apply_transform() will fail
to roll back the pivot-point translation done before multiplying the
transformation matrix due to the "out:" label being slightly misplaced
in clutter_actor_real_apply_transform().

This works properly:
  clutter_actor_set_pivot_point (actor, 0.5, 0.5);
  clutter_actor_set_rotation_angle (actor, CLUTTER_Z_AXIS, 30);

This results in the actor being moved to the pivot-point position:
  clutter_actor_set_pivot_point (actor, 0.5, 0.5);
  clutter_matrix_init_identity(&matrix);
  cogl_matrix_rotate (&matrix, 30, 0, 0, 1.0);
  clutter_actor_set_transform (actor, &matrix);

This also add a conformance test checking that even when using a
pivot-point, no matter how a rotation is set the resulting
transformation matrix will be the same.

Review of attachment 231581 [details] [review]:

looks good.

The following fix has been pushed:
219d0ef actor: rollback pivot translation even on explicit transforms

Created attachment 231587 [details] [review]
actor: rollback pivot translation even on explicit transforms

When setting an explicit transform with clutter_actor_set_transform()
and a non (0,0) pivot-point, clutter_actor_apply_transform() will fail
to roll back the pivot-point translation done before multiplying the
transformation matrix due to the "out:" label being slightly misplaced
in clutter_actor_real_apply_transform().

This works properly:
  clutter_actor_set_pivot_point (actor, 0.5, 0.5);
  clutter_actor_set_rotation_angle (actor, CLUTTER_Z_AXIS, 30);

This results in the actor being moved to the pivot-point position:
  clutter_actor_set_pivot_point (actor, 0.5, 0.5);
  clutter_matrix_init_identity(&matrix);
  cogl_matrix_rotate (&matrix, 30, 0, 0, 1.0);
  clutter_actor_set_transform (actor, &matrix);

This also add a conformance test checking that even when using a
pivot-point, no matter how a rotation is set the resulting
transformation matrix will be the same.