Bug 95029

I must admit I have not tried to look at what happens with grouped graphs or inset graphs, or other pretty "strange" things...

  This is a well-known issue.  The algorithm required is much more complex 
than what we presently do though.  I hope to tackle this soon.

CVS commit by arwalker: 

Use a smarter algorithm to do the re-gridding which moves plots to the grid based on the closest plot relative to their top-left corner.

CCMAIL: 95029-done@bugs.kde.org


  M +69 -44    kstviewobject.cpp   1.103


--- kdeextragear-2/kst/kst/kstviewobject.cpp  #1.102:1.103
@@ -21,4 +21,5 @@
 
 // include files for Qt
+#include <qdeepcopy.h>
 #include <qstylesheet.h>
 
@@ -30,4 +31,5 @@
 #include "kst.h"
 #include "kstdoc.h"
+#include "kstobject.h"
 #include "kstplotgroup.h"
 #include "kstsettings.h"
@@ -459,8 +461,5 @@ void KstViewObject::setOnGrid(bool on_gr
 
 void KstViewObject::cleanup(int cols) {
-  if (_children.count() < 1) {
-    return;
-  }
-
+  if (_children.count() > 0) {
   if (!_onGrid) {
     if (cols <= 0) {
@@ -477,4 +476,7 @@ void KstViewObject::cleanup(int cols) {
   }
 
+    KstViewObjectList childrenCopy;
+    double distance;
+    double minDistance = 0.0;
   int pos = 0;
   int x = 0;
@@ -484,21 +486,43 @@ void KstViewObject::cleanup(int cols) {
   int h = _geom.height() / (lastRow + (_children.count() % _columns > 0 ? 1 : 0));
 
+    childrenCopy = QDeepCopy<KstViewObjectList>(_children);
+
   //kdDebug() << "cleanup with w=" << w << " and h=" << h << endl;
   //kdDebug() << "columns=" << _columns  << endl;
-  for (KstViewObjectList::Iterator i = _children.begin(); i != _children.end(); ++i) {
-    QSize sz(w, h);
+    for (unsigned i = 0; i < _children.count(); ++i) {
+      KstViewObjectList::Iterator nearest = childrenCopy.end();
     QPoint pt(x, y);
-    // Adjust the last column to be sure that we don't spill over
+      QSize sz(w, h);
+
+      //
+      // adjust the last column to be sure that we don't spill over
+      //
     if (pos % _columns == _columns - 1) {
       sz.setWidth(_geom.width() - x);
     }
 
-    // Adjust the last row to be sure that we don't spill over
+      //
+      // adjust the last row to be sure that we don't spill over
+      //
     if ((pos + 1) / _columns > lastRow) {
       sz.setHeight(_geom.height() - y);
     }
 
-    (*i)->move(pt);
-    (*i)->resize(sz);
+      for (KstViewObjectList::Iterator it = childrenCopy.begin(); it != childrenCopy.end(); ++it) {
+        //
+        // find the plot closest to the desired position, based on the top-left corner...
+        //
+        distance  = (double)(( x - (*it)->geometry().x() ) * ( x - (*it)->geometry().x() ));
+        distance += (double)(( y - (*it)->geometry().y() ) * ( y - (*it)->geometry().y() ));
+        if( it == childrenCopy.begin() || distance < minDistance ) {
+          minDistance = distance;
+          nearest = it;
+        }
+      }
+      if (nearest != childrenCopy.end()) {
+        (*nearest)->move(pt);
+        (*nearest)->resize(sz);
+        childrenCopy.remove(*nearest);
+      }
 
     if (++pos % _columns == 0) {
@@ -509,4 +533,5 @@ void KstViewObject::cleanup(int cols) {
     }
   }
+  }
 }
 

kst seems to have reverted to having this behavior again.  

I'm not able to reproduce this. I find that as long as I roughly position the plots where I want them to be then Cleanup Layout will snap them to a grid as I expect. Could you please provide more information - a series of screen captures would probably be best.

I agree, nothing has changed in this code.

> ------- Additional Comments From arwalker sumusltd com  2005-11-15 23:21 -------
> I'm not able to reproduce this. I find that as long as I roughly
> position the plots where I want them to be then Cleanup Layout will
> snap them to a grid as I expect. Could you please provide more
> information - a series of screen captures would probably be best.


Sorry, I was a little misinformed.  The issue is actually a little more
touchy (and perhaps hard to deal with).  If you have a certain layout on
the screen, and ask kst to cleanup the layout to a different number of
columns than you have in *your* layout (or using default tile but your
current number of columns doesn't match what default tile is for that
number of plots in a window) then kst rearranges things semi-randomly,
but in the new number of columns.  

Matt, would you mind submitting a new wishlist/bug report for this and then closing 95029. Thanks.

*** Bug has been marked as fixed ***.

May I suggest to reopen bug #95030 ? Both were related (as the numbers indicate), and I believe it would solve Matt's issue: if you can conveniently set the number of columns in a visible place prior to calling "cleanup layout" there will be less bad surprises. The other feature that will also help is the Undo/Redo mechanism that is planned at some point as it will give distracted users a second chance :-)

On Wednesday 16 November 2005 03:10, Nicolas Brisset wrote:
> 09:10 ------- May I suggest to reopen bug #95030 ? Both were related (as
> the numbers indicate), and I believe it would solve Matt's issue: if you
> can conveniently set the number of columns in a visible place prior to
> calling "cleanup layout" there will be less bad surprises. The other


  The plot dialog allows this.

I know, but I still believe it is in the wrong place: from a usability perspective, a window property (number of columns) should not be in a plot dialog (or at least not only there). I am surprised that nobody else seems to have this complaint as most new users I have introduced to kst came to ask me at some point because they could not find it...
Barth meant that there is not enough room in the toolbar for a combobox to show/set the number of columns of the current window. Maybe now that zoom modes and view objects are all grouped in one pull-down menu, it would be feasible ? :-)

On Wednesday 16 November 2005 12:35, Nicolas Brisset wrote:
> 18:35 ------- I know, but I still believe it is in the wrong place: from a
> usability perspective, a window property (number of columns) should not be


  I think this has to wait until we sort out the rest of the view object UI.  
Then it should be more natural.