Bug 117822

1.2.1 item. Not to be fixed before.

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.

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

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.

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.

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;
 }
 
 

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.

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;

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]

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) { 

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.

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