Bug 329622 - [wish] [Color-Smudge-Brush] new slider : Smudge radius
Summary: [wish] [Color-Smudge-Brush] new slider : Smudge radius
Status: RESOLVED FIXED
Alias: None
Product: krita
Classification: Applications
Component: Brush engines (show other bugs)
Version: 2.8 Pre-Alpha
Platform: Compiled Sources Linux
: NOR normal
Target Milestone: ---
Assignee: Mohit Goyal
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-01-05 16:46 UTC by David REVOY
Modified: 2015-01-07 17:25 UTC (History)
5 users (show)

See Also:
Latest Commit:
Version Fixed In:


Attachments
screenshot of the feature ( in Mypaint ) (59.53 KB, image/png)
2014-01-05 16:46 UTC, David REVOY
Details
[ ^ ZIP : Krita *.kpp preset for dulling, similar to my Mypaint one, for testing. ] (13.41 KB, application/zip)
2014-02-24 08:49 UTC, David REVOY
Details
[ ^ sampling grid ] (41.96 KB, image/jpeg)
2014-03-05 14:01 UTC, David REVOY
Details
Test patch (14.79 KB, patch)
2014-03-07 06:08 UTC, Mohit Goyal
Details
2nd Patch (11.94 KB, patch)
2014-03-10 09:20 UTC, Mohit Goyal
Details
patch 3 (1.52 KB, patch)
2014-03-10 09:34 UTC, Mohit Goyal
Details
Entire Patch (52.12 KB, patch)
2014-03-11 05:44 UTC, Mohit Goyal
Details
Final Patch (16.36 KB, patch)
2014-03-11 10:52 UTC, Mohit Goyal
Details
Fixup (991 bytes, patch)
2014-03-14 14:27 UTC, Mohit Goyal
Details
[image PNG : a comparison of Mypaint smudge radius, and Krita] (153.23 KB, image/png)
2014-03-19 13:20 UTC, David REVOY
Details
Fixes Up the Fixup (5.63 KB, patch)
2014-03-25 06:42 UTC, Mohit Goyal
Details
[ ^ screenrecord expected color mix] (463.23 KB, image/png)
2014-03-25 09:32 UTC, David REVOY
Details
[ ^ Clean 'actual issue' VS 'expected result' document, to avoid confusion ] (74.92 KB, image/png)
2014-03-25 09:57 UTC, David REVOY
Details
Improvement patch (6.56 KB, patch)
2014-03-29 13:21 UTC, Mohit Goyal
Details

Note You need to log in before you can comment on or make changes to this bug.
Description David REVOY 2014-01-05 16:46:02 UTC
Created attachment 84467 [details]
screenshot of the feature ( in Mypaint )

Hi, a wish to extend the possibilities of the Color-Smudge-Brush engine ; Smudge radius. It's certainly not a major feature, but this one add more subtlities to smudge brushes. 

I guess it's certainly linked to the radius krita pick the background color ( sampled ) while doing the smudge mix. On the attachement ;  low value on top , high value on bottom.
Comment 1 wolthera 2014-01-14 13:25:47 UTC
This is actually the sole reason I'm looking forward to seeing the MyPaint brush engine implemented. It allows for smoothing and mixing in very subtle manners and is nice to have when creating subtle gradients.
Comment 2 Halla Rempt 2014-02-21 10:29:43 UTC
Maybe something for Mohit to look into...
Comment 3 Mohit Goyal 2014-02-23 13:26:33 UTC
David, How did you create the image you have attached here ? As in if you can write the steps for mypaint. Thanks :)
Comment 4 Mohit Goyal 2014-02-23 13:41:27 UTC
To be more specific - how did you get the brush to smudge the background color ?
Comment 5 Mohit Goyal 2014-02-23 13:58:20 UTC
Sorry for so many questions -- but in the color smudge brush for krita, when the user selects smudge length -- is there any use for the curve in there ?
Comment 6 David REVOY 2014-02-23 18:11:05 UTC
@Mohit : No problem for the questions :-) I just hope I'll be able to answer them correctly.  

>> David, How did you create the image you have attached here ? As in if you can write the steps for mypaint. Thanks :)
ok, to reproduce, with Mypaint ( wathever version after 0.7 , 1.0 or 1.1+Git ) :
1. I started with the brush deevad/fill ( black marker icon ) in the set#2 group of brush preset, and I painted the background rainbows of solid colors. This is for getting background variations
2. I took the brush deevad/blending ( white coton stick with a drop of water icon ) 
3. I open the 'brush setting editor' dialog ( Ctrl+B  ) , then I expand the subtree menu Smudge > Smudge radius
4. I do a lot of stroke on canvas, incrementing the Smudge radius value a bit.  
Note : You can check the 'Live update' check box on the  'brush setting editor' and see 'live' what the Smudge Radius does while moving the slider interactively. 

>> To be more specific - how did you get the brush to smudge the background color ?
It's part of my brush preset for Mypaint (set#2) deevad/blending ; the 'Smudge' option is set to max ( 1.0 ). 

>> But in the color smudge brush for krita, when the user selects smudge length -- is there any use for the curve in there ?
Yes, there is a usage case. For example, a 'pressure' sensor on the 'Smudge Lengh' ; can better simulate the power of smudging when user do more pressure ( the color keep longer on the canvas ) , and smudge less or shorter when pressure is lower.  With the curve, it's possible to tweak if this happen in a linear way ( default ) or if it should be subtle, hard, or not affecting low pressure , etc...
Comment 7 Mohit Goyal 2014-02-23 22:40:37 UTC
More questions :)
By what I understood looking at the code in mypaint and documentation -- Smudge Radius will do the following :
Depending on the radius set by the user ( -1.6 to 1.6 * the brush radius ), the color of the background will be taken in that area. Hence -- Smudge Radius should only affect the painter objects color and nothing else ? Smudge length affects the opacity, but smudge radius shouldn't. Right ?
Comment 8 Mohit Goyal 2014-02-24 07:36:40 UTC
More and more questions,
This is the description given in the docs for mypaint -
 Smudge Radius:("This modifies the radius of the circle where color is picked up for smudging.0.0 use the brush radius -0.7 half the brush radius. +0.7 twice the brush radius.+1.6 five times the brush radius (slow)")]
Now, what I don't understand is -- how can you take color from a radius ? I mean, will the function mix the colors it gets from all the pixels in that particular radius ?
Comment 9 wolthera 2014-02-24 08:11:44 UTC
I think so. If you look at the settings of the colour picker tool, you'll see it has a sample radius as well. Though this one is in full pixels.
Comment 10 Mohit Goyal 2014-02-24 08:18:39 UTC
thanks :)
So, even for the Smudge radius slider, if I implement a similar function like the color picker -- it should work right ?
Comment 11 David REVOY 2014-02-24 08:49:24 UTC
Created attachment 85300 [details]
[ ^ ZIP : Krita *.kpp preset for dulling, similar to my Mypaint one, for testing.  ]

@Mohit: oh, nice for the Mypaint documentation example. Now I also better understand it myself. 
Yes, about what Wolthera said ; it does looks like the color-picked is a sampled averaged color. The area of sampling is set by the brush radius. 

>> So, even for the Smudge radius slider, if I implement a similar function like the color picker -- it should work right ?
I'm not a developer, so I might say stupid thing. I assume that yes, it should work. What I see while testing in Krita with the preset I attach to here ( zipped ) : the actual smudge engine in Krita pick the background color at the middle of the painting dab 1x1. Even with big brush size. It's easy to test with the preset I provide over solid band of different colors.
Comment 12 Mohit Goyal 2014-02-27 05:32:09 UTC
Hi,
As of now my work on this wish is a liitle slow because of my exams :/ I think I should get some progress by Tuesday but till then however its books and me :(
Comment 13 David REVOY 2014-02-27 07:42:57 UTC
Hi Mohit, Sure, no hurry, no pressure, take all the time necessary to prepare your exams, it's the priority :-) Good luck !
Comment 14 Mohit Goyal 2014-03-05 12:02:12 UTC
Exams over :D Back to Work :D
Comment 15 Mohit Goyal 2014-03-05 12:07:14 UTC
Also -- with that. There were a few questions -
1. Is it necessary to link the smudge radius with the brush radius ? What if they are independent of each other completely ? Or should there be something like mypaint ?
2. The reason for the above question is that in my current algorithm, a radius more than 100 pt can have performance issues - I'll try to put up the first patch by tonight so that you can test it.
3. In the event when the user might want a bigger size than 100 pt, DmirtyK advised me to get random pixel to choose colors from in the radius specified so as to not mar the performance -- so I'll be trying that.
Not questions really -- An update on the progress :P
Comment 16 David REVOY 2014-03-05 14:01:23 UTC
Created attachment 85420 [details]
[ ^ sampling grid ]

Hey Mohit, 

>> 1. Is it necessary to link the smudge radius with the brush radius ? [...]
>> 2. [...] a radius more than 100 pt can have performance issues [...]
Yes they have to be related... unfortunately ( because I can guess how it affect performances ).
Btw, I guess 99% of the usage case would be : 
[ current brush radius ]  =  [ sampling area size ]
Having a fixed size doesn't really help.

>> 3. In the event when the user might want a bigger size than 100 pt, DmirtyK advised me to get random pixel to choose colors from in the radius specified so as to not mar the performance -- so I'll be trying that.
That sounds really cool. Yes, all the pixel inside the area don't have to be sampled in my opinion too. I attach to this post an idea ( *.jpg )  ; a 'sampling grid dot'  the picture just represent various brushes shape in black with white dot on them spaced like a grid to show how proportionally the color could be sampled. I thought it could be more fair for large brush than a random dot selection for picking . But as I'm not coding, I don't know the complexity behind it. Sorry in advance if this idea I'm submitting is something crazy or a total non-sens.
Comment 17 Mohit Goyal 2014-03-05 20:39:58 UTC
@boud -- I'm getting a double free or corruption in the following function  -- when I remove the delete/ Free functions -- it works -- but I obviously have to free up the space allocated. Think you can check it out ? (Its actually just the colorpicker algorithm used )
void KisSmudgeRadiusOption::apply(KisPainter& painter, const KisPaintInformation& info, qreal scale,QPoint pos) const
{
    int smudgeRadius = computeValue(info);
    KisPaintDeviceSP dev = painter.device();
    if (smudgeRadius == 1) {
        KoColor color = painter.paintColor();
        dev->pixel(pos.x(), pos.y(), &color);
    } else {
        // radius 2 ==> 9 pixels, 3 => 9 pixels, etc

        const KoColorSpace* cs = dev->colorSpace();
        int pixelSize = cs-> pixelSize();


        quint8* data = new quint8[pixelSize];
        quint8** pixels = new quint8*[counts[smudgeRadius]];
        qint16* weights = new qint16[counts[smudgeRadius]];

        int i = 0;
        KisRandomConstAccessorSP accessor = dev->createRandomConstAccessorNG(0, 0);

        for (int y = -smudgeRadius; y <= smudgeRadius; y++) {
            for (int x = -smudgeRadius; x <= smudgeRadius; x++) {
                if (((x * x) + (y * y)) < smudgeRadius * smudgeRadius) {

                    accessor->moveTo(pos.x() + x, pos.y() + y);

                    pixels[i] = new quint8[pixelSize];
                    memcpy(pixels[i], accessor->oldRawData(), pixelSize);

                    if (x == 0 && y == 0) {
                        // Because the sum of the weights must be 255,
                        // we cheat a bit, and weigh the center pixel differently in order
                        // to sum to 255 in total
                        // It's -(counts -1), because we'll add the center one implicitly
                        // through that calculation
                        weights[i] = 255 - (counts[smudgeRadius] - 1) * (255 / counts[smudgeRadius]);
                    } else {
                        weights[i] = 255 / counts[smudgeRadius];
                    }
                    i++;
                }
            }
        }
        // Weird, I can't do that directly :/
        const quint8** cpixels = const_cast<const quint8**>(pixels);
        cs->mixColorsOp()->mixColors(cpixels, weights, counts[smudgeRadius], data);
        painter.setPaintColor(KoColor(data, cs));

        for (i = 0; i < counts[smudgeRadius]; i++){
         //   delete[] pixels[i];
        }
        //delete[] pixels;
        //delete[] data;
        free(pixels);
        free(data);
        free(weights);
    }
}
Comment 18 Halla Rempt 2014-03-06 09:15:09 UTC
Hm, well -- free is something you don't use in C++, so that's wrong. Given that it's an exact copy of the color picker code (suggesting we should factor it out and put in a shared place...), I don't know what's going on. Can you give me a patch that I can apply and play with?
Comment 19 Mohit Goyal 2014-03-06 09:19:15 UTC
Sorry,
I used delete as well. It wasnt working. Ill put in the patch
On Mar 6, 2014 2:45 PM, "Boudewijn Rempt" <boud@valdyas.org> wrote:

> https://bugs.kde.org/show_bug.cgi?id=329622
>
> --- Comment #18 from Boudewijn Rempt <boud@valdyas.org> ---
> Hm, well -- free is something you don't use in C++, so that's wrong. Given
> that
> it's an exact copy of the color picker code (suggesting we should factor
> it out
> and put in a shared place...), I don't know what's going on. Can you give
> me a
> patch that I can apply and play with?
>
> --
> You are receiving this mail because:
> You are on the CC list for the bug.
>
Comment 20 Mohit Goyal 2014-03-07 06:08:21 UTC
Created attachment 85460 [details]
Test patch

hi,
This is the first patch for this bug. It doesn't work first of all.
The issue is that whenever radius is going beyond 9 -- there is some leak the memory. That is probably why even the colorpicker code is upto sample radius 9.
I wanted to first test whether this algorithm ( colorpicker one) is working then think of a better implementation but unfortunately I haven't gotten over the first step :(
Comment 21 Mohit Goyal 2014-03-10 09:20:09 UTC
Created attachment 85501 [details]
2nd Patch

In this patch, I have changed the algorithm a bit to make it less memory intensive. However, it is still crash prone. Sometimes it crashes -- sometimes it doesn't and I have no clue why :( Boud -- can you check it out ?
P.S. -- please first apply the first patch then this one
Comment 22 Mohit Goyal 2014-03-10 09:34:29 UTC
Created attachment 85502 [details]
patch 3

With this code it is working - not crashing .
However I have no way of knowing whether the code is actually doing its job. David can you help me out there ? Because I personally see no difference anywhere when I increase or decrease the smudge radius
again -- apply first two patches then this one :P
Comment 23 David REVOY 2014-03-10 11:56:16 UTC
Hey Mohit, I would really like to test, but I never succeed with patching source by the past. I always got error on my way, and not enough skill to repair. Applying three patch in a raw looks big to me. Switching branch is something I know how to do. I don't know how much work is it to make a Git branch with it.
Comment 24 wolthera 2014-03-10 13:51:51 UTC
David, I'll give it a try tonight after my distro is updated.
Comment 25 Mohit Goyal 2014-03-11 05:44:45 UTC
Created attachment 85524 [details]
Entire Patch

Hi,
This is the final patch. It is working without crashing in my system. I still have to add functionality for including the brush size -- however -- I want to know whether it works properly or not. Also, I have changed the algorithm to be scalable to any radius ( that is -- if it working :P) and less memory intensive.
P.S. -- It replaces the entire colorsmudge folder in krita/plugins/paintops. I had to delete my initial folder -- commit the changes and then put in the backup folder with updated code to generate the patch. Hope it works out :)
Comment 26 Mohit Goyal 2014-03-11 05:50:29 UTC
I'll explain the logic in short.
For the entire radius from -radius to +radius on both x axis and y axis -- it takes 16 x 16 samples throughout the area. Instead of saving all pixels separately, it uses only two pixelsized variables. Pixel[0] is the aggregation pixel and pixel[1] is the temporary pixel. in each iteration, the data in pixel[1] is mixed with the data in pixel[0] and saved in pixel[0]. Also the weight for each pixel[0] initially equal to pixel[1] keeps increasing as it is accumulating more and more data every iteration.
Comment 27 Halla Rempt 2014-03-11 09:35:21 UTC
Hm, that final patch doesn't work -- it re-adds al;ready existing files, so it cannot apply.
Comment 28 Mohit Goyal 2014-03-11 10:18:11 UTC
Oh. Okay.
Too put everything in one patch - I deleted the colorsmudge folder in my
local code. Then I got it. However I'll just correct that.
On Mar 11, 2014 3:05 PM, "Boudewijn Rempt" <boud@valdyas.org> wrote:

> https://bugs.kde.org/show_bug.cgi?id=329622
>
> --- Comment #27 from Boudewijn Rempt <boud@valdyas.org> ---
> Hm, that final patch doesn't work -- it re-adds al;ready existing files,
> so it
> cannot apply.
>
> --
> You are receiving this mail because:
> You are on the CC list for the bug.
>
Comment 29 Mohit Goyal 2014-03-11 10:52:00 UTC
Created attachment 85528 [details]
Final Patch

Hopefully this should work. I reverted to the origin/master and then got these files from a backup copy I stored. 
No need to apply the previous patches. Sorry for all the confusion :(
Comment 30 Halla Rempt 2014-03-11 18:50:09 UTC
Yes, it applied and built fine. I haven't had any crashes though!
Comment 31 Mohit Goyal 2014-03-11 18:55:48 UTC
It isnt even crashing in mine anymore :)
On Mar 12, 2014 12:20 AM, "Boudewijn Rempt" <boud@valdyas.org> wrote:

> https://bugs.kde.org/show_bug.cgi?id=329622
>
> --- Comment #30 from Boudewijn Rempt <boud@valdyas.org> ---
> Yes, it applied and built fine. I haven't had any crashes though!
>
> --
> You are receiving this mail because:
> You are on the CC list for the bug.
>
Comment 32 Halla Rempt 2014-03-11 19:44:07 UTC
Git commit 8066a7557682ffcc98660fb6260b74c35faa93d2 by Boudewijn Rempt.
Committed on 11/03/2014 at 19:00.
Pushed by rempt into branch 'master'.

Patch by Mohit Goyal
CCMAIL:mohit.bits2011@gmail.com

M  +2    -0    krita/plugins/paintops/colorsmudge/CMakeLists.txt
M  +12   -1    krita/plugins/paintops/colorsmudge/kis_colorsmudgeop.cpp
M  +2    -0    krita/plugins/paintops/colorsmudge/kis_colorsmudgeop.h
M  +2    -0    krita/plugins/paintops/colorsmudge/kis_colorsmudgeop_settings_widget.cpp
M  +0    -1    krita/plugins/paintops/colorsmudge/kis_rate_option.cpp
A  +135  -0    krita/plugins/paintops/colorsmudge/kis_smudge_radius_option.cpp     [License: GPL (v2+)]
A  +44   -0    krita/plugins/paintops/colorsmudge/kis_smudge_radius_option.h     [License: GPL (v2+)]
A  +36   -0    krita/plugins/paintops/colorsmudge/kis_smudge_radius_option_widget.cpp     [License: GPL (v2+)]
A  +28   -0    krita/plugins/paintops/colorsmudge/kis_smudge_radius_option_widget.h     [License: GPL (v2+)]

http://commits.kde.org/calligra/8066a7557682ffcc98660fb6260b74c35faa93d2
Comment 33 Dmitry Kazakov 2014-03-13 19:32:45 UTC
Bollebib has reported a crash in the new code radius :(
Comment 34 Bollebib 2014-03-13 19:37:12 UTC
http://pastebin.kde.org/poyemjddj

So use a smudge brush ,enable radius,change value ->brushes stop working and you have to force quit krita



11:47:06 - Bollebib1: dmitryK
11:47:06 - Bollebib1: I think there is an issue with the smudge brush
11:47:25 - dmitryK: ?
11:47:45 - Bollebib1: when I play around with it brushes seem to stop working altogether
11:47:54 - Bollebib1: and when I close krita I have to force quite
11:47:57 - Bollebib1: *quit
11:48:30 - Bollebib1: I can reliably trigger the bug
11:48:39 - dmitryK: i need a backtrace :)
11:48:53 - Bollebib1: no backtrace as krita doesn't crash
11:49:15 - dmitryK: Bollebib1: doesn't it hang?
11:49:17 - s7a has left the room (Quit: http://quassel-irc.org - Chat comfortably. Anywhere.).
11:49:31 - Bollebib1: it works fine,it only hangs when closing
11:49:36 - Bollebib1: but brushes don't work
11:50:07 - Bollebib1: open krita
11:50:07 - Bollebib1: choose smudge brush
11:50:07 - Bollebib1: disable color rate
11:50:07 - Bollebib1: enable smudge radius
11:50:07 - Bollebib1: so far it should still work
11:50:07 - Bollebib1: change smudge radius value
11:50:07 - Bollebib1: it won'(t work anymore
11:51:09 - Bollebib1: well I also can't make selections anymore so I assume more is broken
11:52:18 - dmitryK: it might be the yesterdays patch patch by mohit :)
11:52:50 - Bollebib1: uhu,i was updating just because I wanted to check smudge brush
11:52:51 - dmitryK: Bollebib1: try git checkout 099ba48a183dcc0546fa0d8c42a7d9
11:53:51 - Animtim: pomke: ha good, actually just resizing the file to 72 ppi without "adjust print size separately" fixed all sizes directly, even faster than I thought :) Now I need to make a little icon for it and add it in the design templates
11:54:02 - dmitryK: Bollebib1: you can help a bit: run in gdb, trigger the bug, go tto terminal, press Ctrl+C once, then type 'thread apply all bt', then press space seberal times to see full backtrace and paste it here
11:55:52 - Bollebib1: gdb krita? what is the exact command
12:00:57 - Bollebib1: dmitryK: what is the correct procedure?
12:02:08 - dmitryK: Bollebib1: yep, 'gdb krita' then type 'run'
12:03:23 - Bollebib1: ah run,is what I needed
12:05:54 - Bollebib1: dmitryK: trigger the bug but don't quit krita I assume?
12:06:10 - dmitryK: yes
12:09:01 - Bollebib2: dmitryK: http://pastebin.kde.org/poyemjddj
12:09:53 - Bollebib1: dmitryK: do you still need me to checkout to older version?
12:11:06 - Bollebib1: cause if I don't use or need radius I won't enounter the issue,I imagine
12:12:16 - dmitryK: Bollebib1: you can checkout that older revision to avoid that smudge patch :)
12:12:25 - dmitryK: Bollebib1: or you can just not use it :)
12:12:43 - Bollebib1: dmitryK: yeah,I will just not use it for now
12:12:43 - Bollebib1: is the backtrace useful?
12:15:07 - dmitryK: Bollebib1: you painted in both, scratchpad and canvas, right?
12:15:35 - Bollebib1: yes
12:15:35 - Bollebib1: but when I was changing the values I only did scratchpad
12:15:59 - Bollebib1: and after I confirmed the bug was triggered,i tried canvas as well
12:17:14 - dmitryK: Bollebib1: ok, then at least I know how such interesting backtrace has happened :) Well, yes, this is some infinite loop in the smudge radius code :)
Comment 35 Mohit Goyal 2014-03-13 21:16:56 UTC
Oh no. I will take a look in it again.


On Fri, Mar 14, 2014 at 1:07 AM, Bollebib <kwadraatnope@hotmail.com> wrote:

> https://bugs.kde.org/show_bug.cgi?id=329622
>
> Bollebib <kwadraatnope@hotmail.com> changed:
>
>            What    |Removed                     |Added
>
> ----------------------------------------------------------------------------
>                  CC|                            |kwadraatnope@hotmail.com
>
> --- Comment #34 from Bollebib <kwadraatnope@hotmail.com> ---
> http://pastebin.kde.org/poyemjddj
>
> So use a smudge brush ,enable radius,change value ->brushes stop working
> and
> you have to force quit krita
>
>
>
> 11:47:06 - Bollebib1: dmitryK
> 11:47:06 - Bollebib1: I think there is an issue with the smudge brush
> 11:47:25 - dmitryK: ?
> 11:47:45 - Bollebib1: when I play around with it brushes seem to stop
> working
> altogether
> 11:47:54 - Bollebib1: and when I close krita I have to force quite
> 11:47:57 - Bollebib1: *quit
> 11:48:30 - Bollebib1: I can reliably trigger the bug
> 11:48:39 - dmitryK: i need a backtrace :)
> 11:48:53 - Bollebib1: no backtrace as krita doesn't crash
> 11:49:15 - dmitryK: Bollebib1: doesn't it hang?
> 11:49:17 - s7a has left the room (Quit: http://quassel-irc.org - Chat
> comfortably. Anywhere.).
> 11:49:31 - Bollebib1: it works fine,it only hangs when closing
> 11:49:36 - Bollebib1: but brushes don't work
> 11:50:07 - Bollebib1: open krita
> 11:50:07 - Bollebib1: choose smudge brush
> 11:50:07 - Bollebib1: disable color rate
> 11:50:07 - Bollebib1: enable smudge radius
> 11:50:07 - Bollebib1: so far it should still work
> 11:50:07 - Bollebib1: change smudge radius value
> 11:50:07 - Bollebib1: it won'(t work anymore
> 11:51:09 - Bollebib1: well I also can't make selections anymore so I assume
> more is broken
> 11:52:18 - dmitryK: it might be the yesterdays patch patch by mohit :)
> 11:52:50 - Bollebib1: uhu,i was updating just because I wanted to check
> smudge
> brush
> 11:52:51 - dmitryK: Bollebib1: try git checkout
> 099ba48a183dcc0546fa0d8c42a7d9
> 11:53:51 - Animtim: pomke: ha good, actually just resizing the file to 72
> ppi
> without "adjust print size separately" fixed all sizes directly, even
> faster
> than I thought :) Now I need to make a little icon for it and add it in the
> design templates
> 11:54:02 - dmitryK: Bollebib1: you can help a bit: run in gdb, trigger the
> bug,
> go tto terminal, press Ctrl+C once, then type 'thread apply all bt', then
> press
> space seberal times to see full backtrace and paste it here
> 11:55:52 - Bollebib1: gdb krita? what is the exact command
> 12:00:57 - Bollebib1: dmitryK: what is the correct procedure?
> 12:02:08 - dmitryK: Bollebib1: yep, 'gdb krita' then type 'run'
> 12:03:23 - Bollebib1: ah run,is what I needed
> 12:05:54 - Bollebib1: dmitryK: trigger the bug but don't quit krita I
> assume?
> 12:06:10 - dmitryK: yes
> 12:09:01 - Bollebib2: dmitryK: http://pastebin.kde.org/poyemjddj
> 12:09:53 - Bollebib1: dmitryK: do you still need me to checkout to older
> version?
> 12:11:06 - Bollebib1: cause if I don't use or need radius I won't enounter
> the
> issue,I imagine
> 12:12:16 - dmitryK: Bollebib1: you can checkout that older revision to
> avoid
> that smudge patch :)
> 12:12:25 - dmitryK: Bollebib1: or you can just not use it :)
> 12:12:43 - Bollebib1: dmitryK: yeah,I will just not use it for now
> 12:12:43 - Bollebib1: is the backtrace useful?
> 12:15:07 - dmitryK: Bollebib1: you painted in both, scratchpad and canvas,
> right?
> 12:15:35 - Bollebib1: yes
> 12:15:35 - Bollebib1: but when I was changing the values I only did
> scratchpad
> 12:15:59 - Bollebib1: and after I confirmed the bug was triggered,i tried
> canvas as well
> 12:17:14 - dmitryK: Bollebib1: ok, then at least I know how such
> interesting
> backtrace has happened :) Well, yes, this is some infinite loop in the
> smudge
> radius code :)
>
> --
> You are receiving this mail because:
> You are on the CC list for the bug.
>
Comment 36 Mohit Goyal 2014-03-13 21:18:57 UTC
DmitryK, I was having some difficulty here. The issue is in
plugins/paintops/colorsmudge/kis_smudge_radius_option.cc -- apply function.
Can you please check it out ? Thanks


On Fri, Mar 14, 2014 at 2:46 AM, Mohit Goyal <mohit.bits2011@gmail.com>wrote:

> Oh no. I will take a look in it again.
>
>
> On Fri, Mar 14, 2014 at 1:07 AM, Bollebib <kwadraatnope@hotmail.com>wrote:
>
>> https://bugs.kde.org/show_bug.cgi?id=329622
>>
>> Bollebib <kwadraatnope@hotmail.com> changed:
>>
>>            What    |Removed                     |Added
>>
>> ----------------------------------------------------------------------------
>>                  CC|                            |kwadraatnope@hotmail.com
>>
>> --- Comment #34 from Bollebib <kwadraatnope@hotmail.com> ---
>> http://pastebin.kde.org/poyemjddj
>>
>> So use a smudge brush ,enable radius,change value ->brushes stop working
>> and
>> you have to force quit krita
>>
>>
>>
>> 11:47:06 - Bollebib1: dmitryK
>> 11:47:06 - Bollebib1: I think there is an issue with the smudge brush
>> 11:47:25 - dmitryK: ?
>> 11:47:45 - Bollebib1: when I play around with it brushes seem to stop
>> working
>> altogether
>> 11:47:54 - Bollebib1: and when I close krita I have to force quite
>> 11:47:57 - Bollebib1: *quit
>> 11:48:30 - Bollebib1: I can reliably trigger the bug
>> 11:48:39 - dmitryK: i need a backtrace :)
>> 11:48:53 - Bollebib1: no backtrace as krita doesn't crash
>> 11:49:15 - dmitryK: Bollebib1: doesn't it hang?
>> 11:49:17 - s7a has left the room (Quit: http://quassel-irc.org - Chat
>> comfortably. Anywhere.).
>> 11:49:31 - Bollebib1: it works fine,it only hangs when closing
>> 11:49:36 - Bollebib1: but brushes don't work
>> 11:50:07 - Bollebib1: open krita
>> 11:50:07 - Bollebib1: choose smudge brush
>> 11:50:07 - Bollebib1: disable color rate
>> 11:50:07 - Bollebib1: enable smudge radius
>> 11:50:07 - Bollebib1: so far it should still work
>> 11:50:07 - Bollebib1: change smudge radius value
>> 11:50:07 - Bollebib1: it won'(t work anymore
>> 11:51:09 - Bollebib1: well I also can't make selections anymore so I
>> assume
>> more is broken
>> 11:52:18 - dmitryK: it might be the yesterdays patch patch by mohit :)
>> 11:52:50 - Bollebib1: uhu,i was updating just because I wanted to check
>> smudge
>> brush
>> 11:52:51 - dmitryK: Bollebib1: try git checkout
>> 099ba48a183dcc0546fa0d8c42a7d9
>> 11:53:51 - Animtim: pomke: ha good, actually just resizing the file to 72
>> ppi
>> without "adjust print size separately" fixed all sizes directly, even
>> faster
>> than I thought :) Now I need to make a little icon for it and add it in
>> the
>> design templates
>> 11:54:02 - dmitryK: Bollebib1: you can help a bit: run in gdb, trigger
>> the bug,
>> go tto terminal, press Ctrl+C once, then type 'thread apply all bt', then
>> press
>> space seberal times to see full backtrace and paste it here
>> 11:55:52 - Bollebib1: gdb krita? what is the exact command
>> 12:00:57 - Bollebib1: dmitryK: what is the correct procedure?
>> 12:02:08 - dmitryK: Bollebib1: yep, 'gdb krita' then type 'run'
>> 12:03:23 - Bollebib1: ah run,is what I needed
>> 12:05:54 - Bollebib1: dmitryK: trigger the bug but don't quit krita I
>> assume?
>> 12:06:10 - dmitryK: yes
>> 12:09:01 - Bollebib2: dmitryK: http://pastebin.kde.org/poyemjddj
>> 12:09:53 - Bollebib1: dmitryK: do you still need me to checkout to older
>> version?
>> 12:11:06 - Bollebib1: cause if I don't use or need radius I won't
>> enounter the
>> issue,I imagine
>> 12:12:16 - dmitryK: Bollebib1: you can checkout that older revision to
>> avoid
>> that smudge patch :)
>> 12:12:25 - dmitryK: Bollebib1: or you can just not use it :)
>> 12:12:43 - Bollebib1: dmitryK: yeah,I will just not use it for now
>> 12:12:43 - Bollebib1: is the backtrace useful?
>> 12:15:07 - dmitryK: Bollebib1: you painted in both, scratchpad and canvas,
>> right?
>> 12:15:35 - Bollebib1: yes
>> 12:15:35 - Bollebib1: but when I was changing the values I only did
>> scratchpad
>> 12:15:59 - Bollebib1: and after I confirmed the bug was triggered,i tried
>> canvas as well
>> 12:17:14 - dmitryK: Bollebib1: ok, then at least I know how such
>> interesting
>> backtrace has happened :) Well, yes, this is some infinite loop in the
>> smudge
>> radius code :)
>>
>> --
>> You are receiving this mail because:
>> You are on the CC list for the bug.
>>
>
>
>
> --
> Warm Regards,
> Mohit Goyal | +918600046760
> Department of Sponsorship and Marketing(DoSM)
> BITS-Pilani Goa Campus
>
Comment 37 Mohit Goyal 2014-03-14 14:27:08 UTC
Created attachment 85574 [details]
Fixup

For smudgeradius between 10 and 16, the loop increment was 0. So infinite loop I think. Bollebib -- please check it out :)
Comment 38 Bollebib 2014-03-15 09:34:21 UTC
Patch works ,brushes don't lock up anymore



I am however confused on how it works

I try the setup David shows in his first posts,but I do not see a difference in the strokes and their length or effect,what needs to be done?
Comment 39 Dmitry Kazakov 2014-03-19 08:59:49 UTC
Git commit cd6a4952fbf3e5e28efc2c890c0888bd76c1a5fc by Dmitry Kazakov, on behalf of Mohit Goyal.
Committed on 19/03/2014 at 08:58.
Pushed by dkazakov into branch 'master'.

Fix a hangup in the Smudge Radius option

Patch by: Mohit Goyal

M  +2    -2    krita/plugins/paintops/colorsmudge/kis_smudge_radius_option.cpp

http://commits.kde.org/calligra/cd6a4952fbf3e5e28efc2c890c0888bd76c1a5fc
Comment 40 David REVOY 2014-03-19 13:20:01 UTC
Created attachment 85635 [details]
[image PNG : a comparison of Mypaint smudge radius, and Krita]

Hi Mohit, 
I updated to master today, and tested. On my result, I see no change. 

Here is a how-to test the feature :
============================
1. On a grey background canvas, fill a lower part with a saturated color  ( two zones, grey + color )
2. Select the preset Basic_Circle_Wet ( icon: a stylus, with a blue stroke ) and customise it like this in brush editor :
     - Uncheck 'Color Rate'
     - Check 'Smudge Radius'  ( in smudge radius uncheck 'Use Curve' )
3. In Setting > Configure Krita > General , select the Cursor Shape "Brush Outline with Crosshair" and press ok. 
4. Now, try to paint with the center of your brush across 2 solid colors to see if the resulted dab mix the color sampled from the background. 

About the attachement
====================
The attachement PNG gives a screenshot from my result with Krita and Mypaint. 
The black circles and little cross symbolise the "brush cursor shape" and center.
Comment 41 Mohit Goyal 2014-03-22 19:26:30 UTC
I am going through it. Initally I thought I found the problem. I just realized I didn't :(
Comment 42 Mohit Goyal 2014-03-24 22:23:38 UTC
http://imgur.com/puq36IG

This is a screenshot of the effect of smudge radius after heavy correction. The values go from 100 at the top to 2 at the bottom.
Comment 43 Mohit Goyal 2014-03-25 06:42:35 UTC
Created attachment 85723 [details]
Fixes Up the Fixup

Hi,
This patch produces the results as displayed in the last comment. This should be it. The 4th patch I think :(
Comment 44 David REVOY 2014-03-25 09:32:31 UTC
Created attachment 85728 [details]
[ ^ screenrecord expected color mix]

Hi Mohit,
Thanks for keeping working on it and providing a screenshot. 
Only by watching the picture, the color mixing looks wrong, and it doesn't sound like the dab are mixing with the sampled color from the radius. There is black in the color. 
I attach a screenshot of a paint-over your screenshot, to show how Mypaint mix colors  ( M stroke ) comparing to your actual solution in Krita ( K stroke ). 

I'm curious to know how the smudge radius slider from 0 ~ 100 in Krita works.
I would assume something like :
0 --> keep sampling only 1x1px at center of brush ( as smudge brush work in Krita by default in the past )
100 --> sample all the colors from the actual brush radius , 100%

I know Mypaint propose to sample 5x time the brush size, but that's not necessary. What's really necessary is the 100% sampling. I'll do another mockup to help you to understand how it works, because I can see in the results it's not obvious.
Comment 45 David REVOY 2014-03-25 09:57:24 UTC
Created attachment 85729 [details]
[ ^ Clean 'actual issue' VS 'expected result' document, to avoid confusion ]
Comment 46 Dmitry Kazakov 2014-03-25 12:33:32 UTC
Hi, Mohit!

Looks like your samples loop is not uniform enough. Or it is just disabled for some accidental reason.
Comment 47 Mohit Goyal 2014-03-25 12:44:06 UTC
Yes.
Basically - my code is doing two things -
1. Taking 256 samples from the entire smudge radius area.
2. Using two pixels with pixel[0] being the aggregate pixel.pixel [1] is
used to store raw data from each sample. Then its mixed using the built in
function and put into pixel0
Im trying variations now.
On Mar 25, 2014 6:03 PM, "Dmitry Kazakov" <dimula73@gmail.com> wrote:

> https://bugs.kde.org/show_bug.cgi?id=329622
>
> --- Comment #46 from Dmitry Kazakov <dimula73@gmail.com> ---
> Hi, Mohit!
>
> Looks like your samples loop is not uniform enough. Or it is just disabled
> for
> some accidental reason.
>
> --
> You are receiving this mail because:
> You are on the CC list for the bug.
>
Comment 48 Mohit Goyal 2014-03-27 11:04:35 UTC
DmitryK,
I have been trying for quite some time now. I think the error is that I am using 2 pixelsized variables. But whenever I try more -- the program crashes.
Currently- since there are two pixels -- every color is not able to get equal weight. For example -- the first two pixels are mixed with a weight of 128 and 127 ( all weights have to add up to 255). Then next pixel gets a weight of 1/3 * 255 as the aggregation pixel will have a weight of 2/3 * 255 and so on. The importance of each pixel keeps decreasing with the number of iterations.

However, I don't know how to counter this :(
Comment 49 Mohit Goyal 2014-03-27 11:18:00 UTC
Can you push the current smudgeradius patch ? Let David test it out once ? I know its not at par with MyPaint but its better than the previous version in Krita. Plus, maybe David might be able to give a good analysis once he has tested it out on his own
Comment 50 Mohit Goyal 2014-03-29 13:14:38 UTC
http://imgur.com/Vu8THmt

Hi,
I am not sure of the improvement in the result. But it is slightly less black and more mixing. I have also attached the slider value to the brush size. Check it out :)
Comment 51 Mohit Goyal 2014-03-29 13:21:13 UTC
Created attachment 85824 [details]
Improvement patch

Latest Patch for the above improvement
Comment 52 Halla Rempt 2014-04-02 14:15:20 UTC
Git commit a10ff5ac69e89da3786f678277a40f3b099e30e8 by Boudewijn Rempt.
Committed on 02/04/2014 at 14:14.
Pushed by rempt into branch 'master'.

Next step in smudge radius implementation

Patch by Mohit Goyal. Thanks!
CCMAIL:mohit.bits2011@gmail.com

M  +28   -8    krita/plugins/paintops/colorsmudge/kis_colorsmudgeop.cpp
M  +52   -34   krita/plugins/paintops/colorsmudge/kis_smudge_radius_option.cpp
M  +1    -1    krita/plugins/paintops/colorsmudge/kis_smudge_radius_option.h

http://commits.kde.org/calligra/a10ff5ac69e89da3786f678277a40f3b099e30e8
Comment 53 Mohit Goyal 2014-04-09 12:28:05 UTC
I need some help regarding the following :
1. Should the smudge radius also work for Smearing mode ? Right now it is implemented for Dulling mode in the Colorsmudgebrush
2. The result is still yielding darkish colors. Not as dark as before but it still isn't perfect. Can somebody please tell any potential issue that might be doing this ?
Thanks :)
Comment 54 David REVOY 2014-04-23 14:01:08 UTC
Hey Mohit, 
I've made a 15min video feedback in this zip :  http://www.davidrevoy.com/XYZ/2014-04-23_smudge-radius-video-feedback.zip   60MB ( temp link , I'll probably remove it in 3 month  when I clean server ) I hope you'll be able to understand my french accent. 

For your questions :
1. No, no need in smearing mode.
2. I have no ideas. From what I observe;  it's like if the colors are multiplying ( blending mode multiply )  instead of making an average 50%50% opacity in normal mode.
Comment 55 Dmitry Kazakov 2014-05-13 09:35:08 UTC
Hi, Mohit!

There are at least two problems in the radius option:

1) [it might be the reason of wrong blending] You read the old device pixel data from the device (using oldRawData() method of the random accessor), but to achieve the blending we have when the radius option is disabled you should read actual data (which is rawDataConst() method of the random accessor).

2) Generally you also need to use KisCrossDeviceColorPickerInt class and its method pickColor() to pick the background color from the paint device while doing a stroke. It is needed, because in some circumstances (painting on selection masks), one needs to do some color conversions in the meantime, and this class does the thing internally. Using pickColor() method will also fix the point 1).

You can find an example of usage of KisCrossDeviceColorPickerInt in the else branch of checking whether the radius option is activated :)
Comment 56 Halla Rempt 2014-11-10 12:18:09 UTC
Mohit, 

Can you take another look at this one?
Comment 57 wolthera 2014-12-18 21:32:35 UTC
Git commit d905dd37a2c69e5a23f94067e1c217412f8798a1 by Wolthera van Hovell.
Committed on 18/12/2014 at 21:32.
Pushed by woltherav into branch 'calligra/2.9'.

Fix for color smudge radius darkening results

There's still a couple of issues, but at the least the colour don't
become darker.

It was a for loop iterator that was named incorrectly resulting in the
darkening of colours, because the weights were set incorrectly.

M  +7    -4    krita/plugins/paintops/colorsmudge/kis_smudge_radius_option.cpp

http://commits.kde.org/calligra/d905dd37a2c69e5a23f94067e1c217412f8798a1
Comment 58 wolthera 2014-12-23 21:35:24 UTC
Git commit bc8371bb1ef5ded30d7858f236f94cea29f94f60 by Wolthera van Hovell.
Committed on 23/12/2014 at 21:34.
Pushed by woltherav into branch 'calligra/2.9'.

Extra fixes smudge radius

The previous fix was a little wrong, some awkwardness with variable
names.

I also made the smudge-radius less agrssive: It seemed to use the
diameter instead of the radius to calculate itself.

please test.

M  +6    -5    krita/plugins/paintops/colorsmudge/kis_smudge_radius_option.cpp

http://commits.kde.org/calligra/bc8371bb1ef5ded30d7858f236f94cea29f94f60
Comment 59 wolthera 2014-12-23 21:37:06 UTC
Oh, btw, do you guys want me to set the units to 0 to 300(%)? I think right now it may break presets(though not necessarily).

Anyway, 1.0 is the proper radius now.
Comment 60 David REVOY 2015-01-06 13:47:18 UTC
Hi Wolthera,  Thanks for your work on it. I tested, and it works now as I expect. I could already setup better Color-smudge preset with it in local. Especially with low 'kernel' radius of the brush, like 0.5 or 0.6 ( it suit well  brushes with a fading corona or bristtle around a center with more pixel density ). It gives more natural feeling and result in a improved blending color experience.

A minor bug feedback : 
====================
On the brush editor GUI , the sliders still use the naming "strenght" as a label , and it should be renamed to 'radius' in my opinion.

>> Do you guys want me to set the units to 0 to 300(%)? 
Yes, I would prefer it ( if it not breaks too many things ).  "radius 100%" would be really easier to understand than actual "strenght 1.0" . Thanks !
Comment 61 wolthera 2015-01-07 17:25:14 UTC
Git commit 04fa1c5a2ecfdb0d829bb640ee89095cb2d2e3ec by Wolthera van Hovell.
Committed on 07/01/2015 at 17:24.
Pushed by woltherav into branch 'calligra/2.9'.

Set smudge radius to 0-300 range

Also 'edited' the label, but the label is not passed on yet, so that'll
be a later commit.

Anyway, you can now go forth and make your presets using the slider as a
percentage of the basis.

M  +1    -1    krita/plugins/paintops/colorsmudge/kis_colorsmudgeop_settings_widget.cpp
M  +2    -2    krita/plugins/paintops/colorsmudge/kis_smudge_radius_option.cpp

http://commits.kde.org/calligra/04fa1c5a2ecfdb0d829bb640ee89095cb2d2e3ec