[comp.windows.x] Bug and improvement in xperfmon

brossard@litsun.epfl.ch (06/06/90)

RFC-822-Headers:
Received: from litsun12.epfl.ch by SIC.Epfl.CH via INTERNET ; Tue, 5 Jun 90 19:08:32 N

==================
			  X Window System Bug Report
			    xbugs@expo.lcs.mit.edu


VERSION:
    R4

CLIENT MACHINE and OPERATING SYSTEM (not relevant):
    Sun 3/60 running SunOS 4.0.3

DISPLAY TYPE (not relevant):
    Sun color and B&W

WINDOW MANAGER (not relevant):
    twm

AREA:
    xperfmon

SYNOPSIS:
    Two things are included here:
 1- Bug fix, when xperfmon refreshed its window, the number of values was
    erroneously increased by one
 2- Improvment: When a new value is above the previous maximum, update the
    maximum and cause a refresh.

DESCRIPTION:
    File: Perfmon.c, in subroutine refresh_graph, one point too many is
given to the for loop.  To illustrate, specify a slow update time and
cause a few refreshes, you'll see new (empty or repeat) point being
added every time there is a refresh.
 2-  When, for example, the Free memory increased above the old maximum, there
was no way to know how far above it went until the next display shift.  The
routine graph (in Perfmon.c) was changed to check for such a case.  If it
occurs, then it now causes a refresh the same way shift_graph does.
===>  This is done with XClearArea by asking it to generate an expose event
===> Isn't there a better way to generate such an event since the refresh
===> routine starts by doing a XClearWIndow which makes the XClearArea
===> redundant.
===> Also, while we are at it, when an expose event is received, there should
===> be a check to flush out any other expose event on the queue or maybe
===> just to ignore this one and just do the last one?


SAMPLE FIX:

	Note that there are two lines which are >80 characters, check
for a possible bitnet screw up (two lines instead of one).

					Alain Brossard
					brossard@litsun.epfl.ch
PS Anybody know why the free memory reported would be too low?  On a
Sun 3/260 running 4.0.3, top says used+free <<< real and xperfmon picks
up the same amount for free.  Kernel bug?

*** /tmp/,RCSt1a03838	Tue Jun  5 18:31:05 1990
--- Perfmon.c	Tue Jun  5 18:25:54 1990
***************
*** 1,6 ****
--- 1,18 ----
  /*
   * $XConsortium: Perfmon.c,v 1.3 89/09/19 14:44:04 jim Exp $
+  *
+  * $Log:	Perfmon.c,v $
+  * Revision 1.2  90/06/05  16:20:31  brossard
+  * Fixed refresh bug which increased the number of points it was
+  * supposed to draw everytime it refreshed the window.
+  * Now when adding a new point, it checks whether the scale of the graph should
+  * be changed to reflect a new maximum or a new minimum.
+  * The function that adds values to the graph shouldn`t also draw them.
+  * There should be two different functions so that redraw could just
+  * redraw without having to worry about reading in values and incrementing
+  * the count of them.  (Which caused the bug above).
   * 
+  * 
   * Perfmon Performance Monitor Widget
   *
   * Copyright 1989 PCS Computer Systeme GmbH, West Germany
***************
*** 587,603 ****
      *y = Max(tmp, graph_list[graph_num]->max_pix);
      /* If the choice of Max and Min look odd, just remember that pixel values
       * go up as actual values go down; i.e., min_pix is greater than max_pix.
!      */
! }
  
  static void
  graph(pw, vals)
      PerfmonWidget pw;
      int vals[];
! /* This routine adds the values that are given to it to the graph. */
  {
      int i;
      int x1, y1, x2, y2;
  
      Display *dpy = XtDisplay(pw);
      int screen = DefaultScreen(XtDisplay(pw));
--- 599,616 ----
      *y = Max(tmp, graph_list[graph_num]->max_pix);
      /* If the choice of Max and Min look odd, just remember that pixel values
       * go up as actual values go down; i.e., min_pix is greater than max_pix.
!      */ }
  
  static void
  graph(pw, vals)
      PerfmonWidget pw;
      int vals[];
! /* This routine adds the values that are given to it to the graph and
!  draw them. */
  {
      int i;
      int x1, y1, x2, y2;
+     Boolean	resize = FALSE;
  
      Display *dpy = XtDisplay(pw);
      int screen = DefaultScreen(XtDisplay(pw));
***************
*** 607,640 ****
      int NUM_VALS = DisplayWidth(dpy, screen);
  
      if (graph_data.g_width == 0) {
! 	 /* This means that the window has not been drawn yet. */
! 	 return;
      }
  
      for (i = 0; i < graph_set.num_graphs; i++) {
! 	 graph_data.saved_values[graph_data.cur_value][i] = vals[i];
! 	 val_to_pixels(pw, i, graph_data.cur_value, &x2, &y2);
! 	 if (graph_data.cur_value == 0)	{
! 	      /* If this is the leftmost point, then just plot it. */
! 	      XDrawPoint(XtDisplay(pw), XtWindow(pw), pw->perfmon.fgGC, x2, y2);
! 	 } else	{
! 	      /* If this is not the left most point, then draw a line
! 	       * connecting this point with the one to its left.
! 	       */
! 	      val_to_pixels(pw, i, (graph_data.cur_value - 1) % NUM_VALS,
  			    &x1, &y1);
! 	      XDrawLine(XtDisplay(pw), XtWindow(pw), pw->perfmon.fgGC,
  			x1, y1, x2, y2);
! 	 }
      }
      /* Check to see whether the graph needs to be shifted. */
!     if ((graph_data.cur_value + 1) * pw->perfmon.step_size > graph_data.g_width)
! 	 shift_graph(pw);
! 
!     /* Update the current value.  This needs to be done even when the
!      * graph is shifted, as all shift_graph does is move the pointers.
!      */
!     graph_data.cur_value++;
  }
  
  static void
--- 620,671 ----
      int NUM_VALS = DisplayWidth(dpy, screen);
  
      if (graph_data.g_width == 0) {
! 	/* This means that the window has not been drawn yet. */
! 	return;
      }
  
      for (i = 0; i < graph_set.num_graphs; i++) {
! 	graph_data.saved_values[graph_data.cur_value][i] = vals[i];
! 	if( vals[i] > graph_list[i]->max_value ){
! 	    graph_list[i]->max_value = vals[i];
! 	    resize = TRUE;
! 	} else if( vals[i] < graph_list[i]->min_value ){
! 	    graph_list[i]->min_value = vals[i];
! 	    resize = TRUE;
! 	} else if( !resize ) {	/* Don't bother drawing it if resizing */
! 	    val_to_pixels(pw, i, graph_data.cur_value, &x2, &y2);
! 	    if (graph_data.cur_value == 0)	{
! 		  /* If this is the leftmost point, then just plot it. */
! 		  XDrawPoint(XtDisplay(pw), XtWindow(pw), pw->perfmon.fgGC,
! 			x2, y2);
! 	    } else {
! 		  /* If this is not the left most point, then draw a line
! 		   * connecting this point with the one to its left.
! 		   */
! 		  val_to_pixels(pw, i, (graph_data.cur_value - 1) % NUM_VALS,
  			    &x1, &y1);
! 		  XDrawLine(XtDisplay(pw), XtWindow(pw), pw->perfmon.fgGC,
  			x1, y1, x2, y2);
! 	    }
! 	}
      }
      /* Check to see whether the graph needs to be shifted. */
!     if ((graph_data.cur_value + 1) * pw->perfmon.step_size > graph_data.g_width){
! 	shift_graph(pw);
! 	resize = FALSE;	/* already redrawn */
!     }
! /*
!  *	Generate an expose event and force a redraw
!  */
!     if( resize )
! 	XClearArea(XtDisplay(pw), XtWindow(pw), 0, 0,
! 	       graph_data.g_width, pw->core.height, True);
!     else
! 	/* Update the current value.  This needs to be done even when the
! 	 * graph is shifted, as all shift_graph does is move the pointers.
! 	 * The problem is that the increment is done one too many if we redraw
! 	 */
! 	graph_data.cur_value++;
  }
  
  static void
***************
*** 648,654 ****
  
      save_cur_value = graph_data.cur_value;
  
!     for (graph_data.cur_value = 0; graph_data.cur_value <= save_cur_value;)
  	 graph(pw, graph_data.saved_values[graph_data.cur_value]);
  }
  
--- 679,685 ----
  
      save_cur_value = graph_data.cur_value;
  
!     for (graph_data.cur_value = 0; graph_data.cur_value < save_cur_value;)
  	 graph(pw, graph_data.saved_values[graph_data.cur_value]);
  }