Repository navigation
Ignore a transform holding a non-finite number, as the web does - #59229
YevheniiKotyrlo wants to merge 12 commits into
Conversation
Device evidence — before / afterRNTester release builds of Android — Galaxy S23, Android 14
iOS — iPad Pro 12.9-inch (4th generation), iOS 27.0.1
On the iPad, Steelblue pixels per row in the full frames. A square is 11,664 px on the phone and 5,184 on the iPad; a control row loses the corner its marker covers, and — marks a row under the black.
Scene
import * as React from 'react';
import {useEffect, useRef, useState} from 'react';
import {Animated, ScrollView, StyleSheet, Text, View} from 'react-native';
import MyLegacyViewNativeComponent from '../NativeComponentExample/js/MyLegacyViewNativeComponent';
const BUILD = 'unset';
const SUBJECT_COLOR = '#4682B4';
const ROWS = [
{label: 'translateX 40 (control)', style: {transform: [{translateX: 40}]}},
{label: 'translateX NaN', style: {transform: [{translateX: NaN}]}},
{label: 'scale Infinity', style: {transform: [{scale: Infinity}]}},
{
label: 'matrix with NaN',
style: {transform: [{matrix: [NaN, 0, 0, 0, 1, 0, 0, 0, 1]}]},
},
{
label: 'scaleX 2, origin [NaN, 0, 0]',
style: {transform: [{scaleX: 2}], transformOrigin: [NaN, 0, 0]},
},
{label: 'perspective 0', style: {transform: [{perspective: 0}]}},
{label: 'skewX 10deg (control)', style: {transform: [{skewX: '10deg'}]}},
{
label: 'skewX 10deg, translateX NaN',
style: {transform: [{skewX: '10deg'}, {translateX: NaN}]},
},
{
label: 'skewX 10deg, origin [NaN, 0, 0]',
style: {transform: [{skewX: '10deg'}], transformOrigin: [NaN, 0, 0]},
},
{
label: 'translateX 10, scale 1e39',
style: {transform: [{translateX: 10}, {scale: 1e39}]},
},
{
label: 'scale 1e20, scale 1e20',
style: {transform: [{scale: 1e20}, {scale: 1e20}]},
},
];
function Row({label, children}) {
return (
<View style={styles.row}>
<Text style={styles.label}>{label}</Text>
<View style={styles.marker} />
{children}
<View style={styles.marker} />
</View>
);
}
function NativeDriverRow() {
const driven = useRef(new Animated.Value(0)).current;
useEffect(() => {
const loop = Animated.loop(
Animated.timing(driven, {
duration: 120,
toValue: NaN,
useNativeDriver: true,
}),
);
loop.start();
return () => loop.stop();
}, [driven]);
return (
<Row label="native driver translateX NaN, in flight">
<Animated.View
style={[styles.subject, {transform: [{translateX: driven}]}]}
/>
</Row>
);
}
export default function NonFiniteTransformProbe() {
const [applied, setApplied] = useState(false);
useEffect(() => {
setApplied(true);
}, []);
return (
<View style={styles.container}>
<ScrollView contentContainerStyle={styles.content}>
<Text style={styles.build}>{BUILD}</Text>
{ROWS.map(row => (
<Row key={row.label} label={row.label}>
<View style={[styles.subject, applied ? row.style : null]} />
</Row>
))}
<Row label="legacy view translateX 40 (control)">
<MyLegacyViewNativeComponent
color={SUBJECT_COLOR}
style={[styles.legacySubject, {transform: [{translateX: 40}]}]}
/>
</Row>
<Row label="legacy view translateX NaN">
<MyLegacyViewNativeComponent
color={SUBJECT_COLOR}
style={[
styles.legacySubject,
applied ? {transform: [{translateX: NaN}]} : null,
]}
/>
</Row>
<Row label="legacy view scale 3e38, origin top left">
<MyLegacyViewNativeComponent
color={SUBJECT_COLOR}
style={[
styles.legacySubject,
applied
? {transform: [{scale: 3e38}], transformOrigin: 'top left'}
: null,
]}
/>
</Row>
<NativeDriverRow />
</ScrollView>
</View>
);
}
const styles = StyleSheet.create({
container: {flex: 1, backgroundColor: 'white'},
content: {padding: 16, paddingTop: 24, gap: 6},
build: {fontSize: 14, color: 'black'},
row: {flexDirection: 'row', alignItems: 'center', gap: 12, height: 40},
label: {flex: 1, fontSize: 12, color: 'black'},
marker: {width: 16, height: 16, backgroundColor: 'rgb(255, 140, 0)'},
subject: {width: 36, height: 36, backgroundColor: SUBJECT_COLOR},
legacySubject: {width: 36, height: 36},
});On the webThe cases of {
const cases = [
['translateX(10px)', ''],
[`translateX(${NaN}px)`, ''],
[`scale(${Infinity})`, ''],
[`matrix(${NaN}, 0, 0, 1, 0, 0)`, ''],
[`translateX(10px) scaleY(${NaN})`, ''],
[`translateX(${NaN}px)`, 'top left'],
['scaleX(2)', `${NaN}px 0px 0px`],
];
console.table(
cases.map(([transform, transformOrigin]) => {
const view = document.createElement('div');
view.style.cssText = 'position: fixed; left: 0; top: 0; width: 10px; height: 10px';
view.style.transform = transform;
view.style.transformOrigin = transformOrigin;
document.body.append(view);
const {x, y, width, height} = view.getBoundingClientRect();
const computed = getComputedStyle(view).transform;
view.remove();
return {transform, transformOrigin, computed, x, y, width, height};
}),
);
}Chromium 153, Firefox 155 and WebKit 26.6 print the same rows: a transform holding
Cost
Counted with callgrind inside the measured loop alone, as the difference between 110,000 and 10,000 calls; each binary is checked first to resolve or interpolate
One round with this PR scattered on the percentage origin (227.8 to 399.7 ns across its samples); the other two sit at 226.5 and 223.4. From Android 10 a transform with a skew is built by
|
…not finite From Android 10, BaseViewManager hands a transform with a skew to SkewMatrixHelper straight from the props, past the parser's check, so a NaN beside the skew set a NaN animation matrix and hid the view; a non-finite origin turns it about the centre, as elsewhere.
…ne past the largest float is kept Narrowing first turned a finite number past the largest float into infinity, and the check then ignored a transform main draws.
… the matrix is not finite The double check beside a float build let a finite number past the largest float reach the layer as infinity, and no operand check sees a composition that overflows; a perspective of infinity is the one non-finite operand whose matrix comes out finite, so it is checked where it is read.
The default perspective operation stood for none with a depth of 0, so interpolating toward none reached the strongest perspective, and an overshoot drove the depth negative; it now interpolates -1 / depth, with none as an infinite depth: https://drafts.csswg.org/css-transforms-2/#interpolation-of-transform-functions
…s to literal matrices Every non-finite operand but a perspective's makes the matrix non-finite, so the walk over the operands before the parse went over each transform a second time on the native driver's frame path.
toValueUnitIfFinite replaces toValueUnit followed by a separate check at each translate and origin entry, so no call site can read the value before the check, and toFloatIfFinite no longer guards a type its callers already checked.
Single-axis scales, an operand the parser cannot read, a string that parses to infinity, a negative perspective, a translate interpolated beside the perspective rule, the skew path's origin and depth checks, a transform origin RCTConvert keeps, a normal perspective, an angle in radians and a translate with a depth.
…arser as main has it BaseViewProps::resolveTransform, which iOS mounting, Android's prop path through nativeProcessTransform and the C++ readers share, returns the identity when a perspective it reads or the matrix it finishes is not finite, and leaves a non-finite origin out, so the transform turns about the centre. The parser's own checks left a plain view whose transform turned NaN with no transform, so the view stopped forming a native view until the value was finite again, and no operand check saw a composition of finite numbers that overflows. UIView+React's updateTransform checks the matrix it assigns once the origin is composed in, RCTConvert converts a non-finite origin as a missing one, and interpolating a perspective away leaves no perspective operation behind.
…n invalid one as none React Native holds a transform's numbers as floats, so a finite number past the largest float is infinite on Fabric. Animated's native driver and Android's skew path read them as doubles, and iOS Fabric and RCTConvert finish their matrices as doubles, so a value past the largest float, or a composition that overflows a float, was drawn on one path and ignored on another; on the native driver it reached setScaleX and setTranslationX as Float.MAX_VALUE and infinity. Each path now checks at float precision: resolveTransform's matrix, RCTIsFiniteTransform, and the Kotlin result, perspective and origin checks. convertToRadians and RCTConvert read an operand as a float, as the C++ parser holds it. Transform::hasFiniteOperands decides once what a transform holding a non-finite number is: resolveTransform draws it untransformed and Interpolate interpolates it as no transform, so a LayoutAnimation toward perspective: Infinity ends on what the committed props draw instead of snapping to it. resolveTransform returns its matrix instead of a conditional expression's copy of it.
…ths, and keep the operand check private The C++ parser holds an angle with a unit in float degrees and resolves a percentage in floats, and Animated's native driver read a radian angle, a percentage and every other number as a double, so '1e37rad', '1e37%' or numbers past the largest float that cancel out as doubles were drawn there and ignored on Fabric. convertToRadians holds a unit angle in float degrees, resolvePercentage resolves a translate's and an origin's percentage in floats, every number processTransform reads is held as a float, and an origin is checked on the numbers it declares, as resolveTransform checks it. resolveTransformOrigin is the one reader of an origin for processTransform and the skew path, checking each number as it reads it, so a frame parses an origin once, and a percentage that is not a number leaves its axis at the center on both paths, as the skew path read it, where processTransform threw. A legacy view's RCTConvert holds a radian angle in float degrees too, and a matrix's entries as floats, so an entry past the largest float stays infinite through the operations after it. hasFiniteOperands moves to the private TransformUtils.h, so the C++ API snapshots stay as they are, and Interpolate checks each endpoint once. A transform holding a non-finite number is an invalid declaration and interpolates as no transform; a valid one whose matrix overflows still animates toward it, as Chromium and Firefox do, and each frame whose matrix overflows is drawn untransformed. Interpolate's doc says what it does: operation by operation, a perspective as its matrix, stopping at an arbitrary matrix. RCTCATransform3DFromTransformMatrix checks each entry as a float, the predicate resolveTransform uses, in the loop it already has, so the conversion costs what it did. The skew angle past the largest float is tested through BaseViewManager, as its siblings are, so the case compiles against main. A transform string the CSS parser reads is tested through getBoundingClientRect, and the Kotlin path is tested for leaving nothing in its scratch matrix after an ignored transform.
…ess checks as loops Transform.cpp included its own header as "Transform.h" and the operand check's header as <react/renderer/graphics/TransformUtils.h>. With USE_FRAMEWORKS=dynamic the second resolves into React_graphics.framework's copy of the headers, so Transform.h arrived twice, as two files, and test_ios_rntester_dynamic_frameworks failed on redefinitions. Transform.cpp now includes its header as <react/renderer/graphics/Transform.h>, as fromRawValueShared.cpp includes its own. hasFiniteOperands and isFiniteMatrix are range-for loops, as the check in RCTConversions.h is: std::all_of compiled to an out-of-line std::__find_if_not, 46 of the 84 instructions the checks added to a Transform::Interpolate call with one operation.
…d later (#59314) Summary: A view whose `elevation` is `NaN` or infinite ends the app on Android 9 and later, and so do a `DrawerLayoutAndroid`'s `elevation` and the deprecated `scaleX` and `scaleY` props. From API 28, `View.setElevation`, `setScaleX` and `setScaleY` pass their value through `sanitizeFloatPropertyValue` ([`View.java`, line 20100](https://android.googlesource.com/platform/frameworks/base/+/refs/tags/android-16.0.0_r1/core/java/android/view/View.java#20100)), which throws `IllegalArgumentException: Cannot set 'elevation' to Float.NaN` for an app targeting API 28 or later ([line 5903](https://android.googlesource.com/platform/frameworks/base/+/refs/tags/android-16.0.0_r1/core/java/android/view/View.java#5903)). `BaseViewManager.setTransformProperty` already reads every value it hands those setters through its own `sanitizeFloatPropertyValue`, but `BaseViewManager.setElevation`, the deprecated scale setters and `ReactDrawerLayoutManager.setElevation`, which hands `DrawerLayout.setDrawerElevation` its value without `super`, do not. This moves the helper to `com.facebook.react.uimanager.internal.FloatPropertySanitizer`, which the drawer can reach and the public API leaves out, and reads those values through it — the elevations after the dp-to-pixel conversion, so an infinite elevation isn't multiplied back to infinity on a density above 1, and a finite one that overflows in pixels is caught too. A `NaN` takes 0 and ±∞ ±`Float.MAX_VALUE`: the values the framework itself gives an app that does not opt into the throw. A finite value costs the helper's `NaN` test and its two bounds, in setters that run when the prop changes. #59229 makes a transform holding a non-finite value ignored, as the web does. ## Changelog: [ANDROID] [FIXED] - A `NaN` or infinite `elevation`, or deprecated `scaleX` or `scaleY`, no longer crashes the app on Android 9 and later Pull Request resolved: #59314 Test Plan: - `BaseViewManagerTest`: an elevation of `NaN`, `Infinity`, `-Infinity` and `1e39` through the props map, a `NaN` after a finite one, a finite elevation that overflows only in pixels at density 3, and `NaN` and infinite `scaleX` and `scaleY`. `ReactDrawerLayoutManagerTest` (new): a drawer at a `NaN` and an infinite elevation, and at a `Float.MAX_VALUE` elevation at density 3. On `main` the 11 new cases fail on `View`'s own refusal (`Cannot set 'elevation' to Float.NaN`, `Cannot set 'scaleX' to Float.NaN`, …); with this change they pass, and so does the whole ReactAndroid JVM suite (776 tests). - google-java-format and ktfmt pass. - To reproduce, on Android 9 or later: ```jsx <View style={{width: 48, height: 48, backgroundColor: 'white', elevation: NaN}} /> ``` On `main` the app ends with `Cannot set 'elevation' to Float.NaN`, as it does for a `DrawerLayoutAndroid` at `elevation: NaN`, and with `Cannot set 'scaleX' to Float.NaN` for a `View` given `scaleX={NaN}`; with this change each draws. I ran RNTester release builds of `main` and of this change on an Android 14 phone with a root component rendering every case; a screenshot and the scene are in the [evidence comment](#59314 (comment)). Reviewed By: Abbondanzo Differential Revision: D126883976 Pulled By: javache fbshipit-source-id: e87a85ab56a50c5bbb94891888ced851d9a453b9




Summary:
Follow-up to #59133, which keeps a non-finite matrix away from Core Animation on Fabric. Other paths still draw one:
BaseViewProps::resolveTransformhands the matrix on unchecked, and on AndroidnativeProcessTransformgives it toBaseViewManager, which decomposes it and reads aNaNscale as 0: aNaNscale, aNaNmatrix entry or aNaNtranslate with atransformOrigincollapses the view, an infinite translate or scale leaves it undrawn, and so does[{translateX: 10}, {scale: 1e39}], whose scale is infinite once it is a float.getBoundingClientRectandmeasureInWindowread the same matrix and returnNaNon both platforms.RCTConvert CATransform3D:converts the transform andUIView+React'supdateTransformassigns it, with its origin composed in, unchecked.NaNtranslate on Android while the animation runs (TransformHelper's Kotlin path), and from Android 10 a transform with a skew is built straight from the props (SkewMatrixHelper), so aNaNbeside a skew hides the view.On the web,
NaNandInfinityare not CSS numbers (css-values-4 §5.3), so a transform holding one is an invalid declaration that is ignored (CSS 2.1 §4.2), and the element is drawn untransformed; the evidence comment has a snippet that shows it in a browser's console. This draws such a view untransformed on every path, where each path finishes its matrix:BaseViewProps::resolveTransform, which iOS mounting, Android's prop path and the C++ readers share, returns the identity for a transform holding a non-finite operand or one whose matrix is not finite, and turns the transform about the center when its origin is not. The parser is unchanged, so the shadow tree keeps the transform as written and the view's flattening doesn't change while the value isNaN.RCTConvert CATransform3D:returns the identity when the matrix it builds is not finite,updateTransformchecks the matrix once the origin is composed in, and a non-finite origin converts as a missing one.TransformHelper.processTransformand the skew path inBaseViewManagerreturn the identity when the matrix they build is not finite, and turn it about the center when its origin is not.-1 / ±Infinityis a finitem34. A perspective from 0 up to 1px renders as 1px, as css-transforms-2 defines, soperspective: 0no longer produces-1 / 0.React Native holds a transform's numbers as floats — an angle in degrees, a percentage once resolved — so a number past the largest float is infinite on Fabric. Every path now reads and checks them at that precision — iOS's
CGFloatmatrix,RCTConvert, Animated's native driver and the skew path — so such a number, or a composition that overflows a float, draws untransformed everywhere; before, the double paths kept it, and the native driver handedsetTranslationXan infinity. The web converts an out-of-range number to the largest value it supports instead (css-values-4 §5); here that would draw a view scaled by 3.4 × 10³⁸, and on iOS Core Animation already turns the window black for a view scaled by1e10, a valid transform this change leaves asmaindraws it.Transform::Interpolatetreats a transform holding a non-finite number as no transform, so a LayoutAnimation toward one ends on what the committed props draw; a valid transform whose matrix overflows still animates toward it, as Chromium and Firefox do, and each frame whose matrix overflows draws untransformed. It interpolates a perspective as its matrix (§14), so on iOS a LayoutAnimation toward or away from no perspective follows the browsers rather than reaching the strongest perspective on the way.The checks add 95–125 instructions to a
resolveTransformcall (733 → 828 for one operation, 4,479 → 4,604 for four) and 61–197 to aTransform::Interpolatecall under callgrind on Linux, 7–18 ns to aprocessTransformcall and about 40 ns to a skewed transform's update on the JVM, while the iOS conversion costs what it did onmain; the numbers are in the evidence comment.Changelog:
[GENERAL] [FIXED] - Draw a view untransformed when its transform or transform origin holds
NaNorInfinity, as the web doesTest Plan:
ResolveTransformTest(new cases) andTransformTest: a transform holdingNaN,Infinityor-Infinityin any operation's operand, a translate entry or a matrix entry, a string that parses to infinity, a percentage that resolves past the largest float, or a composition that overflows a float resolves to the identity; a finite perspective is kept; a non-finite origin turns the transform about the center; a depth below 1px renders as 1px and a negative one keeps its depth;m34interpolates toward and between perspectives as its matrix does, and a perspective interpolated away keeps the operations beside it; an animation toward or from a transform holding a non-finite number interpolates toward or from no transform, and one toward a composition that overflows draws each frame it can. Built into the Fantom tester's CMake tree on Linux, 12 of the 15 new cases fail onmain— the other three pin what it already does — and all 84 tests in the three files pass with this change.View-itest.js:getBoundingClientRectof a view whose transform holds aNaNtranslate, an infinite scale, aNaNmatrix entry or aNaNafter a finite operation, or aNaNtranslate with atransformOrigin, is its untransformed box, and aNaNtransform origin turns the transform about the center. Onmainthe 6 cases fail in all four flag combinations (Expected 0 but received NaN); with this change they pass, as do the 13 Fantom files that render a transform (621 tests).View-nativeCSSParsing-itest.js: the transform stringscale(3e1000), which the CSS parser reads as infinity, gives the untransformed box; onmainit fails withExpected 0 but received NaN.TransformHelperTest(new),MatrixMathHelperTest,SkewMatrixHelperTestandBaseViewManagerTest: the Kotlin path Animated's native driver takes and the skew path ignore a non-finite operand, one that parses toNaNor infinity ('NaNdeg'), a non-finite origin, and a number past the largest float — a translate, a perspective, an angle in degrees or radians, a skew angle, a percentage, an origin, a product, and numbers that would cancel out as doubles — and render a depth below 1px as 1px; one reader serves both paths' origins, so an origin percentage that is not a number leaves its axis at the center on both, as the skew path read it, where the native driver's path threw, and an ignored transform leaves nothing in the scratch matrix the next one reads. 19 of the 23 new cases fail onmain, the other four pinning what it already does; with this change they pass, and so does the whole ReactAndroid JVM suite (787 tests).RCTConvert_CATransform3DTests(new,RNTesterUnitTests) and two newRCTViewComponentViewTestscases:RCTConvertandupdateTransformignore a non-finite operand or origin, a number past the largest float and a matrix that overflows a float, and render a depth below 1px as 1px; a Fabric view ignores aCGFloatmatrix whose entries overflow a float, and so does the conversion it mounts through. On an iPad (iOS 27.0.1), 8 of the 11RCTConvert_CATransform3DTestsand both newRCTViewComponentViewTestscases fail onmainand pass with this change.RNTesterUnitTestsruns on a device only inside a host app, which I gave it locally as RNTester, asRNTesterIntegrationTestsalready has; the rest of it passes there too, except five tests that fail onmainas well:RCTConvert_NSColorTests testGenerateFallbacks, whose expected fallbacks differ from iOS 27's system colours, and fourRCTBundleURLProviderTeststhat read the device's packager setup. React-RCTFabric's test spec, which no Podfile installs, I ran with a local app host: of its 65 tests onlyRCTTextInputUtilsTest testCopyBackedTextInputPreservesTintColorfails, onmainas well.mainand of this change, with a root component rendering each case between two markers, on an Android phone and on the iPad: onmainthe phone draws only three of the twelve cases, and on the iPadscale 1e20, scale 1e20blacks out every view drawn before it; with this change every case draws untransformed and theNaNorigins turn about the center. Screenshots and per-row pixel counts are in the evidence comment.USE_FRAMEWORKS=dynamic) in Debug and Release.