Skip to content

Commit

Permalink
Skip zero delay (#6398)
Browse files Browse the repository at this point in the history
## Summary

Closes
#6341

Probably we want to fix the comparison inside `withDelay` to start the
delayed animation as soon as the conditions are fulfilled, instead of
waiting an additional frame.

```diff
-if (now - startTime > delayMs || animation.reduceMotion) {
+if (now - startTime >= delayMs || animation.reduceMotion) {
```

With this fix the delay of 0ms is only visible if you are really picky:

<table>
<tr>
<td> BEFORE </td>
<td> AFTER </td>
</tr>
<tr>
<td> 


https://github.com/user-attachments/assets/261d92d2-f302-4240-96c8-3d14352431c2

</td>
<td> 


https://github.com/user-attachments/assets/cf833359-7440-41d1-ab0a-daa3050bae15

</td>
</tr>

### Test plan

Includes simplification of tests of withDelay. Tested on both IOS and
Android
  • Loading branch information
Latropos authored Aug 27, 2024
1 parent afbad1e commit b7c542f
Show file tree
Hide file tree
Showing 3 changed files with 37 additions and 70 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ const NonActiveWidthComponents = ({ delays }: { delays: [number] | [number, numb
withDelay( A, withDelay( B, withDelay( C, some_animation) and
withDelay( A + B + C, some_animation) are equal.
*/
describe('Add three delays', () => {
describe('Compare a sequence of three delays, with one delay of their sum', () => {
async function getSnapshotUpdates(delays: [number] | [number, number, number], componentType: ComponentType) {
await mockAnimationTimer();
const updatesContainer = await recordAnimationUpdates();
Expand All @@ -105,89 +105,56 @@ describe('Add three delays', () => {

const updates = updatesContainer.getUpdates(getTestComponent(componentType));
const updatesNative = await updatesContainer.getNativeSnapshots(getTestComponent(componentType));
updatesNative.forEach(update => {
if ('width' in update && update.width === '[Reanimated] Unable to resolve view') {
// We use this hack because component with inline props gets mounted a bit later
update.width = 0;
}
});

await unmockAnimationTimer();
await clearRenderOutput();

return [updates, updatesNative];
}

test.each([
[[10, 20, 30], 'ACTIVE'],
[[10, 0, 30], 'ACTIVE'],
[[0, 30, 30], 'ACTIVE'],
[[40, 30, 0], 'ACTIVE'],

[[10, 20, 30], 'INLINE'],
[[10, 0, 30], 'INLINE'],
[[0, 30, 30], 'INLINE'],
[[40, 30, 0], 'INLINE'],
] as Array<[[number, number, number], keyof typeof ComponentType]>)(
'Sum of delays ${0} is *****one frame longer***** than the delay of the sum, **${1}** component',
async ([delays, componentType]) => {
const [updates, nativeUpdates] = await getSnapshotUpdates(delays, ComponentType[componentType]);
const delaySum = delays.reduce((current, sum) => sum + current, 0);
const [updatesOneDelay, nativeUpdatesOneDelay] = await getSnapshotUpdates(
[delaySum],
[10, 20, 30],
[16, 32, 64],
[10, 0, 30],
[0, 30, 30],
[40, 30, 0],
[0, 0, 30],
[40, 0, 0],
[0, 55, 0],
[0, 0, 0],
] as Array<[number, number, number]>)('Sum of delays **%p** matches the delay of the sum', async delays => {
const delaySum = delays.reduce((current, sum) => sum + current, 0);

for (const componentType of ['PASSIVE', 'ACTIVE', 'INLINE'] as const) {
const [updatesDelaySequence, nativeUpdatesDelaySequence] = await getSnapshotUpdates(
delays,
ComponentType[componentType],
);

expect([{ width: 0 }, ...updatesOneDelay]).toMatchSnapshots(updates);
expect(updatesOneDelay).toMatchNativeSnapshots(nativeUpdatesOneDelay);
expect(updates).toMatchNativeSnapshots(nativeUpdates);
},
);

test.each([
[[0, 0, 30], 'ACTIVE'],
[[40, 0, 0], 'ACTIVE'],
[[0, 55, 0], 'ACTIVE'],
[[0, 0, 0], 'ACTIVE'],

[[0, 0, 30], 'INLINE'],
[[40, 0, 0], 'INLINE'],
[[0, 55, 0], 'INLINE'],
[[0, 0, 0], 'INLINE'],
] as Array<[[number, number, number], keyof typeof ComponentType]>)(
'Sum of delays ${0} is *****two frames longer***** than the delay of the sum, **${1}** component',
async ([delays, componentType]) => {
const [updates, nativeUpdates] = await getSnapshotUpdates(delays, ComponentType[componentType]);
const delaySum = delays.reduce((current, sum) => sum + current, 0);
const [updatesOneDelay, nativeUpdatesOneDelay] = await getSnapshotUpdates(
[delaySum],
ComponentType[componentType],
);

expect([{ width: 0 }, { width: 0 }, ...updatesOneDelay]).toMatchSnapshots(updates);
expect(updatesOneDelay).toMatchNativeSnapshots(nativeUpdatesOneDelay);
expect(updates).toMatchNativeSnapshots(nativeUpdates);
},
);
const fillerSize = updatesDelaySequence.length - updatesOneDelay.length;
const filler = Array.from({ length: fillerSize }, () => {
return {
width: 0,
};
});

test.each([
[[10, 20, 30], 'PASSIVE'],
[[10, 0, 30], 'PASSIVE'],
[[0, 30, 30], 'PASSIVE'],
[[40, 30, 0], 'PASSIVE'],

[[0, 0, 30], 'PASSIVE'],
[[40, 0, 0], 'PASSIVE'],
[[0, 55, 0], 'PASSIVE'],
[[0, 0, 0], 'PASSIVE'],
] as Array<[[number, number, number], keyof typeof ComponentType]>)(
'Sum of delays ${0} is *****exactly matches***** than the delay of the sum, **${1}** component',
async ([delays, componentType]) => {
const [updates, nativeUpdates] = await getSnapshotUpdates(delays, ComponentType[componentType]);
const delaySum = delays.reduce((current, sum) => sum + current, 0);
const [updatesOneDelay, nativeUpdatesOneDelay] = await getSnapshotUpdates(
[delaySum],
ComponentType[componentType],
);
expect([...filler, ...updatesOneDelay]).toMatchSnapshots(updatesDelaySequence);
expect(fillerSize <= 1).toBe(true); // The additional delay should be at most of one frame

expect(updatesOneDelay).toMatchSnapshots(updates);
expect(updatesOneDelay).toMatchNativeSnapshots(nativeUpdatesOneDelay);
expect(updates).toMatchNativeSnapshots(nativeUpdates);
},
);
expect(updatesDelaySequence).toMatchNativeSnapshots(nativeUpdatesDelaySequence);
}
});
});

const styles = StyleSheet.create({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ const testCases = [
{ testAnimation: TestAnimation.TIMING, delay: 0 },
{ testAnimation: TestAnimation.SEQUENCE, delay: 0 },
{ testAnimation: TestAnimation.TIMING, delay: 100 },
{ testAnimation: TestAnimation.TIMING, delay: 1000 },
{ testAnimation: TestAnimation.TIMING, delay: 500 },
{ testAnimation: TestAnimation.SEQUENCE, delay: 100 },
] as const;

Expand All @@ -170,6 +170,7 @@ for (const { testAnimation, delay } of testCases) {
'Components animated _with_ withDelay in two different ways have matching snapshots',
delaySnapshots,
);

// Create snapshot of static animation consisting of frame {width: 100} repeated multiple times
const fillerSize = delaySnapshots.active.length - noDelaySnapshots.active.length;
const filler = Array.from({ length: fillerSize }, () => {
Expand Down
3 changes: 1 addition & 2 deletions packages/react-native-reanimated/src/animation/delay.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,7 @@ export const withDelay = function <T extends AnimationObject>(
function delay(animation: DelayAnimation, now: Timestamp): boolean {
const { startTime, started, previousAnimation } = animation;
const current: AnimatableValue = animation.current;

if (now - startTime > delayMs || animation.reduceMotion) {
if (now - startTime >= delayMs || animation.reduceMotion) {
if (!started) {
nextAnimation.onStart(
nextAnimation,
Expand Down

0 comments on commit b7c542f

Please sign in to comment.