Re: [Freeglut-developer] Bug (very nasty).

[Richard R. <sf...@ol...>]

On Sun, Sep 14, 2003 at 11:32:05PM -0500, Steve Baker wrote:
> Richard Rauch wrote:
> 
> >Xevent reading does not halt freeglut.  It does halt GLUT.  GLUT
 [...]
> >I hope that you at least are not adverse to calling this a terrible
> >bug and accepting patches---whether or not you particularly care about
> >fixing the bug.
> 
> Steve says "This is a terrible bug and I accept patches".  :-)

Okay, I'm including A patch that works for me.  I put in the framework
for MS-WINDOWS, but can't test it (much less flesh it out).  For BOTH,

  if( check-event )
     ...
  else
  {
     ...
  }

became

  while( check-event )
     ...

  {
     ...
  }


Also, I didn't think too hard about the ordering of the sleep vs. closing
windows.  Maybe the sleep should be moved a little lower?

Sleeping is ONLY done if:

 * There is no idle callback.
 * There is a {fgStructure.Windows.First} (that should really
   be abstracted to a check-for-an-open-window function.)

The code assumes that, for X11, ConnectionNumber( fgDisplay.Dsiplay ) returns
a socket.  This is true for XFree86.  I wound up skimming the GLUT for
ideas and if you unwrap enough stuff in the GLUT, this is what they
are doing, so I think that this will work everywhere that the GLUT works.

The code has all of the #includes that I thought I might need, stuffed in
right before the fgSleepForEvents() (very yucky; they includes should be
trimmed and bubbled to the top).

I attempted to generally follow the style conventions of the code.
The event-loop code seemed to use a 2-space indent (I followed that, there).
Other places use a 4-space indent, so I used that in fgSleepForEvents().
I assume that the name fgSleepForEvents() is okay.

I check for an error-code on select() and printf() when appropriate.  I
know that old GLUT would print error messages in some cases, so I assume
that this is OK.


Alright, the DESCRIPTION of the patch is longer than the patch, practically.
(^&

Here's the code (_freeglut_main.c is the new version; it's a sad story
why freeglut_main.c is the original code, but I did compile against the
new code, and a I tested it, and CPU usage for an idle program drops
to 0 for me, so all is well for me):


 /~~~ freeglut_main.c.patch

--- _freeglut_main.c	Mon Sep 15 02:19:36 2003
+++ freeglut_main.c	Mon Sep 15 02:39:53 2003
@@ -399,30 +399,6 @@
     va_end( ap );
 }
 
-#include <sys/types.h>
-#include <sys/time.h>
-#include <unistd.h>
-#include <errno.h>
-#include <sys/stat.h>
-void fgSleepForEvents( void )
-{
-#ifdef TARGET_HOST_UNIX_X11
-    fd_set fdset;
-    int err;
-    int socket;
-    
-    socket = ConnectionNumber( fgDisplay.Display );
-    FD_ZERO( &fdset );
-    FD_SET( socket, &fdset );
-    
-    err = select( socket+1, &fdset, NULL, NULL, NULL );
-    if( -1 == err )
-        printf( "freeglut select() error: %d\n", errno );
-    
-#elif TARGET_HOST_WIN32
-#endif
-}
-
 /* -- INTERFACE FUNCTIONS -------------------------------------------------- */
 
 /*
@@ -449,7 +425,7 @@
   /*
    * Do we have any event messages pending?
    */
-  while( XPending( fgDisplay.Display ) )
+  if( XPending( fgDisplay.Display ) )
   {
     /*
      * Grab the next event to be processed...
@@ -1000,7 +976,7 @@
       break;
     }
   }
-
+  else
   {
     /*
      * Have all the timers checked.
@@ -1032,7 +1008,7 @@
   /*
    * The windows processing is considerably smaller
    */
-  while( PeekMessage( &stMsg, NULL, 0, 0, PM_NOREMOVE ) )
+  if( PeekMessage( &stMsg, NULL, 0, 0, PM_NOREMOVE ) )
   {
     /*
      * Grab the message now, checking for WM_QUIT
@@ -1046,7 +1022,7 @@
     TranslateMessage( &stMsg );
     DispatchMessage( &stMsg );
   }
-
+  else
   {
     /*
      * Have all the timers checked.
@@ -1072,9 +1048,6 @@
   }
 #endif
 
-  if( fgStructure.Windows.First && !fgState.IdleCallback )
-    fgSleepForEvents();
-
     /* 
      * If an event caused a window to be closed, do the actual closing here
      */

 \___ freeglut_main.c.patch


-- 
  "I probably don't know what I'm talking about."  http://www.olib.org/~rkr/