Fix ButtonEvent position on double-click and fix #4598
Two fixes:
- Only transform the mouse coordinates to world coordinates on first button press, and do not convert them a second time in case of double-clicking
- Snap to intersection of path and guideline upon double-click in the node tool
@pbs3141 moving conv() up in the code path as you suggested is not as easy, because CanvasPrivate::emit_event() is not only called from CanvasPrivate::process_event(), but also from CanvasPrivate::autoscroll_begin(). So if we want to call conv() in only one location, then it has to be in emit_event(). That's a rather natural location for this, because of all code paths going through this call. The other option would be to create a copy of the event when sending it out a second time, as you also suggested, on the second button click. However, that doesn't feel good to me either. Copying the whole event struct in Canvas::on_button_pressed() is slightly wasteful, but would also require some explanation to be added. So I've changed it as follows:
IMHO, this is the most straightforward fix, the easiest to explain to other devs when they would read the code, and with the least impact.
It would be nice though if all Geom::Points had a coordinate system flag attached to them, which would allow us to detect mistakes like this; we could prevent duplicate conversions, and prevent passing points in the wrong coordinate system to some function. I've seen this happening before, and I wouldn't be surprised at all if we have more such bugs lurking around. Could we maybe even implement derived classes from Geom::Point, one for each coordinate system, so that we could detect such errors at compile-time? Not sure though if the huge effort would be really worth it.
See: