Fix composited transform bounds calculations (#6302)

We weren't computing the bounds for composited transforms correctly. We
need to conjugate the transform by the offset in order to get the
correct paint bounds for the composited layer. We now also use the same
math in the non-composited case for consistency.

Also, don't scale the z-coordinate in RenderFittedBox.

Fixes #6293

packages/flutter/lib/src/painting/transforms.dart

@@ -107,13 +107,13 @@ class MatrixUtils {
     return math.max(a, math.max(b, math.max(c, d)));
   }
 
-  /// Returns a rect that bounds the result of applying the given matrix as a
-  /// perspective transform to the given rect.
+  /// Returns a rect that bounds the result of applying the inverse of the given
+  /// matrix as a perspective transform to the given rect.
   ///
   /// This function assumes the given rect is in the plane with z equals 0.0.
   /// The transformed rect is then projected back into the plane with z equals
   /// 0.0 before computing its bounding rect.
-  static Rect transformRect(Rect rect, Matrix4 transform) {
+  static Rect inverseTransformRect(Rect rect, Matrix4 transform) {
     assert(rect != null);
     assert(transform.determinant != 0.0);
     if (isIdentity(transform))

packages/flutter/lib/src/rendering/layer.dart

@@ -445,17 +445,20 @@ class TransformLayer extends OffsetLayer {
   /// The [transform] property must be non-null before the compositing phase of
   /// the pipeline.
   TransformLayer({
-    Offset offset: Offset.zero,
     this.transform
-  }): super(offset: offset);
+  });
 
   /// The matrix to apply
   Matrix4 transform;
 
   @override
   void addToScene(ui.SceneBuilder builder, Offset layerOffset) {
-    Matrix4 effectiveTransform = new Matrix4.translationValues(offset.dx + layerOffset.dx, offset.dy + layerOffset.dy, 0.0)
-      ..multiply(transform);
+    assert(offset == Offset.zero);
+    Matrix4 effectiveTransform = transform;
+    if (layerOffset != Offset.zero) {
+      effectiveTransform = new Matrix4.translationValues(layerOffset.dx, layerOffset.dy, 0.0)
+        ..multiply(transform);
+    }
     builder.pushTransform(effectiveTransform.storage);
     addChildrenToScene(builder, Offset.zero);
     builder.pop();

packages/flutter/lib/src/rendering/object.dart

@@ -241,17 +241,17 @@ class PaintingContext {
   /// * `painter` is a callback that will paint with the [clipRect] applied. This
   ///   function calls the [painter] synchronously.
   void pushClipRect(bool needsCompositing, Offset offset, Rect clipRect, PaintingContextCallback painter) {
-    Rect offsetClipRect = clipRect.shift(offset);
+    final Rect offsetClipRect = clipRect.shift(offset);
     if (needsCompositing) {
       _stopRecordingIfNeeded();
-      ClipRectLayer clipLayer = new ClipRectLayer(clipRect: offsetClipRect);
+      final ClipRectLayer clipLayer = new ClipRectLayer(clipRect: offsetClipRect);
       _appendLayer(clipLayer);
-      PaintingContext childContext = new PaintingContext._(clipLayer, offsetClipRect);
+      final PaintingContext childContext = new PaintingContext._(clipLayer, offsetClipRect);
       painter(childContext, offset);
       childContext._stopRecordingIfNeeded();
     } else {
       canvas.save();
-      canvas.clipRect(clipRect.shift(offset));
+      canvas.clipRect(offsetClipRect);
       painter(this, offset);
       canvas.restore();
     }
@@ -270,13 +270,13 @@ class PaintingContext {
   /// * `painter` is a callback that will paint with the `clipRRect` applied. This
   ///   function calls the `painter` synchronously.
   void pushClipRRect(bool needsCompositing, Offset offset, Rect bounds, RRect clipRRect, PaintingContextCallback painter) {
-    Rect offsetBounds = bounds.shift(offset);
-    RRect offsetClipRRect = clipRRect.shift(offset);
+    final Rect offsetBounds = bounds.shift(offset);
+    final RRect offsetClipRRect = clipRRect.shift(offset);
     if (needsCompositing) {
       _stopRecordingIfNeeded();
-      ClipRRectLayer clipLayer = new ClipRRectLayer(clipRRect: offsetClipRRect);
+      final ClipRRectLayer clipLayer = new ClipRRectLayer(clipRRect: offsetClipRRect);
       _appendLayer(clipLayer);
-      PaintingContext childContext = new PaintingContext._(clipLayer, offsetBounds);
+      final PaintingContext childContext = new PaintingContext._(clipLayer, offsetBounds);
       painter(childContext, offset);
       childContext._stopRecordingIfNeeded();
     } else {
@@ -300,13 +300,13 @@ class PaintingContext {
   /// * `painter` is a callback that will paint with the `clipPath` applied. This
   ///   function calls the `painter` synchronously.
   void pushClipPath(bool needsCompositing, Offset offset, Rect bounds, Path clipPath, PaintingContextCallback painter) {
-    Rect offsetBounds = bounds.shift(offset);
-    Path offsetClipPath = clipPath.shift(offset);
+    final Rect offsetBounds = bounds.shift(offset);
+    final Path offsetClipPath = clipPath.shift(offset);
     if (needsCompositing) {
       _stopRecordingIfNeeded();
-      ClipPathLayer clipLayer = new ClipPathLayer(clipPath: offsetClipPath);
+      final ClipPathLayer clipLayer = new ClipPathLayer(clipPath: offsetClipPath);
       _appendLayer(clipLayer);
-      PaintingContext childContext = new PaintingContext._(clipLayer, offsetBounds);
+      final PaintingContext childContext = new PaintingContext._(clipLayer, offsetBounds);
       painter(childContext, offset);
       childContext._stopRecordingIfNeeded();
     } else {
@@ -327,20 +327,20 @@ class PaintingContext {
   /// * `painter` is a callback that will paint with the `transform` applied. This
   ///   function calls the `painter` synchronously.
   void pushTransform(bool needsCompositing, Offset offset, Matrix4 transform, PaintingContextCallback painter) {
+    final Matrix4 effectiveTransform = new Matrix4.translationValues(offset.dx, offset.dy, 0.0)
+      ..multiply(transform)..translate(-offset.dx, -offset.dy);
     if (needsCompositing) {
       _stopRecordingIfNeeded();
-      TransformLayer transformLayer = new TransformLayer(offset: offset, transform: transform);
+      final TransformLayer transformLayer = new TransformLayer(transform: effectiveTransform);
       _appendLayer(transformLayer);
-      // TODO(abarth): We need to run _paintBounds through the inverse of transform.
-      PaintingContext childContext = new PaintingContext._(transformLayer, _paintBounds);
-      painter(childContext, Offset.zero);
+      final Rect transformedPaintBounds = MatrixUtils.inverseTransformRect(_paintBounds, effectiveTransform);
+      final PaintingContext childContext = new PaintingContext._(transformLayer, transformedPaintBounds);
+      painter(childContext, offset);
       childContext._stopRecordingIfNeeded();
     } else {
-      Matrix4 offsetMatrix = new Matrix4.translationValues(offset.dx, offset.dy, 0.0);
-      Matrix4 transformWithOffset = offsetMatrix * transform;
       canvas.save();
-      canvas.transform(transformWithOffset.storage);
-      painter(this, Offset.zero);
+      canvas.transform(effectiveTransform.storage);
+      painter(this, offset);
       canvas.restore();
     }
   }
@@ -356,9 +356,9 @@ class PaintingContext {
   ///   function calls the `painter` synchronously.
   void pushOpacity(Offset offset, int alpha, PaintingContextCallback painter) {
     _stopRecordingIfNeeded();
-    OpacityLayer opacityLayer = new OpacityLayer(alpha: alpha);
+    final OpacityLayer opacityLayer = new OpacityLayer(alpha: alpha);
     _appendLayer(opacityLayer);
-    PaintingContext childContext = new PaintingContext._(opacityLayer, _paintBounds);
+    final PaintingContext childContext = new PaintingContext._(opacityLayer, _paintBounds);
     painter(childContext, offset);
     childContext._stopRecordingIfNeeded();
   }
@@ -377,13 +377,13 @@ class PaintingContext {
   ///   function calls the `painter` synchronously.
   void pushShaderMask(Offset offset, Shader shader, Rect maskRect, TransferMode transferMode, PaintingContextCallback painter) {
     _stopRecordingIfNeeded();
-    ShaderMaskLayer shaderLayer = new ShaderMaskLayer(
+    final ShaderMaskLayer shaderLayer = new ShaderMaskLayer(
       shader: shader,
       maskRect: maskRect,
       transferMode: transferMode
     );
     _appendLayer(shaderLayer);
-    PaintingContext childContext = new PaintingContext._(shaderLayer, _paintBounds);
+    final PaintingContext childContext = new PaintingContext._(shaderLayer, _paintBounds);
     painter(childContext, offset);
     childContext._stopRecordingIfNeeded();
   }
@@ -395,9 +395,9 @@ class PaintingContext {
   // TODO(abarth): I don't quite understand how this API is supposed to work.
   void pushBackdropFilter(Offset offset, ui.ImageFilter filter, PaintingContextCallback painter) {
     _stopRecordingIfNeeded();
-    BackdropFilterLayer backdropFilterLayer = new BackdropFilterLayer(filter: filter);
+    final BackdropFilterLayer backdropFilterLayer = new BackdropFilterLayer(filter: filter);
     _appendLayer(backdropFilterLayer);
-    PaintingContext childContext = new PaintingContext._(backdropFilterLayer, _paintBounds);
+    final PaintingContext childContext = new PaintingContext._(backdropFilterLayer, _paintBounds);
     painter(childContext, offset);
     childContext._stopRecordingIfNeeded();
   }
@@ -487,7 +487,7 @@ class _SemanticsGeometry {
         } else {
           Matrix4 clipTransform = new Matrix4.identity();
           parent.applyPaintTransform(child, clipTransform);
-          clipRect = MatrixUtils.transformRect(clipRect, clipTransform);
+          clipRect = MatrixUtils.inverseTransformRect(clipRect, clipTransform);
         }
       }
       parent.applyPaintTransform(child, transform);

packages/flutter/lib/src/rendering/proxy_box.dart

@@ -1479,7 +1479,7 @@ class RenderFittedBox extends RenderProxyBox {
       final Rect destinationRect = _alignment.inscribe(sizes.destination, Point.origin & size);
       _hasVisualOverflow = sourceRect.width < childSize.width || sourceRect.height < childSize.width;
       _transform = new Matrix4.translationValues(destinationRect.left, destinationRect.top, 0.0)
-        ..scale(scaleX, scaleY)
+        ..scale(scaleX, scaleY, 1.0)
         ..translate(-sourceRect.left, -sourceRect.top);
     }
   }

packages/flutter/test/widget/transform_test.dart

@@ -3,7 +3,9 @@
 // found in the LICENSE file.
 
 import 'package:flutter_test/flutter_test.dart';
+import 'package:flutter/rendering.dart';
 import 'package:flutter/widgets.dart';
+import 'package:vector_math/vector_math_64.dart';
 
 void main() {
   testWidgets('Transform origin', (WidgetTester tester) async {
@@ -18,9 +20,9 @@ void main() {
               width: 100.0,
               height: 100.0,
               decoration: new BoxDecoration(
-                backgroundColor: new Color(0xFF0000FF)
-              )
-            )
+                backgroundColor: new Color(0xFF0000FF),
+              ),
+            ),
           ),
           new Positioned(
             top: 100.0,
@@ -37,15 +39,15 @@ void main() {
                   },
                   child: new Container(
                     decoration: new BoxDecoration(
-                      backgroundColor: new Color(0xFF00FFFF)
-                    )
-                  )
-                )
-              )
-            )
-          )
-        ]
-      )
+                      backgroundColor: new Color(0xFF00FFFF),
+                    ),
+                  ),
+                ),
+              ),
+            ),
+          ),
+        ],
+      ),
     );
 
     expect(didReceiveTap, isFalse);
@@ -67,9 +69,9 @@ void main() {
               width: 100.0,
               height: 100.0,
               decoration: new BoxDecoration(
-                backgroundColor: new Color(0xFF0000FF)
-              )
-            )
+                backgroundColor: new Color(0xFF0000FF),
+              ),
+            ),
           ),
           new Positioned(
             top: 100.0,
@@ -86,15 +88,15 @@ void main() {
                   },
                   child: new Container(
                     decoration: new BoxDecoration(
-                      backgroundColor: new Color(0xFF00FFFF)
-                    )
-                  )
-                )
-              )
-            )
-          )
-        ]
-      )
+                      backgroundColor: new Color(0xFF00FFFF),
+                    ),
+                  ),
+                ),
+              ),
+            ),
+          ),
+        ],
+      ),
     );
 
     expect(didReceiveTap, isFalse);
@@ -106,46 +108,44 @@ void main() {
 
   testWidgets('Transform offset + alignment', (WidgetTester tester) async {
     bool didReceiveTap = false;
-    await tester.pumpWidget(
-      new Stack(
-        children: <Widget>[
-          new Positioned(
-            top: 100.0,
-            left: 100.0,
-            child: new Container(
-              width: 100.0,
-              height: 100.0,
-              decoration: new BoxDecoration(
-                backgroundColor: new Color(0xFF0000FF)
-              )
-            )
+    await tester.pumpWidget(new Stack(
+      children: <Widget>[
+        new Positioned(
+          top: 100.0,
+          left: 100.0,
+          child: new Container(
+            width: 100.0,
+            height: 100.0,
+            decoration: new BoxDecoration(
+              backgroundColor: new Color(0xFF0000FF),
+            ),
           ),
-          new Positioned(
-            top: 100.0,
-            left: 100.0,
-            child: new Container(
-              width: 100.0,
-              height: 100.0,
-              child: new Transform(
-                transform: new Matrix4.diagonal3Values(0.5, 0.5, 1.0),
-                origin: new Offset(100.0, 0.0),
-                alignment: new FractionalOffset(0.0, 0.5),
-                child: new GestureDetector(
-                  onTap: () {
-                    didReceiveTap = true;
-                  },
-                  child: new Container(
-                    decoration: new BoxDecoration(
-                      backgroundColor: new Color(0xFF00FFFF)
-                    )
-                  )
-                )
-              )
-            )
-          )
-        ]
-      )
-    );
+        ),
+        new Positioned(
+          top: 100.0,
+          left: 100.0,
+          child: new Container(
+            width: 100.0,
+            height: 100.0,
+            child: new Transform(
+              transform: new Matrix4.diagonal3Values(0.5, 0.5, 1.0),
+              origin: new Offset(100.0, 0.0),
+              alignment: new FractionalOffset(0.0, 0.5),
+              child: new GestureDetector(
+                onTap: () {
+                  didReceiveTap = true;
+                },
+                child: new Container(
+                  decoration: new BoxDecoration(
+                    backgroundColor: new Color(0xFF00FFFF),
+                  ),
+                ),
+              ),
+            ),
+          ),
+        ),
+      ],
+    ));
 
     expect(didReceiveTap, isFalse);
     await tester.tapAt(new Point(110.0, 110.0));
@@ -153,4 +153,36 @@ void main() {
     await tester.tapAt(new Point(190.0, 150.0));
     expect(didReceiveTap, isTrue);
   });
+
+  testWidgets('Composited transform offset', (WidgetTester tester) async {
+    await tester.pumpWidget(
+      new Center(
+        child: new SizedBox(
+          width: 400.0,
+          height: 300.0,
+          child: new ClipRect(
+            child: new Transform(
+              transform: new Matrix4.diagonal3Values(0.5, 0.5, 1.0),
+              child: new Opacity(
+                opacity: 0.9,
+                child: new Container(
+                  decoration: new BoxDecoration(
+                    backgroundColor: const Color(0xFF00FF00),
+                  ),
+                ),
+              ),
+            ),
+          ),
+        ),
+      ),
+    );
+
+    List<Layer> layers = tester.layers
+      ..retainWhere((Layer layer) => layer is TransformLayer);
+    expect(layers.length, 2);
+    // The first transform is from the render view.
+    TransformLayer layer = layers[1];
+    Matrix4 transform = layer.transform;
+    expect(transform.getTranslation(), equals(new Vector3(100.0, 75.0, 0.0)));
+  });
 }