Re: Progressive updates for image window of Windowsconsole app

"Russell Lang" <[email protected]> Sat, 10 Jul 2004 08:40:16 +1000
Newsgroups gmane.comp.printing.ghostscript.patches
Message-ID <40EFAB70.11378.B7733442@localhost>
Igor,

> I viewed your patch briefly and got a couple of remarks.
> Sorry if I didn't understand it correctly.
> 
> > + /* Make sure the update interval is at least 10 times
> > + * what it takes to paint the window
> > + GetSystemTime(&t2);
> 
> IMO this logic can't work properly.
> If a single processor system is busy with another task for 1 minute,
> this logic waits for 10 minutes.

Note that UpdateWindow doesn't post a WM_PAINT message to the message 
queue. It sends a WM_PAINT message, which means it calls the window 
procedure directly.  So the only way this could take 1 minute to get 
between those to GetSystemTime calls is if your system is *really* 
busy and the thread scheduler gives that thread no time at all. In 
that case, you don't want to be doing updates.


> > SetTimer(img->hwnd, img->update_timer, img->update_interval, NULL);
> 
> I'm not sure that you correctly understand SetTimer.
> Once a timer is set, it will send messages after each 'interval' milliseconds.
> So a single SetTimer causes many messages until you call KillTimer.
> So you don't need to restart it multiple times,
> just skip ones which come too frequent.

Yes, I understand that.   Stopping the timer makes it easier when 
debugging.  If I am single stepping through a program, I don't want 
the timer to keep causing a thread switch to the other thread in the 
program I am debugging.


Russell

> ----- Original Message ----- 
> From: "Russell Lang" <[email protected]>
> To: <[email protected]>
> Cc: "Ray Johnston" <[email protected]>
> Sent: Friday, July 09, 2004 2:57 PM
> Subject: Re: [gs-code-review] Progressive updates for image window of Windowsconsole app
> 
> 
> > Ray,
> > 
> > I've modified the code to use a timer as preferred by Igor.
> > The code is cleaner.
> > 
> > Log Message:
> > Enable progressive updates for the image window of
> > the MS-Windows ghostscript command line program.
> > Change update method to use a Windows timer.
> > 
> > DETAILS:
> > In the MS-Windows GUI executable (i.e. text window managed by 
> > ghostscript rather than using a system console window), the image 
> > window is progressively updated every second or so as each page 
> > is drawn.  The console executable did not do this.
> > 
> > The update code for both GUI and console executables is changed
> > to use a timer.  The display update code is called whenever 
> > particular device drawing operations happen, and if a timer is 
> > not already running, it starts a timer.  If the timer elapses, 
> > a sync is generated to refresh the window.  The timer is stopped 
> > by any sync, either from the timer or GS.
> > The update interval is shortened from 1s to 100ms, 
> > and this is adjusted to make sure updates account for
> > no more than 10% of processing.
> > 
> > 
> > > Date: Fri, 09 Jul 2004 00:24:49 -0700
> > > From: Ray Johnston <[email protected]>
> > > To: [email protected]
> > > CC: [email protected]
> > > Subject: Re: [gs-code-review] Progressive updates for image window of Windows
> > > 
> > > Russell,
> > > 
> > > I think that this approach is fine. I realize that Igor's
> > > suggestion of a timer is more elegant, but don't want to place
> > > a burden on you (as the primary developer of the display code).
> > > 
> > > If Igor wants to submit a patch to change to the timer, more
> > > power to him. I expect him to put that to code-review so you
> > > will have a chance to comment.
> > > 
> > > One request (if you feel like it) is to change the minimum
> > > update time to 0.2 seconds -- which requires using wMilliseconds
> > > from the SYSTEMTIME. I had done this patch a while ago locally,
> > > but lost it when it got too outdated.
> > > 
> > > If you want to make the additional change to 0.2 millisecond
> > > minimum time update, please go ahead and commit after testing
> > > (I don't think additional c-r is needed). Otherwise commit as
> > > is (and maybe I will change to 0.2 second updates)
> > > 
> > > Regards,
> > > Ray
> > 
> > 
> > 
> 
> 
> --------------------------------------------------------------------------------
> 
> 
> > diff -u l:/cvs/gs/src/dwimg.c src/dwimg.c
> > --- l:/cvs/gs/src/dwimg.c Sat Jul 03 10:51:14 2004
> > +++ src/dwimg.c Fri Jul 09 10:49:35 2004
> > @@ -123,8 +123,7 @@
> >   img->handle = handle;
> >   img->device = device;
> >  
> > - img->update_interval = 1;
> > - memset(&img->update_time, 0, sizeof(img->update_time));
> > + img->update_interval = 100; /* milliseconds */
> >  
> >          img->hmutex = INVALID_HANDLE_VALUE;
> >  
> > @@ -528,50 +527,46 @@
> >  void
> >  image_poll(IMAGE *img)
> >  {
> > -    /* Update the display periodically while Ghostscript is drawing */
> > -    SYSTEMTIME t1;
> > -    SYSTEMTIME t2;
> > -    int delta;
> >      if ((img->bmih.biWidth == 0) || (img->bmih.biHeight == 0))
> >   return;
> > -
> > -    GetSystemTime(&t1);
> > -    delta = (t1.wSecond - img->update_time.wSecond) +
> > - (t1.wMinute - img->update_time.wMinute) * 60 +
> > - (t1.wHour - img->update_time.wHour) * 3600;
> > -    if (img->update_interval < 1)
> > - img->update_interval = 1; /* seconds */
> > -    if (delta < 0)
> > -        img->update_time = t1;
> > -    else if (delta > img->update_interval) {
> > - /* redraw window */
> > - image_sync(img);
> > -
> > - /* Make sure the update interval is at least 10 times
> > - * what it takes to paint the window
> > - */
> > - GetSystemTime(&t2);
> > - delta = (t2.wSecond - t1.wSecond)*1000 + 
> > - (t2.wMilliseconds - t1.wMilliseconds);
> > - if (delta < 0)
> > -     delta += 60000; /* delta = time to redraw */
> > - if (delta > img->update_interval * 100)
> > -     img->update_interval = delta/100;
> > -        img->update_time = t2;
> > +    img->pending_update = 1;
> > +    if (img->update_timer == 0) {
> > + img->update_timer = 1;
> > + SetTimer(img->hwnd, img->update_timer, img->update_interval, NULL);
> >      }
> >  }
> >  
> >  void
> >  image_sync(IMAGE *img)
> >  {
> > +    SYSTEMTIME t1;
> > +    SYSTEMTIME t2;
> > +    int delta;
> > +    if (img->update_timer) {
> > + KillTimer(img->hwnd, img->update_timer);
> > +        img->update_timer = 0;
> > +    }
> > +    img->pending_sync = 0;
> >      if ( !IsWindow(img->hwnd) ) /* some clod closed the window */
> >   create_window(img);
> >  
> >      if ( !IsIconic(img->hwnd) ) {  /* redraw window */
> > + GetSystemTime(&t1);
> >   InvalidateRect(img->hwnd, NULL, 1);
> >   UpdateWindow(img->hwnd);
> > + GetSystemTime(&t2);
> > + /* Make sure the update interval is at least 10 times
> > + * what it takes to paint the window
> > + */
> > + delta = (t2.wSecond - t1.wSecond)*1000 + 
> > + (t2.wMilliseconds - t1.wMilliseconds);
> > + if (delta < 0)
> > +     delta += 60000;
> > + if (delta*10 > img->update_interval)
> > +     img->update_interval = delta*10;
> >      }
> >      image_separations(img);
> > +    img->pending_update = 0;
> >  }
> >  
> >  
> > @@ -1300,6 +1295,9 @@
> >       WriteConsoleInput(hStdin, &ir, 1, &dwWritten); 
> >       }
> >       return 0;
> > + case WM_TIMER:
> > +     image_sync(img);
> > +     return 0;
> >   case WM_PAINT:
> >       {
> >       int sx,sy,wx,wy,dx,dy;
> > diff -u l:/cvs/gs/src/dwimg.h src/dwimg.h
> > --- l:/cvs/gs/src/dwimg.h Sat Jul 03 10:51:14 2004
> > +++ src/dwimg.h Fri Jul 09 10:03:14 2004
> > @@ -51,8 +51,10 @@
> >      IMAGE_DEVICEN devicen[IMAGE_DEVICEN_MAX];
> >  
> >      /* periodic redrawing */
> > -    SYSTEMTIME update_time;