Skip to content

Commit 2f8b42e

Browse files
authored
Merge pull request #2961 from remotion-dev/fix-float-sequences
2 parents d2e1bf0 + ff97bf8 commit 2f8b42e

2 files changed

Lines changed: 70 additions & 120 deletions

File tree

packages/core/src/Sequence.tsx

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -171,7 +171,9 @@ const SequenceRefForwardingFunction: React.ForwardRefRenderFunction<
171171
loopDisplay,
172172
]);
173173

174-
const endThreshold = cumulatedFrom + from + durationInFrames - 1;
174+
// Ceil to support floats
175+
// https://github.com/remotion-dev/remotion/issues/2958
176+
const endThreshold = Math.ceil(cumulatedFrom + from + durationInFrames - 1);
175177
const content =
176178
absoluteFrame < cumulatedFrom + from
177179
? null

packages/core/src/test/nested-sequences.test.tsx

Lines changed: 67 additions & 119 deletions
Original file line numberDiff line numberDiff line change
@@ -2,14 +2,16 @@
22
* @vitest-environment jsdom
33
*/
44
/* eslint-disable react/jsx-no-constructed-context-values */
5-
import {render} from '@testing-library/react';
6-
import {expect, test} from 'vitest';
7-
import {CanUseRemotionHooksProvider} from '../CanUseRemotionHooks.js';
5+
import {cleanup, render} from '@testing-library/react';
6+
import {afterEach, expect, test} from 'vitest';
7+
import {AbsoluteFill} from '../AbsoluteFill.js';
88
import {Sequence} from '../Sequence.js';
99
import {TimelineContext} from '../timeline-position-state.js';
1010
import {useCurrentFrame} from '../use-current-frame.js';
1111
import {WrapSequenceContext} from './wrap-sequence-context.js';
1212

13+
afterEach(cleanup);
14+
1315
test('It should calculate the correct offset in nested sequences', () => {
1416
const NestedChild = () => {
1517
const frame = useCurrentFrame();
@@ -32,34 +34,14 @@ test('It should calculate the correct offset in nested sequences', () => {
3234
);
3335
};
3436

35-
const {queryByText} = render(
36-
<WrapSequenceContext>
37-
<TimelineContext.Provider
38-
value={{
39-
rootId: 'hi',
40-
frame: {
41-
'my-comp': 40,
42-
},
43-
playing: false,
44-
imperativePlaying: {
45-
current: false,
46-
},
47-
playbackRate: 1,
48-
setPlaybackRate: () => {
49-
throw new Error('playback rate');
50-
},
51-
audioAndVideoTags: {
52-
current: [],
53-
},
54-
}}
55-
>
56-
<Sequence from={20} durationInFrames={100}>
57-
<Child />
58-
</Sequence>
59-
</TimelineContext.Provider>
60-
</WrapSequenceContext>,
37+
const content = (
38+
<Sequence from={20} durationInFrames={100}>
39+
<Child />
40+
</Sequence>
6141
);
62-
expect(queryByText(/^frame9$/i)).not.toBe(null);
42+
const result = getForFrame(40, content);
43+
44+
expect(result(/^frame9$/i)).not.toBe(null);
6345
});
6446

6547
test('Negative offset test', () => {
@@ -68,12 +50,27 @@ test('Negative offset test', () => {
6850
return <div>{'frame' + frame}</div>;
6951
};
7052

53+
const content = (
54+
<Sequence from={-200} durationInFrames={300}>
55+
<Sequence from={10} durationInFrames={300}>
56+
<Sequence from={10} durationInFrames={300}>
57+
<NestedChild />
58+
</Sequence>
59+
</Sequence>
60+
</Sequence>
61+
);
62+
63+
const result = getForFrame(40, content);
64+
expect(result(/^frame220/i)).not.toBe(null);
65+
});
66+
67+
const getForFrame = (frame: number, content: React.ReactNode) => {
7168
const {queryByText} = render(
7269
<WrapSequenceContext>
7370
<TimelineContext.Provider
7471
value={{
7572
frame: {
76-
'my-comp': 40,
73+
'my-comp': frame,
7774
},
7875
playing: false,
7976
rootId: 'hi',
@@ -89,19 +86,12 @@ test('Negative offset test', () => {
8986
},
9087
}}
9188
>
92-
<Sequence from={-200} durationInFrames={300}>
93-
<Sequence from={10} durationInFrames={300}>
94-
<Sequence from={10} durationInFrames={300}>
95-
<NestedChild />
96-
</Sequence>
97-
</Sequence>
98-
</Sequence>
89+
{content}
9990
</TimelineContext.Provider>
10091
</WrapSequenceContext>,
10192
);
102-
const result = queryByText(/^frame220/i);
103-
expect(result).not.toBe(null);
104-
});
93+
return queryByText;
94+
};
10595

10696
test('Nested negative offset test', () => {
10797
const NestedChild = () => {
@@ -113,51 +103,20 @@ test('Nested negative offset test', () => {
113103
const endAt = 90;
114104

115105
const content = (
116-
<WrapSequenceContext>
117-
<Sequence from={0 - startFrom} durationInFrames={endAt}>
118-
<NestedChild />
119-
</Sequence>
120-
</WrapSequenceContext>
106+
<Sequence from={0 - startFrom} durationInFrames={endAt}>
107+
<NestedChild />
108+
</Sequence>
121109
);
122110

123-
const getForFrame = (frame: number) => {
124-
const {queryByText} = render(
125-
<WrapSequenceContext>
126-
<TimelineContext.Provider
127-
value={{
128-
frame: {
129-
'my-comp': frame,
130-
},
131-
playing: false,
132-
rootId: 'hi',
133-
imperativePlaying: {
134-
current: false,
135-
},
136-
playbackRate: 1,
137-
setPlaybackRate: () => {
138-
throw new Error('playback rate');
139-
},
140-
audioAndVideoTags: {
141-
current: [],
142-
},
143-
}}
144-
>
145-
{content}
146-
</TimelineContext.Provider>
147-
</WrapSequenceContext>,
148-
);
149-
return queryByText;
150-
};
151-
152-
const frame0 = getForFrame(0);
111+
const frame0 = getForFrame(0, content);
153112
expect(frame0(/^frame40$/i)).not.toBe(null);
154-
const frame39 = getForFrame(39);
113+
const frame39 = getForFrame(39, content);
155114
expect(frame39(/^frame79$/i)).not.toBe(null);
156-
const frame50 = getForFrame(50);
115+
const frame50 = getForFrame(50, content);
157116
expect(frame50(/^frame90$/i)).toBe(null);
158117
});
159118

160-
test.skip('Negative offset edge case', () => {
119+
test('Negative offset edge case', () => {
161120
const NestedChild = () => {
162121
const frame = useCurrentFrame();
163122
return <div>{'frame' + frame}</div>;
@@ -167,49 +126,38 @@ test.skip('Negative offset edge case', () => {
167126
const endAt = 90;
168127

169128
const content = (
170-
<WrapSequenceContext>
171-
<Sequence from={40}>
172-
<Sequence from={0 - startFrom} durationInFrames={endAt}>
173-
<NestedChild />
174-
</Sequence>
129+
<Sequence from={40}>
130+
<Sequence from={0 - startFrom} durationInFrames={endAt}>
131+
<NestedChild />
175132
</Sequence>
176-
</WrapSequenceContext>
133+
</Sequence>
177134
);
178135

179-
const getForFrame = (frame: number) => {
180-
const {queryByText} = render(
181-
<CanUseRemotionHooksProvider>
182-
<TimelineContext.Provider
183-
value={{
184-
frame: {
185-
'my-comp': frame,
186-
},
187-
playing: false,
188-
rootId: 'hi',
189-
imperativePlaying: {
190-
current: false,
191-
},
192-
playbackRate: 1,
193-
setPlaybackRate: () => {
194-
throw new Error('playback rate');
195-
},
196-
audioAndVideoTags: {
197-
current: [],
198-
},
199-
}}
200-
>
201-
{content}
202-
</TimelineContext.Provider>
203-
</CanUseRemotionHooksProvider>,
204-
);
205-
return queryByText;
206-
};
207-
208-
expect(getForFrame(0)(/^frame0/i)).toBe(null);
209-
expect(getForFrame(10)(/^frame10/i)).toBe(null);
210-
expect(getForFrame(40)(/^frame40$/i)).not.toBe(null);
211-
const atFrame80 = getForFrame(80)(/^frame80$/i);
136+
expect(getForFrame(40, content)(/^frame40$/i)).not.toBe(null);
137+
expect(getForFrame(0, content)(/^frame0/i)).toBe(null);
138+
expect(getForFrame(10, content)(/^frame10/i)).toBe(null);
139+
const atFrame80 = getForFrame(80, content)(/^frame80$/i);
212140
expect(atFrame80).not.toBe(null);
213-
const atFrame90 = getForFrame(90)(/^frame90$/i);
141+
const atFrame90 = getForFrame(90, content)(/^frame90$/i);
214142
expect(atFrame90).toBe(null);
215143
});
144+
145+
test('Floats', () => {
146+
const content = (
147+
<AbsoluteFill style={{backgroundColor: 'white'}}>
148+
<Sequence from={5.1000000000000005} durationInFrames={128.4}>
149+
<h1>One</h1>
150+
</Sequence>
151+
<Sequence from={133.5} durationInFrames={96.72}>
152+
<h1>Two</h1>
153+
</Sequence>
154+
</AbsoluteFill>
155+
);
156+
expect(getForFrame(132, content)(/^One$/i)).not.toBe(null);
157+
cleanup();
158+
expect(getForFrame(133, content)(/^One$/i)).not.toBe(null);
159+
cleanup();
160+
expect(getForFrame(133, content)(/^Two$/i)).toBe(null);
161+
cleanup();
162+
expect(getForFrame(134, content)(/^Two$/i)).not.toBe(null);
163+
});

0 commit comments

Comments
 (0)