Repository navigation
Conversation
saveAndSetupCanvas built mCTM as mMatrix x ctm, so under nested transforms (or a transform under a scaled viewBox) the parent's matrix was applied before the element's own. Drawing was unaffected because the canvas is concatenated separately, but everything that reads mCTM was wrong: non-scaling-stroke paths, getScreenCTM()/getCTM() and the element client rects. Use postConcat, and have getCTM() undo the viewBox on the left to match. Fixes software-mansion#3090 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Fails on Android before the CTM order fix (7448 differing pixels) and passes after it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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 #3090.
On Android,
VirtualView.saveAndSetupCanvasbuilt an element's CTM asmMatrix × ctm(mCTM.preConcat(ctm)). With Android'sMatrix, that applies the parent's CTM to a point first and the element's own transform second, so under nested transforms, or a transform under a scaledviewBox, the order is reversed.Drawing was unaffected, because the canvas is concatenated separately, and so were touches, because hit-testing walks each element's inverse matrix. Everything that reads
mCTMwas wrong:vectorEffect="non-scaling-stroke"paths, which draw withmCTMaftercanvas.setMatrix(null)getScreenCTM()andgetCTM()onLayoutand accessibility)iOS reads the CTM from the drawing context (
CGContextGetCTM), so it was already correct.Changes
VirtualView.saveAndSetupCanvas:mCTM.preConcat(ctm)→mCTM.postConcat(ctm), i.e.ctm × mMatrix.RNSVGRenderableManager.getCTM:ctm.preConcat(invViewBoxMatrix)→ctm.postConcat(invViewBoxMatrix). The old order compensated for the reversed CTM and was only right for a single level of transform. With the CTM fixed it has to undo the viewBox from the left.The
VirtualViewline is the same change that was part of #2802 and was reverted with it in #2832, which was about #2802's performance cost (#2831). This PR contains only the matrix order, plus thegetCTMchange #2802 didn't include.Behaviour change
getScreenCTM()now returns the correct matrix for any element with a transform under a scaledviewBox, and bothgetScreenCTM()andgetCTM()are correct for nested transforms. Code that worked around the old values on Android will see different numbers.getCTM()for a single transform level is unchanged.Test Plan
What's required for testing (prerequisites)?
Android emulator or device. I used a Pixel 9 emulator (API 36, pixel ratio 2.625, which the Android E2E references use) with
apps/fabric-example.What are the steps to reproduce (after prerequisites)?
1. Test example
Test3090apps/common/index.tsximport./test, and inapps/common/test/index.tsxrender<Test3090 />.apps/fabric-exampleon Android.non-scaling-stroketriangle should lie on the red reference, and the printed matrices should match the expected values.mainnon-scaling-strokepathgetScreenCTM()a=1.5 d=1.5 e=25 f=10a=1.5 d=1.5 e=90.06 f=45.03getCTM()a=0.5 d=0.5 e=25 f=10a=0.5 d=0.5 e=30 f=15(The
.06/.03come from the 300 dp<Svg>being 787.5 px wide at this density, which rounds to 788. Theaanddvalues are off by the same factor onmaintoo.)2. E2E case
e2e/cases/5.svgnon-scaling-strokeoutline on a square, under two nested<g transform>and aviewBoxscaled ×2. References generated withyarn generateE2eReferences. I committed only the new5.pngfiles, not the regenerated existing ones.main:5.svgfails, 7448 differing pixels (limit 525). The other cases are unchanged.3. Touch events still work
Example app → TouchEvents, with this PR:
<G scale="1.4">under a scaledviewBox,<Circle scale="1.2">inside it): pressing the green circle and the yellow rect gives "Pressed on G", pressing "H" gives "Pressed on Text", and pressing empty space gives nothing.4. Checks
yarn lintandyarn tscpass (after buildinglib/).Compatibility
Not run locally: iOS E2E (the new
ios/5.pngreference is generated at pixel ratio 3 like the others) and the old architecture (Paper). The change is in Java code shared by both architectures.Checklist
README.md(not needed)__tests__folder (addedapps/common/test/Test3090.tsxand E2E case5.svginstead)