Fix #5475: Support tolerance when picking patches #5500
Conversation
|
why did the two |
| if cbook.is_numlike(self._picker): | ||
| radius = self._picker | ||
| else: | ||
| radius = self.get_linewidth() |
There was a problem hiding this comment.
While we are here, can we change the variable name "radius"? It is misleading. I would be fine with delta_r, possibly?
There was a problem hiding this comment.
actually, I see that it is tied to an input variable... grrr...
|
The overridden versions of However, they aren't compatible with how the picking tolerance calculation is supposed to work, which is a distance in pixel space. In order to make them work on a circle, for example, you would have to inverse project the tolerance from pixel space to "patch space" (which is usually (-1, 1)), which could be different in the x and y dimensions in the case of scaling or projection. Rectangle is probably more easily doable, but there's also an advantage to reducing the number of code paths. It's much easier to just use the general tolerance-aware picking we already have -- it's all in C++ so even though mathematically it's more complex than the distance to the center of a circle, in practice I don't think it makes much difference. |
|
the general polygon hit-test code can get computationally expensive as the number of vertices goes up. While I agree that for rectangles the cost/benefit isn't really there since there are only four vertexes, but circles got a lot of vertices, and they also have a very cheap geometric definition of an interior hit. I think it might be worth it to restore the circle case and add support for tolerance, even if it is a rough approximation of the tolerance. |
|
Circles only have 8 vertices because they are approximated with Bézier splines. |
|
But, does the C++ hit-testing code know that? Have we been hit-testing circles with octagons? |
|
The C++ hit-testing understands Bézier curves. They are not approximated with octagons -- the Bézier curve approximation is accurate to around 0.001 of the radius, which is good enough for on-screen display most of the time. |
|
Ok, I hadn't realized that the hit-testing code understood bezier curves (I'll have to take your word on that, I haven't looked at the C++ stuff in a long time). So, does the code comment in the base class method make sense anymore? Should any subclass ever implement their own |
That's a good question. I suppose they could if they could reasonably implement tolerance in physical space. In the general case that's non-straightforward, though. Maybe we just remove the comment? |
|
That's what I am thinking. Outdated comments are as bad a broken code... On Fri, Nov 20, 2015 at 2:30 PM, Michael Droettboom <
|
|
Comment removed. |
Fix #5475: Support tolerance when picking patches
Fix #5475: Support tolerance when picking patches
|
backported to v1.5.x as 8fcbf60 |
No description provided.