Time loop layer issues

I had notice that Timeloop layer works incorrectly when Length parameter is set to some particular values.

For example, try the sample file. You see sequence of images with numbers displayed at each frame: 0, 1, 2, 3, 4, 5, …

It is looped with timeloop layer. And length is 16. We should expect this:

0, 1, 2, 3, ... 14, 15, 0, 1, 2, 3, ...

Instead of that we have:

0, 1, 2, 3, ... 14, 15, 16, 1, 2, 3, ...

See? the loop glitches at the end. Again, it breaks animation timing. Look similar like FPS interpolation in List Importer. And looks like we have same precision loss during calculations.

K, let’s inspect synfig-core/src/modules/lyr_std/timeloop.cpp:

void
Layer_TimeLoop::set_time(Context context, Time t)const
{
	Time time = t;

	if (!only_for_positive_duration || duration > 0)
	{
		if (duration == 0)
			t = link_time;
		else if (duration > 0)
		{
			t -= local_time;
			t -= floor(t / duration) * duration;
			t  = link_time + t;
		}
		else
		{
			t -= local_time;
			t -= floor(t / -duration) * -duration;
			t  = link_time - t;
		}

		// for compatibility with v0.1 layers; before local_time is reached, take a step back
		if (!symmetrical && time < local_time)
			t -= duration;
	}

	context.set_time(t);
}

To fix the issue, I have applied the same solution as with List Importer – we should avoid the loss if we will get convert time to frame number and only after that do the calculations.

I need to determine FPS of the document. I know, that any layer can get document fps via get_canvas()->rend_desc().get_frame_rate() (see this post).

But when I have tried to add this:

float document_fps=get_canvas()->rend_desc().get_frame_rate();

…I’ve got compilation error:

timeloop.cpp:230: error: invalid use of incomplete type 'struct synfig::Canvas'
../../../src/synfig/layer.h:153: error: forward declaration of 'struct synfig::Canvas'

Stuck. I guessed that I need to include some header to timeloop.cpp. Then I remember that I have used “get_canvas()->rend_desc()” in some file when I was fixing List importer. And it was working fine there. I quickly found that this file is synfig-core/src/modules/lyr_std/import.cpp.

I have started to compare the include list at the top of timeloop.cpp and import.cpp.
I guessed that I miss this line:

#include <synfig/canvas.h>

After adding it to timeloop.cpp everything worked well.

CONCLUSION: To use get_canvas()->rend_desc().get_frame_rate() we need #include at the header.

And here is the patch:

 #include <synfig/paramdesc.h>
 #include <synfig/renddesc.h>
 #include <synfig/value.h>
+#include <synfig/canvas.h>

 #endif

@@ -221,24 +222,39 @@ void
 Layer_TimeLoop::set_time(Context context, Time t)const
 {
 	Time time = t;
+
+	float document_fps=get_canvas()->rend_desc().get_frame_rate();

 	if (!only_for_positive_duration || duration > 0)
 	{
 		if (duration == 0)
 			t = link_time;
-		else if (duration > 0)
-		{
-			t -= local_time;
-			t -= floor(t / duration) * duration;
-			t  = link_time + t;
+		else {
+			float t_frames = round(t*document_fps);
+			float duration_frames = round(duration*document_fps);
+			if (duration > 0)
+			{
+				t -= local_time;
+				// Simple formula looks like this:
+				// t -= floor(t / duration) * duration;
+				// but we should make all calculations in frames to avoid round errors
+				t_frames -= floor(t_frames / duration_frames) * duration_frames;
+				// converting back to seconds:
+				t = t_frames / document_fps;
+				t = link_time + t;
+			}
+			else
+			{
+				t -= local_time;
+				// Simple formula looks like this:
+				// t -= floor(t / -duration) * -duration;
+				// but we should make all calculations in frames to avoid round errors
+				t_frames -= floor(t_frames / -duration_frames) * -duration_frames;
+				// converting back to seconds:
+				t = t_frames / document_fps;
+				t = link_time - t;
+			}
 		}
-		else
-		{
-			t -= local_time;
-			t -= floor(t / -duration) * -duration;
-			t  = link_time - t;
-		}
-
 		// for compatibility with v0.1 layers; before local_time is reached, take a step back
 		if (!symmetrical && time < local_time)
 			t -= duration;

The issue is fixed. Sending to patch tracker

NOTE: Actually, if you play with this bug a little, you can see that the interpolation problem comes from file save/loading procedure. I.e. if you open sample file, change duration to 15f and then back to 16f, then everything is alright – you get expected sequence. But if you save and reopen file – then timings are messing out. Anyway, I have no time digging into XML saving routines and patch above fixes the problem.

Advertisements

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s

%d bloggers like this: