From 4e39d13a6f67fa04a9dc424cd20663da9b81301b Mon Sep 17 00:00:00 2001 From: yim Date: Tue, 25 Feb 2025 13:53:03 +0800 Subject: [PATCH] SliverMainAxisGroup multiple PinnedHeaderSliver children (#163528) Fixes: #155758 ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement]. - [x] I signed the [CLA]. - [x] I listed at least one issue that this PR fixes in the description above. - [ ] I updated/added relevant documentation (doc comments with `///`). - [x] I added new tests to check the change I am making, or this PR is [test-exempt]. - [x] I followed the [breaking change policy] and added [Data Driven Fixes] where supported. - [x] All existing and new tests are passing. If you need help, consider asking for advice on the #hackers-new channel on [Discord]. [Contributor Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview [Tree Hygiene]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md [test-exempt]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests [Flutter Style Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md [Features we expect every widget to implement]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement [CLA]: https://cla.developers.google.com/ [flutter/tests]: https://github.com/flutter/tests [breaking change policy]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes [Discord]: https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md [Data Driven Fixes]: https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md --- .../lib/src/rendering/sliver_group.dart | 96 ++++++++++--------- .../widgets/sliver_main_axis_group_test.dart | 84 ++++++++++++++++ 2 files changed, 135 insertions(+), 45 deletions(-) diff --git a/packages/flutter/lib/src/rendering/sliver_group.dart b/packages/flutter/lib/src/rendering/sliver_group.dart index acae62bb53..7b886068d2 100644 --- a/packages/flutter/lib/src/rendering/sliver_group.dart +++ b/packages/flutter/lib/src/rendering/sliver_group.dart @@ -6,7 +6,9 @@ library; import 'dart:math' as math; +import 'dart:ui'; +import 'package:flutter/foundation.dart'; import 'package:vector_math/vector_math_64.dart'; import 'object.dart'; @@ -265,39 +267,46 @@ class RenderSliverMainAxisGroup extends RenderSliver @override void performLayout() { - double offset = 0; + double scrollOffset = 0; + double layoutOffset = 0; double maxPaintExtent = 0; + double paintOffset = constraints.overlap; RenderSliver? child = firstChild; - while (child != null) { final double beforeOffsetPaintExtent = calculatePaintOffset( constraints, from: 0.0, - to: offset, + to: scrollOffset, ); child.layout( constraints.copyWith( - scrollOffset: math.max(0.0, constraints.scrollOffset - offset), - cacheOrigin: math.min(0.0, constraints.cacheOrigin + offset), - overlap: math.max(0.0, constraints.overlap - beforeOffsetPaintExtent), - remainingPaintExtent: constraints.remainingPaintExtent - beforeOffsetPaintExtent, - remainingCacheExtent: - constraints.remainingCacheExtent - - calculateCacheOffset(constraints, from: 0.0, to: offset), - precedingScrollExtent: offset + constraints.precedingScrollExtent, + scrollOffset: math.max(0.0, constraints.scrollOffset - scrollOffset), + cacheOrigin: math.min(0.0, constraints.cacheOrigin + scrollOffset), + overlap: math.max(0.0, _fixPrecisionError(paintOffset - beforeOffsetPaintExtent)), + remainingPaintExtent: _fixPrecisionError( + constraints.remainingPaintExtent - beforeOffsetPaintExtent, + ), + remainingCacheExtent: _fixPrecisionError( + constraints.remainingCacheExtent - + calculateCacheOffset(constraints, from: 0.0, to: scrollOffset), + ), + precedingScrollExtent: scrollOffset + constraints.precedingScrollExtent, ), parentUsesSize: true, ); final SliverGeometry childLayoutGeometry = child.geometry!; + final double childPaintOffset = layoutOffset + childLayoutGeometry.paintOrigin; final SliverPhysicalParentData childParentData = child.parentData! as SliverPhysicalParentData; childParentData.paintOffset = switch (constraints.axis) { - Axis.vertical => Offset(0.0, beforeOffsetPaintExtent), - Axis.horizontal => Offset(beforeOffsetPaintExtent, 0.0), + Axis.vertical => Offset(0.0, childPaintOffset), + Axis.horizontal => Offset(childPaintOffset, 0.0), }; - offset += childLayoutGeometry.scrollExtent; - maxPaintExtent += child.geometry!.maxPaintExtent; + scrollOffset += childLayoutGeometry.scrollExtent; + layoutOffset += childLayoutGeometry.layoutExtent; + maxPaintExtent += childLayoutGeometry.maxPaintExtent; + paintOffset = math.max(childPaintOffset + childLayoutGeometry.paintExtent, paintOffset); child = childAfter(child); assert(() { if (child != null && maxPaintExtent.isInfinite) { @@ -310,48 +319,41 @@ class RenderSliverMainAxisGroup extends RenderSliver }()); } - final double totalScrollExtent = offset; - offset = 0.0; - child = firstChild; - // Second pass to correct out of bound paintOffsets. - while (child != null) { - final double beforeOffsetPaintExtent = calculatePaintOffset( - constraints, - from: 0.0, - to: offset, - ); - final SliverGeometry childLayoutGeometry = child.geometry!; - final SliverPhysicalParentData childParentData = - child.parentData! as SliverPhysicalParentData; - final double remainingExtent = totalScrollExtent - constraints.scrollOffset; - if (childLayoutGeometry.paintExtent > remainingExtent) { - final double paintCorrection = childLayoutGeometry.paintExtent - remainingExtent; - childParentData.paintOffset = switch (constraints.axis) { - Axis.vertical => Offset(0.0, beforeOffsetPaintExtent - paintCorrection), - Axis.horizontal => Offset(beforeOffsetPaintExtent - paintCorrection, 0.0), - }; + final double remainingExtent = math.max(0, scrollOffset - constraints.scrollOffset); + // If the children's paint extent exceeds the remaining scroll extent of the `RenderSliverMainAxisGroup`, + // they need to be corrected. + if (paintOffset > remainingExtent) { + final double paintCorrection = paintOffset - remainingExtent; + paintOffset = remainingExtent; + child = firstChild; + while (child != null) { + final SliverGeometry childLayoutGeometry = child.geometry!; + if (childLayoutGeometry.paintExtent > 0) { + final SliverPhysicalParentData childParentData = + child.parentData! as SliverPhysicalParentData; + childParentData.paintOffset = switch (constraints.axis) { + Axis.vertical => Offset(0.0, childParentData.paintOffset.dy - paintCorrection), + Axis.horizontal => Offset(childParentData.paintOffset.dx - paintCorrection, 0.0), + }; + } + child = childAfter(child); } - offset += child.geometry!.scrollExtent; - child = childAfter(child); } - final double paintExtent = calculatePaintOffset( - constraints, - from: math.min(constraints.scrollOffset, 0), - to: totalScrollExtent, - ); final double cacheExtent = calculateCacheOffset( constraints, from: math.min(constraints.scrollOffset, 0), - to: totalScrollExtent, + to: scrollOffset, ); + final double paintExtent = clampDouble(paintOffset, 0, constraints.remainingPaintExtent); + geometry = SliverGeometry( - scrollExtent: totalScrollExtent, + scrollExtent: scrollOffset, paintExtent: paintExtent, cacheExtent: cacheExtent, maxPaintExtent: maxPaintExtent, hasVisualOverflow: - totalScrollExtent > constraints.remainingPaintExtent || constraints.scrollOffset > 0.0, + scrollOffset > constraints.remainingPaintExtent || constraints.scrollOffset > 0.0, ); // Update the children's paintOffset based on the direction again, which @@ -428,4 +430,8 @@ class RenderSliverMainAxisGroup extends RenderSliver child = childAfter(child); } } + + static double _fixPrecisionError(double number) { + return number.abs() < precisionErrorTolerance ? 0.0 : number; + } } diff --git a/packages/flutter/test/widgets/sliver_main_axis_group_test.dart b/packages/flutter/test/widgets/sliver_main_axis_group_test.dart index a559e20079..6b6525afb2 100644 --- a/packages/flutter/test/widgets/sliver_main_axis_group_test.dart +++ b/packages/flutter/test/widgets/sliver_main_axis_group_test.dart @@ -955,6 +955,90 @@ void main() { expect(renderBox.localToGlobal(Offset.zero), const Offset(0.0, 310.0)); expect(tester.getTopLeft(find.text('1')), const Offset(0.0, 310.0)); }); + + testWidgets('SliverMainAxisGroup multiple PinnedHeaderSliver children', ( + WidgetTester tester, + ) async { + final Size screenSize = tester.view.physicalSize / tester.view.devicePixelRatio; + final ScrollController controller = ScrollController(); + addTearDown(controller.dispose); + Future pumpWidget({Axis scrollDirection = Axis.vertical, bool reverse = false}) async { + Widget buildExtentBox(double size, {Widget? child}) { + return switch (scrollDirection) { + Axis.vertical => SizedBox(height: size, child: child), + Axis.horizontal => SizedBox(width: size, child: child), + }; + } + + await tester.pumpWidget( + _buildSliverMainAxisGroup( + controller: controller, + viewportHeight: screenSize.height, + viewportWidth: screenSize.width, + scrollDirection: scrollDirection, + reverse: reverse, + slivers: [ + PinnedHeaderSliver(child: buildExtentBox(30)), + SliverToBoxAdapter(child: buildExtentBox(30)), + PinnedHeaderSliver(child: buildExtentBox(20, child: const Text('1'))), + SliverToBoxAdapter(child: buildExtentBox(1000)), + ], + ), + ); + } + + await pumpWidget(); + controller.jumpTo(500); + await tester.pumpAndSettle(); + expect(tester.getTopLeft(find.text('1')), const Offset(0, 30)); + + await pumpWidget(scrollDirection: Axis.horizontal); + controller.jumpTo(500); + await tester.pumpAndSettle(); + expect(tester.getTopLeft(find.text('1')), const Offset(30, 0)); + + await pumpWidget(reverse: true); + controller.jumpTo(500); + await tester.pumpAndSettle(); + expect(tester.getTopLeft(find.text('1')), Offset(0, screenSize.height - 50)); + + await pumpWidget(scrollDirection: Axis.horizontal, reverse: true); + controller.jumpTo(500); + await tester.pumpAndSettle(); + expect(tester.getTopLeft(find.text('1')), Offset(screenSize.width - 50, 0)); + }); + + testWidgets('SliverMainAxisGroup precision error', (WidgetTester tester) async { + final ScrollController controller = ScrollController(); + addTearDown(controller.dispose); + await tester.pumpWidget( + MaterialApp( + home: Center( + child: SizedBox( + height: 201, + child: CustomScrollView( + controller: controller, + slivers: const [ + SliverMainAxisGroup( + slivers: [ + SliverToBoxAdapter(child: SizedBox(height: 70)), + PinnedHeaderSliver(child: SizedBox(height: 70)), + SliverToBoxAdapter(child: SizedBox(height: 70)), + PinnedHeaderSliver(child: SizedBox(height: 70)), + SliverToBoxAdapter(child: SizedBox(height: 70)), + PinnedHeaderSliver(child: SizedBox(height: 70)), + ], + ), + ], + ), + ), + ), + ), + ); + controller.jumpTo(60.22678428085297); + await tester.pumpAndSettle(); + expect(tester.takeException(), isNull); + }); } Widget _buildSliverList({