New feature for review

[LukasT.dev@gmail.com]

On Thursday 05 November 2009 14:13:15 Boudewijn Rempt wrote:
> On Wednesday 04 November 2009, LukasT.dev at gmail.com wrote:
> > Is this patch acceptable for the autobrush so far? Can I commit?
> 
> First of all, I don't think the method names are ok:
> 
> void KisBrushOpSettings::changePaintOpSize(qreal x, qreal y) const
> -> I don't where this is called, and the name doesn't make it clear to me
>  what it actually changes.

I name it more abstract so that you, the developer of the paintop, can decide 
what size will be changed. Whether it is diameter, softness, etc. E.g. you can 
make a widget for paintop with GUI and the GUI will tell what will be resized. 
That's why changePaintopSize.
 
> void KisAutoBrushWidget::setAutoBrushDiamter(qreal diameter)
> 
> -> it's diameter, not diamter.

Yes, that is typo.

> 
> Further, I'm not sure whether this is the right approach. I think it would
>  be better to have a scaling factor that is applied to the brush before we
>  apply the scaling for pressure.
> 
> There are a couple of issues that need to be answered beforehand, though:
> 
> * what is the function of brush resizing in the painting workflow: is it to
> create a new brush variant for later reuse, or is it to fit the brush size
>  to the detail level you are working on (and if so, is it useful to also
>  keep the brush size constant while zooming in and out -- I would love
>  that, personally).

MyPaint has the zooming thing. I think it should be added to Krita, but as 
configurable option. E.g. I like the current state :) But I would like to have 
both.
 
> * should the gui reflect the new base brush size, and if so, how should we
> show that for gbr/abr/gih brushes?
> * when should the brush size be reset? If a gbr brush is resized, do we
>  keep the new base size for that brush tip when we select a different brush
>  tip, then select the old one? Or do we apply the factor also to the new
>  brush tip?

I would implement it per brush type. For auto-brush is this patch acceptable? 
 
> (On that note, I think KisBrush and friends really should have been renamed
> KisBrushTip...)
> 

Why?