Bug 95029 - "cleanup layout" should respect current layout
Summary: "cleanup layout" should respect current layout
Status: RESOLVED FIXED
Alias: None
Product: kst
Classification: Applications
Component: general (show other bugs)
Version: 1.x
Platform: unspecified Linux
: NOR wishlist
Target Milestone: ---
Assignee: kst
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2004-12-12 23:03 UTC by Nicolas Brisset
Modified: 2005-11-16 20:12 UTC (History)
0 users

See Also:
Latest Commit:
Version Fixed In:
Sentry Crash Report:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Nicolas Brisset 2004-12-12 23:03:28 UTC
Version:           1.1.0_devel (using KDE 3.2 BRANCH >= 20040204, Mandrake Linux Cooker i586 - Cooker)
Compiler:          gcc version 3.3.2 (Mandrake Linux 10.0 3.3.2-6mdk)
OS:                Linux (i686) release 2.6.3-7mdk

When trying to reorganize plots in a window, the layout mode comes in handy. However, you might often end up with tiny spaces between graphs or incorrect alignments. Imagine yu have moved graphs around e.g. to put all curves from file A on the left and file B on the right. If you now call the "cleanup layout" function, it will mess everything up as it reorders graphs according to some internal order, not the current upper left corners for instance.
The expected behavior would be for it to first order graphs internally according to the position of their upper left corner, and only after that to align them nicely.
Comment 1 Nicolas Brisset 2004-12-12 23:04:29 UTC
I must admit I have not tried to look at what happens with grouped graphs or inset graphs, or other pretty "strange" things...
Comment 2 George Staikos 2004-12-12 23:06:40 UTC
  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.

Comment 3 Andrew Walker 2004-12-15 20:52:21 UTC
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) {
     }
   }
+  }
 }
 


Comment 4 Matthew Truch 2005-11-15 23:03:33 UTC
kst seems to have reverted to having this behavior again.  
Comment 5 Andrew Walker 2005-11-15 23:21:30 UTC
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.
Comment 6 George Staikos 2005-11-15 23:32:13 UTC
I agree, nothing has changed in this code.
Comment 7 Matthew Truch 2005-11-15 23:38:24 UTC
> ------- 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.  
Comment 8 Andrew Walker 2005-11-15 23:44:21 UTC
Matt, would you mind submitting a new wishlist/bug report for this and then closing 95029. Thanks.
Comment 9 Matthew Truch 2005-11-16 00:34:16 UTC
*** Bug has been marked as fixed ***.
Comment 10 Nicolas Brisset 2005-11-16 09:10:05 UTC
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 :-)
Comment 11 George Staikos 2005-11-16 17:35:32 UTC
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.
Comment 12 Nicolas Brisset 2005-11-16 18:35:34 UTC
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 ? :-)
Comment 13 George Staikos 2005-11-16 20:12:22 UTC
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.