Bug 318701 - Support nonzero fill rule for experiment brush
Summary: Support nonzero fill rule for experiment brush
Status: RESOLVED FIXED
Alias: None
Product: krita
Classification: Applications
Component: Brush engines (show other bugs)
Version: 2.6.3
Platform: Arch Linux All
: NOR wishlist
Target Milestone: ---
Assignee: Krita Bugs
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-04-22 06:58 UTC by David Gowers
Modified: 2014-01-08 13:13 UTC (History)
2 users (show)

See Also:
Latest Commit:
Version Fixed In:


Attachments
Implement "Winding fill" and "Hard Edge" options for Experiment brush (6.46 KB, patch)
2014-01-06 05:21 UTC, David Gowers
Details
Simple demo of new options (3.21 KB, image/png)
2014-01-06 05:25 UTC, David Gowers
Details
Set path/paint options in paintRegion instead (6.57 KB, patch)
2014-01-06 22:37 UTC, David Gowers
Details

Note You need to log in before you can comment on or make changes to this bug.
Description David Gowers 2013-04-22 06:58:25 UTC
In one sentence: "Allow the choice of fill rules (odd/even VS non-zero) for experiment brush."

Currently, the experiment brush is always drawn with 'odd/even' fill rule (that is, when the shape intersects itself, it creates holes in the result). This is useful for creating the more exotic shapes. 

For quickly building a silhouette, the 'non-zero' fill rule is more reliable (If the shape intersects itself, it still does not create a hole, so it's easier to make neat coherent shapes at high speed.). 

Examples of how this works:
* Choice between odd/even and nonzero fill rule is found in Inkscape's Fill/Stroke dialog.
* GrafX2's "contour fill" tool is like a nonzero-fill-rule version of experiment brush.
* With Cairo, CAIRO_FILL_RULE_WINDING selects non-zero fill rule (see http://www.cairographics.org/manual/cairo-cairo-t.html )

http://tavmjong.free.fr/INKSCAPE/MANUAL/html/Attributes-Fill-Stroke.html gives a nice visual example of the difference.

Reproducible: Always
Comment 1 Dmitry Kazakov 2013-04-22 12:18:06 UTC
This can be done quite easily, it just needs a UI for configuration
Comment 2 Halla Rempt 2013-06-22 09:40:38 UTC
Confirmed, then.
Comment 3 David Gowers 2014-01-06 05:19:40 UTC
I have implemented this, and also added a 'hard edge' option which makes it much nicer for composition design and cel-type shading.

I wanted to avoid the phrase 'winding fill' in the UI, however the alternative I came up with, 'inclusive fill', was not much of an improvement. I welcome suggestions.

Patch attached.
Comment 4 David Gowers 2014-01-06 05:21:58 UTC
Created attachment 84474 [details]
Implement "Winding fill" and "Hard Edge" options for Experiment brush
Comment 5 David Gowers 2014-01-06 05:25:45 UTC
Created attachment 84475 [details]
Simple demo of new options

Demonstrates effect of drawing a triangle-in-triangle shape with each possible combination of the new settings.
Comment 6 Dmitry Kazakov 2014-01-06 14:12:03 UTC
Hi, D Gowers!

Your patch looks really nice! I have only a small comment about it:

m_path object may be overridden while the stroke is running. E.g. in calls to trySimplifyPath(), so you might need to move the 'm_path.setFillRule(Qt::WindingFill)' line to the beginning of paintRegion() function to ensure the setting is applied in all circumstances.

After checking this issue we can surely push the patch. Do you have a write access to KDE's git repo? If not, I can push it for you.
Comment 7 David Gowers 2014-01-06 22:35:29 UTC
Thank you Dmitry.
I have tested with some more exotic settings, and indeed the winding setting of the path can get lost when smoothing is on. (oddly enough, the antialiasing state is preserved correctly.)
Anyway I have implemented your proposed solution and it appears to fix this. Updated patch attached.

I'd appreciate if you would push it for me, I don't currently have push privileges.
Comment 8 David Gowers 2014-01-06 22:37:35 UTC
Created attachment 84489 [details]
Set path/paint options in paintRegion instead

Also fixed some cosmetic consistency issues.
Comment 9 Dmitry Kazakov 2014-01-07 08:24:58 UTC
Git commit 3e5908f3af3b14317eff7c3b955a89f32dfc54f2 by Dmitry Kazakov, on behalf of David Gowers.
Committed on 07/01/2014 at 08:22.
Pushed by dkazakov into branch 'master'.

Add support for nonzero fill rule for experiment brush

Thanks David Gowers for preparing this patch!

M  +9    -0    krita/plugins/paintops/experiment/kis_experiment_paintop.cpp
M  +3    -0    krita/plugins/paintops/experiment/kis_experiment_paintop.h
M  +6    -0    krita/plugins/paintops/experiment/kis_experimentop_option.cpp
M  +7    -0    krita/plugins/paintops/experiment/kis_experimentop_option.h
M  +18   -0    krita/plugins/paintops/experiment/wdgexperimentoptions.ui

http://commits.kde.org/calligra/3e5908f3af3b14317eff7c3b955a89f32dfc54f2
Comment 10 Dmitry Kazakov 2014-01-07 08:35:57 UTC
Hi, David!

Congrats! Now you have your first patch in KDE repo! :)
Comment 11 Dmitry Kazakov 2014-01-08 13:13:14 UTC
Git commit 6ae1bfc4ae6adb13906f0687b97d19abca0482c7 by Dmitry Kazakov, on behalf of David Gowers.
Committed on 07/01/2014 at 08:22.
Pushed by dkazakov into branch 'calligra/2.8'.

Add support for nonzero fill rule for experiment brush

Thanks David Gowers for preparing this patch!

M  +9    -0    krita/plugins/paintops/experiment/kis_experiment_paintop.cpp
M  +3    -0    krita/plugins/paintops/experiment/kis_experiment_paintop.h
M  +6    -0    krita/plugins/paintops/experiment/kis_experimentop_option.cpp
M  +7    -0    krita/plugins/paintops/experiment/kis_experimentop_option.h
M  +18   -0    krita/plugins/paintops/experiment/wdgexperimentoptions.ui

http://commits.kde.org/calligra/6ae1bfc4ae6adb13906f0687b97d19abca0482c7