New feature for review

[Cyrille Berger]

On Monday 02 November 2009, LukasT.dev at gmail.com wrote:
> On Monday 02 November 2009 09:22:52 Cyrille Berger wrote:
> > On Sunday 01 November 2009, LukasT.dev at gmail.com wrote:
> > > Basically I want to be able to change the size of the brush quickly. So
> > > I implemented the wishlist [2] a bit and now we have two possibilities.
> > > The old patch is not applicable at the trunk anymore :/ I hoped you to
> > > review little faster ;( .
> > >
> > > Here is the new patch [1] and now when you press Shift and drag with
> > > mouse left/right the spray brush changes it's size. Change the cursor
> > > mode to _brush outline_ to see changes.
> >
> > About the patch itself, I think it would be better done using
> > KisPaintOpSettings::mousePressEvent (and probably adding the related
> > mouseReleaseEvent/mouseMoveEvent).
> 
> Why?
> 
> Boud said he wanted something more abstract. So I decided not to bring
>  those events to paintop and I created the method. This way e.g. wheel can
>  be configured to change brush size. Although this is not good example as
>  it is obsolete now.
> 
> I think the code should be in freehand as it is more logical for me. It's
> Freehand business how it manages the events. My feeling of good design...

The problem is that in your patch nothing is abstracted. It introduces a 
"changeBrushSize", and if I remember well the design of the paint op, we 
wanted to hide the idea of "brush", and I think your patch go one step back. 
You are right that it gives some flexibility to freehand tool on what input 
affect the brush size, yet the freehand tool is not supposed to know about 
brush. But then if we have other on canvas setting we want to make we would 
have to extend KisPaintOpSettings to support more of those settings (actually 
we already have a similar setting that allow to set the offset of the duplicate 
op).

So on one hand we might not want to have the KisPaintOpSettings to know about 
events (this would go in the direction of separating UI of painting), and we 
probably do not want to bloat the KisPaintOpSettings with settings that are 
specific to a set of paintops.

So maybe what we need is a new class "Editor", that would be triggered by 
paint tools when the user press shift, that would countains a set of hanles, 
be able to draw an overlay (not so good for UI seperation though), and list 
actions (that can be assigned to keyboard).

With the handles you could change the size of the brush, set the 
position/offset for the duplicate op. Overlay would fix the visualisation issue 
for when the user do not use the outline. And finaly action could countains 
stuff like "increase brush size" that could be mapped to a shortcut.

Just an idea :)

-- 
Cyrille Berger