GNOME Bugzilla – Bug 65030
the brushes should be scalable
Last modified: 2007-07-29 15:33:22 UTC
I think that brush size should be able to customize arbitrary. Like in Pain Shop Pro. Not like it is now, that one have to choose betwwen preformatted brush sizes. E.g. on brush is 19x19 and you can't change the size.
This is already possible in the current version: from the toolbar, select Xtns->Script-Fu->Make Brush->... Using these options, you can create elliptical or rectangular brushes easily.
Well, using Xtns->Script-Fu->Make Brush is the old-fashioned way. Pressing the "New" button in the Brushes dialog is a little easier.
Yes, I should have mentioned that first. However, the Script-Fu way is still useful for creating rectangular or square brushes.
But I was thinking that it might be possible to scale all brushes that were already available. Not necessarily having to create a brush of one's own. You know like in Paint Shop Pro.
OK. I changed the summary of this bug report and I am re-opening it. You wrote: "You know, like in Paint Shop Pro". No, I do not know what you are talking about, unfortunately. I do not have Paint Shop Pro because I do not use Windows, so it would be very helpful if you could give a detailled description of how it works in Paint Shop Pro. Some screenshots could also be useful (easier to explain). Keep in mind that some of the people who will read this bug report (and maybe try to implement that feature) have never seen or used Paint Shop Pro, so make sure that your description is easy to understand. If you create some screenshots, you can attach them to this bug report.
Created attachment 6062 [details] Example1
Created attachment 6063 [details] Example2
Of course. Sorry. I hope these 2 images speak for themselves. Otherwise, please post some additional questions.
Thanks a lot for these nice screenshots. They are very helpful. In your second attachment, you mention that the brushes are probably vector graphics, but I don't think so. They are probably bitmaps, but the original one is very large and all other versions are created by scaling it down. I suppose that Paint Shop Pro generates a new brush bitmap from the original one (using high-quality bicubic interpolation) and caches it in memory or on disk so that you can paint with it easily. This would be a nice addition to the Gimp.
Actually that's exactly what GIMP does to scale the brush if used with a pressure-sensitive graphics tablet. It would be trivial to add brush-scaling for normal use and I think we will see this in GIMP-1.4.
I assume that Paint Shop Pro is also using some special code for round or square brushes, so they can be generated directly by the code instead of having to scale down a bitmap brush. This is probably what the "Shape" option does, as seen in the first screenshot. Hakon, could you give a list of the Shapes that are available?
Square Round Left slash \ Right slash / Horizontal Vertical But then when "custom brush style" s chosen (as opposed to normal) then the "shape" combo is grey, of course.
Also... being able to resize brushes using mouse + modifier key is a very nice feature to have. Corel Photopaint does this quite well. My idea is detailed in this document http://www.midcoast.com.au/~rgcoy/GimpUIIdeas.pdf
I don't think this will make it before the feature freeze. Setting milestone Future. Dave.
Bug #126591 brings up the idea of allowing to rotate the brush as well. I'm sure this has been mentioned before but I'm adding it here for reference.
*** Bug 126591 has been marked as a duplicate of this bug. ***
*** Bug 140376 has been marked as a duplicate of this bug. ***
GIMPressionist has a nice collection of scalable brushes. It might be nice to be able to select those brushes (and manipulate them in a similar way as GIMPressionist can manipulate them) for drawing by hand. Perhaps some source code could be reused? Maybe make all brushes like GIMPressionist, and create a standard dialog for selecting a brush - similar to the standard dialog for selecting a color, and use the same dialog for both places? The new interface might need to read files containing both of the legacy formats.
The GIMPressionist brushes aren't any more or any less scalable than the standard GIMP brushes. They are PGM files so they are just grayscale pixmaps and you could easily convert them all to GIMP brushes if you wanted to. That wouldn't make them scalable though.
GIMP 2.2 will have quite some of the features that are being asked for in this bug report. The parametric brushes have lately been extended to support more shapes. I think what needs to be done here (and I have been asking for a volunteer for this simple job very often already) is to add a small set of scalable brushes to the default GIMP setup. That should make it more obvious that this feature is available. It would also be relatively easy to allow all pixmap brushes to be scaled down. That would be a rather hackish approach though and I think it would make more sense to introduce more brush shapes, perhaps even allow for SVG brushes.
See also bug #157506.
It would be very nice if the brushes could be scaled EASILY with a slider or the mouse wheel. Creating new brushes using a dialog all the time is very time-intensive.
*** Bug 342798 has been marked as a duplicate of this bug. ***
See also bug #322176.
please see also http://bugzilla.gnome.org/show_bug.cgi?id=342798 for an example with picture...
Almost there... 2006-11-16 Michael Natterer <mitch@gimp.org> Made all brushes scalable from the tool options. So far only downscaling is supported since we lack an algorithm for bitmap brush upscaling. Addresses bug #65030. * app/paint/gimppaintoptions.[ch]: added "brush-scale" property. * app/paint/gimpbrushcore.[ch]: separate pressure logic from brush scaling logic and take paint_options->brush_scale into account. Added gimp_brush_core_create_bound_segs() which returns BoundSegs of the correctly scaled brush mask for the brush preview on the canvas. * app/tools/gimpbrushtool.c: use gimp_brush_core_create_bound_segs() instead of doing this here (also removes all knowledge about lowlevel stuff from this file). Connect to notify::brush-scale of the paint options and invalidate the brush core's brush accordingly. * app/tools/gimppaintoptions-gui.c: added brush scale slider.
As I tried this out, I noticed that tool-value-2 should now effect brush scale rather than VBR brush radius.
Already done on my disk. Will commit tomorrow with some more stuff.
Made the [ and ] shortcuts work for all tool tips: 2006-11-17 Michael Natterer <mitch@gimp.org> * app/actions/tools-actions.c * app/actions/tools-commands.[ch]: added actions and callbacks for the new paint options brush scale property. Assigned new shortcuts: '<' and '>' are now changing tool-value-1 and '[' and ']' are changing tool-value-2 * app/actions/context-actions.c: removed the shortcuts from the brush size actions. * app/tools/gimpbrushtool.c * app/tools/gimpcolortool.c: set tool-value-2 to brush-scale and to color-average-radius, so '[' and ']' always affect the size of the tip of the active tool. tool-value-1 is connected to context-opacity so that is changeable using '<' and '>' now.
Created attachment 76792 [details] [review] improve brush scale UI and actions (trivial patch - adjusts brush-scale property to range 0.01..1.0 and actions to adjust by 0.03 rather than 0.10) The step sizes given for brush scale are rather impractical. 0.1 for the smallest modification actions available means only 10 steps of brush size. When scaling up is supported, I expect the maximum brush scale value will be 2.0, until vector brushes become the norm. The minimum brush scale value should be 0.01 or similar, since 0 is always useless. The 0.03 value for stepping used in this patch equates to 33 steps of brush scale being available.
We should revisit this patch when the scaling stuff is complete, the plan is to add upscaling too. Let's think about step sizes when the full range is available.
Okay, here are some cases that fail/misbehave: 1. * create a new brush (radius 15, hardness 1, spacing 3) * set paint tool 'brush scale' to a low value (0.01 triggers it, so does 0.22. the values where it occurs vary depending on the base brush parameters.) * draw a single dab on an image (pencil tool shows this up most obviously) * the resulting strokes are misaligned. This is a result of the brush scale, because setting the radius to 1.2 and scale to 1.0 produces an identical brush outline and draws the dab aligned to the brush outline. 2. Certain scale values cause random noise in the brush and even produce results that do not conform to normal behaviour for the paint tool (the random noise can have varying alpha, even when using pencil tool where the alpha should always be binary) All of the scales that cause random noise are also misaligned, though in the opposite direction (-1,-1 rather than +1,+1) With the brush specified in (1), try the following scales (drawing with pencil tool) * 0.97, 0.94, 0.85, 0.82,0.73, 0.70, 0.64, 0.61, 0.52, 0.43 The trigger values are dependent on the base brush parameters, same as in (1). I suspect the brush alignment is related to the fix for bug #166622, though the random noise bug has been around for a long time.
I just noticed, the brush spacing is considered relative to the original scale of the brush. (ie, if your original brush is 100x100, spacing is 100.0, and scale is 0.5, then the pointer will still need to move 100 pixels before the next dab is laid down, rather than 50 pixels). Hopefully this is an easy fix. I plan to look into it.
I have done this on my disk already, it just doesn't work properly yet. Expect it in CVS soonish.
Spacing scales nicely now: 2006-12-10 Michael Natterer <mitch@gimp.org> * app/paint/gimpbrushcore.c: also scale the brush's spacing (by scaling its x_axis and y_axis). Addresses bug #65030.
So what remains to be done here? Would be nice to have a short summary, preferably categorized into things that need to be done for 2.4 and things that would be nice to have. We can then open new bug reports for them. This one definitely has too many comments by now.
blocker? : Pixel artifacts when drawing with brush scaling - bug #399221 -- It occurs also with non-generated brush, and it causes offsetting by up to 1 pixel in the X and or Y dimension (for instance, you draw a circular dab with low pressure using a brush like .***. ***** ***** ***** .***. but it produces a result like ..... ..... ..... ...*. ..... rather than ..... ..... ..*.. ..... ..... ) Would be nice: Scaling factors > 1.0 Blocker? + would be nice: Brush scale doesn't appear for smudge (nor does it work if you set it to appear.)
Smudge never worked with scaled brushes, therefore the scale GUI is disabled (you will notice that there is also no pressure-size checkbutton)
Yes, but it makes perfect sense to be able to scale the brush before painting a stroke -- it's quite different from scaling it during painting (which doesn't make much sense in the context of smudging). I personally find it very annoying to be able to modify brush size in all but one tool (that tool being one of the ones I most often want to adjust brush size for), and this situation presents me with the following choices: * reattach my shortcuts to the 'virtual brush' size editing commands; now I can edit the size of the brush when using any tool, only if it is a virtual brush (and have to adjust the 'brush scale' slider by mouse if I want to adjust brushes of other types) * edit the brush size using the virtual brush editing dialog (only if it is a virtual brush and smudge is the active tool) This demonstrates a marked gap -- it is impossible to scale pixmap/mask brushes when the smudge tool is active, and it requires special treatment for resizing brushes when using smudge tool.
Ah I see, that's a good point. The smudge tool of course only wants to prevent scaling *while* painting. But then, I would really prefer if you directed your skills at the problem of brush upscaling :-) It's a badly missing feature for all tools, and the algorithm we have currently only scales down. It's IMHO much more of a "blocker" than the smudge issue.
Okay. Interpolation is certainly far less baffling than decimation. In the future, it'd be nice to be able to specify X and Y scale independently (esp. if modulatable by pressure)
I have a patch which fixes both upscaling and downscaling, it ditches the brush_scale specific algorithms and uses scale_region instead. Speed seems to be totally acceptable. However, before submitting (commiting?), there are some inconsistencies to sort out, the main one being: I belive that the Scale-slider should scale length instead of area. My argument is mainly based on that the good 'ol brushes e.g. Circle (19) refers to diameter as the main measuring reference. Doubling scale => doubling main measuring reference => doubling diameter. This length-scaling should be true for all brushes, of course. Now, the reason I'd like to ask this Michael is because your previous code already has prepared for area-scaling. How do you feel about changing area-scaling for length-scaling to get it (in my opinion) more intuitive? I mean, artists most often measure things in lenght-units I guess?
Personally, I like the idea of length based measurements very much in this case. Eventually I'd like to replace the scale slider with a square operating similar to Pixia: You have an inner rectangle showing the original size; X distance from centre specifies X scale, Y distance from centre specifies Y scale, and (maybe) angle from centre specifies rotation (which may be available easily after GEGL is partly integrated). The relevant idea is that you can always see what a brush's 'natural size' is. I think it's important if we use a length based slider, to allow the user to easily reset to 'default' size (at least until we start supporting SVG brushes exclusively -- they arguably don't have a default size.)
Ultimately I'd like the "Scale:" slider to be a "Diameter:" slider, measured in an absolute scale like 'pixels'. From an artist point of view I believe scale is irrelevant, what you want to specify is the diameter of brush you use. I agree though that the original sample size is of interest, which could be solved by a 'set to sample diameter'-button or something similar. This is not really relevant for this specific bug, but I wanted to mention it here anyway. I also will probably separate the upscaling patch and the consistent-length area patch.
Also, there is a problem with discarding the usage of brush_scale : it is the correct way to do things for VBR brushes, and not using brush_scale for VBRs produces a much inferior result. I see no nice way to specify this in the relevant classes, so you'll probably have to special case it.
Seems like I expressed myself a bit foggy, by "brush_scale functions" I meant the only-bitmap, only-downscaling algorithms (well it's the same algorithm but for two bitdeepths) in app/base/brush-scale.c. The VBR brushes-scaling does not use bitmap scaling, but recalculates the mask from it's parameters, but with a scaled radius. The virtual scale_(mask|pixmap) methods of GimpBrush will still be there, and overidden by GimpBrushGenerated.
Created attachment 84365 [details] [review] Brush-Upscaling-First-Try.patch This patch adds support for up- and downscaling of brushes. For bitmapped brushes it uses existing upscaling functions. For VBR brushes it uses, after some refactoring, the existing vector-based "scaling" functions. I have one problem with this patch, and that's the duplicate code in gimp_brush_generated_get_half_width_and_height and gimp_brush_generated_calc. They share code, but the code is hard to share in a clean way, at least now at 02:08 AM ;). Let me know if I miss the obvious solution and please leave other comments if you find other stuff to comment on.
OKAY, the main remaining issue is that scale doesn't properly correspond to brush dimensions. Try this: * make or select a 1pixel brush * set scale to 2.0 - the 1pixel brush should now be 2pixels. * but, it isn't -- you have to set it to >= 2.25 before it grows to 2pixels in size. * even worse, try setting it to scale == 10.0 -- this should be a 10x10 pixel brush, but it's a 3x3 pixel brush (huh?) * The disparity just increases as scale values increase -- Scale == 50 produces a 7x7 brush. How silly is that!? scale == 100.0 finally produces the 10x10 pixel brush that scale == 10.0 should have. * larger brushes also display the problem; it's slightly less obvious. * so do VBRs.
Yes, this is the inconsistency I mentioned earlier. The Scale currently denotes area scaling. A 4.00 scale gives double diameter. 9.00 in scale gives 3 times the diameter etc. I intend to make the scale denote lenght scaling since imo that is more intuitive and useful. But What I first want to do is get rid of this 2.4 milestone because of the sheer amount of comments on it.
sqrt(10) * 1 ≈ 3 sqrt(50) * 1 ≈ 7 sqrt(100) * 1 = 10 See the pattern? ;)
Created attachment 84379 [details] [review] Brush-Upscaling-First-Try-Refined.patch Without duplicate code this time. Should be commitable.
I have committed this patch. I suggest that a new bug is opened for UI changes. 2007-03-11 Michael Schumacher <schumaml@cvs.gnome.org> Made brushes scalable (both up and down) by using existing scaling routines, and also refactored some brush-code. Patch by Martin Nordholts. Fixes bug #65030. * app/paint/gimpbrushcore.c: (gimp_brush_core_calc_brush_length_scale) Refactored 'get brush size' code to where it belongs, in GimpBrush-classes, and renamed gimp_brush_core_calc_brush_size to gimp_brush_core_calc_brush_length_scale. * app/paint/gimppaintoptions.c (gimp_paint_options_class_init): Changed Scale scale to [0.0, 100.0]. * app/base/temp-buf.[ch] (mask_buf_new): Change signature to also take a bpp parameter. * app/base/brush-scale.[ch]: Changed brush_scale_(mask|pixmap) to the new brush_scale_buf, which uses existing scaling routines (scale_region) instead of dedicated down-scaling only routines. * app/tools/gimppaintoptions-gui.c (gimp_paint_options_gui): Made the brush Scale-slider logarithmic. * app/core/gimpbrushgenerated.c: Implemented the new get_scaled_size method inherited from GimpBrush, and modified gimp_brush_generated_calc to use this helper function. * app/core/gimpbrush.[ch]: Added public virtual method get_scaled_size to GimpBrush, overridden by GimpBrushGenerated, which calculates the buffer sizes for a given brush scaled with a given scale. Also changed calls to brush_scale_(mask|pixmap) to the new brush_scale_buf.
Created attachment 84387 [details] [review] make-brush-scale-scale-length-units.patch This patch makes scaling affect length (e.g. diameter) instead of area. Mitch, since (afaik) you are the brain behind area-scaling, could you tell us why length scaling is a bad thing (if you think it is), and if you don't think it is, commit the patch (or ask me to rewrite it if you find flaws), and then close this bug? I suggest we create a new 'Make Smudge tool use scaled brushes' bug and close this one, it has enough comments already.
Please, avoid including changelog diffs in your patches. They are almost certain to fail or conflict -- merely a single new entry in the changelog before your patch is committed can cause this. There is also a convention of using no empty lines between prototypes; the empty lines are used to separate groups of functions. As for the actual result of the patch -- that's INFINITELY better :)
Created attachment 84415 [details] [review] Make-Brush-Scale-Length-Units-[Refined].patch Here's the new patch Since it is far less work to resolve ChangeLog conflicts then to write the ChangeLog entry from scratch for a patch one hasn't wrote oneself, and since providing the ChangeLog entry makes the patch much easier to understand, I think it is irrational not to provide them. Therefore I will keep providing them. Heh, infinitely better, that's a lot 'a lot better' ;)
On a second thought, it would be much more convenient to provide ChangeLog entry separate from the actual patch, so it can be copypasted into ChangeLog. I will do that in the next patch.
Just for the record: the patch that was applied a few hours ago (comment #52) has been reverted by mitch after a quick IRC discussion. Some of this code should be re-designed and it is easier to start from a clean state rather than applying patches on top of patches. Also, patches affecting important parts of the core code should be reviewed before being applied, especially if we want to be able to release 2.4 soon.
Created attachment 84540 [details] [review] Scale-Region-For-Brush-Upscaling.patch The new patch, provided for reference and approval.
Created attachment 84543 [details] [review] Scale-Region-For-Brush-Upscaling.patch Hopefully with no flaws this time
Looks good. Please commit.
Fixed in trunk, revision 22113: 2007-03-14 Martin Nordholts <martinn@svn.gnome.org> Made brushes upscalable by using scale_region. Downscaling still uses functions in app/base/brush-scale.c (because of better performance). Parametric brushes of course recalculates masks instead of using bitmap scaling. Fixes bug #65030. * app/core/gimpbrush.[ch]: Added a private scale_buf method which uses scale_region (with GIMP_INTERPOLATION_LINEAR) to scale its masks and pixmaps. Also added public virtual method scale_size to GimpBrush (overridden by GimpBrushGenerated) which calculates the buffer sizes for a given brush scaled with a given scale. * app/core/gimpbrushgenerated.c: Implemented the new scale_size method inherited from GimpBrush, and modified gimp_brush_generated_calc to use this helper function. * app/tools/gimppaintoptions-gui.c (gimp_paint_options_gui): Made the brush Scale-slider logarithmic. * app/paint/gimppaintoptions.c (gimp_paint_options_class_init): Changed Scale scale to [0.0, 10.0]. * app/paint/gimpbrushcore.c: (gimp_brush_core_calc_brush_size): Refactored 'get brush size' code to where it belongs, in GimpBrush-classes, and allowed scales larger than 1.0. This should probably be closed as fixed now since area vs length scaling is a separate issue.
Just an update: This bug will be closed when a convenient way to reset the Scale scale to 1.0 has been implemented.
*** Bug 422254 has been marked as a duplicate of this bug. ***
Would still be nice to find a convenient way to reset the Scale to 1.0 but if we don't find one soon, then we should close this as FIXED and open a new enhancement request for the Reset issue and set it on the 2.6 milestone.
I have opened bug #461507 for the remaining issue. Closing as FIXED.