mirror of
https://github.com/stablyai/orca.git
synced 2026-09-22 00:02:31 +00:00
fix(mobile): reset recycled paragraph layout before reuse (#16692)
* fix(mobile): quantize chat pinch font scale so a zoom stops re-measuring the list every frame
A user bubble on a 390pt iPhone painted five lines inside a frame that
reserved six, with the last painted line cut through a glyph at the content
edge and "no longer needed." gone.
The paint is React Native's: a `<Text>` with no `numberOfLines` gets a text
container whose `lineBreakMode` is `NSLineBreakByClipping`
(RCTTextLayoutManager.mm). Measure lays out into `{width, CGFLOAT_MAX}`, paint
lays out into the mounted content frame — so a frame one line short does not
re-wrap, it dumps the remainder onto the last fitting line and clips it, with
no ellipsis. Reproduced on-device against the real component with the message
text held constant, so five painted lines can only be truncation.
Two conditions are each necessary, and removing either makes it vanish over
~6000 measured bubble renders: a pooled `RCTParagraphComponentView` carrying a
shorter row's content frame (`prepareForRecycle` clears `state` but not
`_textView.layoutMetrics`), and whole-list re-measure churn while rows enter
and leave that pool.
The churn was ours. `renderItem` closes over `fontScale`, and the pinch handler
committed a new scale on every gesture frame, so one zoom drove hundreds of
full-list re-measures. The pinch is composed `Simultaneous` with the list's own
scroll, so a stray second finger during a scroll started that storm at scales
the user cannot see — matching the report, whose glyph metrics are `fontScale`
1.0 exactly.
`quantizeFontScale` snaps commits to a 5% grid. React bails out of a same-value
`setState`, so gesture noise now commits nothing and a full-range pinch commits
at most ~20 times. Under the churn that produced 67 defects in 6636 bubble
renders, the quantized build measured 0 in 6064 — with a forced-defect control
bubble flagged in 100% of frames of both runs to prove the detector was live.
This removes the trigger we own; it does not close the RN recycling window
itself. That needs a one-line reset in `prepareForRecycle`, which cannot land
here without refreshing the `patchedDependencies` hash under `mobile/`.
* fix(mobile): reset recycled paragraph layout before reuse
This commit is contained in:
@@ -46,3 +46,15 @@ index a3f2e01c130c1a53aee0b39e72927d21f2a6973d..c9d34f8b42644800b5c85e5be45b6ebf
|
||||
AttributedString::Range selectionRange;
|
||||
// ScrollView-like metrics
|
||||
Size contentSize;
|
||||
diff --git a/React/Fabric/Mounting/ComponentViews/Text/RCTParagraphComponentView.mm b/React/Fabric/Mounting/ComponentViews/Text/RCTParagraphComponentView.mm
|
||||
index 79ee7ffe43..956f189a1c 100644
|
||||
--- a/React/Fabric/Mounting/ComponentViews/Text/RCTParagraphComponentView.mm
|
||||
+++ b/React/Fabric/Mounting/ComponentViews/Text/RCTParagraphComponentView.mm
|
||||
@@ -136,6 +136,7 @@ using namespace facebook::react;
|
||||
- (void)prepareForRecycle
|
||||
{
|
||||
[super prepareForRecycle];
|
||||
_textView.state = nullptr;
|
||||
+ _textView.layoutMetrics = EmptyLayoutMetrics;
|
||||
_accessibilityProvider = nil;
|
||||
}
|
||||
|
||||
Generated
+1
-1
@@ -9,7 +9,7 @@ overrides:
|
||||
|
||||
patchedDependencies:
|
||||
react-native@0.83.9:
|
||||
hash: a6502e7d63769bf1fd639cbbaab1cc0b8aebef31d22048e8f2186fb399d0c16e
|
||||
hash: 44876634a8efbb0f2c3f66cd4332be170ec821d1cbfc0264ac80680983e8513d
|
||||
path: patches/react-native@0.83.9.patch
|
||||
|
||||
importers:
|
||||
|
||||
@@ -21,3 +21,28 @@ export function clampFontScale(scale: number): number {
|
||||
}
|
||||
return Math.min(FONT_SCALE_MAX, Math.max(FONT_SCALE_MIN, scale))
|
||||
}
|
||||
|
||||
/** Granularity a pinch is allowed to commit at. */
|
||||
export const FONT_SCALE_STEP = 0.05
|
||||
|
||||
/** Snap a proposed font scale to the nearest committed step.
|
||||
*
|
||||
* Why quantize: the chat's `renderItem` closes over `fontScale`, so every
|
||||
* distinct value re-renders and re-measures every message in the list. A raw
|
||||
* pinch proposes a new scale on each gesture frame, so a single zoom drives
|
||||
* hundreds of whole-list re-measures — and because the pinch is composed
|
||||
* `Simultaneous` with the list's own scroll, an incidental second finger during
|
||||
* a scroll starts that storm at scales indistinguishable from 1. Those
|
||||
* re-measures run while rows are mounting and unmounting, which is when a
|
||||
* recycled iOS paragraph view can repaint with the previous row's content
|
||||
* frame and silently drop the tail of a long message. Snapping to steps keeps a
|
||||
* full-range pinch to at most 20 commits and makes a near-neutral pinch commit
|
||||
* nothing at all. */
|
||||
export function quantizeFontScale(scale: number): number {
|
||||
if (Number.isNaN(scale)) {
|
||||
return 1
|
||||
}
|
||||
const stepped = Math.round(clampFontScale(scale) / FONT_SCALE_STEP) * FONT_SCALE_STEP
|
||||
// Re-clamp: rounding can push the outermost step past the bound.
|
||||
return clampFontScale(Number(stepped.toFixed(2)))
|
||||
}
|
||||
|
||||
@@ -0,0 +1,91 @@
|
||||
import { createElement } from 'react'
|
||||
import { act, create } from 'react-test-renderer'
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
import { FONT_SCALE_STEP, quantizeFontScale } from './mobile-native-chat-message-text'
|
||||
|
||||
const handlers: { start?: () => void; update?: (e: { scale: number }) => void } = {}
|
||||
|
||||
vi.mock('react-native-gesture-handler', () => {
|
||||
const pinch = {
|
||||
runOnJS: () => pinch,
|
||||
onStart: (cb: () => void) => {
|
||||
handlers.start = cb
|
||||
return pinch
|
||||
},
|
||||
onUpdate: (cb: (e: { scale: number }) => void) => {
|
||||
handlers.update = cb
|
||||
return pinch
|
||||
}
|
||||
}
|
||||
return {
|
||||
Gesture: { Simultaneous: (...g: unknown[]) => g, Native: () => ({}), Pinch: () => pinch }
|
||||
}
|
||||
})
|
||||
|
||||
import { useMobileNativeChatPinchGesture } from './use-mobile-native-chat-pinch-gesture'
|
||||
|
||||
describe('quantizeFontScale', () => {
|
||||
it('snaps to the commit step and stays inside the supported range', () => {
|
||||
expect(quantizeFontScale(1)).toBe(1)
|
||||
expect(quantizeFontScale(1.01)).toBe(1)
|
||||
expect(quantizeFontScale(1.024)).toBe(1)
|
||||
expect(quantizeFontScale(1.03)).toBe(1.05)
|
||||
expect(quantizeFontScale(5)).toBe(1.8)
|
||||
expect(quantizeFontScale(0.1)).toBe(0.8)
|
||||
expect(quantizeFontScale(Number.NaN)).toBe(1)
|
||||
})
|
||||
|
||||
it('collapses a full-range pinch to at most one commit per step', () => {
|
||||
const committed = new Set<number>()
|
||||
for (let i = 0; i <= 600; i++) {
|
||||
committed.add(quantizeFontScale(0.7 + i * 0.002))
|
||||
}
|
||||
expect(committed.size).toBeLessThanOrEqual(Math.round(1 / FONT_SCALE_STEP) + 1)
|
||||
})
|
||||
})
|
||||
|
||||
describe('useMobileNativeChatPinchGesture', () => {
|
||||
let renderer: { unmount: () => void } | null = null
|
||||
let latest = 1
|
||||
|
||||
function Probe(): null {
|
||||
latest = useMobileNativeChatPinchGesture().fontScale
|
||||
return null
|
||||
}
|
||||
|
||||
beforeEach(() => {
|
||||
latest = 1
|
||||
act(() => {
|
||||
renderer = create(createElement(Probe))
|
||||
})
|
||||
})
|
||||
|
||||
afterEach(() => {
|
||||
act(() => renderer?.unmount())
|
||||
renderer = null
|
||||
})
|
||||
|
||||
// Why this matters: `renderItem` closes over `fontScale`, so each committed
|
||||
// value re-measures every message in the list. Committing raw gesture scales
|
||||
// drives hundreds of whole-list re-measures per pinch, and a stray second
|
||||
// finger during an ordinary scroll starts that storm at scales the user cannot
|
||||
// even see. Those re-measures are what expose the recycled-paragraph repaint
|
||||
// that silently drops the tail of a long message.
|
||||
it('commits nothing for gesture noise the user cannot see', () => {
|
||||
act(() => handlers.start?.())
|
||||
for (const scale of [1.001, 0.995, 1.008, 1.02, 0.982]) {
|
||||
act(() => handlers.update?.({ scale }))
|
||||
}
|
||||
expect(latest).toBe(1)
|
||||
})
|
||||
|
||||
it('commits a real pinch, on the step grid', () => {
|
||||
act(() => handlers.start?.())
|
||||
act(() => handlers.update?.({ scale: 1.34 }))
|
||||
expect(latest).toBe(1.35)
|
||||
act(() => handlers.update?.({ scale: 1.37 }))
|
||||
expect(latest).toBe(1.35)
|
||||
act(() => handlers.update?.({ scale: 1.42 }))
|
||||
expect(latest).toBe(1.4)
|
||||
})
|
||||
})
|
||||
@@ -1,7 +1,7 @@
|
||||
import { useMemo, useRef, useState } from 'react'
|
||||
import { Gesture } from 'react-native-gesture-handler'
|
||||
import type { ComposedGesture } from 'react-native-gesture-handler'
|
||||
import { clampFontScale } from './mobile-native-chat-message-text'
|
||||
import { quantizeFontScale } from './mobile-native-chat-message-text'
|
||||
|
||||
/** Pinch-to-zoom chat font. `fontScale` is the committed size; `pinchBase`
|
||||
* anchors the live gesture so successive pinches compound rather than reset. */
|
||||
@@ -14,8 +14,8 @@ export function useMobileNativeChatPinchGesture(): {
|
||||
fontScaleRef.current = fontScale
|
||||
const pinchBase = useRef(1)
|
||||
// Why: run the gesture callbacks on the JS thread (not a reanimated worklet) so
|
||||
// they can touch React refs/state and clampFontScale directly — accessing those
|
||||
// from the UI-thread worklet crashes the app.
|
||||
// they can touch React refs/state and quantizeFontScale directly — accessing
|
||||
// those from the UI-thread worklet crashes the app.
|
||||
// Compose the pinch with the list's native scroll as Simultaneous so a
|
||||
// two-finger pinch is recognized even while the scroll view is active —
|
||||
// otherwise the scroll grabs the gesture first and the pinch never fires.
|
||||
@@ -29,7 +29,7 @@ export function useMobileNativeChatPinchGesture(): {
|
||||
pinchBase.current = fontScaleRef.current
|
||||
})
|
||||
.onUpdate((e) => {
|
||||
setFontScale(clampFontScale(pinchBase.current * e.scale))
|
||||
setFontScale(quantizeFontScale(pinchBase.current * e.scale))
|
||||
})
|
||||
),
|
||||
[]
|
||||
|
||||
Reference in New Issue
Block a user