diff --git a/example/lib/examples/item_list.dart b/example/lib/examples/item_list.dart index f5c6f59..fe76c68 100644 --- a/example/lib/examples/item_list.dart +++ b/example/lib/examples/item_list.dart @@ -21,6 +21,22 @@ const _kMaxSlivers = 10; const _kItemsPerSliver = [1, 9, 27, 80, 200, 1000, 2500, 7000, 20000]; class _ReadingOrderTraversalPolicy extends ReadingOrderTraversalPolicy { + _ReadingOrderTraversalPolicy() : super(requestFocusCallback: _requestFocus); + + static void _requestFocus( + FocusNode node, { + ScrollPositionAlignmentPolicy? alignmentPolicy, + double? alignment, + Duration? duration, + Curve? curve, + }) { + if (!node.hasFocus) { + node.requestFocus(); + } + final renderObject = node.context!.findRenderObject(); + renderObject?.safeShowOnScreen(node.context!); + } + @override bool inDirection(FocusNode currentNode, TraversalDirection direction) { // For list traversal the history flutter keeps have weird behavior. diff --git a/example/lib/main.dart b/example/lib/main.dart index 3a42409..dded609 100644 --- a/example/lib/main.dart +++ b/example/lib/main.dart @@ -10,7 +10,7 @@ void main() { }); hierarchicalLoggingEnabled = true; WidgetsFlutterBinding.ensureInitialized(); - // Logger("SuperSliver").level = Level.FINER; + // Logger("SuperSliverList").level = Level.FINER; // Right now the debug bar doesn't work nicely with safe area so // only enable it on desktop platform. diff --git a/example/lib/util/show_on_screen.dart b/example/lib/util/show_on_screen.dart index e997fcb..b5250d6 100644 --- a/example/lib/util/show_on_screen.dart +++ b/example/lib/util/show_on_screen.dart @@ -1,5 +1,6 @@ import "package:flutter/rendering.dart"; import "package:flutter/widgets.dart"; +import "package:super_sliver_list/super_sliver_list.dart"; extension RenderObjectShowOnScreen on RenderObject { /// showOnScreen alternative that doesn't scroll if the widget is already @@ -8,8 +9,6 @@ extension RenderObjectShowOnScreen on RenderObject { BuildContext context, { /// Extra spacing for around the widget in percent of the viewport (0.05 being 5%) double extraSpacingPercent = 0.0, - Duration? duration, - Curve? curve, bool allowScrollingDown = true, bool allowScrollingUp = true, }) { @@ -22,16 +21,18 @@ extension RenderObjectShowOnScreen on RenderObject { if (scrollable == null) return; final minOffset = - viewport.getOffsetToReveal(this, extraSpacingPercent).offset; - final maxOffset = - viewport.getOffsetToReveal(this, 1.0 - extraSpacingPercent).offset; + viewport.getOffsetToRevealExt(this, extraSpacingPercent).offset; final position = scrollable.position; if (position.pixels > minOffset && allowScrollingDown) { - scrollable.position.moveTo(minOffset, duration: duration, curve: curve); - } else if (position.pixels < maxOffset && allowScrollingUp) { - scrollable.position.moveTo(maxOffset, duration: duration, curve: curve); + scrollable.position.moveTo(minOffset); + } else { + final maxOffset = + viewport.getOffsetToRevealExt(this, 1.0 - extraSpacingPercent).offset; + if (position.pixels < maxOffset && allowScrollingUp) { + scrollable.position.moveTo(maxOffset); + } } } } diff --git a/lib/src/render_object.dart b/lib/src/render_object.dart index 1676cbe..e1636f5 100644 --- a/lib/src/render_object.dart +++ b/lib/src/render_object.dart @@ -8,6 +8,7 @@ import "element.dart"; import "extent_manager.dart"; import "layout_budget.dart"; import "layout_pass.dart"; +import "sliver_extensions.dart"; import "super_sliver_list.dart"; final _log = Logger("SuperSliverList"); @@ -21,6 +22,9 @@ class _ChildScrollOffsetEstimation { required this.offset, required this.extent, required this.precedingScrollExtent, + required this.revealingRect, + required this.alignment, + required this.childObstructionExtent, }); /// Index of child for which the offset was estimated. @@ -40,14 +44,17 @@ class _ChildScrollOffsetEstimation { /// for change in preceding sliver extent change during layout. final double precedingScrollExtent; - /// Scroll offset of viewport when estimation was made. - double? viewportScrollOffset; - /// Whether the entire element or only a rect should be revealed. - bool revealingRect = false; + final bool revealingRect; /// Child alignment within viewport. - double? alignment; + final double? alignment; + + /// Child obstruction extent when estimation was made. + final ChildObstructionExtent childObstructionExtent; + + /// Scroll offset of viewport when estimation was made. + double? viewportScrollOffset; } class RenderSuperSliverList extends RenderSliverMultiBoxAdaptor @@ -200,7 +207,14 @@ class RenderSuperSliverList extends RenderSliverMultiBoxAdaptor // we can possibly correct scrollOffset in next performLayout call. final index = indexOf(child as RenderBox); - if (child is _FakeRenderObject && child.needEstimationOnly) { + final offsetToRevealContext = OffsetToRevealContext.current(); + + assert( + offsetToRevealContext == null || + offsetToRevealContext.viewport == getViewport(), + ); + + if (offsetToRevealContext?.estimationOnly == true) { return _extentManager.offsetForIndex(index); } @@ -209,7 +223,28 @@ class RenderSuperSliverList extends RenderSliverMultiBoxAdaptor offset: _extentManager.offsetForIndex(index), extent: _extentManager.getExtent(index), precedingScrollExtent: precedingScrollExtent, + revealingRect: offsetToRevealContext?.rect != null, + alignment: offsetToRevealContext?.alignment, + childObstructionExtent: child.getParentChildObstructionExtent(), ); + + if (offsetToRevealContext != null) { + assert(offsetToRevealContext.estimationOnly == false); + offsetToRevealContext.registerOffsetResolvedCallback((value) { + final offset = value.offset; + // Remember the target scroll position. Scroll correction will only be enforced + // when the viewport scroll position is the same as when the estimation was made. + // This is enforced by [LayoutPass.sanitizeChildScrollOffsetEstimation]. + final position = getViewport()!.offset as ScrollPosition; + // Only remember position if it is within scroll extent. Otherwise + // it will be corrected and it is not possible to check against it. + if (offset >= position.minScrollExtent && + offset <= position.maxScrollExtent) { + _childScrollOffsetEstimation?.viewportScrollOffset = offset; + } + }); + } + _log.fine( "$_logIdentifier remembering estimated offset ${_childScrollOffsetEstimation!.offset} for child $index (preceding extent ${constraints.precedingScrollExtent})", ); @@ -223,9 +258,7 @@ class RenderSuperSliverList extends RenderSliverMultiBoxAdaptor break; } } - final offset = _childScrollOffsetEstimation!.offset; - - return offset; + return _childScrollOffsetEstimation!.offset; } /// Moves the layout offset of this and subsequent children by the given delta. @@ -758,6 +791,16 @@ class RenderSuperSliverList extends RenderSliverMultiBoxAdaptor // the correction is equal to extent difference. final extentDifference = paintExtentOf(c) - estimation.extent; correction += extentDifference * childAlignmentWithinViewport; + + // Obstruction extent on this sliver might have been outdated during the estimation + // so account of the difference. + final obstructionExtentDifference = + c.getParentChildObstructionExtent() - + estimation.childObstructionExtent; + correction -= obstructionExtentDifference.leading * + (1.0 - childAlignmentWithinViewport); + correction += obstructionExtentDifference.trailing * + childAlignmentWithinViewport; } if (correction.abs() > precisionErrorTolerance) { @@ -904,29 +947,16 @@ class RenderSuperSliverList extends RenderSliverMultiBoxAdaptor parent: this, index: index, extent: _extentManager.getExtent(index), - needEstimationOnly: estimationOnly, ); - final offset = getViewport() - ?.getOffsetToReveal( + final viewport = getViewport()!; + return viewport + .getOffsetToRevealExt( renderObject, alignment, + esimationOnly: estimationOnly, rect: rect, ) .offset; - - if (offset != null && !estimationOnly) { - final position = getViewport()!.offset as ScrollPosition; - // Only remember position if it is within scroll extent. Otherwise - // it will be corrected and it is not possible to check against it. - if (offset >= position.minScrollExtent && - offset <= position.maxScrollExtent) { - _childScrollOffsetEstimation?.viewportScrollOffset = offset; - } - _childScrollOffsetEstimation?.revealingRect = rect != null; - _childScrollOffsetEstimation?.alignment = alignment; - } - - return offset ?? 0.0; } @override @@ -941,13 +971,11 @@ class _FakeRenderObject extends RenderBox { @override final RenderObject parent; final double extent; - final bool needEstimationOnly; _FakeRenderObject({ required this.parent, required int index, required this.extent, - required this.needEstimationOnly, }) { parentData = SliverMultiBoxAdaptorParentData()..index = index; } diff --git a/lib/src/sliver_extensions.dart b/lib/src/sliver_extensions.dart new file mode 100644 index 0000000..91a2937 --- /dev/null +++ b/lib/src/sliver_extensions.dart @@ -0,0 +1,238 @@ +import "package:flutter/rendering.dart"; + +class ChildObstructionExtent { + ChildObstructionExtent({ + required this.leading, + required this.trailing, + }); + + /// Amount of space from the leading edge where this sliver obstructs its child. + final double leading; + + /// Amount of space from the trailing edge where this sliver obstructs its child. + final double trailing; + + ChildObstructionExtent operator +(ChildObstructionExtent other) { + return ChildObstructionExtent( + leading: leading + other.leading, + trailing: trailing + other.trailing, + ); + } + + ChildObstructionExtent operator -(ChildObstructionExtent other) { + return ChildObstructionExtent( + leading: leading - other.leading, + trailing: trailing - other.trailing, + ); + } +} + +extension SliverGeometryChildObstruction on SliverGeometry { + /// Returns the extent from edges in which this sliver obstructs + /// its child. + /// + /// This similar to [maxScrollObstructionExtent], with the biggest difference + /// being that [maxScrollObstructionExtent] affects slivers after this one, + /// while this one affects child slivers. + /// + /// For pinned headers this should be the extent of the header, regardless + /// of the scroll offset, layout or paint extent. + /// + /// The value is used to determine offset when revealing child and as such + /// should not be affected by the scroll offset. + ChildObstructionExtent? get childObstructionExtent { + if (this is _SliverGeometryWithChildObstructionExtent) { + return (this as _SliverGeometryWithChildObstructionExtent) + ._childObstructionExtent; + } else { + return null; + } + } + + /// Sets the extent from the edges in which this sliver obstructs + /// its child. + /// + /// See [childObstructionExtent]. + SliverGeometry withChildObstructionExtent( + ChildObstructionExtent? value, + ) { + return _SliverGeometryWithChildObstructionExtent( + scrollExtent: scrollExtent, + paintExtent: paintExtent, + paintOrigin: paintOrigin, + layoutExtent: layoutExtent, + maxPaintExtent: maxPaintExtent, + maxScrollObstructionExtent: maxScrollObstructionExtent, + crossAxisExtent: crossAxisExtent, + hitTestExtent: hitTestExtent, + visible: visible, + hasVisualOverflow: hasVisualOverflow, + cacheExtent: cacheExtent, + scrollOffsetCorrection: scrollOffsetCorrection, + childObstructionExtent: value, + ); + } +} + +extension RenderObjectChildObstuctionExtent on RenderObject { + /// Returns the total child obstruction extent of all slivers + /// above and this render object. + ChildObstructionExtent getParentChildObstructionExtent() { + var childObstructionExtent = + ChildObstructionExtent(leading: 0, trailing: 0); + var parent = this.parent; + while (parent != null && parent is! RenderViewport) { + if (parent is RenderSliver) { + final geometry = parent.geometry; + if (geometry?.childObstructionExtent != null) { + childObstructionExtent += geometry!.childObstructionExtent!; + } + } + parent = parent.parent; + } + return childObstructionExtent; + } +} + +class OffsetToRevealContext { + OffsetToRevealContext({ + required this.viewport, + required this.target, + required this.alignment, + required this.estimationOnly, + required this.rect, + required this.axis, + }); + + final RenderAbstractViewport viewport; + final RenderObject target; + final double alignment; + final bool estimationOnly; + final Rect? rect; + final Axis? axis; + + void registerOffsetResolvedCallback(ValueSetter callback) { + _offsetResolvedCallbacks.add(callback); + } + + final _offsetResolvedCallbacks = >[]; + + static final _contexts = []; + + static OffsetToRevealContext? current() { + if (_contexts.isEmpty) return null; + return _contexts.last; + } +} + +extension RenderViewportExt on RenderAbstractViewport { + /// Extended version of [RenderAbstractViewport.getOffsetToReveal] that + /// takes child scroll obstruction into account and thus works properly + /// with sliver that have [childObstructionExtent] set. + /// + /// In addition this method allows the underlying sliver that is being + /// queried for child scroll offset to access [OffsetToRevealContext] that + /// contains information about the current query. + RevealedOffset getOffsetToRevealExt( + RenderObject target, + double alignment, { + bool esimationOnly = false, + Rect? rect, + Axis? axis, + }) { + final context = OffsetToRevealContext( + viewport: this, + target: target, + alignment: alignment, + estimationOnly: esimationOnly, + rect: rect, + axis: axis, + ); + OffsetToRevealContext._contexts.add(context); + var result = getOffsetToReveal(target, alignment); + OffsetToRevealContext._contexts.removeLast(); + final obstruction = target.getParentChildObstructionExtent(); + + final resolvedAxis = + (this is RenderViewportBase ? (this as RenderViewportBase).axis : axis); + + Rect shiftRect(Rect rect, double offset) { + if (resolvedAxis == Axis.vertical) { + return rect.shift(Offset(0, offset)); + } else { + return rect.shift(Offset(offset, 0)); + } + } + + if (obstruction.leading > 0) { + final offset = obstruction.leading * (1.0 - alignment); + result = RevealedOffset( + offset: result.offset - offset, + rect: shiftRect(result.rect, offset), + ); + } + if (obstruction.trailing > 0) { + final offset = obstruction.trailing * alignment; + result = RevealedOffset( + offset: result.offset + offset, + rect: shiftRect(result.rect, -offset), + ); + } + for (final callback in context._offsetResolvedCallbacks) { + callback(result); + } + return result; + } +} + +class _SliverGeometryWithChildObstructionExtent extends SliverGeometry { + const _SliverGeometryWithChildObstructionExtent({ + required super.scrollExtent, + required super.paintExtent, + required super.paintOrigin, + required super.layoutExtent, + required super.maxPaintExtent, + required super.maxScrollObstructionExtent, + required super.crossAxisExtent, + required super.hitTestExtent, + required super.visible, + required super.hasVisualOverflow, + required super.scrollOffsetCorrection, + required super.cacheExtent, + required ChildObstructionExtent? childObstructionExtent, + }) : _childObstructionExtent = childObstructionExtent; + + @override + SliverGeometry copyWith({ + double? scrollExtent, + double? paintExtent, + double? paintOrigin, + double? layoutExtent, + double? maxPaintExtent, + double? maxScrollObstructionExtent, + double? crossAxisExtent, + double? hitTestExtent, + bool? visible, + bool? hasVisualOverflow, + double? cacheExtent, + }) { + return _SliverGeometryWithChildObstructionExtent( + scrollExtent: scrollExtent ?? this.scrollExtent, + paintExtent: paintExtent ?? this.paintExtent, + paintOrigin: paintOrigin ?? this.paintOrigin, + layoutExtent: layoutExtent ?? this.layoutExtent, + maxPaintExtent: maxPaintExtent ?? this.maxPaintExtent, + maxScrollObstructionExtent: + maxScrollObstructionExtent ?? this.maxScrollObstructionExtent, + crossAxisExtent: crossAxisExtent ?? this.crossAxisExtent, + hitTestExtent: hitTestExtent ?? this.hitTestExtent, + visible: visible ?? this.visible, + hasVisualOverflow: hasVisualOverflow ?? this.hasVisualOverflow, + cacheExtent: cacheExtent ?? this.cacheExtent, + scrollOffsetCorrection: scrollOffsetCorrection, + childObstructionExtent: _childObstructionExtent, + ); + } + + final ChildObstructionExtent? _childObstructionExtent; +} diff --git a/lib/src/super_sliver_list.dart b/lib/src/super_sliver_list.dart index 9f449ec..73ac287 100644 --- a/lib/src/super_sliver_list.dart +++ b/lib/src/super_sliver_list.dart @@ -1,5 +1,6 @@ import "package:flutter/rendering.dart"; import "package:flutter/widgets.dart"; +import "package:logging/logging.dart"; import "animate_to_item.dart"; import "element.dart"; @@ -7,6 +8,8 @@ import "extent_manager.dart"; import "layout_budget.dart"; import "render_object.dart"; +final _log = Logger("SuperSliverList"); + class ExtentController extends ChangeNotifier { ExtentController({ this.onAttached, @@ -51,7 +54,11 @@ class ExtentController extends ChangeNotifier { }) { assert(_delegate != null, "ExtentController is not attached."); final offset = getOffsetToReveal(index, alignment, rect: rect); - scrollController.jumpTo(offset); + if (offset.isFinite) { + scrollController.jumpTo(offset); + } else { + _log.warning("getOffsetToReveal returned non-finite value."); + } } void animateToItem({ diff --git a/lib/super_sliver_list.dart b/lib/super_sliver_list.dart index 37acd3f..11afb44 100644 --- a/lib/super_sliver_list.dart +++ b/lib/super_sliver_list.dart @@ -1,4 +1,5 @@ export "src/layout_budget.dart"; export "src/list_view.dart"; export "src/scroll_physics.dart"; +export "src/sliver_extensions.dart"; export "src/super_sliver_list.dart"; diff --git a/test/super_sliver_list_test.dart b/test/super_sliver_list_test.dart index 3c282f6..ef514a5 100644 --- a/test/super_sliver_list_test.dart +++ b/test/super_sliver_list_test.dart @@ -647,62 +647,62 @@ void main() async { controller.dispose(); }); - }); - testWidgets("delay populating cache area disabled", (tester) async { - final keys = List.generate(50, (index) => GlobalKey()); - final controller = ScrollController(); - await tester.pumpWidget( - Directionality( - textDirection: TextDirection.ltr, - child: Center( - child: SizedBox( - height: 500, - child: CustomScrollView( - cacheExtent: 200, - physics: const ClampingScrollPhysics(), - controller: controller, - slivers: [ - SuperSliverList( - delayPopulatingCacheArea: false, - delegate: SliverChildBuilderDelegate( - (context, index) { - return SizedBox( - key: keys[index], - height: 100, - child: Text("Tile $index"), - ); - }, - childCount: keys.length, + testWidgets("delay populating cache area disabled", (tester) async { + final keys = List.generate(50, (index) => GlobalKey()); + final controller = ScrollController(); + await tester.pumpWidget( + Directionality( + textDirection: TextDirection.ltr, + child: Center( + child: SizedBox( + height: 500, + child: CustomScrollView( + cacheExtent: 200, + physics: const ClampingScrollPhysics(), + controller: controller, + slivers: [ + SuperSliverList( + delayPopulatingCacheArea: false, + delegate: SliverChildBuilderDelegate( + (context, index) { + return SizedBox( + key: keys[index], + height: 100, + child: Text("Tile $index"), + ); + }, + childCount: keys.length, + ), ), - ), - ], + ], + ), ), ), ), - ), - ); - expect(keys[0].currentContext, isNotNull); - - // Cache area - expect(keys[5].currentContext, isNotNull); - expect(keys[6].currentContext, isNotNull); - // After cache area - expect(keys[7].currentContext, isNull); - - // All items replaced, cache area should immediately be populated - controller.jumpTo(2000); - await tester.pump(); - - // Items removed - expect(keys[0].currentContext, isNull); - expect(keys[6].currentContext, isNull); - // Visible content - expect(keys[20].currentContext, isNotNull); - expect(keys[24].currentContext, isNotNull); - // Cache area - expect(keys[19].currentContext, isNotNull); - expect(keys[25].currentContext, isNotNull); - controller.dispose(); + ); + expect(keys[0].currentContext, isNotNull); + + // Cache area + expect(keys[5].currentContext, isNotNull); + expect(keys[6].currentContext, isNotNull); + // After cache area + expect(keys[7].currentContext, isNull); + + // All items replaced, cache area should immediately be populated + controller.jumpTo(2000); + await tester.pump(); + + // Items removed + expect(keys[0].currentContext, isNull); + expect(keys[6].currentContext, isNull); + // Visible content + expect(keys[20].currentContext, isNotNull); + expect(keys[24].currentContext, isNotNull); + // Cache area + expect(keys[19].currentContext, isNotNull); + expect(keys[25].currentContext, isNotNull); + controller.dispose(); + }); }); group("Fuzzer", () { testWidgets("layout multiple slivers scrolling down", (tester) async {