Breaking animations

[Pekka Paalanen]

I noticed that gears may sometimes stop, and then start animating
again. Turned out the underlying problem is a known one,
eglSwapBuffers() dispatching Wayland events.

The patch below is a nice try, but it creates exactly the problem the
comment warns about. It is very difficult to open the window context
menu, the menu does not respond to mouse motion, and clicking "close"
deadlocks the whole program.

This is just a note for people who might wonder about breaking
animations in toytoolkit applications.

- pq


From 0395a7eccd6ed7cd344d69895d39c852f743abf4 Mon Sep 17 00:00:00 2001
From: Pekka Paalanen <ppaalanen at gmail.com>
Date: Wed, 18 Jan 2012 15:59:15 +0200
Subject: [PATCH] window: workaround for stopping animations

The problem is evident in this backtrace:

 0  window_schedule_redraw (window=0xc55d00) at window.c:2117
 1  0x0000000000406338 in frame_callback (data=0xc55cb0, callback=0xf16580, time=4037878823) at gears.c:206
 2  0x00007ffff79d006c in ffi_call_unix64 () from /usr/lib64/libffi.so.5
 3  0x00007ffff79cfa04 in ffi_call (cif=0x626928, fn=0x40629e <frame_callback>, rvalue=<optimized out>,
    avalue=<optimized out>) at ../src/x86/ffi64.c:486
 4  0x00007ffff7bd951d in wl_closure_invoke (closure=0x626878, target=0xf16580,
    func=0x40629e <frame_callback>, data=0xc55cb0) at connection.c:734
 5  0x00007ffff7bd7587 in handle_event (display=0x622790, id=25, opcode=0, size=12) at wayland-client.c:488
 6  0x00007ffff7bd765f in wl_display_iterate (display=0x622790, mask=1) at wayland-client.c:521
 7  0x00007ffff67150a1 in dri2_swap_buffers (drv=0x6283c0, disp=<optimized out>, draw=0xe25680)
    at platform_wayland.c:577
 8  0x00007ffff670b62c in eglSwapBuffers (dpy=0x627690, surface=0xe25680) at eglapi.c:702
 9  0x00007ffff756a4c8 in cairo_gl_surface_swapbuffers (abstract_surface=0xe26000)
    at cairo-gl-surface.c:703
 10 0x00000000004083e9 in window_attach_surface (window=0xc55d00) at window.c:827
 11 0x0000000000408590 in window_flush (window=0xc55d00) at window.c:877
 12 0x000000000040abd5 in idle_redraw (task=0xc55d68, events=0) at window.c:2110
 13 0x000000000040cd82 in display_run (display=0x622660) at window.c:3116
 14 0x0000000000406a42 in main (argc=1, argv=0x7fffffffdeb8) at gears.c:381
(gdb) print window->redraw_scheduled
$1 = 1

Gears sets up a frame callback after rendering a frame of animation, and
relies on the callback to schedule a redraw, which sets up a new frame
callback.

Deferred redraws are executed in idle_redraw(), which via window_flush()
and Cairo may call eglSwapBuffers(). If eglSwapBuffers() needs to block,
it will wait for an event, inadvertently dispatching all events while
waiting. If one of the dispatched events is the animation callback, the
callback will fail to schedule a new redraw, and animation stops.

This patch works around that situation by allowing to schedule a redraw
in the above case. The downside and the proper solution are described in
a comment in the code.
---
 clients/window.c |   19 ++++++++++++++++++-
 1 files changed, 18 insertions(+), 1 deletions(-)

diff --git a/clients/window.c b/clients/window.c
index 4dd9960..2cb2981 100644
--- a/clients/window.c
+++ b/clients/window.c
@@ -2107,8 +2107,25 @@ idle_redraw(struct task *task, uint32_t events)
 
 	window_create_surface(window);
 	widget_redraw(window->widget);
-	window_flush(window);
+
+	/* XXX:
+	 * window_flush() may recurse into eglSwapBuffers(), which
+	 * may call wl_display_iterate(), dispatching events. If any of
+	 * those events calls window_schedule_redraw(), idle_redraw() will
+	 * be called again from display_run() without breaking out from the
+	 * deferred tasks loop. This may cause us looping forever.
+	 *
+	 * But, if we set redraw_scheduled=0 only after window_flush(),
+	 * no redraws will be scheduled from under eglSwapBuffers().
+	 * If eglSwapBuffers() then ends up dispatching an animation frame
+	 * callback, the animation will stop, as it cannot schedule a redraw.
+	 *
+	 * The proper solution is to fix eglSwapBuffers() to not
+	 * dispatch events, and prevent rendering into an in-use
+	 * buffer some other way.
+	 */
 	window->redraw_scheduled = 0;
+	window_flush(window);
 }
 
 void
-- 
1.7.3.4