Fix virtualization logic with horizontal RTL lists (#38529)

Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/38529

VirtualizedList internally represents metrics along the scrolling axis using `offset` (x, y), and `length` (width, height). This one dimensional representation allows the list to be generic to being horizontal or vertical.

Right now offset conversion directly takes the `x` or `y` axis value, but  this is not correct when we are using a horizontal FlatList in RTL.

This change converts most VirtualizedList code handling  `offset,length` coordinates consistently flow from start to end, including in horizontal RTL and in inverted FlatList.

This is paired with a fix to Android native code in D47627115. With these together, basic Horizontal FlatList scenarios should work correctly when laid out in RTL.

Changelog:
[General][Fixed] - Fix virtualization logic with horizontal RTL lists

Reviewed By: vincentriemer

Differential Revision: D46586420

fbshipit-source-id: 79594e197d21871bb493399999e91fc0d6c7b050
This commit is contained in:
Nick Gerleman
2023-07-27 16:26:18 -07:00
committed by Facebook GitHub Bot
parent 30c7e9dfa4
commit 8b39bfadbb
2 changed files with 326 additions and 136 deletions
@@ -8,6 +8,7 @@
* @format
*/
import type {Layout} from 'react-native/Libraries/Types/CoreEventTypes';
import type {Props as VirtualizedListProps} from './VirtualizedListProps';
import {keyExtractor as defaultKeyExtractor} from './VirtualizeUtils';
@@ -23,7 +24,8 @@ export type CellMetrics = {
*/
length: number,
/**
* Offset to the cell along the scrolling axis
* Distance between this cell and the start of the list along the scrolling
* axis
*/
offset: number,
/**
@@ -32,6 +34,13 @@ export type CellMetrics = {
isMounted: boolean,
};
// TODO: `inverted` can be incorporated here if it is moved to an order
// based implementation instead of transform.
export type ListOrientation = {
horizontal: boolean,
rtl: boolean,
};
/**
* Subset of VirtualizedList props needed to calculate cell metrics
*/
@@ -44,52 +53,85 @@ export type CellMetricProps = {
...
};
type UnresolvedCellMetrics = {
index: number,
layout: Layout,
isMounted: boolean,
// The length of list content at the time of layout is needed to correctly
// resolve flow relative offset in RTL. We are lazily notified of this after
// the layout of the cell, unless the cell relayout does not cause a length
// change. To keep stability, we use content length at time of query, or
// unmount if never queried.
listContentLength?: ?number,
};
/**
* Provides an interface to query information about the metrics of a list and its cells.
*/
export default class ListMetricsAggregator {
_averageCellLength = 0;
_frames: {[string]: CellMetrics} = {};
_cellMetrics: {[string]: UnresolvedCellMetrics} = {};
_contentLength: ?number;
_highestMeasuredCellIndex = 0;
_totalCellLength = 0;
_totalCellsMeasured = 0;
_measuredCellsLength = 0;
_measuredCellsCount = 0;
_orientation: ListOrientation = {
horizontal: false,
rtl: false,
};
/**
* Notify the ListMetricsAggregator that a cell has been laid out.
*
* @returns whether the cell layout has changed since last notification
*/
notifyCellLayout(
notifyCellLayout({
cellIndex,
cellKey,
orientation,
layout,
}: {
cellIndex: number,
cellKey: string,
index: number,
length: number,
offset: number,
): boolean {
const next: CellMetrics = {
offset,
length,
index,
orientation: ListOrientation,
layout: Layout,
}): boolean {
this._invalidateIfOrientationChanged(orientation);
const next: UnresolvedCellMetrics = {
index: cellIndex,
layout: layout,
isMounted: true,
};
const curr = this._frames[cellKey];
const curr = this._cellMetrics[cellKey];
if (
!curr ||
next.offset !== curr.offset ||
next.length !== curr.length ||
index !== curr.index
this._selectOffset(next.layout) !== this._selectOffset(curr.layout) ||
this._selectLength(next.layout) !== this._selectLength(curr.layout) ||
(curr.listContentLength != null &&
curr.listContentLength !== this._contentLength)
) {
this._totalCellLength += next.length - (curr ? curr.length : 0);
this._totalCellsMeasured += curr ? 0 : 1;
if (curr) {
const dLength =
this._selectLength(next.layout) - this._selectLength(curr.layout);
this._measuredCellsLength += dLength;
} else {
this._measuredCellsLength += this._selectLength(next.layout);
this._measuredCellsCount += 1;
}
this._averageCellLength =
this._totalCellLength / this._totalCellsMeasured;
this._frames[cellKey] = next;
this._measuredCellsLength / this._measuredCellsCount;
this._cellMetrics[cellKey] = next;
this._highestMeasuredCellIndex = Math.max(
this._highestMeasuredCellIndex,
index,
cellIndex,
);
return true;
} else {
this._frames[cellKey].isMounted = true;
this._cellMetrics[cellKey].isMounted = true;
return false;
}
}
@@ -98,12 +140,31 @@ export default class ListMetricsAggregator {
* Notify ListMetricsAggregator that a cell has been unmounted.
*/
notifyCellUnmounted(cellKey: string): void {
const curr = this._frames[cellKey];
const curr = this._cellMetrics[cellKey];
if (curr) {
this._frames[cellKey] = {...curr, isMounted: false};
this._cellMetrics[cellKey] = {
...curr,
isMounted: false,
listContentLength: curr.listContentLength ?? this._contentLength,
};
}
}
/**
* Notify ListMetricsAggregator that the lists content container has been laid out.
*/
notifyListContentLayout({
orientation,
layout,
}: {
orientation: ListOrientation,
layout: $ReadOnly<{width: number, height: number}>,
}): void {
this._invalidateIfOrientationChanged(orientation);
const newLength = this._selectLength(layout);
this._contentLength = newLength;
}
/**
* Return the average length of the cells which have been measured
*/
@@ -150,17 +211,20 @@ export default class ListMetricsAggregator {
const {data, getItem, getItemCount, getItemLayout} = props;
invariant(
index >= 0 && index < getItemCount(data),
'Tried to get frame for out of range index ' + index,
'Tried to get metrics for out of range cell index ' + index,
);
const keyExtractor = props.keyExtractor ?? defaultKeyExtractor;
const frame = this._frames[keyExtractor(getItem(data, index), index)];
if (!frame || frame.index !== index) {
if (getItemLayout) {
const {length, offset} = getItemLayout(data, index);
return {index, length, offset, isMounted: true};
}
const frame = this._cellMetrics[keyExtractor(getItem(data, index), index)];
if (frame && frame.index === index) {
return this._resolveCellMetrics(frame);
}
return frame;
if (getItemLayout) {
const {length, offset} = getItemLayout(data, index);
return {index, length, offset, isMounted: true};
}
return null;
}
/**
@@ -176,4 +240,93 @@ export default class ListMetricsAggregator {
return frameMetrics.offset + remainder * frameMetrics.length;
}
}
/**
* Returns the length of all ScrollView content along the scrolling axis.
*/
getContentLength(): number {
return this._contentLength ?? 0;
}
/**
* Whether a content length has been observed
*/
hasContentLength(): boolean {
return this._contentLength != null;
}
/**
* Converts a cartesian offset along the x or y axis to a flow-relative
* offset, (e.g. starting from the left in LTR, but right in RTL).
*/
flowRelativeOffset(layout: Layout, referenceContentLength?: ?number): number {
const {horizontal, rtl} = this._orientation;
if (horizontal && rtl) {
const contentLength = referenceContentLength ?? this._contentLength;
invariant(
contentLength != null,
'ListMetricsAggregator must be notified of list content layout before resolving offsets',
);
return contentLength - this._selectOffset(layout);
} else {
return this._selectOffset(layout);
}
}
/**
* Converts a flow-relative offset to a cartesian offset
*/
cartesianOffset(flowRelativeOffset: number): number {
const {horizontal, rtl} = this._orientation;
if (horizontal && rtl) {
invariant(
this._contentLength != null,
'ListMetricsAggregator must be notified of list content layout before resolving offsets',
);
return this._contentLength - flowRelativeOffset;
} else {
return flowRelativeOffset;
}
}
_invalidateIfOrientationChanged(orientation: ListOrientation): void {
if (orientation.rtl !== this._orientation.rtl) {
this._cellMetrics = {};
}
if (orientation.horizontal !== this._orientation.horizontal) {
this._averageCellLength = 0;
this._contentLength = 0;
this._highestMeasuredCellIndex = 0;
this._measuredCellsLength = 0;
this._measuredCellsCount = 0;
}
this._orientation = orientation;
}
_selectLength({
width,
height,
}: $ReadOnly<{width: number, height: number, ...}>): number {
return this._orientation.horizontal ? width : height;
}
_selectOffset({x, y}: $ReadOnly<{x: number, y: number, ...}>): number {
return this._orientation.horizontal ? x : y;
}
_resolveCellMetrics(metrics: UnresolvedCellMetrics): CellMetrics {
metrics.listContentLength ??= this._contentLength;
const {index, layout, isMounted, listContentLength} = metrics;
return {
index,
length: this._selectLength(layout),
isMounted,
offset: this.flowRelativeOffset(layout, listContentLength),
};
}
}
@@ -22,9 +22,11 @@ import type {
RenderItemType,
Separators,
} from './VirtualizedListProps';
import type {CellMetricProps} from './ListMetricsAggregator';
import type {CellMetricProps, ListOrientation} from './ListMetricsAggregator';
import {
I18nManager,
Platform,
RefreshControl,
ScrollView,
View,
@@ -155,24 +157,8 @@ class VirtualizedList extends StateSafePureComponent<Props, State> {
this._scrollMetrics.visibleLength,
);
if (this._scrollRef == null) {
return;
}
if (this._scrollRef.scrollTo == null) {
console.warn(
'No scrollTo method provided. This may be because you have two nested ' +
'VirtualizedLists with the same orientation, or because you are ' +
'using a custom component that does not implement scrollTo.',
);
return;
}
this._scrollRef.scrollTo(
horizontalOrDefault(this.props.horizontal)
? {x: offset, animated}
: {y: offset, animated},
);
// TODO: consider using `ref.scrollToEnd` directly
this.scrollToOffset({animated, offset});
}
// scrollToIndex may be janky without getItemLayout prop
@@ -183,13 +169,8 @@ class VirtualizedList extends StateSafePureComponent<Props, State> {
viewPosition?: number,
...
}): $FlowFixMe {
const {
data,
horizontal,
getItemCount,
getItemLayout,
onScrollToIndexFailed,
} = this.props;
const {data, getItemCount, getItemLayout, onScrollToIndexFailed} =
this.props;
const {animated, index, viewOffset, viewPosition} = params;
invariant(
index >= 0,
@@ -236,22 +217,7 @@ class VirtualizedList extends StateSafePureComponent<Props, State> {
(this._scrollMetrics.visibleLength - frame.length),
) - (viewOffset || 0);
if (this._scrollRef == null) {
return;
}
if (this._scrollRef.scrollTo == null) {
console.warn(
'No scrollTo method provided. This may be because you have two nested ' +
'VirtualizedLists with the same orientation, or because you are ' +
'using a custom component that does not implement scrollTo.',
);
return;
}
this._scrollRef.scrollTo(
horizontal ? {x: offset, animated} : {y: offset, animated},
);
this.scrollToOffset({offset, animated});
}
// scrollToItem may be janky without getItemLayout prop. Required linear scan through items -
@@ -286,12 +252,13 @@ class VirtualizedList extends StateSafePureComponent<Props, State> {
*/
scrollToOffset(params: {animated?: ?boolean, offset: number, ...}) {
const {animated, offset} = params;
const scrollRef = this._scrollRef;
if (this._scrollRef == null) {
if (scrollRef == null) {
return;
}
if (this._scrollRef.scrollTo == null) {
if (scrollRef.scrollTo == null) {
console.warn(
'No scrollTo method provided. This may be because you have two nested ' +
'VirtualizedLists with the same orientation, or because you are ' +
@@ -300,11 +267,18 @@ class VirtualizedList extends StateSafePureComponent<Props, State> {
return;
}
this._scrollRef.scrollTo(
horizontalOrDefault(this.props.horizontal)
? {x: offset, animated}
: {y: offset, animated},
);
const {horizontal, rtl} = this._orientation();
if (horizontal && rtl && !this._listMetrics.hasContentLength()) {
console.warn(
'scrollToOffset may not be called in RTL before content is laid out',
);
return;
}
scrollRef.scrollTo({
animated,
...this._cartesianScrollOffset(offset),
});
}
recordInteraction() {
@@ -621,7 +595,8 @@ class VirtualizedList extends StateSafePureComponent<Props, State> {
const onEndReachedThreshold = onEndReachedThresholdOrDefault(
props.onEndReachedThreshold,
);
const {contentLength, offset, visibleLength} = this._scrollMetrics;
const {offset, visibleLength} = this._scrollMetrics;
const contentLength = this._listMetrics.getContentLength();
const distanceFromEnd = contentLength - visibleLength - offset;
// Wait until the scroll view metrics have been set up. And until then,
@@ -1220,9 +1195,15 @@ class VirtualizedList extends StateSafePureComponent<Props, State> {
new ChildListCollection();
_offsetFromParentVirtualizedList: number = 0;
_prevParentOffset: number = 0;
// $FlowFixMe[missing-local-annot]
_scrollMetrics = {
contentLength: 0,
_scrollMetrics: {
dOffset: number,
dt: number,
offset: number,
timestamp: number,
velocity: number,
visibleLength: number,
zoomScale: number,
} = {
dOffset: 0,
dt: 10,
offset: 0,
@@ -1291,19 +1272,27 @@ class VirtualizedList extends StateSafePureComponent<Props, State> {
}
};
_onCellLayout = (e: LayoutEvent, cellKey: string, index: number): void => {
const layout = e.nativeEvent.layout;
const offset = this._selectOffset(layout);
const length = this._selectLength(layout);
const layoutHasChanged = this._listMetrics.notifyCellLayout(
_onCellLayout = (
e: LayoutEvent,
cellKey: string,
cellIndex: number,
): void => {
const layoutHasChanged = this._listMetrics.notifyCellLayout({
cellIndex,
cellKey,
index,
length,
offset,
);
layout: e.nativeEvent.layout,
orientation: this._orientation(),
});
if (layoutHasChanged) {
this._scheduleCellsToRenderUpdate();
// TODO: We have not yet received parent content length, meaning we do not
// yet have up to date offsets in RTL. This means layout queries done
// when scheduling a new batch may not yet be correct. This is corrected
// when we schedule again in response to `onContentSizeChange`.
const {horizontal, rtl} = this._orientation();
this._scheduleCellsToRenderUpdate({
allowImmediateExecution: !(horizontal && rtl),
});
}
this._triggerRemeasureForChildListsInCell(cellKey);
@@ -1341,9 +1330,9 @@ class VirtualizedList extends StateSafePureComponent<Props, State> {
this.context.getOutermostParentListRef().getScrollRef(),
(x, y, width, height) => {
this._offsetFromParentVirtualizedList = this._selectOffset({x, y});
this._scrollMetrics.contentLength = this._selectLength({
width,
height,
this._listMetrics.notifyListContentLayout({
layout: {width, height},
orientation: this._orientation(),
});
const scrollMetrics = this._convertParentScrollMetrics(
this.context.getScrollMetrics(),
@@ -1415,7 +1404,7 @@ class VirtualizedList extends StateSafePureComponent<Props, State> {
_renderDebugOverlay() {
const normalize =
this._scrollMetrics.visibleLength /
(this._scrollMetrics.contentLength || 1);
(this._listMetrics.getContentLength() || 1);
const framesInLayout = [];
const itemCount = this.props.getItemCount(this.props.data);
for (let ii = 0; ii < itemCount; ii++) {
@@ -1487,14 +1476,48 @@ class VirtualizedList extends StateSafePureComponent<Props, State> {
: metrics.width;
}
_selectOffset(
_flowRelativeScrollOffset(
metrics: $ReadOnly<{
x: number,
y: number,
...
}>,
contentSize: $ReadOnly<{
width: number,
height: number,
...
}>,
): number {
return !horizontalOrDefault(this.props.horizontal) ? metrics.y : metrics.x;
let offset = this._selectOffset(metrics);
const {horizontal, rtl} = this._orientation();
if (horizontal && rtl && Platform.OS !== 'ios') {
offset = this._selectLength(contentSize) - offset;
}
return offset;
}
_cartesianScrollOffset(offset: number): {x?: number, y?: number} {
const {horizontal, rtl} = this._orientation();
const normalizedOffset =
horizontal && rtl && Platform.OS !== 'ios'
? this._listMetrics.getContentLength() - offset
: offset;
const cartOffset = this._listMetrics.cartesianOffset(normalizedOffset);
return horizontal ? {x: cartOffset} : {y: cartOffset};
}
_selectOffset({x, y}: $ReadOnly<{x: number, y: number, ...}>): number {
return this._orientation().horizontal ? x : y;
}
_orientation(): ListOrientation {
return {
horizontal: horizontalOrDefault(this.props.horizontal),
rtl: I18nManager.isRTL,
};
}
_maybeCallOnEdgeReached() {
@@ -1512,9 +1535,10 @@ class VirtualizedList extends StateSafePureComponent<Props, State> {
return;
}
const {contentLength, visibleLength, offset} = this._scrollMetrics;
const {visibleLength, offset} = this._scrollMetrics;
let distanceFromStart = offset;
let distanceFromEnd = contentLength - visibleLength - offset;
let distanceFromEnd =
this._listMetrics.getContentLength() - visibleLength - offset;
// Especially when oERT is zero it's necessary to 'floor' very small distance values to be 0
// since debouncing causes us to not fire this event for every single "pixel" we scroll and can thus
@@ -1548,9 +1572,9 @@ class VirtualizedList extends StateSafePureComponent<Props, State> {
onEndReached &&
this.state.cellsAroundViewport.last === getItemCount(data) - 1 &&
isWithinEndThreshold &&
this._scrollMetrics.contentLength !== this._sentEndForContentLength
this._listMetrics.getContentLength() !== this._sentEndForContentLength
) {
this._sentEndForContentLength = this._scrollMetrics.contentLength;
this._sentEndForContentLength = this._listMetrics.getContentLength();
onEndReached({distanceFromEnd});
}
@@ -1561,9 +1585,9 @@ class VirtualizedList extends StateSafePureComponent<Props, State> {
onStartReached != null &&
this.state.cellsAroundViewport.first === 0 &&
isWithinStartThreshold &&
this._scrollMetrics.contentLength !== this._sentStartForContentLength
this._listMetrics.getContentLength() !== this._sentStartForContentLength
) {
this._sentStartForContentLength = this._scrollMetrics.contentLength;
this._sentStartForContentLength = this._listMetrics.getContentLength();
onStartReached({distanceFromStart});
}
@@ -1605,7 +1629,10 @@ class VirtualizedList extends StateSafePureComponent<Props, State> {
if (this.props.onContentSizeChange) {
this.props.onContentSizeChange(width, height);
}
this._scrollMetrics.contentLength = this._selectLength({height, width});
this._listMetrics.notifyListContentLayout({
layout: {width, height},
orientation: this._orientation(),
});
this._scheduleCellsToRenderUpdate();
this._maybeCallOnEdgeReached();
};
@@ -1623,7 +1650,7 @@ class VirtualizedList extends StateSafePureComponent<Props, State> {
// Child's visible length is the same as its parent's
const visibleLength = metrics.visibleLength;
const dOffset = offset - this._scrollMetrics.offset;
const contentLength = this._scrollMetrics.contentLength;
const contentLength = this._listMetrics.getContentLength();
return {
visibleLength,
@@ -1643,11 +1670,14 @@ class VirtualizedList extends StateSafePureComponent<Props, State> {
const timestamp = e.timeStamp;
let visibleLength = this._selectLength(e.nativeEvent.layoutMeasurement);
let contentLength = this._selectLength(e.nativeEvent.contentSize);
let offset = this._selectOffset(e.nativeEvent.contentOffset);
let offset = this._flowRelativeScrollOffset(
e.nativeEvent.contentOffset,
e.nativeEvent.contentSize,
);
let dOffset = offset - this._scrollMetrics.offset;
if (this._isNestedWithSameOrientation()) {
if (this._scrollMetrics.contentLength === 0) {
if (this._listMetrics.getContentLength() === 0) {
// Ignore scroll events until onLayout has been called and we
// know our offset from our offset from our parent
return;
@@ -1682,7 +1712,6 @@ class VirtualizedList extends StateSafePureComponent<Props, State> {
// For invalid negative values (w/ RTL), set this to 1.
const zoomScale = e.nativeEvent.zoomScale < 0 ? 1 : e.nativeEvent.zoomScale;
this._scrollMetrics = {
contentLength,
dt,
dOffset,
offset,
@@ -1708,7 +1737,34 @@ class VirtualizedList extends StateSafePureComponent<Props, State> {
this._scheduleCellsToRenderUpdate();
};
_scheduleCellsToRenderUpdate() {
_scheduleCellsToRenderUpdate(opts?: {allowImmediateExecution?: boolean}) {
const allowImmediateExecution = opts?.allowImmediateExecution ?? true;
// Only trigger high-priority updates if we've actually rendered cells,
// and with that size estimate, accurately compute how many cells we should render.
// Otherwise, it would just render as many cells as it can (of zero dimension),
// each time through attempting to render more (limited by maxToRenderPerBatch),
// starving the renderer from actually laying out the objects and computing _averageCellLength.
// If this is triggered in an `componentDidUpdate` followed by a hiPri cellToRenderUpdate
// We shouldn't do another hipri cellToRenderUpdate
if (
allowImmediateExecution &&
this._shouldRenderWithPriority() &&
(this._listMetrics.getAverageCellLength() || this.props.getItemLayout) &&
!this._hiPriInProgress
) {
this._hiPriInProgress = true;
// Don't worry about interactions when scrolling quickly; focus on filling content as fast
// as possible.
this._updateCellsToRenderBatcher.dispose({abort: true});
this._updateCellsToRender();
return;
} else {
this._updateCellsToRenderBatcher.schedule();
}
}
_shouldRenderWithPriority(): boolean {
const {first, last} = this.state.cellsAroundViewport;
const {offset, visibleLength, velocity} = this._scrollMetrics;
const itemCount = this.props.getItemCount(this.props.data);
@@ -1743,27 +1799,8 @@ class VirtualizedList extends StateSafePureComponent<Props, State> {
distBottom <
getScrollingThreshold(onEndReachedThreshold, visibleLength));
}
// Only trigger high-priority updates if we've actually rendered cells,
// and with that size estimate, accurately compute how many cells we should render.
// Otherwise, it would just render as many cells as it can (of zero dimension),
// each time through attempting to render more (limited by maxToRenderPerBatch),
// starving the renderer from actually laying out the objects and computing _averageCellLength.
// If this is triggered in an `componentDidUpdate` followed by a hiPri cellToRenderUpdate
// We shouldn't do another hipri cellToRenderUpdate
if (
hiPri &&
(this._listMetrics.getAverageCellLength() || this.props.getItemLayout) &&
!this._hiPriInProgress
) {
this._hiPriInProgress = true;
// Don't worry about interactions when scrolling quickly; focus on filling content as fast
// as possible.
this._updateCellsToRenderBatcher.dispose({abort: true});
this._updateCellsToRender();
return;
} else {
this._updateCellsToRenderBatcher.schedule();
}
return hiPri;
}
_onScrollBeginDrag = (e: ScrollEvent): void => {