Re: [AD] X vsync emulation (timing)

[Peter Wang <tjaden@xxxxxxxxxx>]

Chris wrote:

> Peter Wang wrote:
>
> > The problem is that the background thread runs at 100Hz.  You can't 
> > update retrace_count at some arbitrary rate without introducing 
> > another thread, which is just overkill.
>
>
> True.. but I think the timer should be designed so it attempts to 
> increment at the speed of the reported refresh rate, instead of a flat 
> 100Hz.


How do you propose to do that?

> And while you could do that in the input callback, I don't believe 
> that's where it should be. As well, I think it should be updating 
> retrace_count directly, or else we run the risk of vsync retraces 
> being different than the reported retraces (which I believe we should 
> avoid). 


You're right, there should be a connection between retrace_count and 
vsync.  But I can't think of a clean implementation.  The attached patch 
is more or less the best I could do.  We could add a field to the system 
driver vtable instead of the explicit system_driver->id == 
SYSTEM_XWINDOWS, but I didn't feel like touching all those files yet.

Peter

Index: src/timer.c
===================================================================
RCS file: /cvsroot/alleg/allegro/src/timer.c,v
retrieving revision 1.17
diff -u -r1.17 timer.c
--- src/timer.c	22 Aug 2004 01:57:14 -0000	1.17
+++ src/timer.c	19 Nov 2004 03:30:13 -0000
@@ -77,13 +77,19 @@
    d = timer_delay;
 
    /* deal with retrace synchronisation */
-   vsync_counter -= d; 
+   /* note: the X port updates retrace_count in the bgman thread */
+#ifdef SYSTEM_XWINDOWS
+   if (system_driver->id != SYSTEM_XWINDOWS)
+#endif
+   {
+      vsync_counter -= d; 
 
-   while (vsync_counter <= 0) {
-      vsync_counter += _vsync_speed;
-      retrace_count++;
-      if (retrace_proc)
-	 retrace_proc();
+      while (vsync_counter <= 0) {
+	 vsync_counter += _vsync_speed;
+	 retrace_count++;
+	 if (retrace_proc)
+	    retrace_proc();
+      }
    }
 
    /* process the user callbacks */
Index: src/x/xsystem.c
===================================================================
RCS file: /cvsroot/alleg/allegro/src/x/xsystem.c,v
retrieving revision 1.30
diff -u -r1.30 xsystem.c
--- src/x/xsystem.c	5 Nov 2004 17:08:15 -0000	1.30
+++ src/x/xsystem.c	19 Nov 2004 03:30:15 -0000
@@ -154,6 +154,30 @@
  */
 static void _xwin_bg_handler (int threaded)
 {
+   /* retrace_count is used for vsync emulation (see _xwin_vsync)
+    *
+    * All other ports increment retrace_count in _handle_timer_tick,
+    * but in the X port that would cause a deadlock in the following
+    * situation:
+    *
+    * 1. The main thread calls set_palette, so grabs the X lock.
+    * set_palette waits for a vsync.  Under X vsync() is emulated by
+    * waiting for the retrace_count variable to increment by one.
+    *
+    * 2. The timer thread kicks in and wants to draw the mouse.  It
+    * requests the X lock.  A deadlock occurs because the timer thread
+    * is waiting for the lock held by the main thread, while the main
+    * thread is waiting for the timer thread to increment the
+    * retrace_count.
+    *
+    * Subtle differences in the X port:
+    * - retrace_proc might be run in parallel with a timer callback
+    * - retrace_count increments at 100Hz
+    */
+   retrace_count++;
+   if (retrace_proc)
+      retrace_proc();
+
    _xwin_handle_input();
 }
 
Index: src/x/xwin.c
===================================================================
RCS file: /cvsroot/alleg/allegro/src/x/xwin.c,v
retrieving revision 1.79
diff -u -r1.79 xwin.c
--- src/x/xwin.c	18 Nov 2004 03:45:10 -0000	1.79
+++ src/x/xwin.c	19 Nov 2004 03:30:18 -0000
@@ -2160,15 +2160,17 @@
  */
 void _xwin_vsync(void)
 {
+   int x = retrace_count;
+
    /* This does not wait for the VBI - but it waits until X11 has synchronized,
-    * i.e. until actual changes are visible. So it has a similar effect. A
-    * better solution might be waiting a specific time to simulate a VBI (but
-    * the X11 mutex might be locked by the timer thread), or using the XSync
-    * extension.
+    * i.e. until actual changes are visible. So it has a similar effect.
     */
    XLOCK();
    XSync(_xwin.display, False);
    XUNLOCK();
+
+   while (x == retrace_count)
+      ;
 }