Bug 123576 - Y-axes are not aligned after Cleanup Layout operation
Summary: Y-axes are not aligned after Cleanup Layout operation
Status: RESOLVED FIXED
Alias: None
Product: kst
Classification: Applications
Component: general (show other bugs)
Version: 1.x
Platform: Compiled Sources Linux
: NOR normal
Target Milestone: ---
Assignee: kst
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2006-03-13 22:55 UTC by Andrew Walker
Modified: 2006-03-15 01:36 UTC (History)
0 users

See Also:
Latest Commit:
Version Fixed In:


Attachments
Example of mis-aligned y-axes (36.35 KB, image/png)
2006-03-13 22:56 UTC, Andrew Walker
Details
Proposed patch (5.73 KB, patch)
2006-03-14 23:59 UTC, Andrew Walker
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Andrew Walker 2006-03-13 22:55:25 UTC
Version:           HEAD (using KDE KDE 3.5.0)
Installed from:    Compiled From Sources
OS:                Linux

PROBLEM:

In some cases the y-axes of plots are not aligned as they should be, following a Cleanup Layout operation.

STEPS TO REPRODUCE:

Start Kst
Create a number of plots within the same window
Ensure the y-axis of each plot starts at a discernibly different location on each plot
Select the Cleanup Layout...Default Tile option

RESULTS:

The y-axes are not always aligned for plots that use the same horizontal region of the window

EXPECTED RESULTS:

The y-axes should be aligned when appropriate
Comment 1 Andrew Walker 2006-03-13 22:56:43 UTC
Created attachment 15099 [details]
Example of mis-aligned y-axes
Comment 2 Andrew Walker 2006-03-14 23:59:51 UTC
Created attachment 15123 [details]
Proposed patch

Makes code slightly more legible also.
Comment 3 George Staikos 2006-03-15 00:10:42 UTC
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.
Comment 4 Andrew Walker 2006-03-15 00:23:52 UTC
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.
Comment 5 George Staikos 2006-03-15 00:45:04 UTC
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)
Comment 6 Andrew Walker 2006-03-15 01:36:14 UTC
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();
       }
     }
   }