Bug 123576

Created attachment 15099 [details]
Example of mis-aligned y-axes

Created attachment 15123 [details]
Proposed patch

Makes code slightly more legible also.

Two questions:
1) Why is the comment removed?
2) Is there any way to do this without setting dirty?  We shouldn't have to 
repaint all plots if no alignment changes.

Also that extra ; should be removed.  Those things are great at causing bugs, 
and I suspect that strict gcc 4.x may not compile with it.

1) Because the comment was written for an older version of this code. When Barth rewrote it to reflect how he though it should behave the comment was (incorrectly) left.
2) Probably not a trivial determination to check if the alignment has changed.

On Tuesday 14 March 2006 18:23, Andrew Walker wrote:

> ------- 1) Because the comment was written for an older version of this
> code. When Barth rewrote it to reflect how he though it should behave the
> comment was (incorrectly) left. 2) Probably not a trivial determination to
> check if the alignment has changed.


  Ok, we should leave a comment there to remind us to make a way to update 
based on alignment changes at some point.  The rest is fine.  (and the 
extra ; should still be removed but I can do that after)

SVN commit 518727 by arwalker:

BUG:123576 Ensure y-axes are aligned

 M  +25 -26    kstviewobject.cpp  


--- trunk/extragear/graphics/kst/src/libkstapp/kstviewobject.cpp #518726:518727
@@ -680,7 +680,7 @@
 
 void KstViewObject::cleanup(int cols) {
   KstViewObjectList childrenCopy;
-  double ave_w = 0;
+  double ave_w = 0.0;
   bool dirty = false;
   
   for (KstViewObjectList::ConstIterator i = _children.begin(); i != _children.end(); ++i) {
@@ -695,7 +695,7 @@
   if (cnt < 1) {
     return;
   }
-  ave_w/=double(cnt);
+  ave_w /= double(cnt);
   
   // FIXME: don't allow regrid to a number of columns that will result in
   //        >= height() plots in a column
@@ -721,19 +721,10 @@
   }
   int rows = ( cnt + _columns - 1 ) / _columns;
 
-  //
-  // the following is an attempt to arrange objects on a grid.
-  //  This should behave as both a snap-to-grid when the objects
-  //  are already roughly aligned to the desired grid, but should
-  //  also act to retain the inherent order when this is not the
-  //  case. The inherent order is defined by identifying objects
-  //  from left-to-right and top-to-bottom based on the position
-  //  of their top-left corner.
-  //
   QMemArray<int> plotLoc(rows*_columns); // what plot lives at each grid location
   QMemArray<int> unAssigned(cnt); // what plots haven't got a home yet?
   int n_unassigned = 0;
-  int r,c, CR;
+  int r, c, CR;
   for (int i=0; i<rows*_columns; i++) {
     plotLoc[i] = -1;
   }
@@ -752,14 +743,18 @@
     r = int((childrenCopy[i]->aspectRatio().y+childrenCopy[i]->aspectRatio().h/2)*rows); // use center
     c = int(childrenCopy[i]->aspectRatio().x*_columns+0.5);
 
-    if (c>=_columns) c = _columns-1;
-    if (r>=rows) r = rows-1;
+    if (c >= _columns) {
+      c = _columns-1;
+    }
+    if (r >= rows) {
+      r = rows-1;
+    }
     CR = c + r*_columns;
     if (childrenCopy[i]->dirty()) { // newly added plots get no priority
       dirty = true;
       unAssigned[n_unassigned] = i;
       n_unassigned++;
-    } else if (plotLoc[CR]<0) {
+    } else if (plotLoc[CR] < 0) {
       plotLoc[CR] = i;
     } else { // another plot is already at this grid point
 
@@ -770,7 +765,7 @@
           fabs(double(c) - childrenCopy[i]->aspectRatio().x*_columns);
       d2 = fabs(double(r) - childrenCopy[plotLoc[CR]]->aspectRatio().y*rows) + 
           fabs(double(c) - childrenCopy[plotLoc[CR]]->aspectRatio().x*_columns);      
-      if (d1>=d2) { 
+      if (d1 >= d2) { 
         unAssigned[n_unassigned] = i;
       } else {
         unAssigned[n_unassigned] = plotLoc[CR];
@@ -783,43 +778,46 @@
   // Question: should be dump them in the closest holes?
   CR = 0;
   for (int i=0; i<n_unassigned; i++) {
-    for (;plotLoc[CR]!=-1; CR++){};
+    for (;plotLoc[CR]!=-1; CR++) { }
     plotLoc[CR] = unAssigned[i];
   }
 
   QMemArray<double> HR(rows);
-  double sum_HR=0;
+  double sum_HR = 0.0;
   KstViewObject *ob;
   double hr;
+  
   for (r=0; r<rows; r++) {
     HR[r] = 10.0;
     for (c=0; c<_columns; c++) {
       CR = c + r*_columns;
-      if (plotLoc[CR]>-1) {
+      if (plotLoc[CR] > -1) {
         hr = childrenCopy[plotLoc[CR]]->verticalSizeFactor();
         if (hr < HR[r]) {
           HR[r] = hr;
         }
       }
     }
-    if (HR[r]>9.0) HR[r] = 1.0;
-    sum_HR+= HR[r];
+    if (HR[r] > 9.0) {
+      HR[r] = 1.0;
+    }
+    sum_HR += HR[r];
   }
 
   // now actually move/resize the plots
   int w = _geom.width() / _columns;
   int h = 0;
-  int y=0;
+  int y = 0;
   for (r=0; r<rows; r++) {
-    y+=h;
+    y += h;
     //h = _geom.height() / rows;
     h = int(double(_geom.height()) * HR[r]/sum_HR);
     for (c=0; c<_columns; c++) {
       CR = c + r*_columns;
-      if (plotLoc[CR] >=0) {
+      if (plotLoc[CR] >= 0) {
         QSize sz(w, h);
-        r = CR/_columns;
-        c = CR%_columns;
+        r = CR / _columns;
+        c = CR % _columns;
         QPoint pt(w*c, y);
 
         // adjust the last column to be sure that we don't spill over
@@ -835,6 +833,7 @@
         ob = childrenCopy[plotLoc[CR]];
         ob->move(pt);
         ob->resize(sz);
+        ob->setDirty();
       }
     }
   }