Improve transform matrix animation #4536
Merged
Conversation
This file contains 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
Latropos
force-pushed
the
acynk/matrix_transform
branch
from
ec46098
to
99f5fcd
Compare
|
@Latropos I have same issues with transition the transform value, after back to current screen, the view lost the transform origin, as described here: mateoguzmana/simple-gallery#4 Can you help? |
|
Hi @fukemy, thanks for report, however we don't support this particular thing yet - view loses its transform origin and this is an expected behaviour. If you want to transform your view around a specific anyway, please use this library: https://www.npmjs.com/package/react-native-anchor-point |
tomekzaw
requested changes
Jun 19, 2023
src/reanimated2/animation/transformationMatrix/__tests__/matrixUtils.test.tsx
Outdated
Show resolved
Hide resolved
src/reanimated2/animation/transformationMatrix/__tests__/matrixUtils.test.tsx
Outdated
Show resolved
Hide resolved
<!-- Thanks for submitting a pull request! We appreciate you spending the time to work on these changes. Please follow the template so that the reviewers can easily understand what the code changes affect. --> ## Summary <!-- Explain the motivation for this PR. Include "Fixes #<number>" if applicable. --> Fix sharedElementTransitions link in the documentation ## Test plan <!-- Provide a minimal but complete code snippet that can be used to test out this change along with instructions how to run it and a description of the expected behavior. --> --------- Co-authored-by: Kacper Kapuściak <39658211+kacperkapusciak@users.noreply.github.com>
<!-- Thanks for submitting a pull request! We appreciate you spending the time to work on these changes. Please follow the template so that the reviewers can easily understand what the code changes affect. --> ## Summary Currently our `tsconfig.json` and config for react-native-builder-bob in `package.json` are misconfigured and we are getting type declarations for tests and typetests. PR removes them and restores proper structure. | Before | After | |--------|--------| | <img width="331" alt="Screenshot 2023-05-25 at 08 14 06" src="https://github.com/software-mansion/react-native-reanimated/assets/40713406/31706cda-e844-41f5-87fe-b97d08aec742"> | <img width="321" alt="Screenshot 2023-05-25 at 08 15 17" src="https://github.com/software-mansion/react-native-reanimated/assets/40713406/00782729-84c1-4eea-ae52-0a7b33c66e72"> | `tsconfig.json` doesn't need to be specified in builder-bob configuration as it [takes root tsconfig by default](https://github.com/callstack/react-native-builder-bob#typescript). ## Test plan Unit tests should suffice.
<!-- Thanks for submitting a pull request! We appreciate you spending the time to work on these changes. Please follow the template so that the reviewers can easily understand what the code changes affect. --> ## Summary This PR is a part of ongoing effort to improve Reanimated Babel plugin. - Most of our tests based purely on snapshots, turning them into actual regression tests only. This PR adds an actual check to each test on top of existing snapshot test. On the other hand, some snapshot testing was added to tests that weren't having them but could use them. - Changed output log location from `jest tests fixture` to `/dev/null` - to simplify snapshot update process - we never used that file anyway. - Moved most of string checks to RegExp checks for clarity. ## Test plan No other way than to run the tests and read the tests. --------- Co-authored-by: Tomek Zawadzki <tomasz.zawadzki@swmansion.com>
<!-- Thanks for submitting a pull request! We appreciate you spending the time to work on these changes. Please follow the template so that the reviewers can easily understand what the code changes affect. --> ## Summary Fixes #4485 Before swizzling some methods from react-native-screens, we should check if they exist to avoid crashes. <!-- Explain the motivation for this PR. Include "Fixes #<number>" if applicable. --> ## Test plan <!-- Provide a minimal but complete code snippet that can be used to test out this change along with instructions how to run it and a description of the expected behavior. --> --------- Co-authored-by: Aleksandra Cynk <aleksandracynk@swmansion.com> Co-authored-by: Krzysztof Piaskowy <krzysztof.piaskowy@swmansion.com>
<!-- Thanks for submitting a pull request! We appreciate you spending the time to work on these changes. Please follow the template so that the reviewers can easily understand what the code changes affect. --> ## Summary This PR bumps the following versions: * react-native: 0.72.0-rc.5 → 0.72.0-rc.6 * react-native-screens: 3.20.0 → 3.21.0 * react-native-gesture-handler: 2.11.0 → 2.12.0 ## Test plan <!-- Provide a minimal but complete code snippet that can be used to test out this change along with instructions how to run it and a description of the expected behavior. -->
<!-- Thanks for submitting a pull request! We appreciate you spending the time to work on these changes. Please follow the template so that the reviewers can easily understand what the code changes affect. --> ## Summary Reanimated needs to intercept events while still on the UI thread. To achieve this on iOS, currently we overwrite the instance of `RCTEventDispatcher` with an instance of `REAEventDispatcher` which extends `RCTEventDispatcher` but overrides `sentEvent` method. We need to overwrite event dispatcher as soon as possible, i.e. before native modules are initialized, so that `RNGestureHandlerManager` holds an already overwritten instance (see [here](https://github.com/software-mansion/react-native-gesture-handler/blob/9a3b90df0df3e7f85bb06ae7946aea3be95f717c/ios/RNGestureHandlerManager.mm#L62)). Currently, we call `REAInitializer` in `jsExecutorFactoryForBridge` method. This PR removes some dirty initialization code and uses swizzling instead to achieve the same effect. <img width="1496" alt="screenshot" src="https://github.com/software-mansion/react-native-reanimated/assets/20516055/86ed7bec-b40a-4f2a-a29a-9e72f511e77e"> ## Test plan 1. Build Example app on iOS 2. Check scroll- and gesture-based examples
## Summary
This PR fixes an issue with long animations. On Android, it was possible
to execute ChoreographerCallback twice during the same frame (after
frame drop). In effect, existing animations were canceled due to this
if-condition in `requestAnimationFrame` method:
```js
nativeRequestAnimationFrame((timestamp) => {
if (lastNativeAnimationFrameTimestamp >= timestamp) {
// Make sure we only execute the callbacks once for a given frame
return;
}
...
```
To fix it, I moved this check to the native code. When double execution
happens I skip the execution of the JS callback, but I still need to
schedule another choreographer callback.
## How to test it?
```java
final long[] lastTimestamp = {0};
new GuardedFrameCallback(context) {
@OverRide
protected void doFrameGuarded(long frameTimeNanos) {
if (lastTimestamp[0] == frameTimeNanos) {
Log.w("test");
}
onAnimationFrame(frameTimeNanos);
lastTimestamp[0] = frameTimeNanos;
}
}
```
### Before
- Open SVGExample
- Set a breakpoint in Android Studio at line `Log.w("test");`
- Wait some time until the breakpoint hit
- Pass breakpoint
- Animation will **stop** ❌
### After
- Open SVGExample
- Set a breakpoint in Android Studio at line `Log.w("test");`
- Wait some time until the breakpoint hit
- Pass breakpoint
- Animation will **keep going** ✅
## Summary This PR fixes a bug in v3 which prevents animations from running when Chrome Debugger is attached (or, in general, when web implementation of Reanimated is used). Originally reported by @coayscue. ## Test plan In order to reproduce the issue, you can simply force use web implementation in `PlatformChecker.ts`: ```diff export function isChromeDebugger(): boolean { - return !(global as any).nativeCallSyncHook || (global as any).__REMOTEDEV__; + return true; } ``` When testing gesture-based examples, you will also need to make this change in `gesture.ts`: ```diff export function isRemoteDebuggingEnabled(): boolean { // react-native-reanimated checks if in remote debugging in the same way // @ts-ignore global is available but node types are not included - return !(global as any).nativeCallSyncHook || (global as any).__REMOTEDEV__; + return true; } ```
## Summary In order to upgrade to Android Gradle Plugin (AGP) 8.0, we need to update app build files to accommodate five important build behavior changes. This PR fixes R class reference in `ReanimatedKeyboardEventListener` which will enable us to enable `nonTransitiveRClass` as a followup. This behaviour is enforced in AGP 8.0+. As a side bonus of enabling non-transitive R class, we will get app size decrease on Android, as R classes are quite big. You can read more about it here https://medium.com/androiddevelopers/5-ways-to-prepare-your-app-build-for-android-studio-flamingo-release-da34616bb946 Fixes #4463 ## Test Please test the flows that involve this method call to make sure it doesn't crash and work as intended. Co-authored-by: Krzysztof Piaskowy <krzysztof.piaskowy@swmansion.com>
<!-- Thanks for submitting a pull request! We appreciate you spending the time to work on these changes. Please follow the template so that the reviewers can easily understand what the code changes affect. --> ## Summary ### Problem Android devices use a right-handed coordinate system, where the positive x-axis points to the right, the positive y-axis points up, and the positive z-axis points out of the screen. However iOS uses a left-handed coordinate system, where the positive x-axis points to the right, the positive y-axis points down, and the positive z-axis points into the screen. This is already fixed on android native code, but the problem still exists on android web browser (react native web). ### Fix - Replace quaternion z, y and negate z - Do the same thing on Android native ## Test plan - Web Example App
<!-- Thanks for submitting a pull request! We appreciate you spending the time to work on these changes. Please follow the template so that the reviewers can easily understand what the code changes affect. --> ## Summary This PR splits JSI bindings like `_measure` which have different argument types on Paper and Fabric into two separate names, e.g. `_measurePaper` and `_measureFabric`, so that we can make better use of TypeScript. Additionally, error messages have been improved to provide better developer experience across all configurations. ## Current status <!-- Explain the motivation for this PR. Include "Fixes #<number>" if applicable. --> | Function | Paper UI | Fabric UI | native JS | Web | Chrome Debugger | Jest | fallback | |-|:-:|:-:|:-:|:-:|:-:|:-:|:-:| | `updateProps` | ✅ | ✅ | ❓ | ✅ | ✅ | ✅ | ❓ | | `measure` | ✅ | ✅ |⚠️ | ✅ |⚠️ |⚠️ |⚠️ | | `scrollTo` | ✅ | ✅ | 👻 | ✅ |⚠️ |⚠️ |⚠️ | | `dispatchCommand` |⚠️ | ✅ | 👻 |⚠️ |⚠️ |⚠️ |⚠️ | | `setGestureState` | ✅ | ✅ |⚠️ |⚠️ |⚠️ |⚠️ |⚠️ | | `LayoutAnimations` | n/a | n/a | ✅ |⚠️ |⚠️ |⚠️ |⚠️ | | `useAnimatedKeyboard` | n/a | n/a | ✅ |⚠️ |⚠️ |⚠️ |⚠️ | | `useAnimatedSensor` | n/a | n/a | ✅ | ✅ | ❓ | ❓ | ❓ | Legend: * ✅ officially supported *⚠️ `console.warn` + no-op / return empty value * 👻 no-op (without warning) * ❓ needs to be checked or fixed ## Test plan <!-- Provide a minimal but complete code snippet that can be used to test out this change along with instructions how to run it and a description of the expected behavior. -->
## Summary I've noticed that there are some unnecessary reallocations during returning result from Android's measure function. Without any memory reservation, push_back may cause several reallocations and copy of the data. ## Test plan None --------- Co-authored-by: Michał Mąka <michalmaka@swmansion.com>
<!-- Thanks for submitting a pull request! We appreciate you spending the time to work on these changes. Please follow the template so that the reviewers can easily understand what the code changes affect. --> ## Summary This PR fixes a typo introduced by me in #4576. ## Test plan <!-- Provide a minimal but complete code snippet that can be used to test out this change along with instructions how to run it and a description of the expected behavior. -->
<!-- Thanks for submitting a pull request! We appreciate you spending the time to work on these changes. Please follow the template so that the reviewers can easily understand what the code changes affect. --> ## Summary This PR adds `static __weak REAModule *reaModule;` variable which stores the result of `[[self bridge] moduleForName:@"ReanimatedModule"]` so we can call it only once during initialization instead of on each event. When `REAModule` is deallocated, all `__weak` pointers are automatically replaced with `nil` so it doesn't leak any resources as well as works fine after a reload. ## Test plan <!-- Provide a minimal but complete code snippet that can be used to test out this change along with instructions how to run it and a description of the expected behavior. -->
## Summary The inspiration for this changes are two issues: - #4585 (comment) - 4137d1a Users expect continuous scroll events. But the default value of [scrollEventThrottle](https://reactnative.dev/docs/scrollview#scrolleventthrottle-ios) is 0, which results in the scroll event being sent only once each time the view is scrolled. Which is easy to forget about it and in effect, it seems like a Reanimated issue. I know that this is a change of default ScrollView behavior but I believe that it would be less confusing for the users.
Latropos
force-pushed
the
acynk/matrix_transform
branch
from
d418431
to
2ce350a
Compare
Latropos
force-pushed
the
acynk/matrix_transform
branch
from
2ce350a
to
94d4073
Compare
tomekzaw
reviewed
Jun 26, 2023
## Summary This PR just removes unused parameter `depth` from _startAnimationForTag on iOS.
<!-- Thanks for submitting a pull request! We appreciate you spending the time to work on these changes. Please follow the template so that the reviewers can easily understand what the code changes affect. --> ## Summary This PR renames `nodeManager` to `nodesManager` to match the name of the class. ## Test plan <!-- Provide a minimal but complete code snippet that can be used to test out this change along with instructions how to run it and a description of the expected behavior. -->
Latropos
force-pushed
the
acynk/matrix_transform
branch
from
f1606e8
to
108f761
Compare
Latropos
force-pushed
the
acynk/matrix_transform
branch
from
108f761
to
33c7283
Compare
tomekzaw
reviewed
Jun 28, 2023
Latropos
force-pushed
the
acynk/matrix_transform
branch
from
2224670
to
c685ade
Compare
Latropos
commented
Jul 11, 2023
Latropos
force-pushed
the
acynk/matrix_transform
branch
2 times, most recently
from
355f0b3
to
dfcffda
Compare
Latropos
force-pushed
the
acynk/matrix_transform
branch
from
dfcffda
to
d98677c
Compare
Latropos
force-pushed
the
acynk/matrix_transform
branch
from
2f1d945
to
f3fe4ac
Compare
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
We used to animate transformation matrices the very same way we animate every array - move each value from initial to final position. However this may look weird in case of transformation matrices, since scale and rotation may appear to change randomly - the solution is to decompose actual transformation matrix into:
and then we animate translation, skale and skew the way we used to, but for rotation we calculate all the three angles and get matrix for particular angle. Finally we multiply all the matrices in correct order.
See the recording for comparison. The$\text{\large \color{blue}{\color{lime}u}pper}$ square is animated with improved transform matrix interpolation, the $\text{\large \color{darkorange}{\color{red}l}ower}$ one uses the old technique.
Screen.Recording.2023-06-06.at.13.44.16.mov
Test plan
I've added a new test suite
