From a43095296888ec870b75b81858839cd57caec946 Mon Sep 17 00:00:00 2001 From: yim Date: Sat, 15 Feb 2025 02:05:21 +0800 Subject: [PATCH] Refactor SliverMainAxisGroup for reverse mode. (#161849) Fixes: #159787 We should not refer to `RenderSliverMultiBoxAdaptor`. `RenderSliverMainAxisGroup` is more like `RenderViewport` because their children are both Slivers. *If you had to change anything in the [flutter/tests] repo, include a link to the migration guide as per the [breaking change policy].* ## 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 --------- Co-authored-by: Kate Lovett --- .../lib/src/rendering/sliver_group.dart | 67 +++++++------------ .../widgets/sliver_main_axis_group_test.dart | 27 ++++++-- 2 files changed, 46 insertions(+), 48 deletions(-) diff --git a/packages/flutter/lib/src/rendering/sliver_group.dart b/packages/flutter/lib/src/rendering/sliver_group.dart index d661b83874..acae62bb53 100644 --- a/packages/flutter/lib/src/rendering/sliver_group.dart +++ b/packages/flutter/lib/src/rendering/sliver_group.dart @@ -353,57 +353,36 @@ class RenderSliverMainAxisGroup extends RenderSliver hasVisualOverflow: totalScrollExtent > constraints.remainingPaintExtent || constraints.scrollOffset > 0.0, ); + + // Update the children's paintOffset based on the direction again, which + // must be done after obtaining the `paintExtent`. + child = firstChild; + while (child != null) { + final SliverPhysicalParentData childParentData = + child.parentData! as SliverPhysicalParentData; + childParentData.paintOffset = switch (constraints.axisDirection) { + AxisDirection.up => Offset( + 0.0, + paintExtent - childParentData.paintOffset.dy - child.geometry!.paintExtent, + ), + AxisDirection.left => Offset( + paintExtent - childParentData.paintOffset.dx - child.geometry!.paintExtent, + 0.0, + ), + AxisDirection.right || AxisDirection.down => childParentData.paintOffset, + }; + child = childAfter(child); + } } @override void paint(PaintingContext context, Offset offset) { - if (firstChild == null) { - return; - } - // offset is to the top-left corner, regardless of our axis direction. - // originOffset gives us the delta from the real origin to the origin in the axis direction. - final Offset mainAxisUnit, crossAxisUnit, originOffset; - final bool addExtent; - switch (applyGrowthDirectionToAxisDirection( - constraints.axisDirection, - constraints.growthDirection, - )) { - case AxisDirection.up: - mainAxisUnit = const Offset(0.0, -1.0); - crossAxisUnit = const Offset(1.0, 0.0); - originOffset = offset + Offset(0.0, geometry!.paintExtent); - addExtent = true; - case AxisDirection.right: - mainAxisUnit = const Offset(1.0, 0.0); - crossAxisUnit = const Offset(0.0, 1.0); - originOffset = offset; - addExtent = false; - case AxisDirection.down: - mainAxisUnit = const Offset(0.0, 1.0); - crossAxisUnit = const Offset(1.0, 0.0); - originOffset = offset; - addExtent = false; - case AxisDirection.left: - mainAxisUnit = const Offset(-1.0, 0.0); - crossAxisUnit = const Offset(0.0, 1.0); - originOffset = offset + Offset(geometry!.paintExtent, 0.0); - addExtent = true; - } - RenderSliver? child = lastChild; while (child != null) { - final double mainAxisDelta = childMainAxisPosition(child); - final double crossAxisDelta = childCrossAxisPosition(child); - Offset childOffset = Offset( - originOffset.dx + mainAxisUnit.dx * mainAxisDelta + crossAxisUnit.dx * crossAxisDelta, - originOffset.dy + mainAxisUnit.dy * mainAxisDelta + crossAxisUnit.dy * crossAxisDelta, - ); - if (addExtent) { - childOffset += mainAxisUnit * child.geometry!.paintExtent; - } - if (child.geometry!.visible) { - context.paintChild(child, childOffset); + final SliverPhysicalParentData childParentData = + child.parentData! as SliverPhysicalParentData; + context.paintChild(child, offset + childParentData.paintOffset); } child = childBefore(child); } 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 3a7cba09f1..a559e20079 100644 --- a/packages/flutter/test/widgets/sliver_main_axis_group_test.dart +++ b/packages/flutter/test/widgets/sliver_main_axis_group_test.dart @@ -140,12 +140,12 @@ void main() { expect(find.text('Group 0 Tile 19'), findsOneWidget); expect( tester.getRect(find.text('Group 0 Tile 19')), - const Rect.fromLTRB(0.0, 0.0, 300.0, 300.0), + const Rect.fromLTRB(0.0, 300.0, 300.0, 600.0), ); expect(find.text('Group 1 Tile 0'), findsOneWidget); expect( tester.getRect(find.text('Group 1 Tile 0')), - const Rect.fromLTRB(0.0, 400.0, 300.0, 600.0), + const Rect.fromLTRB(0.0, 100.0, 300.0, 300.0), ); final List renderSlivers = @@ -158,9 +158,9 @@ void main() { expect(first.geometry!.scrollExtent, equals(20 * 300.0)); expect(second.geometry!.scrollExtent, equals(20 * 200.0)); - expect((first.parentData! as SliverPhysicalParentData).paintOffset.dy, equals(0.0)); + expect((first.parentData! as SliverPhysicalParentData).paintOffset.dy, equals(300.0)); expect(first.constraints.scrollOffset, equals(19 * 300.0)); - expect((second.parentData! as SliverPhysicalParentData).paintOffset.dy, equals(1 * 300.0)); + expect((second.parentData! as SliverPhysicalParentData).paintOffset.dy, equals(0.0)); final RenderSliverMainAxisGroup renderGroup = tester.renderObject( find.byType(SliverMainAxisGroup), @@ -936,6 +936,25 @@ void main() { expect(find.text('1'), findsNothing); expect(find.text('1', skipOffstage: false), findsOneWidget); }); + + testWidgets("The localToGlobal of SliverMainAxisGroup's children works in reverse.", ( + WidgetTester tester, + ) async { + await tester.pumpWidget( + _buildSliverMainAxisGroup( + viewportHeight: 400, + reverse: true, + slivers: [ + const SliverToBoxAdapter(child: SizedBox(height: 70)), + const SliverToBoxAdapter(child: SizedBox(height: 20, child: Text('1'))), + const SliverToBoxAdapter(child: SizedBox(height: 700)), + ], + ), + ); + final RenderBox renderBox = tester.renderObject(find.text('1')) as RenderBox; + expect(renderBox.localToGlobal(Offset.zero), const Offset(0.0, 310.0)); + expect(tester.getTopLeft(find.text('1')), const Offset(0.0, 310.0)); + }); } Widget _buildSliverList({