Fix skewX/skewY transforms on Android Q+ (#27649)#56724
Open
qflen wants to merge 2 commits intofacebook:mainfrom
Open
Fix skewX/skewY transforms on Android Q+ (#27649)#56724qflen wants to merge 2 commits intofacebook:mainfrom
qflen wants to merge 2 commits intofacebook:mainfrom
Conversation
64158aa to
44364c8
Compare
|
Thank you @qflen! |
`BaseViewManager.setTransformProperty` decomposes the transform array through `MatrixMathHelper.decomposeMatrix`, but only consumes the `translation`, `rotationDegrees`, `scale`, and `perspective` fields when applying to the View. The `skew[]` field, computed correctly by the math layer, is dropped; `View` exposes no `setSkew` so there has been no application path. Views with `skewX` / `skewY` end up rendered as rotated-and-scaled rectangles instead of true parallelograms (facebook#27649). Add a guarded dispatch immediately after the `transforms == null` reset: when the array contains `skewX` / `skewY` and is otherwise 2D-affine, build a Skia `Matrix` directly from the operations and apply it via `View.setAnimationMatrix` on Android Q+. All other transform shapes (`rotateX`, `rotateY`, `perspective`, raw 4x4 `matrix`, `translateZ`) continue to flow through the existing decompose path unchanged. A new top-level Kotlin helper `SkewMatrixHelper` exposes three `@JvmStatic` functions: `hasSkewTransform`, `isAffine2DTransform`, and `buildAffine2DMatrix`. The new `R.id.skew_animation_matrix` view tag records that an animation matrix is currently applied so the cleanup path doesn't fire `setAnimationMatrix(null)` on every animation frame of every non-skew View. The new RNTester example `Skew (facebook#27649)` under Transforms exercises six skew shapes plus a useNativeDriver animated skewX, mirrors the matching iOS scene, and is keyed by `transform-skew-27649` for deep-linking via `rntester://example/TransformExample/skew-27649`. Test plan: - ./gradlew :packages:react-native:ReactAndroid:testDebugUnitTest --tests 'com.facebook.react.uimanager.SkewMatrixHelperTest*' (17/17) - yarn jest packages/react-native/Libraries/StyleSheet/__tests__/processTransform-test.js (16 tests + 19 snapshots) - yarn flow, yarn lint, ./gradlew ktfmtCheck (all clean) Closes facebook#27649.
44364c8 to
9b90807
Compare
`View.setAnimationMatrix(Matrix)` composes for drawing but `View.getMatrix()` does not include `mAnimationMatrix` in its return value. RN's hit test in `TouchTargetHelper.getChildPoint` inverse-maps touch coordinates through `child.matrix` and falls back to the original rectangular bounds. Net result: after the rendering fix, skewed Views render as parallelograms but tip taps that fall outside the rectangular bounds miss, and rect-corner taps that are visually empty post-skew still register. iOS gets parity for free because UIKit hit-testing inverse-maps through the layer's `CATransform3D`. Store the affine `Matrix` on `R.id.skew_animation_matrix` (instead of `Boolean.TRUE`) and have `TouchTargetHelper.getChildPoint` consult the tag, falling back to `child.matrix` when absent. Net effect: hit testing follows the rendered parallelogram on both platforms. Verified empirically by sweeping tap coordinates 1 px on either side of every parallelogram edge: - (100, 555): inside parallelogram top-left tip / outside rect bounds. Before this commit: missed. After: registers as `skewX 20deg`. - (330, 731): inside parallelogram bottom-right tip / outside rect. Same flip. - (208, 640): rect center / parallelogram center. Registers in both states. - (350, 640): outside parallelogram at vertical-pivot y. Misses in both states (correct).
9b90807 to
bea6baf
Compare
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.
Summary
Fixes #27649.
On Android,
skewX/skewYtransforms are silently dropped during view-prop application: the matrix math layer correctly extracts the shear intoMatrixDecompositionContext.skew[], butBaseViewManager.setTransformPropertyreads onlytranslation,rotationDegrees,scale, andperspectivefrom the decomposition context and never consumes theskew[]field. Views withskew*end up rendered as rotated-and-scaled rectangles instead of true parallelograms.This PR adds a single dispatch in
BaseViewManager.setTransformProperty: when the transform array containsskewX/skewYand is otherwise 2D-affine, build aMatrixdirectly from the operations and apply it viaView.setAnimationMatrixon Android Q+. All other transform shapes (rotateX,rotateY,perspective, raw 4x4matrix,translateZ) continue to flow through the existing decompose-and-set-View-props code unchanged.Root cause
packages/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/BaseViewManager.java(the pre-fix range~573-635) iterates the decomposedtranslation,rotationDegrees,scale, andperspectivefields onto the View. Theskew[]field onMatrixDecompositionContext, computed correctly byMatrixMathHelper.decomposeMatrix, is never read. AndroidViewexposes property setters for translation, rotation around pivot, scale, and camera distance, but nosetSkewX/setSkewY, so there has historically been no application path for the residual shear.@quantizor's trace in #27649 (comment) identified the exact site.
Fix
A new top-level Kotlin helper
SkewMatrixHelper(inpackages/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/SkewMatrixHelper.kt) exposes three@JvmStaticfunctions:hasSkewTransform(transforms), linear scan, returns true iff any element key isskewXorskewY.isAffine2DTransform(transforms), linear scan, returns false onmatrix,perspective,rotateX,rotateY, ortranslatewith a non-zero Z component.buildAffine2DMatrix(transforms, viewWidthDip, viewHeightDip, transformOrigin), walks the array left-to-right and applies each operation to aMatrixviapreRotate/preScale/preSkew/preTranslatearound the resolved pivot. Composition is pre-multiplication so the rightmost array entry is applied first to the point, matching CSS / iOS conventions andMatrixMathHelper.multiplyIntoinTransformHelper.processTransform.BaseViewManager.setTransformPropertyadds a guarded dispatch immediately after thetransforms == nullreset block:The
R.id.skew_animation_matrixtag (declared inids.xml) holds the affineMatrixitself. A smallclearSkewAnimationMatrixIfActive(view)helper is invoked from thetransforms == nullbranch and from the existing decompose-path tail, gating theview.setAnimationMatrix(null)call on the tag. Without this gate, every animated rotate / scale / translate frame on every View would firesetAnimationMatrix(null), which unconditionally invalidates the RenderNode and would be a per-frame regression for non-skew animations.View.getMatrix()does not composemAnimationMatrixinto its return value, so the React-side hit-test traversal inTouchTargetHelper.ktwould otherwise still see the original rectangular bounds. To close that gap, theR.id.skew_animation_matrixview tag stores the affineMatrixitself (rather thanBoolean.TRUE), andTouchTargetHelper.getChildPointchecks the tag and uses it as the inverse-mapping matrix when present. Net effect: hit testing follows the rendered parallelogram on both platforms, matching iOS /CATransform3Dbehavior. Invalidation, layer caching, and accessibility-bounds reporting come for free from the existingsetAnimationMatrixplumbing.Pre-Q
Gated to API 29+ to mirror the existing
view.setAnimationMatrix(null)cleanup atBaseViewManager.java:118. On API 24-28 (small and declining install share in 2026), skew continues to be silently dropped, matching today's behavior. AndroidX Transitions usessetAnimationMatrixvia reflection on pre-Q; if that ever becomes a priority for skew on pre-Q, the same shim could be added here without touching the dispatch shape.Why not the prior attempts
Fix skewX on Android and in the JS decomposition #28862 (
wcandillon, May 2020), fixed a JS-side decomposition bug whereskew[0]was zeroed by a duplicate orthogonalization. That fix landed via Phabricator (commit797367c0890a38ec51cfaf7bd90b9cc7db0e97c7) and is preserved in currentmain. It correcteddecomposeMatrixbut did not address the application layer; this PR is the missing application path.feat: Use animation matrix For Skew prop #38494 (
xxrlzzz, Jul 2023, closed Apr 2024), closest in spirit to this fix. It built a SkiaMatrixand applied viasetAnimationMatrixfor a broader class of 2D transforms, plus a reflection shim for pre-Q. @javache's review raised two concerns: (1) the SDK doc framing ofsetAnimationMatrixas an animation API, and (2) "two divergent code paths" complexity. This PR addresses both: (1) AndroidXandroidx.transition.ViewUtilsApi21invokes the same API for static transforms in production today, and the API was promoted to public in API 29; the precedent is established. (2) The dispatch is tightened tohasSkew && isAffine2D, so the new path runs only for transforms that are broken under the existing path, the rotate / scale / translate / rotateX / rotateY / perspective code stays bit-identical. The reflection shim is intentionally not adopted; pre-Q skew remains dropped.Simulate skew on Android #42676 (
piaskowyk+bartlomiejbloniarz, Jan 2024, closed Aug 2025), simulated skew via 3D rotation + non-uniform scale + perspective hack. The 2x2 sub-matrix matches a true skew, but the 4x4 differs in the third row/column, so composition with realrotateX/rotateYproduces wrong results. The PR author acknowledged this in the description. Not the right shape.Changelog:
[ANDROID] [FIXED] - skewX / skewY transforms now render correctly on Android Q+.
Test Plan
Unit tests
./gradlew :packages:react-native:ReactAndroid:testDebugUnitTest --tests 'com.facebook.react.uimanager.SkewMatrixHelperTest*', 17 / 17 pass (predicate cases forhasSkewTransformandisAffine2DTransform; matrix-math cases forbuildAffine2DMatrixcovering pure skewX, scale-then-translate ordering, view-center pivot default, andtransformOriginoverrides via Number values and "%" strings).yarn jest packages/react-native/Libraries/StyleSheet/__tests__/processTransform-test.js, 16 / 16 + 19 / 19 snapshots, unchanged../gradlew ktfmtCheck,yarn lint,yarn flow, all clean.Manual verification (Android, rn_test AVD: Pixel 8 Pro, Android 16, arm64-v8a)
RNTester -> APIs -> Transforms -> "Skew (#27649)" (the new permanent example added in this PR).
Hit testing follows the rendered parallelogram. Without commit 2, the rendering fix would land alone: parallelograms render but
TouchTargetHelper.getChildPointwould still inverse-map throughchild.matrix(which doesn't composemAnimationMatrix) and clip to the original rectangle. Verified empirically by sweeping tap coordinates 1 px on either side of every parallelogram edge on the skewX box (rect bounds[116, 549] [300, 732]on the rn_test AVD):(100, 555): inside parallelogram top-left tip, outside the rect. With only commit 1: misses. With commit 2 added: registers asskewX 20deg.(330, 731): bottom-right tip. Same flip.(208, 640): parallelogram / rect center. Registers either way.(350, 640): outside the parallelogram at vertical-pivot y. Misses either way (correct).The new
Skew (#27649)example also includes auseNativeDriver: trueAnimated.timinginterpolating skewX from0degto20deg. Native-driven animations re-emit the transform array per frame viaTransformAnimatedNode.collectViewUpdates -> setTransformProperty, so the dispatch runs each frame and the skew animates smoothly.iOS
iOS already handles skewX / skewY correctly via
CATransform3D(Paper:RCTConvert+Transform.msetsnext.m21 = tanf(skew)for skewX andnext.m12 = tanf(skew)for skewY; Fabric:RCTViewComponentView.mmcallsresolveTransform->RCTCATransform3DFromTransformMatrix->layer.transform). This PR does not touch that path; the AFTER Android rendering above matches the existing iOS rendering.Negative case
Existing transform examples (Translate-Rotate-Scale, Perspective-Rotate-Animation, Rotate-Scale, Transform-using-a-string, Transform-Matrix-2D / 3D) render bit-identically to
origin/main. ThehasSkewTransformpredicate filters them out of the new path, so they go through the unchanged decompose-and-set-View-props code. The newsetAnimationMatrix(null)clearing call on the fallthrough path is gated by theR.id.skew_animation_matrixview tag, so it fires only on the skew -> non-skew transition; non-skew animations have no per-frame regression.