Bug 135381

Hmm, yeah heh.

SVN commit 608911 by johnflux:

Number of horizontal lines is off by one, plus it no longer tries to 'smart' about showing the top bar, plus fix integer rounding errors (   a*b/c  is better than a*(b/c)  
when using ints! )

CCBUG:135381


 M  +2 -2      FancyPlotterSettings.cc  
 M  +23 -14    SignalPlotter.cc  


--- branches/KDE/3.5/kdebase/ksysguard/gui/SensorDisplayLib/FancyPlotterSettings.cc #608910:608911
@@ -155,8 +155,8 @@
   boxLayout->addWidget( label, 1, 2 );
 
   mHorizontalLinesCount = new KIntNumInput( 0, groupBox );
-  mHorizontalLinesCount->setMinValue( 2 );
-  mHorizontalLinesCount->setMaxValue( 20 );
+  mHorizontalLinesCount->setMinValue( 1 );
+  mHorizontalLinesCount->setMaxValue( 100 );
   QWhatsThis::add( mHorizontalLinesCount, i18n( "Enter the number of horizontal lines here." ) );
   boxLayout->addWidget( mHorizontalLinesCount , 1, 3 );
   label->setBuddy( mHorizontalLinesCount );
--- branches/KDE/3.5/kdebase/ksysguard/gui/SensorDisplayLib/SignalPlotter.cc #608910:608911
@@ -429,21 +429,21 @@
         range = 1.0;
     }
     // Massage the range so that the grid shows some nice values.
-    double step = range / mHorizontalLinesCount;
+    double step = range / (mHorizontalLinesCount+1);
     double dim = pow( 10, floor( log10( step ) ) ) / 2;
-    range = dim * ceil( step / dim ) * mHorizontalLinesCount;
+    range = dim * ceil( step / dim ) * (mHorizontalLinesCount+1);
   }
   double maxValue = minValue + range;
 
   int top = 0;
-  if ( mShowTopBar && h > ( mFontSize + 2 + mHorizontalLinesCount * 10 ) ) {
+  if ( mShowTopBar && h > ( mFontSize/*top bar size*/ + 2/*padding*/ +5/*smallest reasonable size for a graph*/ ) ) {
     /* Draw horizontal bar with current sensor values at top of display. */
     p.setPen( mHorizontalLinesColor );
     int x0 = w / 2;
     p.setFont( QFont( p.font().family(), mFontSize ) );
     top = p.fontMetrics().height();
     h -= top;
-    int h0 = top - 2;
+    int h0 = top - 2; // h0 is our new top.  It's at least 5 pixels high
     p.drawText(0, 0, x0, top - 2, Qt::AlignCenter, mTitle );
 
     p.drawLine( x0 - 1, 1, x0 - 1, h0 );
@@ -607,26 +607,35 @@
     delete[] prevVals;
   }
 
-  /* Draw horizontal lines and values. Lines are drawn when the
-   * height is greater than 10 times hCount + 1, values are shown
-   * when width is greater than 60 */
-  if ( mShowHorizontalLines && h > ( 10 * ( mHorizontalLinesCount + 1 ) ) ) {
+  /* Draw horizontal lines and values. Lines are always drawn.
+   * Values are only draw when width is greater than 60 */
+  if ( mShowHorizontalLines ) {
     p.setPen( mHorizontalLinesColor );
     p.setFont( QFont( p.font().family(), mFontSize ) );
     QString val;
-    for ( uint y = 1; y < mHorizontalLinesCount; y++ ) {
-      p.drawLine( 0, top + y * ( h / mHorizontalLinesCount ), w - 2,
-                  top + y * ( h / mHorizontalLinesCount ) );
+
+    /* top = 0 or  font.height    depending on whether there's a topbar or not
+     * h = graphing area.height   - i.e. the actual space we have to draw inside
+     *
+     * Note we are drawing from 0,0 as the top left corner.  So we have to add on top to get to the top of where we are drawing
+     * so top+h is the height of the widget
+     */ 
+    for ( uint y = 1; y <= mHorizontalLinesCount; y++ ) {
+
+      int y_coord =  top + (y * h) / (mHorizontalLinesCount+1);  //Make sure it's y*h first to avoid rounding bugs
+      p.drawLine( 0, y_coord, w - 2, y_coord );
+
       if ( mShowLabels && h > ( mFontSize + 1 ) * ( mHorizontalLinesCount + 1 )
            && w > 60 ) {
-        val = QString( "%1" ).arg( maxValue - y * ( range / mHorizontalLinesCount ) );
-        p.drawText( 6, top + y * ( h / mHorizontalLinesCount ) - 1, val );
+        val = QString::number(maxValue - (y * range) / (mHorizontalLinesCount+1 ) );
+        p.drawText( 6, y_coord  - 1, val );  //draw the text one pixel raised above the line
       }
     }
 
+    //Draw the bottom most (minimum) number as well
     if ( mShowLabels && h > ( mFontSize + 1 ) * ( mHorizontalLinesCount + 1 )
          && w > 60 ) {
-      val = QString( "%1" ).arg( minValue );
+      val = QString::number( minValue );
       p.drawText( 6, top + h - 2, val );
     }
   }

SVN commit 608912 by johnflux:

 Number of horizontal lines is off by one, plus it no longer tries to 'smart' about showing the top bar, plus fix integer rounding errors (   a*b/c  is better than 
a*(b/c) when using ints! ) 

This was fixed in kde 3.5 as well in commit revision 608911 so closing bug

BUG: 135381 



 M  +2 -2      FancyPlotterSettings.cc  
 M  +19 -11    SignalPlotter.cc  


--- trunk/KDE/kdebase/workspace/ksysguard/gui/SensorDisplayLib/FancyPlotterSettings.cc #608911:608912
@@ -166,8 +166,8 @@
   boxLayout->addWidget( label, 1, 2 );
 
   mHorizontalLinesCount = new KIntNumInput( 0, groupBox );
-  mHorizontalLinesCount->setMinimum( 2 );
-  mHorizontalLinesCount->setMaximum( 20 );
+  mHorizontalLinesCount->setMinimum( 1 );
+  mHorizontalLinesCount->setMaximum( 100 );
   mHorizontalLinesCount->setWhatsThis( i18n( "Enter the number of horizontal lines here." ) );
   boxLayout->addWidget( mHorizontalLinesCount , 1, 3 );
   label->setBuddy( mHorizontalLinesCount );
--- trunk/KDE/kdebase/workspace/ksysguard/gui/SensorDisplayLib/SignalPlotter.cc #608911:608912
@@ -434,9 +434,9 @@
         range = 1.0;
     }
     // Massage the range so that the grid shows some nice values.
-    double step = range / mHorizontalLinesCount;
+    double step = range / (mHorizontalLinesCount+1);
     double dim = pow( 10, floor( log10( step ) ) ) / 2;
-    range = dim * ceil( step / dim ) * mHorizontalLinesCount;
+    range = dim * ceil( step / dim ) * (mHorizontalLinesCount+1);
   }
   double maxValue = minValue + range;
 
@@ -445,7 +445,7 @@
     int x0 = w / 2;
     top = p.fontMetrics().height();
     //Before we continue, check if there's enough room.  So enough room for a bar at the top, plus horizontal lines each of a size with room for a scale
-    if( h <= (top + 2 + mHorizontalLinesCount * qMax(10, top)) ) {
+    if( h <= (top/*top bar size*/ + 2/*padding*/ +5/*smallest reasonable size for a graph*/ ) ) {
       top = 0;
     } else {
       h -= top;
@@ -644,16 +644,23 @@
     }
   }
 
-  /* Draw horizontal lines and values. Lines are drawn when the
-   * height is greater than 10 times hCount + 1, values are shown
-   * when width is greater than 60 */
+  /* Draw horizontal lines and values. Lines are always drawn.
+   * Values are only draw when width is greater than 60 */
   int fontheight = p.fontMetrics().height();
-  if ( mShowHorizontalLines && h > ( 10 * ( mHorizontalLinesCount + 1 ) ) ) { 
+  if ( mShowHorizontalLines ) { 
     QString val;
-    for ( uint y = 1; y < mHorizontalLinesCount; y++ ) {
+    /* top = 0 or  font.height    depending on whether there's a topbar or not
+     * h = graphing area.height   - i.e. the actual space we have to draw inside
+     *
+     * Note we are drawing from 0,0 as the top left corner.  So we have to add on top to get to the top of where we are drawing
+     * so top+h is the height of the widget
+     */
+
+    for ( uint y = 1; y <= mHorizontalLinesCount; y++ ) {
+      int y_coord =  top + (y * h) / (mHorizontalLinesCount+1);  //Make sure it's y*h first to avoid rounding bugs
+
       p.setPen( mHorizontalLinesColor );
-      p.drawLine( 0, top + y * ( h / mHorizontalLinesCount ), w - 2,
-                  top + y * ( h / mHorizontalLinesCount ) );
+      p.drawLine( 0, y_coord, w - 2, y_coord);
       if ( mShowLabels && h > ( fontheight + 1 ) * ( mHorizontalLinesCount + 1 )
            && w > 60 ) {
 	double value = (maxValue - y * ( range / mHorizontalLinesCount ))/mScaleDownBy;
@@ -661,10 +668,11 @@
         QString number = KGlobal::locale()->formatNumber( value, (value >= 100)?0:2);
         val = QString( "%1 %2" ).arg( number, mUnit );
         p.setPen( mFontColor );
-        p.drawText( 6, top + y * ( h / mHorizontalLinesCount ) - 2, val );
+        p.drawText( 6, y_coord - 2, val );
       }
     }
 
+    //Draw the bottom most (minimum) number as well
     if ( mShowLabels && h > ( fontheight + 1 ) * ( mHorizontalLinesCount + 1 )
          && w > 60 ) {
       int value = (int)(minValue / mScaleDownBy);

Thanks Chris Moore for reporting this.

High quality bug reports such as these really do make a huge difference.  

John Tapsell <johnflux@gmail.com> writes:

> Thanks Chris Moore for reporting this.
>
> High quality bug reports such as these really do make a huge
> difference.


Not a problem.  High quality fixes such as these make a huge
difference to whether I continue using the software or not.  I'm not
particularly fussed whether I use KDE or GNOME, but if I find bugs I
report against KDE get fixed and GNOME bugs don't, then I'm sure I'll
find myself running KDE most of the time (he says from his XFCE4
session)...