GNOME Bugzilla – Bug 84145
Narrow straight lines are imprecise
Last modified: 2003-05-23 13:25:14 UTC
Imprecision in the "straight line" feature of the Pen tool can be seen when using a thin brush. The actual kind of imprecision depends on the zoom level. Here's an example: 1. Create a new image; the default 256x256 rgb will do. 2. Select the pencil tool 3. Select the "pixel (1x1 square)" brush 4. Make sure the zoom level is 1:1 5. Place a pixel at position(100,110) with a single click. 6. Change to a different drawing color. 7. Move mouse to position(110,100). Hold down shift and click. 8. Now zoom in to 1600% and see the result: A nice line from position (100,109) to (109,100). One would have expected a line from (100,110) to (110,100). 9. Undo the drawing actions. Then repeat steps 4-8, but this time holding shift AND ctrl before drawing the line. The result is now a line from (100,109) to (108,101). 9. While zoomed, try to draw some more diagonal lines. Most of them will look pretty horrible, mostly twice as thick as the lines drawn in 1:1. Increasing the brush spacing to 100% makes them look a little less horrible, but they are still seldom uniform and don't always connect the points one clicks to draw them.
I can confirm this. It's somewhat easier to see if you do it with a 5x5 or 7x7 image zoomed to 1600%. Create one of these, and try to draw a sw-ne line. It seems like the co-ordinates for the line are not being set properly in the paint core - specifically, in paint_core_button_press() and paint_core_interpolate() in app/paint_core.c, there is an issue with curx and cury being set incorrectly. There's a rounding error creeping in somewhere, and it's not immediately obvious to me where. One good thing is that the bug is also in 1.3, in spite of the huge changes in the pait core. So that's good news, because it means that it's somewhere in the common code. Dave.
Created attachment 9026 [details] [review] This fixes bug for 1.2. Analysis included at top.
Nice patch. I'd prefer to test it on GIMP-1.3 and backport the changes after they have been tested a bit. However this patch looks clean enough to get applied to 1.2.
After applying the patch the line preview for a 45 degrees drawn by X11 is still not a perfect 45 degree line as can easily be seen in the middle of the line. If we really dare to touch this code we should as well fix it correctly and also display a proper line preview.
Created attachment 9213 [details] [review] A much improved patch, this time against 1.3.7
The latest patch (for 1.3.x and later backporting to 1.2.x) takes a much saner approach to 15-degree constrained lines. It also fixes the preview problems noted by Sven; they turned out to be a general rounding problem in gimpdrawtool: gimpdrawtool uses gdisplay_transform_coords_f() followed by RINT() to compute pixel coordinates for GTK. However, the original coordinate system - and hence the one after gdisplay_transform_coords_f() - is one where the integers corrspond to the boundaries between pixels, not the pixel centers themselves. Therefore, to find the pixel that covers a particular floating-point coordinate, one must use round-down, not round-to-nearest. Because of this, my patch changes RINT() to floor() in all pixel calculations in gimpdrawtool.c [which is the only place where gdisplay_transform_coords_f() is called]. If someone more familiar with the intentions about coordinate semantics disagrees with this analysis, please speak up. However, this is not the whole problem with the preview. If the user is preparing to draw a line with the 1x1 aliasing brush in 1:1 mode, he might reasonably assume that the pixel covered by the preview line are exactly the ones that his line on the image will cover. But the former set of pixels are computed by the X server, or the Windows GDI, or whatever ,using an algorithm we have neither good knowledge nor control of, and the latter is computed by gimp_paint_core_interpolate(). So it might be the case that the user will get a surprise when the line is actually drawn (unless it happens to he horizontal, vertical or 45-degrees, in which case there is little room for disagreement). The only way to reach a really full solution is to stop relying on GTK for drawing the preview line, and I'm not sure we really want to do that.
Just some quick thoughts; I haven't tried the patch yet and only looked at it shortly: Does it really make sense to center the endpoints of straight lines? Wouldn't it be more useful if one could draw straight lines with the full resolution offered by a zoomed-in view?
IMO supbixel positioning should remain possible with the Pen tool. However, the Pencil tool should round the coordinates to the centers of the pixel.
I think the strategic debate about which behavior is actually desirable is better had in a mailing list than through a bug tracking system. So I've started a thread about this on gimp-developer: <yahy9dh8mdu.fsf_-_@pc-043.diku.dk>
I suggest moving this bug to target 1.2.5. It's not a blocker, and it's been an awfully long time since we had a 1.2 bug fix release (Feb 11th 2002) Dave.
What was the outcome of this decision? I think that the patch attached to bug #69773 pretty much closes this one too. Dave.
The two bugs are related but not the same. Bug #69773 concerns how a line is drawn once its endpoints have been selected (a paint-core problem); that has been solved satisfactory. Bug #84145, however, concerns how the GUI tool translates mouse clicks to paint-core requests. There is some disagreement about what the Right Thing to do is - I want an easy way to draw a line from THIS pixel's center to THAT pixel's center, whereas others want to be able to use zoomed mode to position the line endpoints with subpixel increments. I tried to starte a debate on gimp-devel, but soon thereafter the list server began bouncing my emails to it. In any case, there is still at least two undisputed bugs here: 1) In 1:1 mode, mouse clicks should translate to the *center* of the pixel one points to rather than its NW corner. Otherwise attempts to position "/"-sloping lines by the mouse will be off by one. 2) The preview has similar round-off errors; see the analysis in my comment of 2002-06-13 22:34
Ah. If there was no decision, I'm moving this back to 1.2.6. My 2c - a click should register as co-ordinate the center of the screen pixel clicked. So if we're at 1:1, the clicks will be in the middle of the pixel, at 1:2 offsets will be [{0.25, 0.75},{0.25, 0.75}], etc. I don't think there's any other sane way to do it. The NW corner behaviour is definitely not the right thing to do. Changing milestone, and reccommending translating mouse clicks to the middle of the screen pixel clicked. Dave.
Let me also throw my 2c: shifting all coordinates to the middle of the screen pixel is most probably the Right Thing to do for the paint brush and most of the tools. But some special code is needed for the pencil and for the "hard edge" eraser: the coordinates have to be shifted back to the edge of the image pixels (especially before doing things like drawing straight lines). Otherwise, it would not be possible to use the pencil for precise pixel work on small bitmap images.
I'm afraid I don't see how that follows. The granularity available to the gimp when someone clicks is a screen pixel. If we're at 1:1, this corresponds to 1 image pixel. At 1:2, it's a quarter of an image pixel, and so on. We have to have some behaviour when someone clicks on a screen pixel which maps that click to an image pixel co-ordinate. At the moment, that mapping is very simple - take the origin of the screen pixel, get the corresponding sub-pixel, and take the top-left hand corner of the sub-pixel as the clicked co-ordinate. Taking the middle of the screen pixel (that is, the middle of the image sub-pixel corresponding to the screen pixel) is another simple way of mapping screen pixels to image coordinates. To me your "otherwise" above comes from a misunderstanding of what's being proposed (this disagreement is the exact reason that the discussion was moved to the mailing list, as I recall, although there was very little discussion on it). Transforming screen co-ordinates to pixel co-ordinates in the latter way has, as far as I can tell, no effect on the effectivemess or otherwise of the pencil or hard-edge eraser tools. Those tools will have the same precision, the only difference would be where the click was considered to have landed. It seems obvious to me that the current behaviour is the wrong thing to do, it would be nice if you could explain to me how the current behaviour better reflects the needs of the pencil tool than the newer behaviour would. Cheers, Dave.
Please keep in mind that such changes are not applicable to gimp-1.2. Only localized bug-fixes should go into the stable tree. Changing the behaviour of tools is definitely not an option unless the current behaviour is so buggy that it makes the tools unusable. I don't think this is the case.
Fair enough. I agree. Moving target to 1.3, since there's a patch attached here (which I don't think has been applied yet), this bug can probably be closed pretty quickly. If there is disagreement with the change, discussion should go to the mailing list. Dave.
Beware that the 1.3.x patch above always rounds shift-clicks to *image* pixel centers even in zoomed mode. This is apparently not what everybody want (even though it is what *I* want), so you guys may consider commenting out most of the gimp_paint_tool_round_line() function that the patch adds. What I had in mind when I tried to take the discussion to the mailing list was to have this behavior depend on a (global?) tool option, but too much proliferation of strange checkboxes that change behavior is not necessarily a Good Thing.
The patch did not apply to current CVS without problems. I didn't apply the changes to GimpDrawTool since that part of the patch was rejected. Since it addressed a different problem, it should be OK to leave it out for now since I believe we took care of this problem already. If you think the problem is still there, please provide an updated patch. 2003-05-23 Sven Neumann <sven@gimp.org> * app/paint/gimppaintcore.c * app/tools/gimppainttool.c: applied a patch from Henning Makholm <henning@makholm.net> that improves drawing of narrow straight lines by moving the endpoints to pixel centers. Fixes bug #84145.
Oh well, the patch is now in for everyone to play with. If people dislike the new behaviour, please open a new bug-report.