~eliasnaur/gio-patches

app/internal, internal, op: implement affine transforms v2 PROPOSED

Péter Szilágyi: 1
 app/internal, internal, op: implement affine transforms

 7 files changed, 445 insertions(+), 86 deletions(-)
Export patchset (mbox)
How do I use this?

Copy & paste the following snippet into your terminal to import this patchset into git:

curl -s https://lists.sr.ht/~eliasnaur/gio-patches/patches/9128/mbox | git am -3
Learn more about email & git

[PATCH v2] app/internal, internal, op: implement affine transforms Export this patch

Currently Gio has a simplistic transformation operation implemented
that's essentially an offset vector. This is enough for many layout
tasks, but falls short when animations come into the picture (e.g.
spinner).

This CL replaces the current transform op with one base on affine
transformation matrices, thus enabling advanced operations such as
scaling, rotation and shearing.

Since it's expected that most UIs will not use (or use in very limited
portions) the advanced transforms, the CL also special cases the offset
use case, so both calculations and serializations using only translation
will take optimized coda paths. This is handled under the hood.

Whilst the CL does implement all the affine transforms, they cannot be
readily used yet for the widgets, as the `gpu` package does not use the
transforms correctly. Instead of transforming the input coordinates, it
transforms an offset vector and uses that to only offset the inputs.
This needs to be fixed but I considered it out of scope of the affine
calculations and left it for a followup CL.

Signed-off-by: Péter Szilágyi <peterke@gmail.com>
---
 app/internal/gpu/gpu.go       |   5 +-
 app/internal/input/pointer.go |   5 +-
 internal/opconst/ops.go       |  41 +++--
 internal/ops/ops.go           |  23 ---
 op/op.go                      |  38 ----
 op/transform.go               | 331 ++++++++++++++++++++++++++++++++++
 op/transform_test.go          |  88 +++++++++
 7 files changed, 445 insertions(+), 86 deletions(-)
 delete mode 100644 internal/ops/ops.go
 create mode 100644 op/transform.go
 create mode 100644 op/transform_test.go

diff --git a/app/internal/gpu/gpu.go b/app/internal/gpu/gpu.go
index b9b3f43..c7dc31f 100644
--- a/app/internal/gpu/gpu.go
+++ b/app/internal/gpu/gpu.go
@@ -722,9 +722,8 @@ func (d *drawOps) collectOps(r *ops.Reader, state drawState) int {
 loop:
 	for encOp, ok := r.Decode(); ok; encOp, ok = r.Decode() {
 		switch opconst.OpType(encOp.Data[0]) {
-		case opconst.TypeTransform:
-			dop := ops.DecodeTransformOp(encOp.Data)
-			state.t = state.t.Multiply(op.TransformOp(dop))
+		case opconst.TypeTransformOffset, opconst.TypeTransform:
+			state.t = state.t.Apply(op.DecodeTransform(encOp.Data))
 		case opconst.TypeAux:
 			aux = encOp.Data[opconst.TypeAuxLen:]
 			// The first data byte stores whether the MaxY
diff --git a/app/internal/input/pointer.go b/app/internal/input/pointer.go
index d2fffd6..2a227a1 100644
--- a/app/internal/input/pointer.go
+++ b/app/internal/input/pointer.go
@@ -85,9 +85,8 @@ func (q *pointerQueue) collectHandlers(r *ops.Reader, events *handlerEvents, t o
 				pass: pass,
 			})
 			node = len(q.hitTree) - 1
-		case opconst.TypeTransform:
-			dop := ops.DecodeTransformOp(encOp.Data)
-			t = t.Multiply(op.TransformOp(dop))
+		case opconst.TypeTransformOffset, opconst.TypeTransform:
+			t = t.Apply(op.DecodeTransform(encOp.Data))
 		case opconst.TypePointerInput:
 			op := decodePointerInputOp(encOp.Data, encOp.Refs)
 			q.hitTree = append(q.hitTree, hitNode{
diff --git a/internal/opconst/ops.go b/internal/opconst/ops.go
index 6bca3d0..7099ffb 100644
--- a/internal/opconst/ops.go
+++ b/internal/opconst/ops.go
@@ -10,7 +10,8 @@ const firstOpIndex = 200
 const (
 	TypeMacroDef OpType = iota + firstOpIndex
 	TypeMacro
-	TypeTransform
+	TypeTransform       // Generic affine transformation
+	TypeTransformOffset // Offset only affine transformation
 	TypeLayer
 	TypeInvalidate
 	TypeImage
@@ -29,24 +30,25 @@ const (
 )
 
 const (
-	TypeMacroDefLen     = 1 + 4 + 4
-	TypeMacroLen        = 1 + 4 + 4 + 4
-	TypeTransformLen    = 1 + 4*2
-	TypeLayerLen        = 1
-	TypeRedrawLen       = 1 + 8
-	TypeImageLen        = 1
-	TypePaintLen        = 1 + 4*4
-	TypeColorLen        = 1 + 4
-	TypeAreaLen         = 1 + 1 + 4*4
-	TypePointerInputLen = 1 + 1
-	TypePassLen         = 1 + 1
-	TypeKeyInputLen     = 1 + 1
-	TypeHideInputLen    = 1
-	TypePushLen         = 1
-	TypePopLen          = 1
-	TypeAuxLen          = 1
-	TypeClipLen         = 1 + 4*4
-	TypeProfileLen      = 1
+	TypeMacroDefLen        = 1 + 4 + 4
+	TypeMacroLen           = 1 + 4 + 4 + 4
+	TypeTransformLen       = 1 + 4*18 // Affine transform matrix + its inverse
+	TypeTransformOffsetLen = 1 + 4*2  // X and Y offset
+	TypeLayerLen           = 1
+	TypeRedrawLen          = 1 + 8
+	TypeImageLen           = 1
+	TypePaintLen           = 1 + 4*4
+	TypeColorLen           = 1 + 4
+	TypeAreaLen            = 1 + 1 + 4*4
+	TypePointerInputLen    = 1 + 1
+	TypePassLen            = 1 + 1
+	TypeKeyInputLen        = 1 + 1
+	TypeHideInputLen       = 1
+	TypePushLen            = 1
+	TypePopLen             = 1
+	TypeAuxLen             = 1
+	TypeClipLen            = 1 + 4*4
+	TypeProfileLen         = 1
 )
 
 func (t OpType) Size() int {
@@ -54,6 +56,7 @@ func (t OpType) Size() int {
 		TypeMacroDefLen,
 		TypeMacroLen,
 		TypeTransformLen,
+		TypeTransformOffsetLen,
 		TypeLayerLen,
 		TypeRedrawLen,
 		TypeImageLen,
diff --git a/internal/ops/ops.go b/internal/ops/ops.go
deleted file mode 100644
index c8a9f89..0000000
--- a/internal/ops/ops.go
@@ -1,23 +0,0 @@
-// SPDX-License-Identifier: Unlicense OR MIT
-
-package ops
-
-import (
-	"encoding/binary"
-	"math"
-
-	"gioui.org/f32"
-	"gioui.org/internal/opconst"
-	"gioui.org/op"
-)
-
-func DecodeTransformOp(d []byte) op.TransformOp {
-	bo := binary.LittleEndian
-	if opconst.OpType(d[0]) != opconst.TypeTransform {
-		panic("invalid op")
-	}
-	return op.TransformOp{}.Offset(f32.Point{
-		X: math.Float32frombits(bo.Uint32(d[1:])),
-		Y: math.Float32frombits(bo.Uint32(d[5:])),
-	})
-}
diff --git a/op/op.go b/op/op.go
index 9cc67e3..98888fe 100644
--- a/op/op.go
+++ b/op/op.go
@@ -64,10 +64,8 @@ package op
 
 import (
 	"encoding/binary"
-	"math"
 	"time"
 
-	"gioui.org/f32"
 	"gioui.org/internal/opconst"
 )
 
@@ -110,12 +108,6 @@ type InvalidateOp struct {
 	At time.Time
 }
 
-// TransformOp applies a transform to the current transform.
-type TransformOp struct {
-	// TODO: general transformations.
-	offset f32.Point
-}
-
 type pc struct {
 	data int
 	refs int
@@ -256,33 +248,3 @@ func (r InvalidateOp) Add(o *Ops) {
 		}
 	}
 }
-
-// Offset the transformation.
-func (t TransformOp) Offset(o f32.Point) TransformOp {
-	return t.Multiply(TransformOp{o})
-}
-
-// Invert the transformation.
-func (t TransformOp) Invert() TransformOp {
-	return TransformOp{offset: t.offset.Mul(-1)}
-}
-
-// Transform a point.
-func (t TransformOp) Transform(p f32.Point) f32.Point {
-	return p.Add(t.offset)
-}
-
-// Multiply by a transformation.
-func (t TransformOp) Multiply(t2 TransformOp) TransformOp {
-	return TransformOp{
-		offset: t.offset.Add(t2.offset),
-	}
-}
-
-func (t TransformOp) Add(o *Ops) {
-	data := o.Write(opconst.TypeTransformLen)
-	data[0] = byte(opconst.TypeTransform)
-	bo := binary.LittleEndian
-	bo.PutUint32(data[1:], math.Float32bits(t.offset.X))
-	bo.PutUint32(data[5:], math.Float32bits(t.offset.Y))
-}
diff --git a/op/transform.go b/op/transform.go
new file mode 100644
index 0000000..d5c68ce
--- /dev/null
+++ b/op/transform.go
@@ -0,0 +1,331 @@
+// SPDX-License-Identifier: Unlicense OR MIT
+
+package op
+
+import (
+	"encoding/binary"
+	"math"
+
+	"gioui.org/f32"
+	"gioui.org/internal/opconst"
+)
+
+// TransformOp applies an transformation (translate, rotate, scale, shear).
+type TransformOp struct {
+	inited  bool          // Whether the matrix was inited (if not, use identity on first op)
+	offset  bool          // Whether only translations are done (small to transfer, easy to invert)
+	matrix  [3][3]float32 // Affine transformation matrix (translate, rotate, scale, shear)
+	inverse [3][3]float32 // Inverse of the transformation matrix to undo it
+}
+
+// Offset the transformation with the given vector.
+func (t TransformOp) Offset(amount f32.Point) TransformOp {
+	return t.Apply(TransformOp{
+		inited: true,
+		offset: true,
+		matrix: [3][3]float32{
+			{1, 0, amount.X},
+			{0, 1, amount.Y},
+			{0, 0, 1},
+		},
+		inverse: [3][3]float32{
+			{1, 0, -amount.X},
+			{0, 1, -amount.Y},
+			{0, 0, 1},
+		},
+	})
+}
+
+// Scale the transformation by the given factors. If either of the components is
+// zero the method will panic.
+func (t TransformOp) Scale(factor f32.Point) TransformOp {
+	return t.Apply(TransformOp{
+		inited: true,
+		matrix: [3][3]float32{
+			{factor.X, 0, 0},
+			{0, factor.Y, 0},
+			{0, 0, 1},
+		},
+		inverse: [3][3]float32{
+			{1 / factor.X, 0, 0},
+			{0, 1 / factor.Y, 0},
+			{0, 0, 1},
+		},
+	})
+}
+
+// Rotate the transformation by the given angle (in radians) counter clockwise.
+func (t TransformOp) Rotate(angle float32) TransformOp {
+	sin, cos := math.Sincos(float64(angle))
+
+	return t.Apply(TransformOp{
+		inited: true,
+		matrix: [3][3]float32{
+			{float32(cos), -float32(sin), 0},
+			{float32(sin), float32(cos), 0},
+			{0, 0, 1},
+		},
+		inverse: [3][3]float32{
+			{float32(cos), float32(sin), 0},
+			{-float32(sin), float32(cos), 0},
+			{0, 0, 1},
+		},
+	})
+}
+
+// Shear the transformation by the given angles (in radians). If either of the
+// components is a multiple of PI/2, the method will panic.
+func (t TransformOp) Shear(angles f32.Point) TransformOp {
+	tx := float32(math.Tan(float64(angles.X)))
+	ty := float32(math.Tan(float64(angles.Y)))
+
+	return t.Apply(TransformOp{
+		inited: true,
+		matrix: [3][3]float32{
+			{1, tx, 0},
+			{ty, 1, 0},
+			{0, 0, 1},
+		},
+		inverse: [3][3]float32{
+			{1, -tx, 0},
+			{-ty, 1, 0},
+			{0, 0, 1},
+		},
+	})
+}
+
+// Apply appends a transformation on top of the current stack. Note, internally
+// this method multiplies the matrices in reverse (T2 x T) to keep the user's
+// expected order (final 2D transform product goes on the right side of the eq).
+func (t TransformOp) Apply(op TransformOp) TransformOp {
+	// Make sure uninitialized matrices don't produce junk results
+	switch {
+	case !t.inited && !op.inited:
+		// Both matrices uninitialized, return the identity and also fake a
+		// pure offset transform hoping only that will be needed.
+		return TransformOp{
+			inited: true,
+			offset: true,
+			matrix: [3][3]float32{
+				{1, 0, 0},
+				{0, 1, 0},
+				{0, 0, 1},
+			},
+			inverse: [3][3]float32{
+				{1, 0, 0},
+				{0, 1, 0},
+				{0, 0, 1},
+			},
+		}
+
+	case !t.inited:
+		// Origin matrix uninitualized, assume identity noop multiplication
+		return op
+
+	case !op.inited:
+		// Operator matrix uninitualized, assume identity noop multiplication
+		return t
+	}
+	// Both transformations are initialized already, try to take a shortcut if
+	// both are simple offset transforms.
+	if t.offset && op.offset {
+		return TransformOp{
+			inited: true,
+			offset: true,
+			matrix: [3][3]float32{
+				{1, 0, t.matrix[0][2] + op.matrix[0][2]},
+				{0, 1, t.matrix[1][2] + op.matrix[1][2]},
+				{0, 0, 1},
+			},
+			inverse: [3][3]float32{
+				{1, 0, t.inverse[0][2] + op.inverse[0][2]},
+				{0, 1, t.inverse[1][2] + op.inverse[1][2]},
+				{0, 0, 1},
+			},
+		}
+	}
+	// Both matrices valid, do an actual multiplication, but reverse the order
+	// so that the operator is actually applied on top of the stack, not below.
+	// At the same time, calculate the actual order transform of the inverse
+	// matrices to be able to undo a transform too.
+	m1, m2 := op.matrix, t.matrix
+	i1, i2 := op.inverse, t.inverse
+
+	return TransformOp{
+		inited: true,
+		offset: op.offset && t.offset,
+		matrix: [3][3]float32{
+			{
+				m1[0][0]*m2[0][0] + m1[0][1]*m2[1][0] + m1[0][2]*m2[2][0],
+				m1[0][0]*m2[0][1] + m1[0][1]*m2[1][1] + m1[0][2]*m2[2][1],
+				m1[0][0]*m2[0][2] + m1[0][1]*m2[1][2] + m1[0][2]*m2[2][2],
+			},
+			{
+				m1[1][0]*m2[0][0] + m1[1][1]*m2[1][0] + m1[1][2]*m2[2][0],
+				m1[1][0]*m2[0][1] + m1[1][1]*m2[1][1] + m1[1][2]*m2[2][1],
+				m1[1][0]*m2[0][2] + m1[1][1]*m2[1][2] + m1[1][2]*m2[2][2],
+			},
+			{
+				m1[2][0]*m2[0][0] + m1[2][1]*m2[1][0] + m1[2][2]*m2[2][0],
+				m1[2][0]*m2[0][1] + m1[2][1]*m2[1][1] + m1[2][2]*m2[2][1],
+				m1[2][0]*m2[0][2] + m1[2][1]*m2[1][2] + m1[2][2]*m2[2][2],
+			},
+		},
+		inverse: [3][3]float32{
+			{
+				i2[0][0]*i1[0][0] + i2[0][1]*i1[1][0] + i2[0][2]*i1[2][0],
+				i2[0][0]*i1[0][1] + i2[0][1]*i1[1][1] + i2[0][2]*i1[2][1],
+				i2[0][0]*i1[0][2] + i2[0][1]*i1[1][2] + i2[0][2]*i1[2][2],
+			},
+			{
+				i2[1][0]*i1[0][0] + i2[1][1]*i1[1][0] + i2[1][2]*i1[2][0],
+				i2[1][0]*i1[0][1] + i2[1][1]*i1[1][1] + i2[1][2]*i1[2][1],
+				i2[1][0]*i1[0][2] + i2[1][1]*i1[1][2] + i2[1][2]*i1[2][2],
+			},
+			{
+				i2[2][0]*i1[0][0] + i2[2][1]*i1[1][0] + i2[2][2]*i1[2][0],
+				i2[2][0]*i1[0][1] + i2[2][1]*i1[1][1] + i2[2][2]*i1[2][1],
+				i2[2][0]*i1[0][2] + i2[2][1]*i1[1][2] + i2[2][2]*i1[2][2],
+			},
+		},
+	}
+}
+
+// Invert the transformation. Since all inverse transforms are calculated on the
+// fly along the actual transforms, this method just returns the already computed
+// matrices.
+func (t TransformOp) Invert() TransformOp {
+	return TransformOp{
+		inited:  true,
+		offset:  t.offset,
+		matrix:  t.inverse,
+		inverse: t.matrix,
+	}
+}
+
+// Transform a point in the 2D space according to the applied transform stack.
+func (t TransformOp) Transform(p f32.Point) f32.Point {
+	// Skip operation if the transform is the noop identity
+	if !t.inited {
+		return p
+	}
+	// Special case offset-only transforms as they are fast to compute
+	if t.offset {
+		return p.Add(f32.Point{X: t.matrix[0][2], Y: t.matrix[1][2]})
+	}
+	// No luck, complex transform needed, execute the matric multiplication
+	m := t.matrix
+	return f32.Point{
+		X: p.X*m[0][0] + p.Y*m[0][1] + m[0][2],
+		Y: p.X*m[1][0] + p.Y*m[1][1] + m[1][2],
+	}
+}
+
+// Add inserts this transformation op onto the end of the ops stack.
+func (t TransformOp) Add(o *Ops) {
+	bo := binary.LittleEndian
+
+	// If the transform is offset only (usual case), we can cheat by serializing
+	// only the offset vector instead of the entire transformation matrix
+	if !t.inited || t.offset {
+		data := o.Write(opconst.TypeTransformOffsetLen)
+		data[0] = byte(opconst.TypeTransformOffset)
+
+		bo.PutUint32(data[1:], math.Float32bits(t.matrix[0][2]))
+		bo.PutUint32(data[5:], math.Float32bits(t.matrix[1][2]))
+
+		return
+	}
+	// Seems we have a full affine transformation, serialize the entire matrix
+	data := o.Write(opconst.TypeTransformLen)
+	data[0] = byte(opconst.TypeTransform)
+
+	m := t.matrix  // Surely inited, because uninited is handled as offset-only
+	i := t.inverse // Surely inited, because uninited is handled as offset-only
+
+	bo.PutUint32(data[1:], math.Float32bits(m[0][0]))
+	bo.PutUint32(data[5:], math.Float32bits(m[0][1]))
+	bo.PutUint32(data[9:], math.Float32bits(m[0][2]))
+	bo.PutUint32(data[13:], math.Float32bits(m[1][0]))
+	bo.PutUint32(data[17:], math.Float32bits(m[1][1]))
+	bo.PutUint32(data[21:], math.Float32bits(m[1][2]))
+	bo.PutUint32(data[25:], math.Float32bits(m[2][0]))
+	bo.PutUint32(data[29:], math.Float32bits(m[2][1]))
+	bo.PutUint32(data[33:], math.Float32bits(m[2][2]))
+
+	bo.PutUint32(data[37:], math.Float32bits(i[0][0]))
+	bo.PutUint32(data[41:], math.Float32bits(i[0][1]))
+	bo.PutUint32(data[45:], math.Float32bits(i[0][2]))
+	bo.PutUint32(data[49:], math.Float32bits(i[1][0]))
+	bo.PutUint32(data[53:], math.Float32bits(i[1][1]))
+	bo.PutUint32(data[57:], math.Float32bits(i[1][2]))
+	bo.PutUint32(data[61:], math.Float32bits(i[2][0]))
+	bo.PutUint32(data[65:], math.Float32bits(i[2][1]))
+	bo.PutUint32(data[69:], math.Float32bits(i[2][2]))
+}
+
+// DecodeTransform parses a transform matrics from a binary format.
+func DecodeTransform(d []byte) TransformOp {
+	bo := binary.LittleEndian
+
+	// Ensure we have a valid decoding call
+	if opconst.OpType(d[0]) != opconst.TypeTransformOffset && opconst.OpType(d[0]) != opconst.TypeTransform {
+		panic("invalid op")
+	}
+	// If the transform is offset only (usual case), we can cheat by deserializing
+	// only the offset vector instead of the entire transformation matrix
+	if opconst.OpType(d[0]) == opconst.TypeTransformOffset {
+		return TransformOp{
+			inited: true,
+			offset: true,
+			matrix: [3][3]float32{
+				{1, 0, math.Float32frombits(bo.Uint32(d[1:]))},
+				{0, 1, math.Float32frombits(bo.Uint32(d[5:]))},
+				{0, 0, 1},
+			},
+			inverse: [3][3]float32{
+				{1, 0, -math.Float32frombits(bo.Uint32(d[1:]))},
+				{0, 1, -math.Float32frombits(bo.Uint32(d[5:]))},
+				{0, 0, 1},
+			},
+		}
+	}
+	// Seems we have a full affine transformation, deserialize the entire matrix
+	return TransformOp{
+		inited: true,
+		matrix: [3][3]float32{
+			{
+				math.Float32frombits(bo.Uint32(d[1:])),
+				math.Float32frombits(bo.Uint32(d[5:])),
+				math.Float32frombits(bo.Uint32(d[9:])),
+			},
+			{
+				math.Float32frombits(bo.Uint32(d[13:])),
+				math.Float32frombits(bo.Uint32(d[17:])),
+				math.Float32frombits(bo.Uint32(d[21:])),
+			},
+			{
+				math.Float32frombits(bo.Uint32(d[25:])),
+				math.Float32frombits(bo.Uint32(d[29:])),
+				math.Float32frombits(bo.Uint32(d[33:])),
+			},
+		},
+		inverse: [3][3]float32{
+			{
+				math.Float32frombits(bo.Uint32(d[37:])),
+				math.Float32frombits(bo.Uint32(d[41:])),
+				math.Float32frombits(bo.Uint32(d[45:])),
+			},
+			{
+				math.Float32frombits(bo.Uint32(d[49:])),
+				math.Float32frombits(bo.Uint32(d[53:])),
+				math.Float32frombits(bo.Uint32(d[57:])),
+			},
+			{
+				math.Float32frombits(bo.Uint32(d[61:])),
+				math.Float32frombits(bo.Uint32(d[65:])),
+				math.Float32frombits(bo.Uint32(d[69:])),
+			},
+		},
+	}
+}
diff --git a/op/transform_test.go b/op/transform_test.go
new file mode 100644
index 0000000..e9c5d27
--- /dev/null
+++ b/op/transform_test.go
@@ -0,0 +1,88 @@
+// SPDX-License-Identifier: Unlicense OR MIT
+
+package op
+
+import (
+	"math"
+	"testing"
+
+	"gioui.org/f32"
+)
+
+// eq checks floating point equality at some epsilon precision.
+func eq(a, b float32) bool {
+	return math.Abs(float64(a)-float64(b)) < 0.00001
+}
+
+func TestTransformOffset(t *testing.T) {
+	p := f32.Point{X: 1, Y: 2}
+	o := f32.Point{X: 2, Y: -3}
+
+	r := TransformOp{}.Offset(o).Transform(p)
+	if !eq(r.X, 3) || !eq(r.Y, -1) {
+		t.Errorf("offset transformation mismatch: have %v, want {3 -1}", r)
+	}
+	i := TransformOp{}.Offset(o).Invert().Transform(r)
+	if !eq(i.X, p.X) || !eq(i.Y, p.Y) {
+		t.Errorf("offset transformation inverse mismatch: have %v, want %v", i, p)
+	}
+}
+
+func TestTransformScale(t *testing.T) {
+	p := f32.Point{X: 1, Y: 2}
+	s := f32.Point{X: -1, Y: 2}
+
+	r := TransformOp{}.Scale(s).Transform(p)
+	if !eq(r.X, -1) || !eq(r.Y, 4) {
+		t.Errorf("scale transformation mismatch: have %v, want {-1 4}", r)
+	}
+	i := TransformOp{}.Scale(s).Invert().Transform(r)
+	if !eq(i.X, p.X) || !eq(i.Y, p.Y) {
+		t.Errorf("scale transformation inverse mismatch: have %v, want %v", i, p)
+	}
+}
+
+func TestTransformRotate(t *testing.T) {
+	p := f32.Point{X: 1, Y: 0}
+	a := float32(math.Pi / 2)
+
+	r := TransformOp{}.Rotate(a).Transform(p)
+	if !eq(r.X, 0) || !eq(r.Y, 1) {
+		t.Errorf("rotate transformation mismatch: have %v, want {0 1}", r)
+	}
+	i := TransformOp{}.Rotate(a).Invert().Transform(r)
+	if !eq(i.X, p.X) || !eq(i.Y, p.Y) {
+		t.Errorf("rotate transformation inverse mismatch: have %v, want %v", i, p)
+	}
+}
+
+func TestTransformShear(t *testing.T) {
+	p := f32.Point{X: 1, Y: 1}
+	a := f32.Point{X: math.Pi / 4, Y: 0}
+
+	r := TransformOp{}.Shear(a).Transform(p)
+	if !eq(r.X, 2) || !eq(r.Y, 1) {
+		t.Errorf("shear transformation mismatch: have %v, want {2 1}", r)
+	}
+	i := TransformOp{}.Shear(a).Invert().Transform(r)
+	if !eq(i.X, p.X) || !eq(i.Y, p.Y) {
+		t.Errorf("shear transformation inverse mismatch: have %v, want %v", i, p)
+	}
+}
+
+func TestTransformMultiply(t *testing.T) {
+	p := f32.Point{X: 1, Y: 2}
+	o := f32.Point{X: 2, Y: -3}
+	s := f32.Point{X: -1, Y: 2}
+	a := float32(-math.Pi / 2)
+	h := f32.Point{X: math.Pi / 4, Y: 0}
+
+	r := TransformOp{}.Offset(o).Scale(s).Rotate(a).Shear(h).Transform(p)
+	if !eq(r.X, 1) || !eq(r.Y, 3) {
+		t.Errorf("complex transformation mismatch: have %v, want {1 3}", r)
+	}
+	i := TransformOp{}.Offset(o).Scale(s).Rotate(a).Shear(h).Invert().Transform(r)
+	if !eq(i.X, p.X) || !eq(i.Y, p.Y) {
+		t.Errorf("complex transformation inverse mismatch: have %v, want %v", i, p)
+	}
+}
-- 
2.20.1
Thank your for working on this.

Adding all these operations to TransformOp bloats the op package and strays
from the idea that Ops are simply descriptions, and not (supposed to be)
useful in themselves.

Therefore, I think you should add type Affine2D to the f32 package with
the meat of this implementation. Unlike a general transformation, Affine2D
only takes up a float32[3][2], right?

Then, TransformOp.Invert and TransformOp.Apply should be moved to
internal packages.

Finally, two constructors should exist for TransformOp (the zero value of
TransformOp is the identity transform):

	func Offset(off f32.Point) TransformOp
	func Affine2D(aff f32.Affine2D) TransformOp

I expect that in the future we'll have

	func Affine3D(aff f32.Affine3D) TransformOp

as well as the general

	func Transform2D(aff f32.Transform2D) TransformOp
	func Transform3D(aff f32.Transform3D) TransformOp

(Note that f32.Transform2D is a 3x3 matrix, f32.Transform3D is a 4x4 matrix.)


Finally, I'm not sure maintaining and serializing inverse affine transformations
is worth it. Inverting a 2d affine transformation is equivalent to inverting the
top-left 2x2 matrix. See

	https://stackoverflow.com/questions/2624422/efficient-4x4-matrix-inverse-affine-transform

Inverting a 2x2 matrix is only one division and a few multiplications, see

	https://www.chilimath.com/lessons/advanced-algebra/inverse-of-a-2x2-matrix/

On Sun Nov 24, 2019 at 12:35 AM Péter Szilágyi wrote:
View this thread in the archives