Complete.Org: Mailing Lists: Archives: freeciv-dev: March 2005:
[Freeciv-Dev] Re: (PR#12634) ftwl: Rewrite main redraw loop (repost)
Home

[Freeciv-Dev] Re: (PR#12634) ftwl: Rewrite main redraw loop (repost)

[Top] [All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index] [Thread Index]
To: per@xxxxxxxxxxx
Subject: [Freeciv-Dev] Re: (PR#12634) ftwl: Rewrite main redraw loop (repost)
From: "Raimar Falke" <i-freeciv-lists@xxxxxxxxxxxxx>
Date: Fri, 25 Mar 2005 07:18:38 -0800
Reply-to: bugs@xxxxxxxxxxx

<URL: http://bugs.freeciv.org/Ticket/Display.html?id=12634 >

On Fri, Mar 25, 2005 at 04:27:09AM -0800, Raimar Falke wrote:
> 
> <URL: http://bugs.freeciv.org/Ticket/Display.html?id=12634 >
> 
> On Fri, Mar 25, 2005 at 03:53:47AM -0800, Per I. Mathisen wrote:
> > I found the problem with the SDL backend. SDL was innocent of all charges,
> > and the same was the SDL backend code. Turns out the problem was that the
> > redraw loop was ... pretty much non-existent. Basically there would be
> > redraws at random intervals due to other stuff happening, but not as a
> > result of actual map movement. The x11 backend had fixed this by inserting
> > a call to the high-level sw_paint_all() function directly into the
> > low-level backend code. The sdl backend did not have this incredible
> > kludge.
> 
> > So, I decided to make a change I had long wanted, making ftwl a
> > constant-frame-rate application. This is nice for doing animations. The
> > framerate should probably be either user set or dynamic, depending on
> > speed of graphics hardware, but so far a fixed FPS of 20 at least makes
> > the graphics smooth pretty much irrespective of backend choice.
> 
> Sorry but I dislike this. FTWL (the library) is event driven. It
> should redraw itself IFF this is required. Note that however that the
> FTWL client may do something like this (constant redraw for
> animations).
> 
> For the starting problem I propose something different. This is also
> needed to implement the add_idle_callback().
> 
> be_next_event is splitted. The first part will return immediately. It
> will fetch a queued event or return with no event. The second part
> will waiting for the network or timeout.
> 
> Now the common FTWL code does something like:
>   forever() {
>     while(be_next_event_non_blocking()) {
>       handle;
>     }
> 
>     execute idle callback
> 
>     // to make the user happy because the next step may take some time
>     sw_paint_all(); 
> 
>     be_next_event_blocking();
>     handle;
>   }
> 
> Do you know the best part? No changes to the backends are needed
> because with the timeout feature of the existing be_next_event() we
> can do the blocking and a non-blocking version already. Still it may
> be cleaner to really create these two functions.

The patch. I tested it successfully with X11 and SDL.

        Raimar

-- 
 email: i-freeciv-lists@xxxxxxxxxxxxx
  "Debugging is twice as hard as writing the code in the first place.
   Therefore, if you write the code as cleverly as possible, you are,
   by definition, not smart enough to debug it."
    -- Brian W. Kernighan

Index: client/gui-ftwl/gui_main.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/client/gui-ftwl/gui_main.c,v
retrieving revision 1.12
diff -u -u -r1.12 gui_main.c
--- client/gui-ftwl/gui_main.c  22 Mar 2005 20:04:42 -0000      1.12
+++ client/gui-ftwl/gui_main.c  25 Mar 2005 15:14:45 -0000
@@ -312,11 +312,7 @@
   function should be called sometimes soon, and passed the 'data' pointer
   as its data.
 ****************************************************************************/
-void add_idle_callback(void (callback)(void *), void *data)
+void add_idle_callback(void (callback) (void *), void *data)
 {
-  /* PORTME */
-
-  /* This is a reasonable fallback if it's not ported. */
-  freelog(LOG_DEBUG, "Unimplemented add_idle_callback.");
-  (callback)(data);
+  sw_add_timeout(-1, callback, data);
 }
Index: utility/ftwl/back_end.h
===================================================================
RCS file: /home/freeciv/CVS/freeciv/utility/ftwl/back_end.h,v
retrieving revision 1.6
diff -u -u -r1.6 back_end.h
--- utility/ftwl/back_end.h     25 Mar 2005 11:31:36 -0000      1.6
+++ utility/ftwl/back_end.h     25 Mar 2005 15:14:46 -0000
@@ -144,7 +144,8 @@
 /* ===== other ===== */
 void be_init(const struct ct_size *screen_size, bool fullscreen);
 bool be_supports_fullscreen(void);
-void be_next_event(struct be_event *event, struct timeval *timeout);
+void be_next_non_blocking_event(struct be_event *event);
+void be_next_blocking_event(struct be_event *event, struct timeval *timeout);
 void be_add_net_input(int sock);
 void be_remove_net_input(void);
 void be_copy_osda_to_screen(struct osda *src);
Index: utility/ftwl/be_sdl.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/utility/ftwl/be_sdl.c,v
retrieving revision 1.4
diff -u -u -r1.4 be_sdl.c
--- utility/ftwl/be_sdl.c       9 Oct 2004 11:59:38 -0000       1.4
+++ utility/ftwl/be_sdl.c       25 Mar 2005 15:14:46 -0000
@@ -216,23 +216,34 @@
 /*************************************************************************
   ...
 *************************************************************************/
-void be_next_event(struct be_event *event, struct timeval *timeout)
+void be_next_non_blocking_event(struct be_event *event)
+{
+  SDL_Event sdl_event;
+
+  event->type = BE_NO_EVENT;
+
+  while (SDL_PollEvent(&sdl_event)) {
+    if (copy_event(event, &sdl_event)) {
+      break;
+    }
+  }
+}
+
+/*************************************************************************
+  ...
+*************************************************************************/
+void be_next_blocking_event(struct be_event *event, struct timeval *timeout)
 {
-  /*
-   * There are 3 event sources
-   * 1) data on the network socket
-   * 2) timeout
-   * 3) SDL keyboard and mouse events
-   */
   Uint32 timeout_end =
       SDL_GetTicks() + timeout->tv_sec * 1000 + timeout->tv_usec / 1000;
 
   for (;;) {
-    SDL_Event sdl_event;
 
-    event->type = BE_NO_EVENT;
-    event->key.key = 0;
-    event->key.type = NUM_BE_KEYS;
+    /* Test for already queued events like mouse and keyboard */
+    be_next_non_blocking_event(event);
+    if (event->type != BE_NO_EVENT) {
+      return;
+    }
 
     /* Test the network socket. */
     if (other_fd != -1) {
@@ -240,7 +251,6 @@
       int ret;
       struct timeval zero_timeout;
 
-      /* No event available: block on input socket until one is */
       FD_ZERO(&readfds);
       FD_ZERO(&exceptfds);
 
@@ -259,25 +269,12 @@
       }
     }
 
-    /* The timeout */
+    /* Test for the timeout */
     if (SDL_GetTicks() >= timeout_end) {
       event->type = BE_TIMEOUT;
       return;
     }
 
-    /* Normal SDL events */
-    while (SDL_PollEvent(&sdl_event)) {
-      if (copy_event(event, &sdl_event)) {
-#if 0
-        /* For debugging sdl slowness - remove me when done */
-        if (event->type == BE_KEY_PRESSED) {
-          printf("sending event %d:%d\n", event->key.key, event->key.type);
-        }
-#endif
-        return;
-      }
-    }
-
     /* Wait 10ms and do polling */
     SDL_Delay(10);
     assert(event->type == BE_NO_EVENT);
Index: utility/ftwl/be_x11_ximage.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/utility/ftwl/be_x11_ximage.c,v
retrieving revision 1.2
diff -u -u -r1.2 be_x11_ximage.c
--- utility/ftwl/be_x11_ximage.c        7 Oct 2004 15:24:45 -0000       1.2
+++ utility/ftwl/be_x11_ximage.c        25 Mar 2005 15:14:46 -0000
@@ -219,7 +219,7 @@
 /*************************************************************************
   ...
 *************************************************************************/
-void be_next_event(struct be_event *event, struct timeval *timeout)
+void be_next_non_blocking_event(struct be_event *event)
 {
   XEvent xevent;
 
@@ -227,7 +227,6 @@
   event->type = BE_NO_EVENT;
 
   if (XCheckMaskEvent(display, -1 /*all events */ , &xevent)) {
-    //printf("got event %d\n", xevent.type);
     if (copy_event(event, &xevent)) {
       return;
     } else {
@@ -235,46 +234,52 @@
       goto restart;
     }
   }
+}
 
-  sw_paint_all();
+/*************************************************************************
+  ...
+*************************************************************************/
+void be_next_blocking_event(struct be_event *event, struct timeval *timeout)
+{
+  fd_set readfds, exceptfds;
+  int ret, highest = x11fd;
 
-  {
-    fd_set readfds, exceptfds;
-    int ret, highest = x11fd;
-
-    /* No event available: block on input socket until one is */
-    FD_ZERO(&readfds);
-    FD_SET(x11fd, &readfds);
-
-    FD_ZERO(&exceptfds);
-
-    if (other_fd != -1) {
-      FD_SET(other_fd, &readfds);
-      FD_SET(other_fd, &exceptfds);
-      if (other_fd > highest) {
-       highest = other_fd;
-      }
-    }
-
-    ret = select(highest + 1, &readfds, NULL, &exceptfds, timeout);
-    if (ret == 0) {
-      // timed out
-      event->type = BE_TIMEOUT;
-    } else if (ret > 0) {
-      if (other_fd != -1 && (FD_ISSET(other_fd, &readfds) ||
-                            FD_ISSET(other_fd, &exceptfds))) {
-       event->type = BE_DATA_OTHER_FD;
-       event->socket = other_fd;
-       return;
-      }
-      /* new data on the x11 fd */
-      goto restart;
-    } else if (errno == EINTR) {
-      goto restart;
-    } else {
-      assert(0);
+restart:
+  event->type = BE_NO_EVENT;
+
+  /* No event available: block on input socket until one is */
+  FD_ZERO(&readfds);
+  FD_SET(x11fd, &readfds);
+
+  FD_ZERO(&exceptfds);
+
+  if (other_fd != -1) {
+    FD_SET(other_fd, &readfds);
+    FD_SET(other_fd, &exceptfds);
+    if (other_fd > highest) {
+      highest = other_fd;
     }
   }
+
+  ret = select(highest + 1, &readfds, NULL, &exceptfds, timeout);
+  if (ret == 0) {
+    // timed out
+    event->type = BE_TIMEOUT;
+  } else if (ret > 0) {
+    if (other_fd != -1 && (FD_ISSET(other_fd, &readfds) ||
+                          FD_ISSET(other_fd, &exceptfds))) {
+      event->type = BE_DATA_OTHER_FD;
+      event->socket = other_fd;
+    }
+    /* 
+     * New data on the x11 fd. return with BE_NO_EVENT and let the
+     * caller handle it. 
+     */
+  } else if (errno == EINTR) {
+    goto restart;
+  } else {
+    assert(0);
+  }
 }
 
 /*************************************************************************
Index: utility/ftwl/widget.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/utility/ftwl/widget.c,v
retrieving revision 1.6
diff -u -u -r1.6 widget.c
--- utility/ftwl/widget.c       22 Jan 2005 19:45:45 -0000      1.6
+++ utility/ftwl/widget.c       25 Mar 2005 15:14:47 -0000
@@ -323,6 +323,76 @@
 /*************************************************************************
   ...
 *************************************************************************/
+static void handle_event(const struct be_event *event,
+                        void (*input_callback) (int socket))
+{
+  switch (event->type) {
+  case BE_DATA_OTHER_FD:
+    input_callback(event->socket);
+    break;
+  case BE_EXPOSE:
+    flush_all_to_screen();
+    break;
+  case BE_TIMEOUT:
+    handle_callbacks();
+    break;
+
+  case BE_MOUSE_MOTION:
+    {
+      struct sw_widget *widget = search_widget(&event->position, EV_MOUSE);
+
+      handle_mouse_motion(widget, &event->position);
+    }
+    break;
+
+  case BE_MOUSE_PRESSED:
+    {
+      struct sw_widget *widget = search_widget(&event->position, EV_MOUSE);
+
+      handle_mouse_press(widget, &event->position, event->button,
+                        event->state);
+    }
+    break;
+
+  case BE_MOUSE_RELEASED:
+    {
+      struct sw_widget *widget = search_widget(&event->position, EV_MOUSE);
+
+      handle_mouse_release(widget, &event->position, event->button);
+    }
+    break;
+
+  case BE_KEY_PRESSED:
+    {
+      bool handled = FALSE;
+      struct sw_widget *widget;
+
+      assert(ct_key_is_valid(&event->key));
+
+      if (selected_widget_gets_keyboard && selected_widget) {
+       widget = selected_widget;
+      } else {
+       widget = search_widget(&event->position, EV_KEYBOARD);
+      }
+      if (widget && widget->key) {
+       handled = widget->key(widget, &event->key, widget->key_data);
+      }
+      if (!handled) {
+       handled = deliver_key(&event->key);
+      }
+      if (!handled) {
+       printf("WARNING: unhandled key stroke\n");
+      }
+    }
+    break;
+  case BE_NO_EVENT:
+    break;
+  }
+}
+
+/*************************************************************************
+  ...
+*************************************************************************/
 void sw_mainloop(void (*input_callback)(int socket))
 {
   sw_paint_all();
@@ -331,74 +401,31 @@
     struct be_event event;
     struct timeval timeout;
 
-    handle_callbacks();
-
-    get_select_timeout(&timeout);    
-    be_next_event(&event, &timeout);
-
-    switch (event.type) {
-    case BE_DATA_OTHER_FD:
-      input_callback(event.socket);
-      break;
-    case BE_EXPOSE:
-      flush_all_to_screen();
-      break;
-    case BE_TIMEOUT:
+    while (TRUE) {
       handle_callbacks();
-      break;
-
-    case BE_MOUSE_MOTION:
-      {
-       struct sw_widget *widget =
-           search_widget(&event.position, EV_MOUSE);
 
-       handle_mouse_motion(widget, &event.position);
+      be_next_non_blocking_event(&event);
+      if (event.type == BE_NO_EVENT) {
+       break;
       }
-      break;
-
-    case BE_MOUSE_PRESSED:
-      {
-       struct sw_widget *widget =
-           search_widget(&event.position, EV_MOUSE);
+      handle_event(&event, input_callback);
+    }
 
-       handle_mouse_press(widget, &event.position, event.button, event.state);
-      }
-      break;
+    /* No events queued. We are idle. */
+    handle_idle_callbacks();
 
-    case BE_MOUSE_RELEASED:
-      {
-       struct sw_widget *widget =
-           search_widget(&event.position, EV_MOUSE);
+    /* 
+     * Since the next step may take some while, we update the screen
+     * to make the user happy.
+     */
+    sw_paint_all();
 
-       handle_mouse_release(widget, &event.position, event.button);
-      }
-      break;
+    /* Wait for the server, the network or the timeout */
+    get_select_timeout(&timeout);    
+    be_next_blocking_event(&event, &timeout);
 
-    case BE_KEY_PRESSED:
-      {
-       bool handled = FALSE;
-       struct sw_widget *widget;
-
-       assert(ct_key_is_valid(&event.key));
-
-       if (selected_widget_gets_keyboard && selected_widget) {
-         widget = selected_widget;
-       } else {
-         widget = search_widget(&event.position, EV_KEYBOARD);
-       }
-       if (widget && widget->key) {
-         handled = widget->key(widget, &event.key, widget->key_data);
-       }
-       if (!handled) {
-         handled = deliver_key(&event.key);
-       }
-       if (!handled) {
-         printf("WARNING: unhandled key stroke\n");
-       }
-      }
-      break;
-    case BE_NO_EVENT:
-      break;
+    if (event.type != BE_NO_EVENT) {
+      handle_event(&event, input_callback);
     }
   }
 }
Index: utility/ftwl/widget_p.h
===================================================================
RCS file: /home/freeciv/CVS/freeciv/utility/ftwl/widget_p.h,v
retrieving revision 1.3
diff -u -u -r1.3 widget_p.h
--- utility/ftwl/widget_p.h     22 Jan 2005 19:45:45 -0000      1.3
+++ utility/ftwl/widget_p.h     25 Mar 2005 15:14:47 -0000
@@ -188,6 +188,7 @@
 extern struct widget_list *deferred_destroyed_widgets;
 
 void handle_callbacks(void);
+void handle_idle_callbacks(void);
 void get_select_timeout(struct timeval *timeout);
 
 struct sw_widget *create_widget(struct sw_widget *parent,
Index: utility/ftwl/widget_timeout.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/utility/ftwl/widget_timeout.c,v
retrieving revision 1.4
diff -u -u -r1.4 widget_timeout.c
--- utility/ftwl/widget_timeout.c       22 Jan 2005 19:45:45 -0000      1.4
+++ utility/ftwl/widget_timeout.c       25 Mar 2005 15:14:47 -0000
@@ -53,6 +53,12 @@
   void *data;
 };
 
+struct idle_callback {
+  struct timeval time;
+  void (*callback) (void *data);
+  void *data;
+};
+
 #define SPECLIST_TAG callback
 #define SPECLIST_TYPE struct callback
 #include "speclist.h"
@@ -61,8 +67,17 @@
     TYPED_LIST_ITERATE(struct callback, list, item)
 #define callback_list_iterate_end  LIST_ITERATE_END
 
+#define SPECLIST_TAG idle_callback
+#define SPECLIST_TYPE struct idle_callback
+#include "speclist.h"
+
+#define idle_callback_list_iterate(list, item) \
+    TYPED_LIST_ITERATE(struct idle_callback, list, item)
+#define idle_callback_list_iterate_end  LIST_ITERATE_END
+
 static struct callback_list *callback_list = NULL;
-static bool callback_list_list_has_been_initialised = FALSE;
+static struct idle_callback_list *idle_callback_list = NULL;
+static bool callback_lists_has_been_initialised = FALSE;
 static int id_counter = 1;
 
 /*************************************************************************
@@ -70,39 +85,55 @@
 *************************************************************************/
 static void ensure_init(void)
 {
-  if (!callback_list_list_has_been_initialised) {
+  if (!callback_lists_has_been_initialised) {
     callback_list = callback_list_new();
-    callback_list_list_has_been_initialised = TRUE;
+    idle_callback_list = idle_callback_list_new();
+    callback_lists_has_been_initialised = TRUE;
   }
 }
 
 /*************************************************************************
-  ...
+  If msec equals -1 it is an idle callback. This means that it will
+  only executed if the system is idle. Also note that in this case the
+  return value will be -1. I.e. you can't remove idle timeouts.
 *************************************************************************/
 int sw_add_timeout(int msec, void (*callback) (void *data), void *data)
 {
-  struct callback *p = fc_malloc(sizeof(*p));
+  if (msec == -1) {
+    struct idle_callback *p = fc_malloc(sizeof(*p));
 
-  //printf("add_timeout(%dms)\n", msec);
-  gettimeofday(&p->time, NULL);
+    p->callback = callback;
+    p->data = data;
 
-  p->time.tv_sec += msec / 1000;
-  msec %= 1000;
+    ensure_init();
+    idle_callback_list_prepend(idle_callback_list, p);
+    return -1;
+  } else {
+    struct callback *p = fc_malloc(sizeof(*p));
 
-  p->time.tv_usec += msec * 1000;
-  while (p->time.tv_usec > 1000 * 1000) {
-    p->time.tv_usec -= 1000 * 1000;
-    p->time.tv_sec++;
-  }
+    assert(msec >= 0);
 
-  p->callback = callback;
-  p->data = data;
-  p->id=id_counter;
-  id_counter++;
+    //printf("add_timeout(%dms)\n", msec);
+    gettimeofday(&p->time, NULL);
 
-  ensure_init();
-  callback_list_prepend(callback_list, p);
-  return p->id;
+    p->time.tv_sec += msec / 1000;
+    msec %= 1000;
+
+    p->time.tv_usec += msec * 1000;
+    while (p->time.tv_usec > 1000 * 1000) {
+      p->time.tv_usec -= 1000 * 1000;
+      p->time.tv_sec++;
+    }
+
+    p->callback = callback;
+    p->data = data;
+    p->id = id_counter;
+    id_counter++;
+
+    ensure_init();
+    callback_list_prepend(callback_list, p);
+    return p->id;
+  }
 }
 
 /*************************************************************************
@@ -161,6 +192,29 @@
 /*************************************************************************
   ...
 *************************************************************************/
+void handle_idle_callbacks(void)
+{
+  ensure_init();
+
+  for (;;) {
+    bool change = FALSE;
+
+    idle_callback_list_iterate(idle_callback_list, callback) {
+      idle_callback_list_unlink(idle_callback_list, callback);
+      callback->callback(callback->data);
+      free(callback);
+      change = TRUE;
+      break;
+    } idle_callback_list_iterate_end;
+    if (!change) {
+      break;
+    }
+  }
+}
+
+/*************************************************************************
+  ...
+*************************************************************************/
 void get_select_timeout(struct timeval *timeout)
 {
   struct callback *earliest = NULL;

[Prev in Thread] Current Thread [Next in Thread]