New feature for review

Boudewijn Rempt boud at valdyas.org
Thu Nov 5 14:13:15 CET 2009


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.

void KisAutoBrushWidget::setAutoBrushDiamter(qreal diameter)

-> it's diameter, not diamter.

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).

* 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?

(On that note, I think KisBrush and friends really should have been renamed 
KisBrushTip...)

-- 
Boudewijn Rempt | http://www.valdyas.org


More information about the kimageshop mailing list