From 6fa2cf279e671a83ed910c405a6b530d701c29a5 Mon Sep 17 00:00:00 2001 From: m1roxx Date: Fri, 28 Aug 2026 15:24:42 +0500 Subject: [PATCH] [two_dimensional_scrollables] Exclude trailing pinned spans from the non-pinned range RenderTableViewport lays out and paints the table as nine regions: leading pinned, regular and trailing pinned rows crossed with the same three column categories. _updateFirstAndLastVisibleCell binary searches for the last regular row and column of the visible range, and when no regular span reaches the trailing edge of the layout target it fell back to the last index in the metrics map, which is a trailing pinned span whenever trailingPinnedRowCount or trailingPinnedColumnCount is greater than zero. The regular range then overlapped the trailing pinned range, so the same vicinity was visited by two regions in a single layout and paint pass. That wastes work, and it also throws: "Expected to re-use an element at ...", "TableViewCell for ... could not be found", and a null paintOffset while computing span decoration bounds. _updateColumnMetrics and _updateRowMetrics already cap the range with the same rule that _lastRegularColumnIndex and _lastRegularRowIndex express, so this reuses those getters and keeps the old fallback for the infinite case where they are null and trailing pinned spans cannot exist. Fixes https://github.com/flutter/flutter/issues/185842 --- .../two_dimensional_scrollables/CHANGELOG.md | 4 + .../lib/src/table_view/table.dart | 8 +- .../two_dimensional_scrollables/pubspec.yaml | 2 +- .../test/table_view/table_test.dart | 111 ++++++++++++++++++ 4 files changed, 122 insertions(+), 3 deletions(-) diff --git a/packages/two_dimensional_scrollables/CHANGELOG.md b/packages/two_dimensional_scrollables/CHANGELOG.md index 3eb9f1d569c0..c2acc0f460e6 100644 --- a/packages/two_dimensional_scrollables/CHANGELOG.md +++ b/packages/two_dimensional_scrollables/CHANGELOG.md @@ -1,3 +1,7 @@ +## 0.5.5 + +* Fixes `TableView` laying out and painting trailing pinned rows and columns twice, which could throw when the table was scrolled to the end. + ## 0.5.4 * Fixes memory leaks. diff --git a/packages/two_dimensional_scrollables/lib/src/table_view/table.dart b/packages/two_dimensional_scrollables/lib/src/table_view/table.dart index 94b104aec8b5..7e8fc05daa35 100644 --- a/packages/two_dimensional_scrollables/lib/src/table_view/table.dart +++ b/packages/two_dimensional_scrollables/lib/src/table_view/table.dart @@ -1087,7 +1087,9 @@ class RenderTableViewport extends RenderTwoDimensionalViewport { (span) => !span.isPinned && span.trailingOffset >= _targetTrailingColumnPixel, ); if (_firstNonPinnedColumn != null) { - _lastNonPinnedColumn ??= _columnMetrics.length - 1; + // Trailing pinned columns are laid out and painted separately, so the + // range of regular columns must never extend into them. + _lastNonPinnedColumn ??= _lastRegularColumnIndex ?? _columnMetrics.length - 1; } if (_rowMetrics.isNotEmpty) { @@ -1116,7 +1118,9 @@ class RenderTableViewport extends RenderTwoDimensionalViewport { (span) => !span.isPinned && span.trailingOffset >= _targetTrailingRowPixel, ); if (_firstNonPinnedRow != null) { - _lastNonPinnedRow ??= _rowMetrics.length - 1; + // Trailing pinned rows are laid out and painted separately, so the range + // of regular rows must never extend into them. + _lastNonPinnedRow ??= _lastRegularRowIndex ?? _rowMetrics.length - 1; } } diff --git a/packages/two_dimensional_scrollables/pubspec.yaml b/packages/two_dimensional_scrollables/pubspec.yaml index d112e4be1113..92ce2815d9ad 100644 --- a/packages/two_dimensional_scrollables/pubspec.yaml +++ b/packages/two_dimensional_scrollables/pubspec.yaml @@ -1,6 +1,6 @@ name: two_dimensional_scrollables description: Widgets that scroll using the two dimensional scrolling foundation. -version: 0.5.4 +version: 0.5.5 repository: https://github.com/flutter/packages/tree/main/packages/two_dimensional_scrollables issue_tracker: https://github.com/flutter/flutter/issues?q=is%3Aissue+is%3Aopen+label%3A%22p%3A+two_dimensional_scrollables%22+ diff --git a/packages/two_dimensional_scrollables/test/table_view/table_test.dart b/packages/two_dimensional_scrollables/test/table_view/table_test.dart index 8a507699b96a..71208a0687b6 100644 --- a/packages/two_dimensional_scrollables/test/table_view/table_test.dart +++ b/packages/two_dimensional_scrollables/test/table_view/table_test.dart @@ -39,6 +39,20 @@ TableSpan getMouseTrackingSpan( ); } +// Counts how many times a span's decoration is painted, keyed by span index. +class CountingSpanDecoration extends TableSpanDecoration { + const CountingSpanDecoration({required this.index, required this.paintCounts}); + + final int index; + final Map paintCounts; + + @override + void paint(TableSpanDecorationPaintDetails details) { + paintCounts.update(index, (int count) => count + 1, ifAbsent: () => 1); + super.paint(details); + } +} + void main() { group('TableView.builder', () { testWidgets('creates correct delegate', (WidgetTester tester) async { @@ -4008,6 +4022,103 @@ void main() { expect(tester.getRect(find.text('R9 C9')).top, 300); }); + // Regression test for https://github.com/flutter/flutter/issues/185842. + testWidgets('Trailing pinned columns are excluded from the non-pinned column range', ( + WidgetTester tester, + ) async { + final horizontalController = ScrollController(); + addTearDown(horizontalController.dispose); + final paintCounts = {}; + + await tester.pumpWidget( + MaterialApp( + home: Scaffold( + body: SizedBox( + height: 400, + width: 400, + child: TableView.builder( + columnCount: 20, + rowCount: 4, + trailingPinnedColumnCount: 1, + horizontalDetails: ScrollableDetails.horizontal(controller: horizontalController), + columnBuilder: (int index) => TableSpan( + extent: const FixedTableSpanExtent(100), + backgroundDecoration: CountingSpanDecoration( + index: index, + paintCounts: paintCounts, + ), + ), + rowBuilder: (int index) => const TableSpan(extent: FixedTableSpanExtent(100)), + cellBuilder: (BuildContext context, TableVicinity vicinity) { + return TableViewCell(child: Text('R${vicinity.row} C${vicinity.column}')); + }, + ), + ), + ), + ), + ); + + // Once scrolled to the end, no regular column reaches the trailing edge of + // the layout target. That is where the range of non-pinned columns used to + // fall back to the last column of the table, which is a trailing pinned + // one, making it part of both the non-pinned and the trailing pinned + // region. + paintCounts.clear(); + horizontalController.jumpTo(horizontalController.position.maxScrollExtent); + await tester.pump(); + + expect(tester.takeException(), isNull); + // The trailing pinned column is decorated once, by the trailing pinned + // region only. + expect(paintCounts[19], 1); + expect(find.text('R0 C19'), findsOneWidget); + }); + + // Regression test for https://github.com/flutter/flutter/issues/185842. + testWidgets('Trailing pinned rows are excluded from the non-pinned row range', ( + WidgetTester tester, + ) async { + final verticalController = ScrollController(); + addTearDown(verticalController.dispose); + final paintCounts = {}; + + await tester.pumpWidget( + MaterialApp( + home: Scaffold( + body: SizedBox( + height: 400, + width: 400, + child: TableView.builder( + columnCount: 4, + rowCount: 20, + trailingPinnedRowCount: 1, + verticalDetails: ScrollableDetails.vertical(controller: verticalController), + columnBuilder: (int index) => const TableSpan(extent: FixedTableSpanExtent(100)), + rowBuilder: (int index) => TableSpan( + extent: const FixedTableSpanExtent(100), + backgroundDecoration: CountingSpanDecoration( + index: index, + paintCounts: paintCounts, + ), + ), + cellBuilder: (BuildContext context, TableVicinity vicinity) { + return TableViewCell(child: Text('R${vicinity.row} C${vicinity.column}')); + }, + ), + ), + ), + ), + ); + + paintCounts.clear(); + verticalController.jumpTo(verticalController.position.maxScrollExtent); + await tester.pump(); + + expect(tester.takeException(), isNull); + expect(paintCounts[19], 1); + expect(find.text('R19 C0'), findsOneWidget); + }); + testWidgets('Intersections of leading and trailing pinned', (WidgetTester tester) async { const span = TableSpan(extent: FixedTableSpanExtent(100)); await tester.pumpWidget(