From 3c8cbef973cdc7f99531674f60447c258af057da Mon Sep 17 00:00:00 2001 From: Adam Barth Date: Tue, 9 Feb 2016 01:24:51 -0800 Subject: [PATCH] Rationalize RenderViewport and RenderVirtualViewport These classes now share more code and have feature parity. --- packages/flutter/lib/src/material/tabs.dart | 2 +- packages/flutter/lib/src/rendering/grid.dart | 11 + packages/flutter/lib/src/rendering/list.dart | 15 +- .../flutter/lib/src/rendering/viewport.dart | 199 +++++++++--------- packages/flutter/lib/src/widgets/basic.dart | 24 ++- .../flutter/lib/src/widgets/scrollable.dart | 14 +- .../flutter/test/rendering/viewport_test.dart | 2 +- .../lib/src/commands/analyze.dart | 1 + 8 files changed, 141 insertions(+), 127 deletions(-) diff --git a/packages/flutter/lib/src/material/tabs.dart b/packages/flutter/lib/src/material/tabs.dart index 101d8784727..9b34dd4ec51 100644 --- a/packages/flutter/lib/src/material/tabs.dart +++ b/packages/flutter/lib/src/material/tabs.dart @@ -771,7 +771,7 @@ class _TabBarState extends ScrollableState> implements TabBarSelect onSizeChanged: _handleViewportSizeChanged, child: new Viewport( scrollDirection: Axis.horizontal, - scrollOffset: new Offset(scrollOffset, 0.0), + paintOffset: scrollOffsetToPixelDelta(scrollOffset), child: contents ) ); diff --git a/packages/flutter/lib/src/rendering/grid.dart b/packages/flutter/lib/src/rendering/grid.dart index ccefe6ca5de..b91c0a26798 100644 --- a/packages/flutter/lib/src/rendering/grid.dart +++ b/packages/flutter/lib/src/rendering/grid.dart @@ -314,10 +314,12 @@ class RenderGrid extends RenderVirtualViewport { int virtualChildBase: 0, int virtualChildCount, Offset paintOffset: Offset.zero, + Painter overlayPainter, LayoutCallback callback }) : _delegate = delegate, _virtualChildBase = virtualChildBase, super( virtualChildCount: virtualChildCount, paintOffset: paintOffset, + overlayPainter: overlayPainter, callback: callback ) { assert(delegate != null); @@ -338,6 +340,15 @@ class RenderGrid extends RenderVirtualViewport { _delegate = newDelegate; } + void set scrollDirection(Axis value) { + assert(() { + if (value != Axis.vertical) + throw new RenderingError('RenderGrid doesn\'t yet support horizontal scrolling.'); + return true; + }); + super.scrollDirection = value; + } + int get virtualChildCount => super.virtualChildCount ?? childCount; /// The virtual index of the first child. diff --git a/packages/flutter/lib/src/rendering/list.dart b/packages/flutter/lib/src/rendering/list.dart index dcfcfce1d0d..1c02bc4ff8d 100644 --- a/packages/flutter/lib/src/rendering/list.dart +++ b/packages/flutter/lib/src/rendering/list.dart @@ -11,7 +11,7 @@ import 'viewport.dart'; /// Parent data for use with [RenderList]. class ListParentData extends ContainerBoxParentDataMixin { } -class RenderList extends RenderVirtualViewport implements HasScrollDirection { +class RenderList extends RenderVirtualViewport { RenderList({ List children, double itemExtent, @@ -19,13 +19,15 @@ class RenderList extends RenderVirtualViewport implements HasScr int virtualChildCount, Offset paintOffset: Offset.zero, Axis scrollDirection: Axis.vertical, + Painter overlayPainter, LayoutCallback callback }) : _itemExtent = itemExtent, _padding = padding, - _scrollDirection = scrollDirection, super( virtualChildCount: virtualChildCount, paintOffset: paintOffset, + scrollDirection: scrollDirection, + overlayPainter: overlayPainter, callback: callback ) { addAll(children); @@ -50,15 +52,6 @@ class RenderList extends RenderVirtualViewport implements HasScr markNeedsLayout(); } - Axis get scrollDirection => _scrollDirection; - Axis _scrollDirection; - void set scrollDirection (Axis newValue) { - if (_scrollDirection == newValue) - return; - _scrollDirection = newValue; - markNeedsLayout(); - } - void setupParentData(RenderBox child) { if (child.parentData is! ListParentData) child.parentData = new ListParentData(); diff --git a/packages/flutter/lib/src/rendering/viewport.dart b/packages/flutter/lib/src/rendering/viewport.dart index f348b3a59a9..0ef2c3048ec 100644 --- a/packages/flutter/lib/src/rendering/viewport.dart +++ b/packages/flutter/lib/src/rendering/viewport.dart @@ -18,26 +18,22 @@ abstract class HasScrollDirection { Axis get scrollDirection; } -/// A render object that's bigger on the inside. +/// A base class for render objects that are bigger on the inside. /// -/// The child of a viewport can layout to a larger size than the viewport -/// itself. If that happens, only a portion of the child will be visible through -/// the viewport. The portion of the child that is visible is controlled by the -/// scroll offset. -/// -/// Viewport is the core scrolling primitive in the system, but it can be used -/// in other situations. -class RenderViewport extends RenderBox with RenderObjectWithChildMixin - implements HasScrollDirection { - - RenderViewport({ - RenderBox child, - Offset scrollOffset: Offset.zero, - Axis scrollDirection: Axis.vertical - }) : _scrollOffset = scrollOffset, - _scrollDirection = scrollDirection { - assert(_offsetIsSane(scrollOffset, scrollDirection)); - this.child = child; +/// This class holds the common fields for viewport render objects but does not +/// have a child model. See [RenderViewport] for a viewport with a single child +/// and [RenderVirtualViewport] for a viewport with multiple children. +class RenderViewportBase extends RenderBox implements HasScrollDirection { + RenderViewportBase( + Offset paintOffset, + Axis scrollDirection, + Painter overlayPainter + ) : _paintOffset = paintOffset, + _scrollDirection = scrollDirection, + _overlayPainter = overlayPainter { + assert(paintOffset != null); + assert(scrollDirection != null); + assert(_offsetIsSane(_paintOffset, scrollDirection)); } bool _offsetIsSane(Offset offset, Axis direction) { @@ -52,13 +48,14 @@ class RenderViewport extends RenderBox with RenderObjectWithChildMixin _scrollOffset; - Offset _scrollOffset; - void set scrollOffset(Offset value) { - if (value == _scrollOffset) + Offset get paintOffset => _paintOffset; + Offset _paintOffset; + void set paintOffset(Offset value) { + assert(value != null); + if (value == _paintOffset) return; assert(_offsetIsSane(value, scrollDirection)); - _scrollOffset = value; + _paintOffset = value; markNeedsPaint(); markNeedsSemanticsUpdate(); } @@ -71,13 +68,69 @@ class RenderViewport extends RenderBox with RenderObjectWithChildMixin _scrollDirection; Axis _scrollDirection; void set scrollDirection(Axis value) { + assert(value != null); if (value == _scrollDirection) return; - assert(_offsetIsSane(scrollOffset, value)); + assert(_offsetIsSane(_paintOffset, value)); _scrollDirection = value; markNeedsLayout(); } + Painter get overlayPainter => _overlayPainter; + Painter _overlayPainter; + void set overlayPainter(Painter value) { + if (_overlayPainter == value) + return; + if (attached) + _overlayPainter?.detach(); + _overlayPainter = value; + if (attached) + _overlayPainter?.attach(this); + markNeedsPaint(); + } + + void attach() { + super.attach(); + _overlayPainter?.attach(this); + } + + void detach() { + super.detach(); + _overlayPainter?.detach(); + } + + Offset get _paintOffsetRoundedToIntegerDevicePixels { + final double devicePixelRatio = ui.window.devicePixelRatio; + int dxInDevicePixels = (_paintOffset.dx * devicePixelRatio).round(); + int dyInDevicePixels = (_paintOffset.dy * devicePixelRatio).round(); + return new Offset(dxInDevicePixels / devicePixelRatio, + dyInDevicePixels / devicePixelRatio); + } + + void applyPaintTransform(RenderBox child, Matrix4 transform) { + final Offset effectivePaintOffset = _paintOffsetRoundedToIntegerDevicePixels; + super.applyPaintTransform(child, transform.translate(effectivePaintOffset.dx, effectivePaintOffset.dy)); + } + +} + +/// A render object that's bigger on the inside. +/// +/// The child of a viewport can layout to a larger size than the viewport +/// itself. If that happens, only a portion of the child will be visible through +/// the viewport. The portion of the child that is visible is controlled by the +/// paint offset. +class RenderViewport extends RenderViewportBase with RenderObjectWithChildMixin { + + RenderViewport({ + RenderBox child, + Offset paintOffset: Offset.zero, + Axis scrollDirection: Axis.vertical, + Painter overlayPainter + }) : super(paintOffset, scrollDirection, overlayPainter) { + this.child = child; + } + BoxConstraints _getInnerConstraints(BoxConstraints constraints) { BoxConstraints innerConstraints; switch (scrollDirection) { @@ -135,41 +188,30 @@ class RenderViewport extends RenderBox with RenderObjectWithChildMixin> - extends RenderBox with ContainerRenderObjectMixin, - RenderBoxContainerDefaultsMixin { + extends RenderViewportBase with ContainerRenderObjectMixin, + RenderBoxContainerDefaultsMixin { RenderVirtualViewport({ int virtualChildCount, - Offset paintOffset, LayoutCallback callback, + Offset paintOffset: Offset.zero, + Axis scrollDirection: Axis.vertical, Painter overlayPainter }) : _virtualChildCount = virtualChildCount, - _paintOffset = paintOffset, _callback = callback, - _overlayPainter = overlayPainter; + super(paintOffset, scrollDirection, overlayPainter); int get virtualChildCount => _virtualChildCount; int _virtualChildCount; @@ -206,21 +248,7 @@ abstract class RenderVirtualViewport _paintOffset; - Offset _paintOffset; - void set paintOffset(Offset value) { - assert(value != null); - if (value == _paintOffset) - return; - _paintOffset = value; - markNeedsPaint(); - markNeedsSemanticsUpdate(); - } - - /// Called during [layout] to determine the grid's children. + /// Called during [layout] to determine the render object's children. /// /// Typically the callback will mutate the child list appropriately, for /// example so the child list contains only visible children. @@ -233,39 +261,12 @@ abstract class RenderVirtualViewport _overlayPainter; - Painter _overlayPainter; - void set overlayPainter(Painter value) { - if (_overlayPainter == value) - return; - if (attached) - _overlayPainter?.detach(); - _overlayPainter = value; - if (attached) - _overlayPainter?.attach(this); - markNeedsPaint(); - } - - void attach() { - super.attach(); - _overlayPainter?.attach(this); - } - - void detach() { - super.detach(); - _overlayPainter?.detach(); - } - - void applyPaintTransform(RenderBox child, Matrix4 transform) { - super.applyPaintTransform(child, transform.translate(paintOffset.dx, paintOffset.dy)); - } - bool hitTestChildren(HitTestResult result, { Point position }) { - return defaultHitTestChildren(result, position: position + -paintOffset); + return defaultHitTestChildren(result, position: position + -_paintOffsetRoundedToIntegerDevicePixels); } void _paintContents(PaintingContext context, Offset offset) { - defaultPaint(context, offset + paintOffset); + defaultPaint(context, offset + _paintOffsetRoundedToIntegerDevicePixels); _overlayPainter?.paint(context, offset); } diff --git a/packages/flutter/lib/src/widgets/basic.dart b/packages/flutter/lib/src/widgets/basic.dart index f0f68e411d1..de76b13bcb6 100644 --- a/packages/flutter/lib/src/widgets/basic.dart +++ b/packages/flutter/lib/src/widgets/basic.dart @@ -797,11 +797,12 @@ class Viewport extends OneChildRenderObjectWidget { Viewport({ Key key, this.scrollDirection: Axis.vertical, - this.scrollOffset: Offset.zero, + this.paintOffset: Offset.zero, + this.overlayPainter, Widget child }) : super(key: key, child: child) { assert(scrollDirection != null); - assert(scrollOffset != null); + assert(paintOffset != null); } /// The direction in which the child is permitted to be larger than the viewport @@ -814,14 +815,25 @@ class Viewport extends OneChildRenderObjectWidget { /// The offset at which to paint the child. /// /// The offset can be non-zero only in the [scrollDirection]. - final Offset scrollOffset; + final Offset paintOffset; - RenderViewport createRenderObject() => new RenderViewport(scrollDirection: scrollDirection, scrollOffset: scrollOffset); + /// Paints an overlay over the viewport. + /// + /// Often used to paint scroll bars. + final Painter overlayPainter; + + RenderViewport createRenderObject() => new RenderViewport( + scrollDirection: scrollDirection, + paintOffset: paintOffset, + overlayPainter: overlayPainter + ); void updateRenderObject(RenderViewport renderObject, Viewport oldWidget) { // Order dependency: RenderViewport validates scrollOffset based on scrollDirection. - renderObject.scrollDirection = scrollDirection; - renderObject.scrollOffset = scrollOffset; + renderObject + ..scrollDirection = scrollDirection + ..paintOffset = paintOffset + ..overlayPainter = overlayPainter; } } diff --git a/packages/flutter/lib/src/widgets/scrollable.dart b/packages/flutter/lib/src/widgets/scrollable.dart index 2e9686fdbc8..d2318ab3b63 100644 --- a/packages/flutter/lib/src/widgets/scrollable.dart +++ b/packages/flutter/lib/src/widgets/scrollable.dart @@ -467,15 +467,17 @@ class ScrollNotification extends Notification { class ScrollableViewport extends Scrollable { ScrollableViewport({ Key key, - this.child, double initialScrollOffset, Axis scrollDirection: Axis.vertical, + ViewportAnchor scrollAnchor: ViewportAnchor.start, ScrollListener onScrollStart, ScrollListener onScroll, - ScrollListener onScrollEnd + ScrollListener onScrollEnd, + this.child }) : super( key: key, scrollDirection: scrollDirection, + scrollAnchor: scrollAnchor, initialScrollOffset: initialScrollOffset, onScrollStart: onScrollStart, onScroll: onScroll, @@ -514,18 +516,12 @@ class _ScrollableViewportState extends ScrollableState { )); } - Offset get _scrollOffsetVector { - if (config.scrollDirection == Axis.horizontal) - return new Offset(scrollOffset, 0.0); - return new Offset(0.0, scrollOffset); - } - Widget buildContent(BuildContext context) { return new SizeObserver( onSizeChanged: _handleViewportSizeChanged, child: new Viewport( - scrollOffset: _scrollOffsetVector, scrollDirection: config.scrollDirection, + paintOffset: scrollOffsetToPixelDelta(scrollOffset), child: new SizeObserver( onSizeChanged: _handleChildSizeChanged, child: config.child diff --git a/packages/flutter/test/rendering/viewport_test.dart b/packages/flutter/test/rendering/viewport_test.dart index bdf74ab12b4..0ba19765c30 100644 --- a/packages/flutter/test/rendering/viewport_test.dart +++ b/packages/flutter/test/rendering/viewport_test.dart @@ -24,7 +24,7 @@ void main() { ), child: size); - RenderViewport viewport = new RenderViewport(child: red, scrollOffset: new Offset(0.0, -10.0)); + RenderViewport viewport = new RenderViewport(child: red, paintOffset: new Offset(0.0, 10.0)); layout(viewport); HitTestResult result; diff --git a/packages/flutter_tools/lib/src/commands/analyze.dart b/packages/flutter_tools/lib/src/commands/analyze.dart index 7dbe03832aa..7d6742f8b81 100644 --- a/packages/flutter_tools/lib/src/commands/analyze.dart +++ b/packages/flutter_tools/lib/src/commands/analyze.dart @@ -298,6 +298,7 @@ class AnalyzeCommand extends FlutterCommand { new RegExp(r'\[lint\] Prefer using lowerCamelCase for constant names.'), // sometimes we have no choice (e.g. when matching other platforms) new RegExp(r'\[lint\] Avoid defining a one-member abstract class when a simple function will do.'), // too many false-positives; code review should catch real instances new RegExp(r'\[info\] TODO.+'), + new RegExp('\\[warning\\] Missing concrete implementation of \'RenderObject\\.applyPaintTransform\''), // https://github.com/dart-lang/sdk/issues/25232 new RegExp(r'[0-9]+ (error|warning|hint|lint).+found\.'), new RegExp(r'^$'), ];