philogb / jit

The JavaScript InfoVis Toolkit provides tools for creating Interactive Data Visualizations for the Web
http://thejit.org
Other
1.51k stars 297 forks source link

EdgeHelper.line.contains() is not correct #112

Closed saneth closed 12 years ago

saneth commented 12 years ago

The current implementation of EdgeHelper.line.contains() leads to some lines not being detected properly.

    var min = Math.min,
        max = Math.max,
        minPosX = min(posFrom.x, posTo.x),
        maxPosX = max(posFrom.x, posTo.x),
        minPosY = min(posFrom.y, posTo.y),
        maxPosY = max(posFrom.y, posTo.y);

    if(pos.x >= minPosX && pos.x <= maxPosX
        && pos.y >= minPosY && pos.y <= maxPosY) {
      if(Math.abs(posTo.x - posFrom.x) <= epsilon) {
        return true;
      }
      var dist = (posTo.y - posFrom.y) / (posTo.x - posFrom.x) * (pos.x - posFrom.x) + posFrom.y;
      return Math.abs(dist - pos.y) <= epsilon;
    }
    return false;
  }

So the function draws a virtual rectangle in which mouse position distance will be tested.

But this rectangle ignores the epsilon value and thus some valid positions will never be considered.

This is especially true for (near) horizontal or vertical lines because in that cases the virtual rectangle is very thin.

I patched by doing this :

  var min = Math.min,
        max = Math.max,
        minPosX = min(posFrom.x, posTo.x) - epsilon,
        maxPosX = max(posFrom.x, posTo.x) + epsilon,
        minPosY = min(posFrom.y, posTo.y) - epsilon,
        maxPosY = max(posFrom.y, posTo.y) + epsilon;
philogb commented 12 years ago

Thanks for the patch! Could you provide this as a pull request so I can merge it?

Thanks,

shex commented 12 years ago

Should be closed: https://github.com/philogb/jit/commit/88e63c51ef12d587d19a8b4c84b83b0dff09e5d2

philogb commented 12 years ago

Thanks for the heads up