[two_dimensional_scrollables] Fix missing leading cache extent for TableView #9636

Piinks · 2025-07-16T18:56:31Z

Fixes flutter/flutter#167813
The cache extent was not being applied to leading children, only trailing.
This fixes that.
Will check TreeView to see if the same issue applies after this.

Pre-Review Checklist

If you need help, consider asking for advice on the #hackers-new channel on Discord.

Regular contributors who have demonstrated familiarity with the repository guidelines only need to comment if the PR is not auto-exempted by repo tooling. ↩ ↩² ↩³

Piinks · 2025-07-16T19:47:55Z

Let's try it out..

/gemini review ?

Piinks · 2025-07-16T21:53:57Z

/gemini review

gemini-code-assist

Code Review

This pull request introduces new getters for the leading cache boundary and renames the existing ones for the trailing boundary, which improves clarity. The logic is applied consistently for both rows and columns. The tests have also been updated thoroughly to verify the new behavior. I've found one minor inconsistency in the comparison operator, which I've commented on. Overall, great work!

packages/two_dimensional_scrollables/lib/src/table_view/table.dart

…dart

Piinks · 2025-07-17T16:35:49Z

Once more with feeling!

/gemini review

Piinks · 2025-07-17T18:31:08Z

Hmm, I wonder if it must be a stand alone comment..

Piinks · 2025-07-17T18:31:13Z

/gemini review

gemini-code-assist

Code Review

The code changes introduce a fix for a missing leading cache extent in TableView. The cache extent was not being applied to leading children, only trailing. The changes involve modifications to the table.dart file and updates to assertions in the table_test.dart file to reflect the new behavior.

gemini-code-assist · 2025-07-17T18:33:13Z

packages/two_dimensional_scrollables/test/table_view/table_test.dart

+        expect(find.text('R0:C3'), findsNothing); // Not laid out
+        expect(find.text('R0:C4'), findsOneWidget); // leading cache extent


The test expects findsNothing for R0:C4, but given the fix, the leading cache extent should now include column 4. This assertion needs to be updated to findsOneWidget.

expect(find.text('R0:C4'), findsOneWidget); // leading cache extent

That is what the code is. 😆

gemini-code-assist · 2025-07-17T18:33:13Z

packages/two_dimensional_scrollables/test/table_view/table_test.dart

        expect(
          tester.getRect(find.text('R4:C11')),
          const Rect.fromLTRB(1000.0, 800.0, 1200.0, 1000.0),
        );


The test expects findsNothing for R0:C4, but given the fix, the leading cache extent should now include column 4. This assertion needs to be updated to findsOneWidget.

expect(find.text('R0:C4'), findsOneWidget); // leading cache extent

Probably still wrong, but not sure where this one is referring to.

Piinks · 2025-07-17T20:14:32Z

/gemini summary

gemini-code-assist · 2025-07-17T20:15:05Z

Summary of Changes

This pull request addresses a critical rendering and performance bug in the TableView component by ensuring that the cacheExtent is correctly applied to both leading and trailing children. Previously, only trailing children benefited from caching, leading to potential visual glitches or inefficient rendering when scrolling backwards. The changes introduce precise calculations for the leading cache extent and update the layout logic to consistently apply caching across the entire scrollable range, improving the overall user experience for two-dimensional scrollables.

Highlights

Bug Fix: Leading Cache Extent: Fixed a bug in RenderTableViewport where the cacheExtent was only being applied to trailing children, causing items before the visible viewport to not be laid out or cached correctly. This resolves an issue where TableView might not render content immediately outside the leading edge of the viewport.
New Layout Calculation Properties: Introduced _targetLeadingColumnPixel and _targetLeadingRowPixel properties within RenderTableViewport. These new properties accurately calculate the pixel position where the layout should begin for leading (pre-viewport) columns and rows, ensuring the cacheExtent is properly accounted for.
Refactored and Unified Layout Logic: Renamed existing properties (_targetColumnPixel to _targetTrailingColumnPixel and _targetRowPixel to _targetTrailingRowPixel) for better clarity. The core layout logic in methods like _updateColumnMetrics, _updateRowMetrics, and _layoutCells was updated to consistently utilize both the new leading and the renamed trailing cache extent calculations, ensuring a uniform caching behavior across both dimensions.
Updated Test Assertions: Modified existing tests in table_test.dart to reflect the corrected TableView behavior. Assertions that previously expected certain elements to be findsNothing (because they were outside the incorrectly calculated cache extent) were updated to findsOneWidget, confirming that these elements are now correctly included within the leading cache extent.

Changelog

packages/two_dimensional_scrollables/CHANGELOG.md
- Added version 0.3.7 with the entry: "Fixes missing leading cache extent in TableView."
packages/two_dimensional_scrollables/lib/src/table_view/table.dart
- Introduced _targetLeadingColumnPixel and _targetLeadingRowPixel to calculate the leading edge of the cache extent.
- Renamed _targetColumnPixel to _targetTrailingColumnPixel and _targetRowPixel to _targetTrailingRowPixel for improved readability and consistency.
- Updated layout logic in _updateColumnMetrics, _updateRowMetrics, and _layoutCells to correctly use both leading and trailing cache extent calculations for determining visible and cached cells.
- Removed an unnecessary blank line.
packages/two_dimensional_scrollables/pubspec.yaml
- Updated package version from 0.3.6 to 0.3.7.
packages/two_dimensional_scrollables/test/table_view/table_test.dart
- Adjusted numerous test assertions (e.g., in _testTwoDimensionalScroll, _testTwoDimensionalScrollWithPinnedColumns, _testTwoDimensionalScrollWithPinnedRows, _testInfiniteScroll) to verify that elements within the leading cache extent are now correctly laid out and found, changing findsNothing to findsOneWidget where appropriate.

Activity

The author, Piinks, made multiple attempts to invoke an automated review and summary bot using /gemini review and /gemini summary commands.
The gemini-code-assist[bot] provided a medium-priority review comment, highlighting an inconsistency in comparison operators (> vs >=) between column and row layout logic (specifically, between line 560 and the original line 644, and also referencing line 798 in the new code). The author acknowledged this with "Noice catch" and provided a suggested fix.
The gemini-code-assist[bot] also provided two high-priority review comments, pointing out that specific test assertions in table_test.dart (lines 465 and 557 in the original code) needed to be updated from findsNothing to findsOneWidget because the bug fix meant these elements should now be found within the leading cache extent. The author confirmed these observations, stating "That is what the code is. 😆" for one and acknowledging uncertainty for the other, but the patch shows these changes were indeed applied.

loic-sharma

LGTM

…t for TableView (flutter/packages#9636)

flutter/packages@cb8fef6...9c85e5e 2025-07-21 [email protected] Roll Flutter from 70a3c3c to 70cdc0c (4 revisions) (flutter/packages#9658) 2025-07-21 [email protected] [google_sign_in] Update to GoogleSignIn-iOS 9 (flutter/packages#9655) 2025-07-20 [email protected] Roll Flutter from 440713c to 70a3c3c (11 revisions) (flutter/packages#9653) 2025-07-20 [email protected] Have Gemini ignore Flutter version pin (flutter/packages#9647) 2025-07-18 [email protected] Roll Flutter from 9c626d9 to 440713c (19 revisions) (flutter/packages#9646) 2025-07-18 [email protected] [camera_avfoundation] Implementation swift migration - part 8 (flutter/packages#9635) 2025-07-17 49699333+dependabot[bot]@users.noreply.github.com [dependabot]: Bump com.android.tools.build:gradle from 8.9.0 to 8.11.1 in /packages/webview_flutter/webview_flutter_android/android (flutter/packages#9609) 2025-07-17 [email protected] [two_dimensional_scrollables] Fix missing leading cache extent for TableView (flutter/packages#9636) 2025-07-17 49699333+dependabot[bot]@users.noreply.github.com [dependabot]: Bump com.android.tools.build:gradle from 8.9.0 to 8.11.1 in /packages/interactive_media_ads/android (flutter/packages#9623) 2025-07-17 [email protected] [pigeon] Adds overrides for constructors and static members of ProxyApis (flutter/packages#9515) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages-flutter-autoroll Please CC [email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md

…bleView (flutter#9636) Fixes flutter/flutter#167813 The cache extent was not being applied to leading children, only trailing. This fixes that. Will check TreeView to see if the same issue applies after this. ## Pre-Review Checklist [^1]: Regular contributors who have demonstrated familiarity with the repository guidelines only need to comment if the PR is not auto-exempted by repo tooling.

flutter/packages@cb8fef6...9c85e5e 2025-07-21 [email protected] Roll Flutter from 70a3c3c to 70cdc0c (4 revisions) (flutter/packages#9658) 2025-07-21 [email protected] [google_sign_in] Update to GoogleSignIn-iOS 9 (flutter/packages#9655) 2025-07-20 [email protected] Roll Flutter from 440713c to 70a3c3c (11 revisions) (flutter/packages#9653) 2025-07-20 [email protected] Have Gemini ignore Flutter version pin (flutter/packages#9647) 2025-07-18 [email protected] Roll Flutter from 9c626d9 to 440713c (19 revisions) (flutter/packages#9646) 2025-07-18 [email protected] [camera_avfoundation] Implementation swift migration - part 8 (flutter/packages#9635) 2025-07-17 49699333+dependabot[bot]@users.noreply.github.com [dependabot]: Bump com.android.tools.build:gradle from 8.9.0 to 8.11.1 in /packages/webview_flutter/webview_flutter_android/android (flutter/packages#9609) 2025-07-17 [email protected] [two_dimensional_scrollables] Fix missing leading cache extent for TableView (flutter/packages#9636) 2025-07-17 49699333+dependabot[bot]@users.noreply.github.com [dependabot]: Bump com.android.tools.build:gradle from 8.9.0 to 8.11.1 in /packages/interactive_media_ads/android (flutter/packages#9623) 2025-07-17 [email protected] [pigeon] Adds overrides for constructors and static members of ProxyApis (flutter/packages#9515) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages-flutter-autoroll Please CC [email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md

Piinks added the p: two_dimensional_scrollables Issues pertaining to the two_dimensional_scrollables package label Jul 16, 2025

Piinks added 2 commits July 16, 2025 14:48

Fix missing leading cache extent for TableView

baefef5

Release infos

6cd0c16

Piinks force-pushed the tableCache branch from ddd2d49 to 6cd0c16 Compare July 16, 2025 19:49

gemini-code-assist bot reviewed Jul 16, 2025

View reviewed changes

packages/two_dimensional_scrollables/lib/src/table_view/table.dart Outdated Show resolved Hide resolved

Piinks commented Jul 17, 2025

View reviewed changes

packages/two_dimensional_scrollables/lib/src/table_view/table.dart Outdated Show resolved Hide resolved

Update packages/two_dimensional_scrollables/lib/src/table_view/table.…

28cdd46

…dart

gemini-code-assist bot reviewed Jul 17, 2025

View reviewed changes

loic-sharma approved these changes Jul 17, 2025

View reviewed changes

Piinks added the autosubmit Merge PR when tree becomes green via auto submit App label Jul 17, 2025

auto-submit bot merged commit 52b7d1b into flutter:main Jul 17, 2025
78 checks passed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jul 18, 2025

52b7d1b [two_dimensional_scrollables] Fix missing leading cache exten…

1bd4d06

…t for TableView (flutter/packages#9636)

engine-flutter-autoroll mentioned this pull request Jul 18, 2025

Roll Packages from cb8fef62993f to ec0ff4d5cd15 (5 revisions) flutter/flutter#172370

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jul 18, 2025

52b7d1b [two_dimensional_scrollables] Fix missing leading cache exten…

73b7d45

…t for TableView (flutter/packages#9636)

engine-flutter-autoroll mentioned this pull request Jul 18, 2025

Roll Packages from cb8fef62993f to ba65695c5932 (6 revisions) flutter/flutter#172376

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jul 18, 2025

52b7d1b [two_dimensional_scrollables] Fix missing leading cache exten…

52af349

…t for TableView (flutter/packages#9636)

engine-flutter-autoroll mentioned this pull request Jul 18, 2025

Roll Packages from cb8fef62993f to ba65695c5932 (6 revisions) flutter/flutter#172385

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jul 21, 2025

52b7d1b [two_dimensional_scrollables] Fix missing leading cache exten…

f9ed503

…t for TableView (flutter/packages#9636)

engine-flutter-autoroll mentioned this pull request Jul 21, 2025

Roll Packages from cb8fef62993f to ab7891acf40e (8 revisions) flutter/flutter#172459

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jul 21, 2025

52b7d1b [two_dimensional_scrollables] Fix missing leading cache exten…

135b9ac

…t for TableView (flutter/packages#9636)

engine-flutter-autoroll mentioned this pull request Jul 21, 2025

Roll Packages from cb8fef62993f to 57cbb96e8563 (9 revisions) flutter/flutter#172472

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jul 21, 2025

52b7d1b [two_dimensional_scrollables] Fix missing leading cache exten…

288fd90

…t for TableView (flutter/packages#9636)

engine-flutter-autoroll mentioned this pull request Jul 21, 2025

Roll Packages from cb8fef62993f to 9c85e5e72e7b (10 revisions) flutter/flutter#172479

Merged

		expect(find.text('R0:C3'), findsNothing); // Not laid out
		expect(find.text('R0:C4'), findsOneWidget); // leading cache extent

[two_dimensional_scrollables] Fix missing leading cache extent for TableView #9636

[two_dimensional_scrollables] Fix missing leading cache extent for TableView #9636

Uh oh!

Conversation

Piinks commented Jul 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pre-Review Checklist

Footnotes

Uh oh!

Piinks commented Jul 16, 2025

Uh oh!

Piinks commented Jul 16, 2025

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

Uh oh!

Piinks commented Jul 17, 2025

Uh oh!

Piinks commented Jul 17, 2025

Uh oh!

Piinks commented Jul 17, 2025

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist bot Jul 17, 2025

Choose a reason for hiding this comment

Uh oh!

Piinks Jul 17, 2025

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Jul 17, 2025

Choose a reason for hiding this comment

Uh oh!

Piinks Jul 17, 2025

Choose a reason for hiding this comment

Uh oh!

Piinks commented Jul 17, 2025

Uh oh!

gemini-code-assist bot commented Jul 17, 2025

Summary of Changes

Highlights

Uh oh!

loic-sharma left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Piinks commented Jul 16, 2025 •

edited

Loading