Bug 120329 - The datawizard should remember the selection order
Summary: The datawizard should remember the selection order
Status: RESOLVED FIXED
Alias: None
Product: kst
Classification: Applications
Component: general (show other bugs)
Version: 1.x
Platform: unspecified Solaris
: HI wishlist
Target Milestone: ---
Assignee: kst
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2006-01-17 17:09 UTC by Nicolas Brisset
Modified: 2006-04-11 02:11 UTC (History)
0 users

See Also:
Latest Commit:
Version Fixed In:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Nicolas Brisset 2006-01-17 17:09:47 UTC
Version:           1.2.0_devel (using KDE 3.4.0, compiled sources)
Compiler:          gcc version 3.4.3
OS:                SunOS (sun4u) release 5.8

When you create a new visualization with the datawizard, it might happen that you take some time to think carefully about the variables you want to plot and their order, and select them accordingly...
Then you are bound to be disappointed when you notice that the wizard generates curves/plots according to its own rules :-(
Even though the layout mode allows to fix the resulting disorder quickly, it would be very nice to solve the problem "upstream" by remembering the order in which Y variables are selected and reusing it subsequently when creating the curves.
Comment 1 Andrew Walker 2006-01-19 02:26:07 UTC
I suspect that if we want to do something like this we'd have to move away from a simple field list to something more like how the curve lists in the plot dialog are handled: i.e. a list of Available and Used fields.
Comment 2 Andrew Walker 2006-01-19 21:15:12 UTC
See also the comments attached to 120330
Comment 3 Netterfield 2006-01-23 19:55:47 UTC
SVN commit 501719 by netterfield:

BUG: 95029
BUG: 116460
CCBUG: 120329
Rework the cleanup layout system to be a lot smarter: existing layout
should now be respected a lot more.
(some of the referenced bugs were already closed, but I am 
re-closing them).

-Place plots into a window in the order they are listed...  Previously
 they would be placed randomly.

-If there are non-dirty plots in the window, then "Default tile"
 guesses the current number of columns based on the width of plots
 already in the image, rather than using sqrt(NC).  If there are no
 clean plots, then it uses sqrt(NC) as before.

-Keep plots in the tile position closest to the current plot position.
 If there would be two in the same tile, move the further to the next
 unfilled tile location.  This way empty tiles in the middle of the
 layout stay empty if there is nothing to fill it.

-If some of the plots have their axis supressed, resize them to make
 the size of the plot area more approximately equal.




 M  +24 -0     kst2dplot.cpp  
 M  +3 -0      kst2dplot.h  
 M  +121 -38   kstviewobject.cpp  
 M  +6 -0      kstviewobject.h  


--- trunk/extragear/graphics/kst/kst/kst2dplot.cpp #501718:501719
@@ -6548,6 +6548,30 @@
                PlotRegion.height());
 }
 
+double Kst2DPlot::verticalSizeFactor() {
+  // roughtly, geometry/dataRect.  But use an estimate rather
+  // than calculating it, so that we get stable results.
+  // this is used by cleanup to do a better job with
+  // sizing plots with supressed borders.
+  double f = 1.3;
+  if (_suppressTop) 
+    f -= 0.1;
+  if (_suppressBottom) 
+    f -= 0.2;
+  
+  return f;
+}
+
+double Kst2DPlot::horizontalSizeFactor() {
+  double f = 1.3;
+  if (_suppressLeft) 
+    f -= 0.25;
+  if (_suppressRight) 
+    f -= 0.05;
+
+  return f;
+}
+
 namespace {
 KstViewObject *create_Kst2DPlot() {
   return new Kst2DPlot;
--- trunk/extragear/graphics/kst/kst/kst2dplot.h #501718:501719
@@ -344,6 +344,9 @@
   
   virtual QRect contentsRect() const;
 
+  double verticalSizeFactor();
+  double horizontalSizeFactor();
+  
 signals:
   void modified();
 
--- trunk/extragear/graphics/kst/kst/kstviewobject.cpp #501718:501719
@@ -648,24 +648,36 @@
 
 void KstViewObject::cleanup(int cols) {
   KstViewObjectList childrenCopy;
-
+  double ave_w = 0;
+  bool dirty = false;
+  
   for (KstViewObjectList::ConstIterator i = _children.begin(); i != _children.end(); ++i) {
     if ((*i)->followsFlow()) {
       childrenCopy.append(*i);
+      ave_w += (*i)->aspectRatio().w;
     }
   }
 
   int cnt = childrenCopy.count();
+  
   if (cnt < 1) {
     return;
   }
-
+  ave_w/=double(cnt);
+  
   // FIXME: don't allow regrid to a number of columns that will result in
   //        >= height() plots in a column
-  if (!_onGrid) {
-    if (cols <= 0) {
+  if (cols <= 0) {
+    if (ave_w>0) { // guess current column alignment based on the average width of existing plots
+      cols = int(1.0/ave_w+0.5); 
+      if (cols>cnt) {
+        cols = int(sqrt(cnt));
+      }
+    } else {
       cols = int(sqrt(cnt));
     }
+  }
+  if (!_onGrid) {
     _onGrid = true;
     _columns = QMAX(1, cols);
   } else {
@@ -686,51 +698,115 @@
   //  from left-to-right and top-to-bottom based on the position
   //  of their top-left corner.
   //
-  double minDistance = 0.0;
-  int pos = 0;
-  int x = 0;
-  int y = 0;
-  int w = _geom.width() / _columns;
-  int h = _geom.height() / rows;
 
   //kstdDebug() << "cleanup with w=" << w << " and h=" << h << endl;
   //kstdDebug() << "columns=" << _columns  << endl;
-  for (int i = 0; i < cnt; ++i) {
-    KstViewObjectList::Iterator nearest = childrenCopy.end();
-    QPoint pt(x, y);
-    QSize sz(w, h);
+  int plotLoc[rows*_columns]; // what plot lives at each grid location
+  int unAssigned[cnt]; // what plots haven't got a home yet?
+  int n_unassigned = 0;
+  int r,c, C;
+  for (int i=0; i<rows*_columns; i++) {
+    plotLoc[i] = -1;
+  }
 
-    // adjust the last column to be sure that we don't spill over
-    if (pos % _columns == _columns - 1) {
-      sz.setWidth(_geom.width() - x);
-    }
+  // put the plots on a grid.  Each plot goes to the closest grid location
+  // unless there is another plot which is closer, in which case the 
+  // plot gets dumped into an un-assigned list for placement into a
+  // random un-filled grid space.
+  // the Location is defined relative to the top left corner.
+  // QUESTION: should we use the center instead?
+  // NOTE: the choice of grid location assumes a regular grid, which is
+  // broken when surpressed axis/labels is taken into account.  This
+  // could have an effect if the plots are grown by >50%.  Using the center
+  // instead of the top left would reduce this...
+  for (int i=0; i<cnt; i++) {
+    //r = int(childrenCopy[i]->aspectRatio().y*rows+0.5);
+    r = int((childrenCopy[i]->aspectRatio().y+childrenCopy[i]->aspectRatio().h/2)*rows);
+    c = int(childrenCopy[i]->aspectRatio().x*_columns+0.5);
 
-    // adjust the last row to be sure that we don't spill over
-    if (pos / _columns == rows - 1) {
-      sz.setHeight(_geom.height() - y);
-    }
+    if (c>=_columns) c = _columns-1;
+    if (r>=rows) r = rows-1;
+    printf("%s %d %d %d\n", childrenCopy[i]->tagName().latin1(), childrenCopy[i]->dirty(), r,c);
+    C = c + r*_columns;
+    if (childrenCopy[i]->dirty()) { // newly added plots get no priority
+      dirty = true;
+      unAssigned[n_unassigned] = i;
+      n_unassigned++;
+    } else if (plotLoc[C]<0) {
+      plotLoc[C] = i;
+    } else { // another plot is already at this grid point
 
-    for (KstViewObjectList::Iterator it = childrenCopy.begin(); it != childrenCopy.end(); ++it) {
-      double distance = rows * d2i(ceil(double(rows) * 2.0 * (*it)->aspectRatio().y)) + (*it)->aspectRatio().x * rows + (*it)->aspectRatio().y;
-
-      if (it == childrenCopy.begin() || distance < minDistance) {
-        minDistance = distance;
-        nearest = it;
+      double d1, d2;
+      // put the further of the two in the unassigned list
+      d1 = fabs(double(r) - childrenCopy[i]->aspectRatio().y*rows) + 
+          fabs(double(c) - childrenCopy[i]->aspectRatio().x*_columns);
+      d2 = fabs(double(r) - childrenCopy[plotLoc[C]]->aspectRatio().y*rows) + 
+          fabs(double(c) - childrenCopy[plotLoc[C]]->aspectRatio().x*_columns);      
+      if (d1>=d2) { 
+        unAssigned[n_unassigned] = i;
+      } else {
+        unAssigned[n_unassigned] = plotLoc[C];
+        plotLoc[C] = i;
       }
+      n_unassigned++;
     }
+  }
+  // now dump the unassigned plots in random holes.
+  // FIXME: instead, dump them in the closest hole.
+  C = 0;
+  for (int i=0; i<n_unassigned; i++) {
+    for (;plotLoc[C]!=-1; C++){};
+    plotLoc[C] = unAssigned[i];
+  }
 
-    if (nearest != childrenCopy.end()) {
-      KstViewObjectPtr vop = *nearest;
-      vop->move(pt);
-      vop->resize(sz);
-      childrenCopy.remove(vop);
+  double HR[rows], sum_HR=0;
+  KstViewObject *ob;
+  double hr;
+  for (r=0; r<rows; r++) {
+    HR[r] = 10.0;
+    for (c=0; c<_columns; c++) {
+      C = c + r*_columns;
+      if (plotLoc[C]>-1) {
+        hr = childrenCopy[plotLoc[C]]->verticalSizeFactor();
+        if (hr < HR[r]) {
+          HR[r] = hr;
+        }
+      }
     }
+    if (HR[r]>9.0) HR[r] = 1.0;
+    sum_HR+= HR[r];
+  }
 
-    if (++pos % _columns == 0) {
-      x = 0;
-      y += h;
-    } else {
-      x += w;
+  // now actually move/resize the plots
+  int w = _geom.width() / _columns;
+  int h = 0;
+  int y=0;
+  for (r=0; r<rows; r++) {
+    y+=h;
+    //h = _geom.height() / rows;
+    h = int(double(_geom.height()) * HR[r]/sum_HR);
+    for (c=0; c<_columns; c++) {
+      C = c + r*_columns;
+      if (plotLoc[C] >=0) {
+        QSize sz(w, h);
+        r = C/_columns;
+        c = C%_columns;
+        QPoint pt(w*c, y);
+
+        // adjust the last column to be sure that we don't spill over
+        if (c == _columns-1) {
+          sz.setWidth(_geom.width() - w*c);
+        }
+
+        // adjust the last row to be sure that we don't spill over
+        if (r == rows - 1) {
+          sz.setHeight(_geom.height() - y);
+        }
+
+        ob = childrenCopy[plotLoc[C]];
+        ob->move(pt);
+        ob->resize(sz);
+      }
     }
   }
 }
@@ -936,7 +1012,14 @@
   setContentsRect(rect);
 }
 
+double KstViewObject::verticalSizeFactor() {
+  return 1.0;
+}
 
+double KstViewObject::horizontalSizeFactor() {
+  return 1.0;
+}
+
 QString KstViewObject::menuTitle() const {
   return tagName();
 }
--- trunk/extragear/graphics/kst/kst/kstviewobject.h #501718:501719
@@ -101,6 +101,12 @@
     // arrows relative to the meaningful contents of the object.
     virtual QRect dataRect() const;
     virtual void setDataRect(const QRect& rect);
+
+    // with cleanup, use this factor to decide if more space should be used
+    // in the row.  1.0 means that the dataRect = geometry.  >1 means dataRect < geometry.
+    virtual double verticalSizeFactor();
+    virtual double horizontalSizeFactor();
+    
     virtual void move(const QPoint& to);
 
     // Draw a focus highlight
Comment 4 Andrew Walker 2006-01-26 19:51:37 UTC
Should be fixed for 1.2.1 release
Comment 5 Andrew Walker 2006-03-21 23:12:36 UTC
SVN commit 521207 by arwalker:

CCBUG:120329 CCBUG:120330 First draft of revised data wizard.

 M  +1 -1      Makefile.am  
 M  +324 -270  datawizard.ui  
 M  +3 -3      kst.cpp  
 A             kstdatawizard_i.cpp   [License: GPL (v2+)]
 A             kstdatawizard_i.h   [License: GPL (v2+)]
Comment 6 Andrew Walker 2006-03-21 23:48:13 UTC
The selection order can now be defined by the user and is remembered during curve creation.
Comment 7 George Staikos 2006-03-22 00:06:17 UTC
Reopened until the patch is fixed.
Comment 8 Nicolas Brisset 2006-04-04 23:36:55 UTC
It would be nice to close this and bug #120330, as the code seems to be written and it is just a matter of where to put it. If we don't do it now, there may appear conflicts later on...
Comment 9 George Staikos 2006-04-11 02:11:36 UTC
SVN commit 528439 by staikos:

two patches that I'm stuck committing together:

1) remove KstPoint and make the two functions namespaced (KstCurvePointSymbol)
2) merge back the DataWizard patch by Andrew, with a few bugs fixed and a few
bugs added (UI related)

We decided not to keep the x/y selected label.

BUG: 120329, 120330


 M  +0 -2      extensions/js/bind_point.h  
 M  +435 -169  libkstapp/datawizard.ui  
 M  +265 -189  libkstapp/datawizard.ui.h  
 M  +3 -3      libkstapp/kstcurvedialog_i.cpp  
 M  +1 -1      libkstapp/kstcurvedifferentiate_i.cpp  
 M  +1 -1      libkstapp/ksteqdialog_i.cpp  
 M  +1 -1      libkstapp/kstfilterdialog_i.cpp  
 M  +1 -1      libkstapp/kstfitdialog_i.cpp  
 M  +1 -1      libkstapp/ksthsdialog_i.cpp  
 M  +1 -1      libkstapp/kstpsddialog_i.cpp  
 M  +1 -1      libkstmath/Makefile.am  
 A             libkstmath/kstcurvepointsymbol.cpp   libkstmath/kstpoint.cpp#526781 [License: GPL (v2+)]
 A             libkstmath/kstcurvepointsymbol.h   libkstmath/kstpoint.h#526781 [License: GPL (v2+)]
 D             libkstmath/kstpoint.cpp  
 D             libkstmath/kstpoint.h  
 M  +13 -15    libkstmath/kstvcurve.cpp  
 M  +4 -4      libkstmath/kstvcurve.h  
 M  +1 -1      widgets/curveappearancewidget.ui  
 M  +3 -8      widgets/curveappearancewidget.ui.h  
 M  +1 -1      widgets/datarangewidget.ui