Code review comment for lp:~andreas-pokorny/mir/add-geometry-transformation

All points I mentioned yesterday...

(1) "Transformation" is probably too generic a term for something with very specific operations like this. It needs a name that better distinguishes it from a common "mat4 transformation".

(2) Rotation is somewhat limited. We presently support rotations around arbitrary axes whereas this proposal limits that to only 3 possible axes. Although given the render_surfaces demo is probably going to be the only user of this, that's OK, but...

(3) As render_surfaces is the only user of rotations, rotations should not be explicitly built into the class. They should be provided by render_surfaces as simply a matrix.

(4) Only supporting one order of operations is too restrictive:
161 + * The transformation is defined as:
162 + * translate(translation) * translate(size*scale/2) * rotation(y,x,z) * scale * translate(-size*scale/2)
Transformations are arbitrary and should always be arbitrary. Imposing only one order and fixed set of operations would be an unacceptable step backwards.

Overall, I think the Transformation class is a mistake. Too restrictive in that it makes transformations less flexible, and more complicated at the same time. We should only deal in mat4's and never try to disassemble a mat4 into separate transformations.