Fix matrix multiplication logic for transforms#10112
Conversation
As reported in microsoft#10111, float4x4's operator* does not produce the same output as expected on other RN platforms. This change ports the logic from RN's MatrixMath.js to ensure multiplication of transforms produces the same result on Windows as on iOS and Android. Fixes microsoft#10111
rozele
added
Area: Animation
Area: View Managers
Area: View Style Props
acoates-ms
added
the
AutoMerge
|
Hello @acoates-ms! Because this pull request has the Do note that I've been instructed to only help merge pull requests of this repository that have been opened for at least 10 hours, a condition that will be fulfilled in about 9 hours 2 minutes. No worries though, I will be back when the time is right! 😉 p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (
|
acoates-ms
removed
the
AutoMerge
asklar
left a comment
asklar
left a comment
There was a problem hiding this comment.
marking requesting changes so we can understand where the difference is coming from
ghost
added
Needs: Author Feedback
Thanks @asklar. It was indeed an issue with ordering, iOS and Android multiply in the reverse order. Fixed, but left the Happy to not use the helper, reverse the order of arguments for I also found it quite strange that we would need to reimplement matrix multiply. |
asklar
changed the title
|
@rozele , any chance you can port this to the 0.69 branch too? |
ghost
merged commit * Uses custom matrix multiplication logic for transforms As reported in microsoft#10111, float4x4's operator* does not produce the same output as expected on other RN platforms. This change ports the logic from RN's MatrixMath.js to ensure multiplication of transforms produces the same result on Windows as on iOS and Android. Fixes microsoft#10111 * yarn format * Change files * Use float4x4 operator* but get the order correct
|
@acoates-ms done - #10121 |
* Uses custom matrix multiplication logic for transforms As reported in #10111, float4x4's operator* does not produce the same output as expected on other RN platforms. This change ports the logic from RN's MatrixMath.js to ensure multiplication of transforms produces the same result on Windows as on iOS and Android. Fixes #10111 * yarn format * Change files * Use float4x4 operator* but get the order correct
* Uses custom matrix multiplication logic for transforms As reported in microsoft#10111, float4x4's operator* does not produce the same output as expected on other RN platforms. This change ports the logic from RN's MatrixMath.js to ensure multiplication of transforms produces the same result on Windows as on iOS and Android. Fixes microsoft#10111 * yarn format * Change files * Use float4x4 operator* but get the order correct
3 participants
Fixes #10111
Description
Type of Change
Erase all that don't apply.
Why
As reported in #10111, float4x4's operator* does not produce the same output as expected on other RN platforms.
Resolves #10111
What
This change ports the logic from RN's MatrixMath.js to ensure multiplication of transforms produces the same result on Windows as on iOS and Android.
Screenshots
One more for good measure - notice that the JS driven animation just sends transform props (rather than computed matrix) now that we forked
processTransform.windows.js, so the animation doesn't match expectations from the native driver / iOS:Before:
React.Native.Playground.Win32.2022-06-14.13-30-28.mp4
After:
playground.2022-06-14.13-30-13.mp4
iOS:
Screen.Recording.2022-06-14.at.1.35.15.PM.mov
Testing
See screenshots / recordings above, confirmed that transforms compose as expected on iOS.
Microsoft Reviewers: Open in CodeFlow