New feature for review

Hello,

I added new feature to freehand and paintops. Scroll mouse button change the
size of the paintop.

Here is the patch for review:
http://lukast.mediablog.sk/patches/0001-Feature-Scroll-wheel-changes-the-size-
of-the-spray.patch

I'm sorry for Sven, it is git format :/

What I'm doing:
I reimplement the event from KoTool in KisTool and then in Freehand tool.
In freehand tool I pass the event to the setting object of the paintop if no
key modifiers are present (if CTRL is down, Zoom tool is in action)
Paintop setting object changes the GUI object.

The freehand code also handle the update of the brush outline.
There has to be some check code added but I was lazy, coz you can discard this
patch and I don't want to waste my time :)

You can't recognize the size change without the brush outline as user. You
would have to check the GUI, now the paintop settings GUI is not detachable :(
so it is quite cumbersome.

I would like t support mypaint way of changing the brush size. That means F
and G if I remember correctly. In the nicest case it should be configurable,
but I need guidance on this (Oslo?).

Also for future:
Paintop settings GUI can be used for the action. E.g. you can select in GUI
what will be changed by your scroll event and how much.
E.g. you can map the wheel event to scale of the spray or to changing the
diameter. Or particle count.

Maybe some other modifier can be used for two modes (e.g. Shift is free now,
Alt is for panning ).

Review please!

Lukas

http://lukast.mediablog.sk/patches/0001-Feature-Scroll-wheel-changes-the-size-
of-the-spray.patch
_______________________________________________
kimageshop mailing list
kimageshop@...
https://mail.kde.org/mailman/listinfo/kimageshop

On Tuesday 27 October 2009 23:06:33 you wrote:

> Hello,
>
> I added new feature to freehand and paintops. Scroll mouse button change
>  the size of the paintop.
>
> Here is the patch for review:
> http://lukast.mediablog.sk/patches/0001-Feature-Scroll-wheel-changes-the-si
> ze- of-the-spray.patch
>
> I'm sorry for Sven, it is git format :/
>
> What I'm doing:
> I reimplement the event from KoTool in KisTool and then in Freehand tool.
> In freehand tool I pass the event to the setting object of the paintop if
>  no key modifiers are present (if CTRL is down, Zoom tool is in action)
>  Paintop setting object changes the GUI object.
>
> The freehand code also handle the update of the brush outline.
> There has to be some check code added but I was lazy, coz you can discard
>  this patch and I don't want to waste my time :)
>
> You can't recognize the size change without the brush outline as user. You
> would have to check the GUI, now the paintop settings GUI is not detachable
>  :( so it is quite cumbersome.
>
> I would like t support mypaint way of changing the brush size. That means F
> and G if I remember correctly. In the nicest case it should be
>  configurable, but I need guidance on this (Oslo?).
>
> Also for future:
> Paintop settings GUI can be used for the action. E.g. you can select in GUI
> what will be changed by your scroll event and how much.
> E.g. you can map the wheel event to scale of the spray or to changing the
> diameter. Or particle count.
>
> Maybe some other modifier can be used for two modes (e.g. Shift is free
>  now, Alt is for panning ).
>
> Review please!
>
> Lukas
>
> http://lukast.mediablog.sk/patches/0001-Feature-Scroll-wheel-changes-the-si
> ze- of-the-spray.patch
>

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.

If you would move up/down, different behaviour can be coded, like the wish list
is saying e.g. setting the softness in default paintop. Better brush outline
preview should be available but that is long-time aim.

The patch has bug -- it moves with the mouse position. You probably want to
change the size and not move with the mouse on the canvas.

Please review! I want to commit and develop in trunk if possible.

[1] http://lukast.mediablog.sk/patches/wishlist-change-brush-size-with-shift-
left-click.patch
[2] https://bugs.kde.org/show_bug.cgi?id=145777
_______________________________________________
kimageshop mailing list
kimageshop@...
https://mail.kde.org/mailman/listinfo/kimageshop

LukasT.dev@... wrote:

> On Tuesday 27 October 2009 23:06:33 you wrote:
>  
>> Hello,
>>
>> I added new feature to freehand and paintops. Scroll mouse button change
>>  the size of the paintop.
>>
>> Here is the patch for review:
>> http://lukast.mediablog.sk/patches/0001-Feature-Scroll-wheel-changes-the-si
>> ze- of-the-spray.patch
>>
>> I'm sorry for Sven, it is git format :/
>>
>> What I'm doing:
>> I reimplement the event from KoTool in KisTool and then in Freehand tool.
>> In freehand tool I pass the event to the setting object of the paintop if
>>  no key modifiers are present (if CTRL is down, Zoom tool is in action)
>>  Paintop setting object changes the GUI object.
>>
>> The freehand code also handle the update of the brush outline.
>> There has to be some check code added but I was lazy, coz you can discard
>>  this patch and I don't want to waste my time :)
>>
>> You can't recognize the size change without the brush outline as user. You
>> would have to check the GUI, now the paintop settings GUI is not detachable
>>  :( so it is quite cumbersome.
>>
>> I would like t support mypaint way of changing the brush size. That means F
>> and G if I remember correctly. In the nicest case it should be
>>  configurable, but I need guidance on this (Oslo?).
>>
>> Also for future:
>> Paintop settings GUI can be used for the action. E.g. you can select in GUI
>> what will be changed by your scroll event and how much.
>> E.g. you can map the wheel event to scale of the spray or to changing the
>> diameter. Or particle count.
>>
>> Maybe some other modifier can be used for two modes (e.g. Shift is free
>>  now, Alt is for panning ).
>>
>> Review please!
>>
>> Lukas
>>
>> http://lukast.mediablog.sk/patches/0001-Feature-Scroll-wheel-changes-the-si
>> ze- of-the-spray.patch
>>
>>    
>
> 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.
>  

Nice patch !
It is both intuitive and fast. In my case, it's much better than using
the mouse wheel, as it is usable with the stylus.
> If you would move up/down, different behaviour can be coded, like the wish list
> is saying e.g. setting the softness in default paintop. Better brush outline
> preview should be available but that is long-time aim.
>  
I think up/down should do the same, because instictively we try to
resize the brush as a shape or windows (dragging the corner for 1:1
proportions). Also, it would be too hard to move perfectly horizontally
or vertically. The only exception would be for changing the ratio, but
not all paintops have a ratio option.

Hm, actually, I think it should be depending of the distance from
center, whichever direction is used. So a 1cm movement in diagonal would
give the same result than 1cm horizontally.

> The patch has bug -- it moves with the mouse position. You probably want to
> change the size and not move with the mouse on the canvas.
>
> Please review! I want to commit and develop in trunk if possible.
>
> [1] http://lukast.mediablog.sk/patches/wishlist-change-brush-size-with-shift-
> left-click.patch
> [2] https://bugs.kde.org/show_bug.cgi?id=145777
> _______________________________________________
> kimageshop mailing list
> kimageshop@...
> https://mail.kde.org/mailman/listinfo/kimageshop
>
>
>  

_______________________________________________
kimageshop mailing list
kimageshop@...
https://mail.kde.org/mailman/listinfo/kimageshop

> 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.
>
> If you would move up/down, different behaviour can be coded, like the wish
>  list is saying e.g. setting the softness in default paintop. Better brush
>  outline preview should be available but that is long-time aim.
I like it a lot, I already want this for all paintops  :D
I suggest using a visual aid like an horizontal line across the canvas and
centred in the middle of the cursor so is clear that you're modifying
something.

I'm not sure if vertical movement  should resize as well, I think moving the
cursor left/right is good enough, and it's probably easier than moving the
hand vertically (if you want to draw a straight line, which is easier?,
horizontal or vertical?)

as for changing the  softness with vertical movement, I like the idea so I did
like to see it, but as enkithan said it may prove to be difficult to move just
in one direction. Anyway a better feedback than the one suggested in the bug
report (the 0-100 number) would be a second circle inside the cursor

For what I can see, shift+click conflicts with the duplicate paintop, maybe
change the duplicate paintop so it uses ctrl or something else?

> The patch has bug -- it moves with the mouse position. You probably want to
> change the size and not move with the mouse on the canvas.
yes, cursor should not move.

> Please review! I want to commit and develop in trunk if possible.

> [1]
>  http://lukast.mediablog.sk/patches/wishlist-change-brush-size-with-shift-
>  left-click.patch
> [2] https://bugs.kde.org/show_bug.cgi?id=145777
> _______________________________________________
> kimageshop mailing list
> kimageshop@...
> https://mail.kde.org/mailman/listinfo/kimageshop
>

--
~ m4v
_______________________________________________
kimageshop mailing list
kimageshop@...
https://mail.kde.org/mailman/listinfo/kimageshop

[Cyrille Berger]

On Sunday 01 November 2009, LukasT.dev@... 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).
 
> If you would move up/down, different behaviour can be coded, like the wish
>  list is saying e.g. setting the softness in default paintop. Better brush
>  outline preview should be available but that is long-time aim.

A more complicated alternative to do that would be to show an "on-canvas
editor" (I think we discussed that in the past), so pressing "shift" would
show some handles for size, softness, spikes, rotation, and then the user
would click on the handle and move it. The advantage of the editor approach is
that it solves the issues of the different behaviours, as well as giving visual
clue even if the user do not use the outline mode. Inconvenient, it is much
more work.

So if you do not have time to do the full editor, my suggestion for that patch
is to use the KisPaintOpSettings::mousePressEvent instead of introducing a
changeSizeBrush. And leave the wish open as a reminder for the full editor
thing.

About this patch vs the wheel patch, I think having two way of changing the
size of the brush is an abuse of a limited ressource (shortcuts and mouse
"gestures"), so I would favor picking only one of the two. And as enkithan
mentioned it, the shift+mouse has the advantage of working for tablet, and as
I said above, in the future to implement a full blown "on-canvas brush
editor".

--
Cyrille Berger
_______________________________________________
kimageshop mailing list
kimageshop@...
https://mail.kde.org/mailman/listinfo/kimageshop

On Monday 02 November 2009 09:22:52 Cyrille Berger wrote:

> On Sunday 01 November 2009, LukasT.dev@... 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...

> > If you would move up/down, different behaviour can be coded, like the
> > wish list is saying e.g. setting the softness in default paintop. Better
> > brush outline preview should be available but that is long-time aim.
>
> A more complicated alternative to do that would be to show an "on-canvas
> editor" (I think we discussed that in the past), so pressing "shift" would
> show some handles for size, softness, spikes, rotation, and then the user
> would click on the handle and move it. The advantage of the editor approach
>  is that it solves the issues of the different behaviours, as well as
>  giving visual clue even if the user do not use the outline mode.
>  Inconvenient, it is much more work.

Yep, I prefer that code evolve. So far we don't have anything and I would
start with this and I'm open to on-canvas editor for future.
 
> So if you do not have time to do the full editor, my suggestion for that
>  patch is to use the KisPaintOpSettings::mousePressEvent instead of
>  introducing a changeSizeBrush. And leave the wish open as a reminder for
>  the full editor thing.

I will CCBUG the wish.

> About this patch vs the wheel patch, I think having two way of changing the
> size of the brush is an abuse of a limited ressource (shortcuts and mouse
> "gestures"), so I would favor picking only one of the two. And as enkithan
> mentioned it, the shift+mouse has the advantage of working for tablet, and
>  as I said above, in the future to implement a full blown "on-canvas brush
>  editor".
>

Ok, wheel is obsolete for me now.
_______________________________________________
kimageshop mailing list
kimageshop@...
https://mail.kde.org/mailman/listinfo/kimageshop

> > 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.
>
> Nice patch !
> It is both intuitive and fast. In my case, it's much better than using
> the mouse wheel, as it is usable with the stylus.

Cool!
 
> I think up/down should do the same, because instictively we try to
> resize the brush as a shape or windows (dragging the corner for 1:1
> proportions). Also, it would be too hard to move perfectly horizontally
> or vertically. The only exception would be for changing the ratio, but
> not all paintops have a ratio option.

You don't have to move perfectly. The code detects if your movement is more
vertical then horizontal or wise-versa.

_______________________________________________
kimageshop mailing list
kimageshop@...
https://mail.kde.org/mailman/listinfo/kimageshop

> I like it a lot, I already want this for all paintops  :D
> I suggest using a visual aid like an horizontal line across the canvas and
> centred in the middle of the cursor so is clear that you're modifying
> something.

I can turn on the brush outline for the moment of changing the size of the
brush if you don't use brush outline.
 
> I'm not sure if vertical movement  should resize as well, I think moving
>  the cursor left/right is good enough, and it's probably easier than moving
>  the hand vertically (if you want to draw a straight line, which is
>  easier?, horizontal or vertical?)

> as for changing the  softness with vertical movement, I like the idea so I
>  did like to see it, but as enkithan said it may prove to be difficult to
>  move just in one direction. Anyway a better feedback than the one
>  suggested in the bug report (the 0-100 number) would be a second circle
>  inside the cursor

Vertical resizing is detectable as I mention in previous mail. So it just adds
another short-cut that we can somehow use. You don't have to draw straight
line, just your movement should be more vertical then horizontal.

> For what I can see, shift+click conflicts with the duplicate paintop, maybe
> change the duplicate paintop so it uses ctrl or something else?

So far the event is used before the resizing the brush. That means that
Shift+Click will work for duplicate but the resizing will not work for
duplicate op so far :/ I agree that CTRL+click should be used for duplicate.
That way we are compatible with the GIMP.
 
> > The patch has bug -- it moves with the mouse position. You probably want
> > to change the size and not move with the mouse on the canvas.
>
> yes, cursor should not move.
Fixed locally :)

QCursor::setPos used for it and works.
_______________________________________________
kimageshop mailing list
kimageshop@...
https://mail.kde.org/mailman/listinfo/kimageshop

[Cyrille Berger]

On Monday 02 November 2009, LukasT.dev@... wrote:

> On Monday 02 November 2009 09:22:52 Cyrille Berger wrote:
> > On Sunday 01 November 2009, LukasT.dev@... 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
_______________________________________________
kimageshop mailing list
kimageshop@...
https://mail.kde.org/mailman/listinfo/kimageshop

On Monday 02 November 2009 13:44:07 Cyrille Berger wrote:

> On Monday 02 November 2009, LukasT.dev@... wrote:
> > On Monday 02 November 2009 09:22:52 Cyrille Berger wrote:
> > > On Sunday 01 November 2009, LukasT.dev@... 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 let's rename it to changePaintopSize and you hide the idea of the "brush"
:)

Freehand now controls the brush outline. You can't turn on and off the outline
in the paintop settings. That's another reason I would put it in the freehand
tool.

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

The biggest problem I see is the conversion of the various measures like
pixels in documents vs pixels in widget + zoom + points + another
complication. If this is in KisPaintOpSettings , it drags flake in and that's
something boud was trying to avoid (there is bug report about it)

My feature plan for 2.2 also contains brush outlines.
I would love to see paintops to work just in document measures and the tool
should handle the conversions. Current brush outline code breaks this. Maybe
we should talk this in Oslo and design something more abstract.

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

At least I would like to see this patch in 2.2 if there is nobody working on
something better. But we also want to stabilize our API so it can contradict.

boud, your opinion?

_______________________________________________
kimageshop mailing list
kimageshop@...
https://mail.kde.org/mailman/listinfo/kimageshop

On Sunday 01 November 2009, LukasT.dev@... 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 ;( .

I told you on irc that while I liked the goal, I didn't like the idea of
passing on input events to the paintops, didn't I? So I like the patch better,
and I would be ok with committing as a temporary measure, but I don't think
the architecture is good enough for so important a feature.

--
Boudewijn Rempt | http://www.valdyas.org
_______________________________________________
kimageshop mailing list
kimageshop@...
https://mail.kde.org/mailman/listinfo/kimageshop

On Monday 02 November 2009, LukasT.dev@... wrote:
> On Monday 02 November 2009 13:44:07 Cyrille Berger wrote:

> > 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 let's rename it to changePaintopSize and you hide the idea of the
>  "brush"

I would like something like this:

 * actuators: keyboard shortcuts, wheel (with delta), pressure, special ways
of moving the cursor.
 * those are associated with (aspects of) a paintop option and shown in a
combobox in the gui, like in photoshop 7. The association of an actuator and
an option is not exclusive, so mouse wheel might actuate changes in both size
and jitter, for instance.
 * KisTool would be responsible for sending the actuator signals to the active
paintop

Interaction question: are changes made in this way sticky? I.e., would they
change the preset, or would they change a local copy of the preset settings in
the paintop, and be forgotten when we switch to a new paint?

However, this scheme needs quite a bit of coding -- I once started on it, but
had to stop because there were other, more pressing issues.

I'm also considering whether we shouldn't have _two_ paintop settings widgets:
the big one with all the options in the dropdown, and a smaller one that fits
in the toolbar with the most useful options for every paintop.

In the end, I think we really need to do some serious interaction design here
instead of muddling one change on top of another.

> Freehand now controls the brush outline. You can't turn on and off the
> outline in the paintop settings. That's another reason I would put it in
> the freehand tool.

Do we want that? I think that the cursor type should stay some global setting.

> > 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.
>
> The biggest problem I see is the conversion of the various measures like
> pixels in documents vs pixels in widget + zoom + points + another
> complication. If this is in KisPaintOpSettings , it drags flake in and
>  that's something boud was trying to avoid (there is bug report about it)

Paintops should only work in pixel sizes and not be concerned about converting
from document to view.

> My feature plan for 2.2 also contains brush outlines.
> I would love to see paintops to work just in document measures and the tool
> should handle the conversions. Current brush outline code breaks this.

Yes, exactly.

>  Maybe we should talk this in Oslo and design something more abstract.

Ok.

> > 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 :)
>
> At least I would like to see this patch in 2.2 if there is nobody working
>  on something better. But we also want to stabilize our API so it can
>  contradict.
>
> boud, your opinion?

I'm fine with committing this for now and working towards a real solution
afterwards. It helps our users right now.

--
Boudewijn Rempt | http://www.valdyas.org
_______________________________________________
kimageshop mailing list
kimageshop@...
https://mail.kde.org/mailman/listinfo/kimageshop

On Monday 02 November 2009, Cyrille Berger wrote:

> A more complicated alternative to do that would be to show an "on-canvas
> editor" (I think we discussed that in the past), so pressing "shift" would
> show some handles for size, softness, spikes, rotation, and then the user
> would click on the handle and move it. The advantage of the editor approach
> is  that it solves the issues of the different behaviours, as well as
> giving visual clue even if the user do not use the outline mode.

I'm not sure this answers to the same users' need as Lukas' patch: when I
quickly want to change the brush size, I'd prefer to have as little
distraction from other options or decorations on screen as possible.

--
Boudewijn Rempt | http://www.valdyas.org
_______________________________________________
kimageshop mailing list
kimageshop@...
https://mail.kde.org/mailman/listinfo/kimageshop

On Monday 02 November 2009 20:41:30 Boudewijn Rempt wrote:

> On Monday 02 November 2009, Cyrille Berger wrote:
> > A more complicated alternative to do that would be to show an "on-canvas
> > editor" (I think we discussed that in the past), so pressing "shift"
> > would show some handles for size, softness, spikes, rotation, and then
> > the user would click on the handle and move it. The advantage of the
> > editor approach is  that it solves the issues of the different
> > behaviours, as well as giving visual clue even if the user do not use the
> > outline mode.
>
> I'm not sure this answers to the same users' need as Lukas' patch: when I
> quickly want to change the brush size, I'd prefer to have as little
> distraction from other options or decorations on screen as possible.
>

I commited the patch that adds the Shift+drag changes the paintop size to
trunk. I started to patch all paintops.

So far deform, spray, sumi, chalk and grid brush works.
Problems:
* I change the GUI and it triggers the preview repaint so there is little slow
down at the beginning. I don't know how to fix it. What I can imagine is to
turn of the connection of the signal and slot and then connect it again? I
made brush outlines for all the paintops so that you can see the size change.

* I want to use RasterOP_XOR for brush outlines and it gives black square in
OpenGL canvas. Maybe it is Qt bug, maybe we setup the canvas wrong, maybe I
setup the painter wrong in the setting object in the paintop. Check the bug
with spray.

Other paintops work and are implemented more general so it is harder to get
it. I prepared the patch for the autobrush:

http://lukast.mediablog.sk/patches/auto-brush.patch

So far it works.The outline is ugly because of non-functional
KisBoundaryPainter.
 
Boud proposed for other brushes in the dialog generic way:
Introduce factor which will scale the brush in the same manner as the pressure
does.

Is this patch acceptable for the autobrush so far? Can I commit?
_______________________________________________
kimageshop mailing list
kimageshop@...
https://mail.kde.org/mailman/listinfo/kimageshop

On Wednesday 04 November 2009, LukasT.dev@... 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
_______________________________________________
kimageshop mailing list
kimageshop@...
https://mail.kde.org/mailman/listinfo/kimageshop

On Thursday 05 November 2009 14:13:15 Boudewijn Rempt wrote:
> On Wednesday 04 November 2009, LukasT.dev@... 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?

_______________________________________________
kimageshop mailing list
kimageshop@...
https://mail.kde.org/mailman/listinfo/kimageshop

On Jueves 05 Noviembre 2009 10:13:15 Boudewijn Rempt escribió:
> * 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).
"fit the brush size to the detail level you are working" is mostly my case,
I don't really see myself saving several variants (in size) of the same brush
.

> * 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 did not understand well, I think each brush tip should remember its size, so
if I select a brush 'x' it will have the size of the last time I used it, not
the size of previous brush.

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

--
~ m4v
_______________________________________________
kimageshop mailing list
kimageshop@...
https://mail.kde.org/mailman/listinfo/kimageshop

On Saturday 07 November 2009, Elián Hanisch wrote:

> On Jueves 05 Noviembre 2009 10:13:15 Boudewijn Rempt escribió:
> > * 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).
>
> "fit the brush size to the detail level you are working" is mostly my case,
> I don't really see myself saving several variants (in size) of the same
>  brush .

Then making resizing part of a big on-canvas brush editing project wouldn't
make much sense -- a means for quickly changing the size is more useful.

> I did not understand well, I think each brush tip should remember its size,
>  so if I select a brush 'x' it will have the size of the last time I used
>  it, not the size of previous brush.

Ok. That should be doable.


--
Boudewijn Rempt | http://www.valdyas.org
_______________________________________________
kimageshop mailing list
kimageshop@...
https://mail.kde.org/mailman/listinfo/kimageshop

I will fix the name and add diameter() to the code.
Can I commit or drop that patch for autobrush?

Lukas
_______________________________________________
kimageshop mailing list
kimageshop@...
https://mail.kde.org/mailman/listinfo/kimageshop

On Sábado 07 Noviembre 2009 11:18:37 Boudewijn Rempt escribió:

> On Saturday 07 November 2009, Elián Hanisch wrote:
> > On Jueves 05 Noviembre 2009 10:13:15 Boudewijn Rempt escribió:
> > > * 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).
> >
> > "fit the brush size to the detail level you are working" is mostly my
> > case, I don't really see myself saving several variants (in size) of the
> > same brush .
>
> Then making resizing part of a big on-canvas brush editing project wouldn't
> make much sense -- a means for quickly changing the size is more useful.

yes, I agree that the on-canvas brush editor would get in the way if I just
want to change the size quickly.
But I would still want to have the on-canvas editor, is indeed a neat idea for
other options, like changing the brush's softness, or for other complicated
paintops. For example in sumi-e there are two ways of resizing it, by chaging
the radius option or the scale factor, I usually have to play with both so the
brush is big enough and yet doesn't eat my cpu away with each stroke.

> > I did not understand well, I think each brush tip should remember its
> > size, so if I select a brush 'x' it will have the size of the last time I
> > used it, not the size of previous brush.
>
> Ok. That should be doable.
>

--
~ m4v
_______________________________________________
kimageshop mailing list
kimageshop@...
https://mail.kde.org/mailman/listinfo/kimageshop