[two_dimensional_scrollables] Exclude trailing pinned spans from the non-pinned range - #12666
Draft
m1roxx wants to merge 1 commit into
Draft
[two_dimensional_scrollables] Exclude trailing pinned spans from the non-pinned range#12666m1roxx wants to merge 1 commit into
m1roxx wants to merge 1 commit into
Conversation
…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 flutter/flutter#185842
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
RenderTableViewportlays out and paints the table as nine regions: leading pinned, regular andtrailing pinned rows, each crossed with the same three column categories.
_updateFirstAndLastVisibleCellbinary searches for the last regular row and column of the visiblerange, and when no regular span reaches the trailing edge of the layout target it fell back to the
last index in the metrics map:
Whenever
trailingPinnedRowCountortrailingPinnedColumnCountis greater than zero, that lastindex is a trailing pinned span. The regular range then overlaps the trailing pinned range, and
the same
TableVicinityis visited by two regions in a single layout and paint pass._updateColumnMetricsand_updateRowMetricsalready cap the same value correctly, with the rulethat
_lastRegularColumnIndexand_lastRegularRowIndexexpress, so the two code paths disagreed.This PR makes
_updateFirstAndLastVisibleCelluse those getters. The old fallback is kept for thecase where they are null — infinite spans with no null terminator — where trailing pinned spans
cannot exist, since
_firstTrailingPinnedColumnand_firstTrailingPinnedRowboth require anon-null span count.
Fixes flutter/flutter#185842
Beyond the duplicated work
The overlap is not only wasted layout. Scrolling a table to the end with a trailing pinned span
throws, because two regions claim the same cell:
and, once the spans carry a decoration, the non-pinned region reaches a cell whose
paintOffsetwasnever set for it:
Dragging through a 60x60 table with pinned and trailing pinned rows and columns throws
Expected to re-use an element at ...andTableViewCell for (row: 1, column: 58) could not be foundbefore this change, and nothing after it. That is the same subsystem and trigger asflutter/flutter#190800, but the specific assertion reported there did not
reproduce in my runs, so I am not claiming this fixes it.
Tests
Two regression tests are added to
table_test.dart, one per axis. Each scrolls a table with atrailing pinned span to the end, asserts that no exception is thrown, and asserts that the trailing
pinned span's decoration is painted exactly once — by the trailing pinned region only. Both fail
before this change.
Counting
cellBuildercalls does not work as a test here:buildOrObtainChildForcaches childrenfor the frame and
RenderObject.layoutearly-returns on unchanged constraints, so the duplicatedvisit is invisible from the delegate. The new
CountingSpanDecorationhelper observes it throughthe public
TableSpanDecoration.paintAPI instead.Pre-Review Checklist
[shared_preferences]///).Footnotes
Regular contributors who have demonstrated familiarity with the repository guidelines only need to comment if the PR is not auto-exempted by repo tooling. ↩ ↩2