GNOME Bugzilla – Bug 548631
Lack of feedback when brush scale becomes < 1px
Last modified: 2008-09-08 19:07:22 UTC
How to reproduce: 1. Create a new image 2. Select Brush tool, pick 1 pixel brush. 3. Reduce brush scale to 0.49 or less 4. Attempt to draw. Nothing happens. (This can be reproduced with other brushes,given appropriate scale factors) What should happen: Either a) A message is shown warning the user that their current brush will have no effect because it is too small. b) The brush scale is clamped at the <1pix threshold so that the brush may not become completely ineffective. Note that for b) the clamping should probably be performed using rounding rather than truncation, since in fact, in the above case, 0.99 of a pixel is impossible and therefore in that case the brush scale should be limited to >=1.0. Both a) and b) are surprising, unfortunately. a) is surprising because it's not normal to allow the user to shoot themselves in the foot, b) is surprising because it would silently bump up the scale if you were selecting between brushes and went from a large to a small to another large. Hopefully there is a better option than a).
I don't see why a brush smaller than 1 pixel should not have any effect. And indeed it does have an effect. Only if you make the brush a lot smaller does it vanish completely. Also there is feedback, the brush outline disappears. This is only a trivial problem.
You may be using the Circle (1) brush, which is actually 3x3 pixels. The following is a list of specific brush/scale combinations to use to get this result: * '1 pixel' * 0.50 (you can use a clipboard brush in place of this, just copy a single p3ixel), * 3px ie 'Circle (1)' * 0.25 * 5px ie. 'Circle (3)' * 0.08, * 7px ie. 'Circle (5)' * 0.05 * 9px ie. 'Circle (7)' * 0.03, * 16px ie. 'Pencil Sketch' * 0.03, * Try making a clipboard brush thats a solid 32x32 rectangle and set it to scale 0.01. Despite it being completely solid, at scale 0.01 it is completely unsolid/ineffective.
I don't see why solution (b) would introduce the problems you described. One would of course not clamp the brush scale factor. The brush size should be clamped to the minimum size where the brush still has an effect.
Oh, thanks for that idea Sven :) I implemented that, patch follows.
Created attachment 118139 [details] [review] Clamp internal brushcore scale to ensure both dimensions are at least 1 pixel This is labeled 0.9 because I think it may have some style issues; I didn't manage to find an example of similar code to base my changes style off of. It works fully in all the circumstances I've tested in, including very narrow/long brushes.
Thanks for the patch, here are my comments: * You forgot to add a block to the (scale > 0.0)-if. (It's not a one-line-if any longer.) * The XXX in your comment doesn't fit there, that tag is typically used where something needs to be fixed or isn't working properly. * I feel uncomfortable that the scale limitation is applied at two different places, it should be applied once and then propagated to the rest of the system. I think the gimp_brush_core_create_bound_segs() should have a dependency to what gimp_paint_options_get_dynamic_size() puts in core->scale. This is not in the scope of this bug report though...
* It is not easy to see what the result of the scale limitation logic is for someone that doesn't already now. A comment explaining what the end effect of that logic is wouldn't hurt.
* fixed * okay, removed; I actually put XXX there because I thought perhaps that clause needed to be removed entirely now. * NOT fixed. Dynamic size can vary -- drawn boundary size must not vary, and when gimp_brush_core_create_bound_segs() is called, gimp_paint_options_get_dynamic_size() has not yet been called anyway. * I moved the clamping into a macro. * Do we need to bail out for brush scale == 0 any longer? (see lines near line ~791 of gimpbrushcore.c, which only initializes the brush mask when brush scale > 0.0.) Perhaps brush scale == 0.0 has some hidden meaning I do not understand.
Created attachment 118154 [details] [review] revised patch
Why does your patch limit the size to 1.0? Brushes smaller than 1.0 do have an effect and there are cases where it makes sense to use such a brush. The limit should probably be 0.5 or perhaps even smaller.
You're right -- my data above indicates that (exactly) 0.50 is a safe threshold for all brushes, and testing bears it out. I've not bothered to attach an updated patch, since it would be so trivially different from the prev one.
Created attachment 118196 [details] [review] Use threshold of 0.5 I've tested this and found that .5 is the absolute minimum (eg. see what happens when you use .49). Technically the brush scaling code could probably cope properly with scales < (.5 / size) by simply reducing the opacity relative to (0.5 / size), but currently it doesn't, it just produces an empty mask. And, IMO the fix in this patch is the correct fix, since reducing the opacity beyond 50% would result in hard-edged drawing having no effect.
Thanks. I have moved this code to its own function (instead of using a macro) and committed to trunk: 2008-09-08 Sven Neumann <sven@gimp.org> * app/paint/gimpbrushcore.c: based on a patch from David Gowers clamp the brush scale so that the brush never becomes smaller than 0.5 pixels. Fixes bug #548631.