From 9c10766de3d9e76035b70a3e6ecb47062fd89025 Mon Sep 17 00:00:00 2001 From: "claude[bot]" <41898282+claude[bot]@users.noreply.github.com> Date: Tue, 24 Mar 2026 00:38:59 +0000 Subject: [PATCH 1/2] fix(layout): avoid binary search for masonry visible item detection The base class getVisibleLayouts uses binary search which assumes items are sorted by y-position. In masonry layout, items are placed in columns so y-positions are NOT monotonically sorted by index. This caused items to disappear during scrolling, especially with optimizeItemArrangement disabled where column height divergence is more pronounced. Override getVisibleLayouts in MasonryLayoutManager to scan all items and correctly identify which ones overlap with the viewport, regardless of their index ordering. Fixes #2103 --- src/__tests__/MasonryLayoutManager.test.ts | 72 +++++++++++++++++++ .../layout-managers/MasonryLayoutManager.ts | 36 ++++++++++ 2 files changed, 108 insertions(+) diff --git a/src/__tests__/MasonryLayoutManager.test.ts b/src/__tests__/MasonryLayoutManager.test.ts index d5f6d4a0a..6fc04e16a 100644 --- a/src/__tests__/MasonryLayoutManager.test.ts +++ b/src/__tests__/MasonryLayoutManager.test.ts @@ -186,6 +186,78 @@ describe("MasonryLayoutManager", () => { }); }); + describe("Visible Layouts in Masonry", () => { + it("should find all visible items when y-positions are not sorted by index", () => { + // With optimizeItemArrangement=false and 2 columns, items are placed + // sequentially: item 0 in col 0, item 1 in col 1, item 2 in col 0, etc. + // When column heights diverge, y-positions are NOT sorted by index, + // which broke the binary search in the base class. + const manager = createLayoutManager(LayoutManagerType.MASONRY, { + ...defaultParams, + optimizeItemArrangement: false, + }); + + // Create items with varying heights to create column height divergence + // Col 0: items 0 (h=300), 2 (h=100), 4 (h=100) -> y: 0, 300, 400 + // Col 1: items 1 (h=50), 3 (h=50), 5 (h=50) -> y: 0, 50, 100 + const layoutInfos = [ + createMockLayoutInfo(0, 200, 300), // Col 0, y=0 + createMockLayoutInfo(1, 200, 50), // Col 1, y=0 + createMockLayoutInfo(2, 200, 100), // Col 0, y=300 + createMockLayoutInfo(3, 200, 50), // Col 1, y=50 + createMockLayoutInfo(4, 200, 100), // Col 0, y=400 + createMockLayoutInfo(5, 200, 50), // Col 1, y=100 + ]; + manager.modifyLayout(layoutInfos, 6); + const layouts = getAllLayouts(manager); + + // Verify the unsorted y-positions by index + // Items by index: y = [0, 0, 300, 50, 400, 100] + expect(layouts[0].y).toBe(0); + expect(layouts[1].y).toBe(0); + expect(layouts[2].y).toBe(300); + expect(layouts[3].y).toBe(50); + expect(layouts[4].y).toBe(400); + expect(layouts[5].y).toBe(100); + + // Viewport from y=40 to y=110 should include items at: + // - Item 1: y=0, h=50 -> ends at 50 > 40 ✓, starts at 0 < 110 ✓ + // - Item 3: y=50, h=50 -> ends at 100 > 40 ✓, starts at 50 < 110 ✓ + // - Item 5: y=100, h=50 -> ends at 150 > 40 ✓, starts at 100 < 110 ✓ + const visible = manager.getVisibleLayouts(40, 110); + // Must include items 1, 3, and 5 (all in viewport) + expect(visible.includes(1)).toBe(true); + expect(visible.includes(3)).toBe(true); + expect(visible.includes(5)).toBe(true); + }); + + it("should handle viewport that only sees one column", () => { + const manager = createLayoutManager(LayoutManagerType.MASONRY, { + ...defaultParams, + optimizeItemArrangement: false, + }); + + // Col 0: item 0 (h=50), item 2 (h=50) -> y: 0, 50 + // Col 1: item 1 (h=300), item 3 (h=50) -> y: 0, 300 + const layoutInfos = [ + createMockLayoutInfo(0, 200, 50), // Col 0, y=0 + createMockLayoutInfo(1, 200, 300), // Col 1, y=0 + createMockLayoutInfo(2, 200, 50), // Col 0, y=50 + createMockLayoutInfo(3, 200, 50), // Col 1, y=300 + ]; + manager.modifyLayout(layoutInfos, 4); + + // Viewport from y=60 to y=120: only item 1 (y=0, h=300) is visible + // Item 0: y=0, h=50, ends at 50 < 60 -> NOT visible + // Item 1: y=0, h=300, ends at 300 > 60, starts at 0 < 120 -> visible + // Item 2: y=50, h=50, ends at 100 > 60, starts at 50 < 120 -> visible + // Item 3: y=300, h=50, starts at 300 > 120 -> NOT visible + const visible = manager.getVisibleLayouts(60, 120); + expect(visible.includes(1)).toBe(true); + expect(visible.includes(2)).toBe(true); + }); + }); + describe("Empty Layout", () => { it("should return zero size for empty layout", () => { const manager = createLayoutManager( diff --git a/src/recyclerview/layout-managers/MasonryLayoutManager.ts b/src/recyclerview/layout-managers/MasonryLayoutManager.ts index 5ba0b5f80..3b47a2150 100644 --- a/src/recyclerview/layout-managers/MasonryLayoutManager.ts +++ b/src/recyclerview/layout-managers/MasonryLayoutManager.ts @@ -5,6 +5,7 @@ import { RVLayoutInfo, RVLayoutManager, } from "./LayoutManager"; +import { ConsecutiveNumbers } from "../helpers/ConsecutiveNumbers"; /** * MasonryLayoutManager implementation that arranges items in a masonry/pinterest-style layout. @@ -321,6 +322,41 @@ export class RVMasonryLayoutManagerImpl extends RVLayoutManager { } } + /** + * Override getVisibleLayouts to avoid using binary search on the full + * layouts array. In masonry layout, item y-positions are NOT sorted by + * index (items are placed in columns), so binary search produces wrong + * results causing items to disappear. Instead, we scan each column + * independently where y-positions ARE sorted, and combine the results. + */ + getVisibleLayouts( + unboundDimensionStart: number, + unboundDimensionEnd: number + ): ConsecutiveNumbers { + let firstVisibleIndex = -1; + let lastVisibleIndex = -1; + const layoutCount = this.layouts.length; + + for (let i = 0; i < layoutCount; i++) { + const layout = this.layouts[i]; + const position = layout.y; + const size = layout.height; + + // Item is visible if it overlaps with the viewport + if (position + size > unboundDimensionStart && position < unboundDimensionEnd) { + if (firstVisibleIndex === -1) { + firstVisibleIndex = i; + } + lastVisibleIndex = i; + } + } + + if (firstVisibleIndex !== -1 && lastVisibleIndex !== -1) { + return new ConsecutiveNumbers(firstVisibleIndex, lastVisibleIndex); + } + return ConsecutiveNumbers.EMPTY; + } + // TODO: For masonry, the "last row" is the last item in each column. // Override isInLastRow if ItemSeparatorComponent support is needed // for masonry layouts. From 20c935ce15b4a831198c1003b9fec12176d2a47c Mon Sep 17 00:00:00 2001 From: "claude[bot]" <41898282+claude[bot]@users.noreply.github.com> Date: Tue, 24 Mar 2026 19:33:10 +0000 Subject: [PATCH 2/2] fix(layout): use per-column binary search for masonry visibility detection Replace O(n) linear scan with per-column binary search for O(numColumns * log(n/numColumns)) performance. Within each masonry column, y-positions are monotonically sorted, so binary search is valid. Column-to-item mappings are built during recomputeLayouts (which already iterates all items) so the visibility hot path stays fast during scrolling. Co-authored-by: Talha Naqvi --- src/__tests__/MasonryLayoutManager.test.ts | 12 +-- .../layout-managers/MasonryLayoutManager.ts | 94 +++++++++++++++---- 2 files changed, 83 insertions(+), 23 deletions(-) diff --git a/src/__tests__/MasonryLayoutManager.test.ts b/src/__tests__/MasonryLayoutManager.test.ts index 6fc04e16a..beb5f48dd 100644 --- a/src/__tests__/MasonryLayoutManager.test.ts +++ b/src/__tests__/MasonryLayoutManager.test.ts @@ -202,11 +202,11 @@ describe("MasonryLayoutManager", () => { // Col 1: items 1 (h=50), 3 (h=50), 5 (h=50) -> y: 0, 50, 100 const layoutInfos = [ createMockLayoutInfo(0, 200, 300), // Col 0, y=0 - createMockLayoutInfo(1, 200, 50), // Col 1, y=0 + createMockLayoutInfo(1, 200, 50), // Col 1, y=0 createMockLayoutInfo(2, 200, 100), // Col 0, y=300 - createMockLayoutInfo(3, 200, 50), // Col 1, y=50 + createMockLayoutInfo(3, 200, 50), // Col 1, y=50 createMockLayoutInfo(4, 200, 100), // Col 0, y=400 - createMockLayoutInfo(5, 200, 50), // Col 1, y=100 + createMockLayoutInfo(5, 200, 50), // Col 1, y=100 ]; manager.modifyLayout(layoutInfos, 6); const layouts = getAllLayouts(manager); @@ -240,10 +240,10 @@ describe("MasonryLayoutManager", () => { // Col 0: item 0 (h=50), item 2 (h=50) -> y: 0, 50 // Col 1: item 1 (h=300), item 3 (h=50) -> y: 0, 300 const layoutInfos = [ - createMockLayoutInfo(0, 200, 50), // Col 0, y=0 + createMockLayoutInfo(0, 200, 50), // Col 0, y=0 createMockLayoutInfo(1, 200, 300), // Col 1, y=0 - createMockLayoutInfo(2, 200, 50), // Col 0, y=50 - createMockLayoutInfo(3, 200, 50), // Col 1, y=300 + createMockLayoutInfo(2, 200, 50), // Col 0, y=50 + createMockLayoutInfo(3, 200, 50), // Col 1, y=300 ]; manager.modifyLayout(layoutInfos, 4); diff --git a/src/recyclerview/layout-managers/MasonryLayoutManager.ts b/src/recyclerview/layout-managers/MasonryLayoutManager.ts index 3b47a2150..5ac8f7a5b 100644 --- a/src/recyclerview/layout-managers/MasonryLayoutManager.ts +++ b/src/recyclerview/layout-managers/MasonryLayoutManager.ts @@ -1,3 +1,5 @@ +import { ConsecutiveNumbers } from "../helpers/ConsecutiveNumbers"; + import { LayoutParams, RVDimension, @@ -5,7 +7,6 @@ import { RVLayoutInfo, RVLayoutManager, } from "./LayoutManager"; -import { ConsecutiveNumbers } from "../helpers/ConsecutiveNumbers"; /** * MasonryLayoutManager implementation that arranges items in a masonry/pinterest-style layout. @@ -19,6 +20,8 @@ export class RVMasonryLayoutManagerImpl extends RVLayoutManager { private columnHeights: number[]; /** Current column index for sequential placement */ private currentColumn = 0; + /** Per-column sorted lists of item indices for efficient visibility detection */ + private columnItems: number[][] = []; /** If there's a span change for masonry layout, we need to recompute all the widths */ private fullRelayoutRequired = false; @@ -156,6 +159,8 @@ export class RVMasonryLayoutManagerImpl extends RVLayoutManager { this.placeItemSequentially(layout, span); } } + // Rebuild per-column item mappings for efficient visibility detection + this.rebuildColumnItems(); } /** @@ -323,31 +328,86 @@ export class RVMasonryLayoutManagerImpl extends RVLayoutManager { } /** - * Override getVisibleLayouts to avoid using binary search on the full - * layouts array. In masonry layout, item y-positions are NOT sorted by - * index (items are placed in columns), so binary search produces wrong - * results causing items to disappear. Instead, we scan each column - * independently where y-positions ARE sorted, and combine the results. + * Rebuilds per-column item index arrays from layout positions. + * Within each column, items are sorted by y-position (guaranteed by + * placement order since column heights only increase). + */ + private rebuildColumnItems(): void { + const columnWidth = this.boundedSize / this.maxColumns; + this.columnItems = Array.from({ length: this.maxColumns }, () => []); + for (let i = 0; i < this.layouts.length; i++) { + const layout = this.layouts[i]; + const col = Math.min( + Math.round(layout.x / columnWidth), + this.maxColumns - 1 + ); + this.columnItems[col].push(i); + } + } + + /** + * Override getVisibleLayouts to use per-column binary search instead of + * a single binary search on the full layouts array. In masonry layout, + * y-positions are NOT sorted by index (items are placed in columns), but + * they ARE sorted within each column. Binary searching each column gives + * O(numColumns * log(n/numColumns)) performance. */ getVisibleLayouts( unboundDimensionStart: number, unboundDimensionEnd: number ): ConsecutiveNumbers { + if (this.layouts.length === 0) { + return ConsecutiveNumbers.EMPTY; + } + let firstVisibleIndex = -1; let lastVisibleIndex = -1; - const layoutCount = this.layouts.length; - - for (let i = 0; i < layoutCount; i++) { - const layout = this.layouts[i]; - const position = layout.y; - const size = layout.height; - // Item is visible if it overlaps with the viewport - if (position + size > unboundDimensionStart && position < unboundDimensionEnd) { - if (firstVisibleIndex === -1) { - firstVisibleIndex = i; + for (const items of this.columnItems) { + if (items.length === 0) continue; + + // Binary search for first visible item in this column: + // find first item where y + height > unboundDimensionStart + let lo = 0; + let hi = items.length - 1; + let colFirst = -1; + while (lo <= hi) { + const mid = (lo + hi) >>> 1; + const layout = this.layouts[items[mid]]; + if (layout.y + layout.height > unboundDimensionStart) { + colFirst = mid; + hi = mid - 1; + } else { + lo = mid + 1; + } + } + if (colFirst === -1) continue; + + // Binary search for last visible item in this column: + // find last item where y < unboundDimensionEnd + lo = colFirst; + hi = items.length - 1; + let colLast = -1; + while (lo <= hi) { + const mid = (lo + hi) >>> 1; + const layout = this.layouts[items[mid]]; + if (layout.y < unboundDimensionEnd) { + colLast = mid; + lo = mid + 1; + } else { + hi = mid - 1; } - lastVisibleIndex = i; + } + if (colLast === -1) continue; + + // Update global min/max with item indices from this column + const colFirstIndex = items[colFirst]; + const colLastIndex = items[colLast]; + if (firstVisibleIndex === -1 || colFirstIndex < firstVisibleIndex) { + firstVisibleIndex = colFirstIndex; + } + if (lastVisibleIndex === -1 || colLastIndex > lastVisibleIndex) { + lastVisibleIndex = colLastIndex; } }