From 343a0668c3f056ae301970bf3fea90a5cdb98cd0 Mon Sep 17 00:00:00 2001 From: icartsh Date: Wed, 2 Sep 2026 12:57:39 +0900 Subject: [PATCH] [two_dimensional_scrollables] Fix trailing pinned spans being laid out twice `RenderTableViewport._updateFirstAndLastVisibleCell` binary searches for the last non-pinned span whose trailing offset reaches the target trailing pixel. When the whole table fits inside the viewport plus the cache extent, that search finds nothing and the code falls back to the last index of the metrics map. With `trailingPinnedColumnCount` (or `trailingPinnedRowCount`) greater than zero, that index is a pinned span. `layoutChildSequence` then requests the same vicinity twice: once for the non-pinned quadrant and once for the trailing pinned quadrant. The first layout is hidden because `_needsDelegateRebuild` routes both calls through `_buildChild`, but any later layout without a delegate rebuild (a scroll, for example) takes the `_reuseChild` path and trips `assert(elementToReuse != null)` on the second request: 'elementToReuse != null': Expected to re-use an element at (row: 0, column: 2), but none was found. Exclude the trailing pinned spans from the fallback, matching what `_updateColumnMetrics`/`_updateRowMetrics` already do when they compute the same value. Fixes flutter/flutter#192152 --- .../two_dimensional_scrollables/CHANGELOG.md | 4 + .../lib/src/table_view/table.dart | 10 ++- .../two_dimensional_scrollables/pubspec.yaml | 2 +- .../test/table_view/table_test.dart | 86 +++++++++++++++++++ 4 files changed, 99 insertions(+), 3 deletions(-) diff --git a/packages/two_dimensional_scrollables/CHANGELOG.md b/packages/two_dimensional_scrollables/CHANGELOG.md index 3eb9f1d569c0..2dab5ebb9118 100644 --- a/packages/two_dimensional_scrollables/CHANGELOG.md +++ b/packages/two_dimensional_scrollables/CHANGELOG.md @@ -1,3 +1,7 @@ +## 0.5.5 + +* Fixes a crash when trailing pinned rows or columns fall within the viewport's cache extent. + ## 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..10b417b16098 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,10 @@ class RenderTableViewport extends RenderTwoDimensionalViewport { (span) => !span.isPinned && span.trailingOffset >= _targetTrailingColumnPixel, ); if (_firstNonPinnedColumn != null) { - _lastNonPinnedColumn ??= _columnMetrics.length - 1; + // The last column of the metrics may be a trailing pinned column, which + // is laid out separately. Exclude them so the same column is not laid out + // in both the non-pinned and the trailing pinned quadrants. + _lastNonPinnedColumn ??= _columnMetrics.length - 1 - delegate.trailingPinnedColumnCount; } if (_rowMetrics.isNotEmpty) { @@ -1116,7 +1119,10 @@ class RenderTableViewport extends RenderTwoDimensionalViewport { (span) => !span.isPinned && span.trailingOffset >= _targetTrailingRowPixel, ); if (_firstNonPinnedRow != null) { - _lastNonPinnedRow ??= _rowMetrics.length - 1; + // The last row of the metrics may be a trailing pinned row, which is laid + // out separately. Exclude them so the same row is not laid out in both + // the non-pinned and the trailing pinned quadrants. + _lastNonPinnedRow ??= _rowMetrics.length - 1 - delegate.trailingPinnedRowCount; } } 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..8b8b78c54c06 100644 --- a/packages/two_dimensional_scrollables/test/table_view/table_test.dart +++ b/packages/two_dimensional_scrollables/test/table_view/table_test.dart @@ -4085,6 +4085,92 @@ void main() { expect(mergedRect.top, 200); expect(mergedRect.bottom, 400); }); + + testWidgets('Trailing pinned columns are not laid out twice when the table fits the viewport', ( + WidgetTester tester, + ) async { + // Regression test for https://github.com/flutter/flutter/issues/192152 + final verticalController = ScrollController(); + addTearDown(verticalController.dispose); + + await tester.pumpWidget( + MaterialApp( + home: Scaffold( + body: SizedBox( + height: 300, + width: 400, + // All of the columns fit within the viewport, so the search for the + // last visible non-pinned column comes up empty and falls back to + // the last column of the metrics - which is a trailing pinned one. + child: TableView.builder( + verticalDetails: ScrollableDetails.vertical(controller: verticalController), + columnCount: 3, + rowCount: 100, + pinnedRowCount: 1, + trailingPinnedColumnCount: 1, + columnBuilder: (int index) => const TableSpan(extent: FixedTableSpanExtent(100)), + rowBuilder: (int index) => const TableSpan(extent: FixedTableSpanExtent(36)), + cellBuilder: (BuildContext context, TableVicinity vicinity) { + return TableViewCell(child: Text('R${vicinity.row} C${vicinity.column}')); + }, + ), + ), + ), + ), + ); + expect(find.text('R0 C2'), findsOneWidget); + + // Lay out again without rebuilding the delegate, which exercises the child + // reuse path. The trailing pinned column must not be requested twice. + verticalController.jumpTo(1); + await tester.pump(); + + expect(tester.takeException(), isNull); + expect(find.text('R0 C2'), findsOneWidget); + }); + + testWidgets('Trailing pinned rows are not laid out twice when the table fits the viewport', ( + WidgetTester tester, + ) async { + // Regression test for https://github.com/flutter/flutter/issues/192152 + final horizontalController = ScrollController(); + addTearDown(horizontalController.dispose); + + await tester.pumpWidget( + MaterialApp( + home: Scaffold( + body: SizedBox( + height: 400, + width: 300, + // All of the rows fit within the viewport, so the search for the + // last visible non-pinned row comes up empty and falls back to the + // last row of the metrics - which is a trailing pinned one. + child: TableView.builder( + horizontalDetails: ScrollableDetails.horizontal(controller: horizontalController), + columnCount: 100, + rowCount: 3, + pinnedColumnCount: 1, + trailingPinnedRowCount: 1, + columnBuilder: (int index) => const TableSpan(extent: FixedTableSpanExtent(36)), + rowBuilder: (int index) => const TableSpan(extent: FixedTableSpanExtent(100)), + cellBuilder: (BuildContext context, TableVicinity vicinity) { + return TableViewCell(child: Text('R${vicinity.row} C${vicinity.column}')); + }, + ), + ), + ), + ), + ); + expect(find.text('R2 C0'), findsOneWidget); + + // Lay out again without rebuilding the delegate, which exercises the child + // reuse path. The trailing pinned row must not be requested twice. + horizontalController.jumpTo(1); + await tester.pump(); + + expect(tester.takeException(), isNull); + expect(find.text('R2 C0'), findsOneWidget); + }); } class _NullBuildContext implements BuildContext, TwoDimensionalChildManager {