Viewport fails to clip its AssetImage child

We were getting confused about our layer tree offsets, which caused us
to set an incorrect cull rect for the recording inside a viewport.

This patch does away with layer tree offsets almost entirely. We now use
them only at repaint boundaries, which is where we want the layer tree
to be mutable.

Fixes #1460

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

@@ -17,11 +17,6 @@ export 'basic_types.dart';
 /// are uploaded into the engine and displayed by the compositor. This class is
 /// the base class for all composited layers.
 abstract class Layer {
-  Layer({ this.offset: Offset.zero });
-
-  /// Offset from parent in the parent's coordinate system.
-  Offset offset;
-
   /// This layer's parent in the layer tree
   ContainerLayer get parent => _parent;
   ContainerLayer _parent;
@@ -95,7 +90,6 @@ abstract class Layer {
   void debugDescribeSettings(List<String> settings) {
     if (debugOwner != null)
       settings.add('owner: $debugOwner');
-    settings.add('offset: $offset');
   }
 
   String debugDescribeChildren(String prefix) => '';
@@ -103,16 +97,13 @@ abstract class Layer {
 
 /// A composited layer containing a [Picture]
 class PictureLayer extends Layer {
-  PictureLayer({ Offset offset: Offset.zero })
-    : super(offset: offset);
-
   /// The picture recorded for this layer
   ///
   /// The picture's coodinate system matches this layer's coodinate system
   ui.Picture picture;
 
   void addToScene(ui.SceneBuilder builder, Offset layerOffset) {
-    builder.addPicture(offset + layerOffset, picture);
+    builder.addPicture(layerOffset, picture);
   }
 }
 
@@ -120,11 +111,10 @@ class PictureLayer extends Layer {
 /// certain performance statistics within it.
 class PerformanceOverlayLayer extends Layer {
   PerformanceOverlayLayer({
-    Offset offset: Offset.zero,
     this.overlayRect,
     this.optionsMask,
     this.rasterizerThreshold
-  }) : super(offset: offset);
+  });
 
   /// The rectangle in this layer's coodinate system that the overlay should occupy.
   Rect overlayRect;
@@ -136,7 +126,7 @@ class PerformanceOverlayLayer extends Layer {
 
   void addToScene(ui.SceneBuilder builder, Offset layerOffset) {
     assert(optionsMask != null);
-    builder.addPerformanceOverlay(optionsMask, overlayRect.shift(offset + layerOffset));
+    builder.addPerformanceOverlay(optionsMask, overlayRect.shift(layerOffset));
     builder.setRasterizerTracingThreshold(rasterizerThreshold);
   }
 }
@@ -144,8 +134,6 @@ class PerformanceOverlayLayer extends Layer {
 
 /// A composited layer that has a list of children
 class ContainerLayer extends Layer {
-  ContainerLayer({ Offset offset: Offset.zero }) : super(offset: offset);
-
   /// The first composited layer in this layer's child list
   Layer get firstChild => _firstChild;
   Layer _firstChild;
@@ -230,7 +218,7 @@ class ContainerLayer extends Layer {
   }
 
   void addToScene(ui.SceneBuilder builder, Offset layerOffset) {
-    addChildrenToScene(builder, offset + layerOffset);
+    addChildrenToScene(builder, layerOffset);
   }
 
   /// Uploads all of this layer's children to the engine
@@ -261,9 +249,26 @@ class ContainerLayer extends Layer {
   }
 }
 
+class OffsetLayer extends ContainerLayer {
+  OffsetLayer({ this.offset: Offset.zero });
+
+  /// Offset from parent in the parent's coordinate system.
+  Offset offset;
+
+  void addToScene(ui.SceneBuilder builder, Offset layerOffset) {
+    addChildrenToScene(builder, offset + layerOffset);
+  }
+
+  void debugDescribeSettings(List<String> settings) {
+    super.debugDescribeSettings(settings);
+    settings.add('offset: $offset');
+  }
+}
+
+
 /// A composite layer that clips its children using a rectangle
 class ClipRectLayer extends ContainerLayer {
-  ClipRectLayer({ Offset offset: Offset.zero, this.clipRect }) : super(offset: offset);
+  ClipRectLayer({ this.clipRect });
 
   /// The rectangle to clip in the parent's coordinate system
   Rect clipRect;
@@ -271,9 +276,8 @@ class ClipRectLayer extends ContainerLayer {
   // instead of in the coordinate system of this layer?
 
   void addToScene(ui.SceneBuilder builder, Offset layerOffset) {
-    Offset childOffset = offset + layerOffset;
-    builder.pushClipRect(clipRect.shift(childOffset));
-    addChildrenToScene(builder, childOffset);
+    builder.pushClipRect(clipRect.shift(layerOffset));
+    addChildrenToScene(builder, layerOffset);
     builder.pop();
   }
 
@@ -285,7 +289,7 @@ class ClipRectLayer extends ContainerLayer {
 
 /// A composite layer that clips its children using a rounded rectangle
 class ClipRRectLayer extends ContainerLayer {
-  ClipRRectLayer({ Offset offset: Offset.zero, this.clipRRect }) : super(offset: offset);
+  ClipRRectLayer({ this.clipRRect });
 
   /// The rounded-rect to clip in the parent's coordinate system
   ui.RRect clipRRect;
@@ -293,9 +297,8 @@ class ClipRRectLayer extends ContainerLayer {
   // instead of in the coordinate system of this layer?
 
   void addToScene(ui.SceneBuilder builder, Offset layerOffset) {
-    Offset childOffset = offset + layerOffset;
-    builder.pushClipRRect(clipRRect.shift(childOffset));
-    addChildrenToScene(builder, childOffset);
+    builder.pushClipRRect(clipRRect.shift(layerOffset));
+    addChildrenToScene(builder, layerOffset);
     builder.pop();
   }
 
@@ -307,7 +310,7 @@ class ClipRRectLayer extends ContainerLayer {
 
 /// A composite layer that clips its children using a path
 class ClipPathLayer extends ContainerLayer {
-  ClipPathLayer({ Offset offset: Offset.zero, this.clipPath }) : super(offset: offset);
+  ClipPathLayer({ this.clipPath });
 
   /// The path to clip in the parent's coordinate system
   Path clipPath;
@@ -315,9 +318,8 @@ class ClipPathLayer extends ContainerLayer {
   // in the coordinate system of this layer?
 
   void addToScene(ui.SceneBuilder builder, Offset layerOffset) {
-    Offset childOffset = offset + layerOffset;
-    builder.pushClipPath(clipPath.shift(childOffset));
-    addChildrenToScene(builder, childOffset);
+    builder.pushClipPath(clipPath.shift(layerOffset));
+    addChildrenToScene(builder, layerOffset);
     builder.pop();
   }
 
@@ -328,8 +330,8 @@ class ClipPathLayer extends ContainerLayer {
 }
 
 /// A composited layer that applies a transformation matrix to its children
-class TransformLayer extends ContainerLayer {
-  TransformLayer({ Offset offset: Offset.zero, this.transform }) : super(offset: offset);
+class TransformLayer extends OffsetLayer {
+  TransformLayer({ Offset offset: Offset.zero, this.transform }): super(offset: offset);
 
   /// The matrix to apply
   Matrix4 transform;
@@ -351,7 +353,7 @@ class TransformLayer extends ContainerLayer {
 
 /// A composited layer that makes its children partially transparent
 class OpacityLayer extends ContainerLayer {
-  OpacityLayer({ Offset offset: Offset.zero, this.alpha }) : super(offset: offset);
+  OpacityLayer({ this.alpha });
 
   /// The amount to multiply into the alpha channel
   ///
@@ -360,9 +362,8 @@ class OpacityLayer extends ContainerLayer {
   int alpha;
 
   void addToScene(ui.SceneBuilder builder, Offset layerOffset) {
-    Offset childOffset = offset + layerOffset;
     builder.pushOpacity(alpha);
-    addChildrenToScene(builder, childOffset);
+    addChildrenToScene(builder, layerOffset);
     builder.pop();
   }
 
@@ -374,7 +375,7 @@ class OpacityLayer extends ContainerLayer {
 
 /// A composited layer that applies a shader to hits children.
 class ShaderMaskLayer extends ContainerLayer {
-  ShaderMaskLayer({ Offset offset: Offset.zero, this.shader, this.maskRect, this.transferMode }) : super(offset: offset);
+  ShaderMaskLayer({ this.shader, this.maskRect, this.transferMode });
 
   /// The shader to apply to the children.
   ui.Shader shader;
@@ -386,9 +387,8 @@ class ShaderMaskLayer extends ContainerLayer {
   TransferMode transferMode;
 
   void addToScene(ui.SceneBuilder builder, Offset layerOffset) {
-    Offset childOffset = offset + layerOffset;
-    builder.pushShaderMask(shader, maskRect.shift(childOffset), transferMode);
-    addChildrenToScene(builder, childOffset);
+    builder.pushShaderMask(shader, maskRect.shift(layerOffset), transferMode);
+    addChildrenToScene(builder, layerOffset);
     builder.pop();
   }
 

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

@@ -69,7 +69,7 @@ class PaintingContext {
   static void repaintCompositedChild(RenderObject child) {
     assert(child.isRepaintBoundary);
     assert(child.needsPaint);
-    child._layer ??= new ContainerLayer();
+    child._layer ??= new OffsetLayer();
     child._layer.removeAllChildren();
     assert(() {
       child._layer.debugOwner = child.debugOwner ?? child.runtimeType;
@@ -110,12 +110,12 @@ class PaintingContext {
         return true;
       });
     }
-    _appendLayer(child._layer, offset);
+    child._layer.offset = offset;
+    _appendLayer(child._layer);
   }
 
-  void _appendLayer(Layer layer, Offset offset) {
+  void _appendLayer(Layer layer) {
     assert(!_isRecording);
-    layer.offset = offset;
     _containerLayer.append(layer);
   }
 
@@ -190,11 +190,11 @@ class PaintingContext {
   void pushPerformanceOverlay(Offset offset, int optionsMask, int rasterizerThreshold, Size size) {
     _stopRecordingIfNeeded();
     PerformanceOverlayLayer performanceOverlayLayer = new PerformanceOverlayLayer(
-      overlayRect: new Rect.fromLTWH(0.0, 0.0, size.width, size.height),
+      overlayRect: new Rect.fromLTWH(offset.dx, offset.dy, size.width, size.height),
       optionsMask: optionsMask,
       rasterizerThreshold: rasterizerThreshold
     );
-    _appendLayer(performanceOverlayLayer, offset);
+    _appendLayer(performanceOverlayLayer);
   }
 
   /// Push a rectangular clip rect.
@@ -203,12 +203,13 @@ class PaintingContext {
   /// is clipped by the given clip. The given clip should not incorporate the
   /// painting offset.
   void pushClipRect(bool needsCompositing, Offset offset, Rect clipRect, PaintingContextCallback painter) {
+    Rect offsetClipRect = clipRect.shift(offset);
     if (needsCompositing) {
       _stopRecordingIfNeeded();
-      ClipRectLayer clipLayer = new ClipRectLayer(clipRect: clipRect);
-      _appendLayer(clipLayer, offset);
-      PaintingContext childContext = new PaintingContext._(clipLayer, clipRect);
-      painter(childContext, Offset.zero);
+      ClipRectLayer clipLayer = new ClipRectLayer(clipRect: offsetClipRect);
+      _appendLayer(clipLayer);
+      PaintingContext childContext = new PaintingContext._(clipLayer, offsetClipRect);
+      painter(childContext, offset);
       childContext._stopRecordingIfNeeded();
     } else {
       canvas.save();
@@ -224,16 +225,18 @@ class PaintingContext {
   /// is clipped by the given clip. The given clip should not incorporate the
   /// painting offset.
   void pushClipRRect(bool needsCompositing, Offset offset, Rect bounds, ui.RRect clipRRect, PaintingContextCallback painter) {
+    Rect offsetBounds = bounds.shift(offset);
+    ui.RRect offsetClipRRect = clipRRect.shift(offset);
     if (needsCompositing) {
       _stopRecordingIfNeeded();
-      ClipRRectLayer clipLayer = new ClipRRectLayer(clipRRect: clipRRect);
-      _appendLayer(clipLayer, offset);
-      PaintingContext childContext = new PaintingContext._(clipLayer, bounds);
-      painter(childContext, Offset.zero);
+      ClipRRectLayer clipLayer = new ClipRRectLayer(clipRRect: offsetClipRRect);
+      _appendLayer(clipLayer);
+      PaintingContext childContext = new PaintingContext._(clipLayer, offsetBounds);
+      painter(childContext, offset);
       childContext._stopRecordingIfNeeded();
     } else {
-      canvas.saveLayer(bounds.shift(offset), _disableAntialias);
-      canvas.clipRRect(clipRRect.shift(offset));
+      canvas.saveLayer(offsetBounds, _disableAntialias);
+      canvas.clipRRect(offsetClipRRect);
       painter(this, offset);
       canvas.restore();
     }
@@ -245,12 +248,14 @@ class PaintingContext {
   /// is clipped by the given clip. The given clip should not incorporate the
   /// painting offset.
   void pushClipPath(bool needsCompositing, Offset offset, Rect bounds, Path clipPath, PaintingContextCallback painter) {
+    Rect offsetBounds = bounds.shift(offset);
+    Path offsetClipPath = clipPath.shift(offset);
     if (needsCompositing) {
       _stopRecordingIfNeeded();
-      ClipPathLayer clipLayer = new ClipPathLayer(clipPath: clipPath);
-      _appendLayer(clipLayer, offset);
-      PaintingContext childContext = new PaintingContext._(clipLayer, bounds);
-      painter(childContext, Offset.zero);
+      ClipPathLayer clipLayer = new ClipPathLayer(clipPath: offsetClipPath);
+      _appendLayer(clipLayer);
+      PaintingContext childContext = new PaintingContext._(clipLayer, offsetBounds);
+      painter(childContext, offset);
       childContext._stopRecordingIfNeeded();
     } else {
       canvas.saveLayer(bounds.shift(offset), _disableAntialias);
@@ -268,8 +273,9 @@ class PaintingContext {
   void pushTransform(bool needsCompositing, Offset offset, Matrix4 transform, PaintingContextCallback painter) {
     if (needsCompositing) {
       _stopRecordingIfNeeded();
-      TransformLayer transformLayer = new TransformLayer(transform: transform);
-      _appendLayer(transformLayer, offset);
+      TransformLayer transformLayer = new TransformLayer(offset: offset, transform: transform);
+      _appendLayer(transformLayer);
+      // TODO(abarth): We need to run _paintBounds through the inverse of transform.
       PaintingContext childContext = new PaintingContext._(transformLayer, _paintBounds);
       painter(childContext, Offset.zero);
       childContext._stopRecordingIfNeeded();
@@ -290,9 +296,9 @@ class PaintingContext {
   void pushOpacity(Offset offset, int alpha, PaintingContextCallback painter) {
     _stopRecordingIfNeeded();
     OpacityLayer opacityLayer = new OpacityLayer(alpha: alpha);
-    _appendLayer(opacityLayer, offset);
+    _appendLayer(opacityLayer);
     PaintingContext childContext = new PaintingContext._(opacityLayer, _paintBounds);
-    painter(childContext, Offset.zero);
+    painter(childContext, offset);
     childContext._stopRecordingIfNeeded();
   }
 
@@ -307,9 +313,9 @@ class PaintingContext {
       maskRect: maskRect,
       transferMode: transferMode
     );
-    _appendLayer(shaderLayer, offset);
+    _appendLayer(shaderLayer);
     PaintingContext childContext = new PaintingContext._(shaderLayer, _paintBounds);
-    painter(childContext, Offset.zero);
+    painter(childContext, offset);
     childContext._stopRecordingIfNeeded();
   }
 }
@@ -1156,11 +1162,11 @@ abstract class RenderObject extends AbstractNode implements HitTestTarget {
   /// getter changes.
   bool get alwaysNeedsCompositing => false;
 
-  ContainerLayer _layer;
+  OffsetLayer _layer;
   /// The compositing layer that this render object uses to repaint.
   ///
   /// Call only when [isRepaintBoundary] is true.
-  ContainerLayer get layer {
+  OffsetLayer get layer {
     assert(isRepaintBoundary);
     assert(!_needsPaint);
     return _layer;
@@ -1522,7 +1528,7 @@ abstract class RenderObject extends AbstractNode implements HitTestTarget {
       if (!node._needsSemanticsUpdate) {
         node._needsSemanticsUpdate = true;
         _nodesNeedingSemantics.add(node);
-      }      
+      }
     } else {
       // The shape of the semantics tree around us may have changed.
       // The worst case is that we may have removed a branch of the
@@ -1541,7 +1547,7 @@ abstract class RenderObject extends AbstractNode implements HitTestTarget {
       if (!node._needsSemanticsUpdate) {
         node._needsSemanticsUpdate = true;
         _nodesNeedingSemantics.add(node);
-      }      
+      }
     }
   }