Bug 117822 - double click on a curve only works if you are near an actual data point
Summary: double click on a curve only works if you are near an actual data point
Status: RESOLVED FIXED
Alias: None
Product: kst
Classification: Applications
Component: general (show other bugs)
Version: 1.x
Platform: unspecified Linux
: VLO normal
Target Milestone: ---
Assignee: kst
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2005-12-06 21:14 UTC by Matthew Truch
Modified: 2007-10-16 03:52 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 Matthew Truch 2005-12-06 21:14:00 UTC
Version:           1.2.0_svn_485855 (using KDE KDE 3.4.2)
OS:                Linux

Double clicking on a curve brings up the edit cuve dialog.  Very handy.

However, before you could click on any visible part of the curve (if it's plot as a line)
to get this functionality.   Now you have to click on the actual datapoint.

Steps to reproduce:
Plot data; make sure you plot as lines.  
If the data isn't that sparse, zoom in so there are only a handful of data points visible.
Double click on the curve (on the line, but between data points).  

Expected behavior: curve dialog pops up.
Actual behavior: nothing happens.
Comment 1 George Staikos 2005-12-06 21:34:16 UTC
1.2.1 item. Not to be fixed before.
Comment 2 Andrew Walker 2005-12-07 01:40:36 UTC
I don't believe this behaviour has changed recently. 

When a plot is double-clicked the nearest curve is found - in terms of the y-distance from the nearest data point. As long as this y-distance of the nearest curve is less than 5 pixels (the current value) then the curve dialog will be launched.

The 5 pixels criterion may be too small, but this is open to debate.
Comment 3 Netterfield 2005-12-07 03:01:49 UTC
The problem comes when we have a low density curve with long lines between 
points.  It is not always clear where the points are, and clicking on the 
line doesn't work; as you say, you have to click near the point.

This is a >=1.2.1 issue.

cbn
Comment 4 Duncan Hanson 2006-05-09 01:46:30 UTC
I've committed a fix which should fix this problem. If possible, the lines of the curve are now used to gauge distance rather than the data points. 

There will still be problems for unordered data.

Exceptionally steep curves may (?) also cause difficulty, because the user will have more trouble getting their cursor within 5 y-pixels of the curve. 

Duncan.
Comment 5 Netterfield 2006-05-09 19:45:55 UTC
The 5 pixel limit is too small (or is not working).
Try doubling it...

With lines turned off, it seems that double clicking where the line would be still calls up the curve dialog - especially if the line would be near-horizontal.  If the lines are turned off, then the old behavior (active near a data point) should be maintained.
Comment 6 Duncan Hanson 2006-05-10 02:06:37 UTC
SVN commit 539223 by dhanson:

BUG:117822 fixed Barth's report, as well as some other problems which I missed the first time round.

 M  +4 -3      libkstapp/kst2dplot.cpp  
 M  +14 -6     libkstmath/kstvcurve.cpp  


--- trunk/extragear/graphics/kst/src/libkstapp/kst2dplot.cpp #539222:539223
@@ -3689,7 +3689,7 @@
       (*i)->point(i_near_x, near_x, near_y);
       distance = fabs(ypos - near_y);
       
-      if (distance < best_distance || !rc ) {
+      if (distance < best_distance || !rc) {
         newypos = near_y;
         newxpos = near_x;
         best_distance = distance;
@@ -6079,10 +6079,11 @@
     dx_per_pix = pow(10.0, dx_per_pix);
   }
   dx_per_pix -= xpos;
+  double dx = 5*dx_per_pix;
 
   for (KstBaseCurveList::Iterator i = Curves.begin(); i != Curves.end(); ++i) {
     (*i)->readLock();
-    double distance = (*i)->distanceToPoint(xpos, dx_per_pix, ypos);
+    double distance = (*i)->distanceToPoint(xpos, dx, ypos);
     (*i)->readUnlock();
     if (isYLog()) {
       distance = log(distance);
@@ -6093,7 +6094,7 @@
     }
   }
 
-  if (curve && best_distance/fabs((ymax - ymin) / pr.height()) <= 5) {
+  if (curve && fabs(best_distance * pr.height() / (ymax - ymin)) <= 5) {
     curve->readLock();
     KstDataObjectPtr provider = curve->providerDataObject();
     if (provider) {
--- trunk/extragear/graphics/kst/src/libkstmath/kstvcurve.cpp #539222:539223
@@ -1613,12 +1613,14 @@
 
 
 double KstVCurve::distanceToPoint(double xpos, double dx, double ypos) const {
-// find the y distance between the curve and a point.
+// find the y distance between the curve and a point. return 1.0E300 if this distance is undefined. i don't want to use -1 because it will make the code which uses this function messy.
   KstVectorPtr xv = *_inputVectors.find(COLOR_XVECTOR);
   if (!xv) {
-    return 0; // anything better we can do?
+    return 1.0E300; // anything better we can do?
   }
 
+  double distance = 1.0E300; // default value.
+
   if (hasLines() && xv->isRising()) {
   // if hasLines then we should find the distance between the curve and the point, not the data and the point. if isRising because it is (probably) to slow to use this technique if the data is unordered.
     // borrowed from indexNearX. use binary search to find the indices immediately above and below our xpos.
@@ -1641,16 +1643,22 @@
     point(i_bot, x_bot, y_bot);
     point(i_top, x_top, y_top);
 
-    double near_y = (y_top - y_bot) * (xpos - x_bot) + y_bot; // calculate y value for line segment between x_bot and x_top.
-
-    return fabs(ypos - near_y);
+    if (x_bot < xpos && x_top > xpos) {
+      double near_y = (y_top - y_bot) * (xpos - x_bot) + y_bot; // calculate y value for line segment between   x_bot and x_top.
+      distance = fabs(ypos - near_y);
+    }
   } else {
   // !hasLines- find the distance between the data and the point.
     int i_near_x = getIndexNearXY(xpos, dx, ypos);
     double near_x, near_y;
     point(i_near_x, near_x, near_y);
-    return fabs(ypos - near_y);
+
+    if (fabs(near_x - xpos) < dx) {
+      distance = fabs(ypos - near_y);
+    }
   }
+
+  return distance;
 }
 
 
Comment 7 Netterfield 2006-05-10 03:53:47 UTC
It is still very tough to actually hit a line, and sometimes seems to miss.
If the line is horizontal, it isn't bad, but if it goes up and down, it is nearly impossible to hit.
Comment 8 Duncan Hanson 2006-05-11 04:05:29 UTC
SVN commit 539587 by dhanson:

BUG:117822 some changes to KstVCurve::distanceToPoint. still plenty of room for improvement

 M  +16 -13    kstvcurve.cpp  


--- trunk/extragear/graphics/kst/src/libkstmath/kstvcurve.cpp #539586:539587
@@ -1619,10 +1619,19 @@
     return 1.0E300; // anything better we can do?
   }
 
-  double distance = 1.0E300; // default value.
+  double distance = 1.0E300;
 
+  int i_near_x = getIndexNearXY(xpos, dx, ypos);
+  double near_x, near_y;
+  point(i_near_x, near_x, near_y);
+
+  if (fabs(near_x - xpos) < dx) {
+    distance = fabs(ypos - near_y); // initial estimate.
+  }
+
   if (hasLines() && xv->isRising()) {
   // if hasLines then we should find the distance between the curve and the point, not the data and the point. if isRising because it is (probably) to slow to use this technique if the data is unordered.
+    
     // borrowed from indexNearX. use binary search to find the indices immediately above and below our xpos.
     int i_top = NS - 1;
     int i_bot = 0;
@@ -1643,19 +1652,13 @@
     point(i_bot, x_bot, y_bot);
     point(i_top, x_top, y_top);
 
-    if (x_bot < xpos && x_top > xpos) {
-      double near_y = (y_top - y_bot) * (xpos - x_bot) + y_bot; // calculate y value for line segment between   x_bot and x_top.
-      distance = fabs(ypos - near_y);
+    if (x_bot <= xpos && x_top >= xpos) {
+      near_y = (y_top - y_bot) / (x_top - x_bot) * (xpos - x_bot) + y_bot; // calculate y value for line segment between x_bot and x_top.
+      
+      if (fabs(ypos - near_y) < distance) {
+        distance = fabs(ypos - near_y);
+      }
     }
-  } else {
-  // !hasLines- find the distance between the data and the point.
-    int i_near_x = getIndexNearXY(xpos, dx, ypos);
-    double near_x, near_y;
-    point(i_near_x, near_x, near_y);
-
-    if (fabs(near_x - xpos) < dx) {
-      distance = fabs(ypos - near_y);
-    }
   }
 
   return distance;
Comment 9 Duncan Hanson 2006-05-11 04:06:53 UTC
This should only happen if there are multiple data points per pixel on
the graph which you're looking at- is this the case? Zooming in will of
course make selecting the line easier. I agree that it is difficult to
select a line when the point density is high- but I can't think of any
clean solution. The problem is that the lines which are graphed differ
from the ones you would expect from looking at the data points. I
suppose we could give KstVCurve::distanceToPoint the graphics context
information and basically re-implement the paint process for more
WYSIWYG behaviour- but I don't know if it is worth it.

Are there any better ideas?

Duncan.

On Wed, 2006-05-10 at 01:53 +0000, netterfield@astro.utoronto.ca wrote: 
[bugs.kde.org quoted mail]
Comment 10 Duncan Hanson 2006-06-07 20:08:30 UTC
This bug can still apply if axes are logarithmic.


------- Additional Comment #4 From Duncan Hanson 2006-06-07 19:43 -------
SVN commit 549185 by dhanson:

CCBUG:125299 partial fix. change the way that dx_per_pix is calculated slightly. redo calculation of dy_per_pix. there will still be problems with 1) clicking on lines in logarithmic graphs. I don't see an easy way to solve this.

 M  +19 -10    kst2dplot.cpp  


--- trunk/extragear/graphics/kst/src/libkstapp/kst2dplot.cpp #549184:549185
@@ -6081,6 +6081,7 @@
 
 
 void Kst2DPlot::mouseDoubleClickEvent(QWidget *view, QMouseEvent *e) {
+  // allow user to edit a curve if click was close enough.
   Q_UNUSED(view)
   KstBaseCurvePtr curve;
   QRect pr = GetPlotRegion();
@@ -6091,28 +6092,36 @@
 
   getCursorPos(pos, xpos, ypos, xmin, xmax, ymin, ymax);
 
-  // convert 1 pixel to plot units.
-  double dx_per_pix = double(pos.x() + 2 - pr.left()) / double(pr.width()) * (xmax - xmin) + xmin;
-  if (isXLog()) {
-    dx_per_pix = pow(_xLogBase, dx_per_pix);
+  // calculate max x distance.
+  double dx_per_pix;
+  if (!isXLog()) {
+    dx_per_pix = (xmax - xmin)/pr.width();
+  } else {
+    dx_per_pix = xpos*log(_xLogBase)*(xmax - xmin)/pr.width();
   }
-  dx_per_pix -= xpos;
-  double dx = 5*dx_per_pix;
+  double dx = fabs(5.0*dx_per_pix); //~5 pixels.
 
+  // calculate max y distance.
+  double dy_per_pix;
+  if (!isYLog()) {
+    dy_per_pix = (ymin - ymax)/pr.height();
+  } else {
+    dy_per_pix = ypos*log(_yLogBase)*(ymin - ymax)/pr.height();
+  }
+  double dy = fabs(5.0*dy_per_pix); //~5 pixels.
+
   for (KstBaseCurveList::Iterator i = Curves.begin(); i != Curves.end(); ++i) {
     (*i)->readLock();
     double distance = (*i)->distanceToPoint(xpos, dx, ypos);
     (*i)->readUnlock();
-    if (isYLog()) {
-      distance = log(distance);
-    }
+
     if (distance < best_distance || !curve) {
       best_distance = distance;
       curve = *i;
     }
   }
 
-  if (curve && fabs(best_distance * pr.height() / (ymax - ymin)) <= 5) {
+  if (curve && fabs(best_distance) <= dy) {
     curve->readLock();
     KstDataObjectPtr provider = curve->providerDataObject();
     if (provider) { 
Comment 11 Andrew Walker 2007-05-04 03:05:21 UTC
The exising algorithm does not give the correct results as it is looking for the y pixel-distance between the cursor position and the point on the line directly above or below the cursor.

This will miss very steep lines.

The only full solution would be to search for the shortest distance to the curve line segments for all curve line segements that are no further than +-5 pixels in x from the cursor position.

This is potentially time consuming for very large vectors, but is probably not a huge issue as the action is only taken at a user's request.

For the logarithmic case we simply need to perform all calculations on the line segment in pixel space.
Comment 12 Andrew Walker 2007-10-16 03:52:53 UTC
SVN commit 725684 by arwalker:

BUG:117822 provide greatly improved curve selection on a mouse double-click. Can be used with lines and points, logarithmic and linear, reversed or not.

 M  +35 -23    libkstapp/kst2dplot.cpp  
 M  +1 -1      libkstmath/kstbasecurve.h  
 M  +9 -5      libkstmath/kstimage.cpp  
 M  +1 -1      libkstmath/kstimage.h  
 M  +238 -49   libkstmath/kstvcurve.cpp  
 M  +1 -1      libkstmath/kstvcurve.h  


WebSVN link: http://websvn.kde.org/?view=rev&revision=725684