Summary: | [wish] [Color-Smudge-Brush] new slider : Smudge radius | ||
---|---|---|---|
Product: | [Applications] krita | Reporter: | David REVOY <info> |
Component: | Brush engines | Assignee: | Mohit Goyal <mohit.bits2011> |
Status: | RESOLVED FIXED | ||
Severity: | normal | CC: | dimula73, griffinvalley, halla, kwadraatnope, mohit.bits2011 |
Priority: | NOR | ||
Version: | 2.8 Pre-Alpha | ||
Target Milestone: | --- | ||
Platform: | Compiled Sources | ||
OS: | Linux | ||
Latest Commit: | http://commits.kde.org/calligra/04fa1c5a2ecfdb0d829bb640ee89095cb2d2e3ec | Version Fixed In: | |
Sentry Crash Report: | |||
Attachments: |
screenshot of the feature ( in Mypaint )
[ ^ ZIP : Krita *.kpp preset for dulling, similar to my Mypaint one, for testing. ] [ ^ sampling grid ] Test patch 2nd Patch patch 3 Entire Patch Final Patch Fixup [image PNG : a comparison of Mypaint smudge radius, and Krita] Fixes Up the Fixup [ ^ screenrecord expected color mix] [ ^ Clean 'actual issue' VS 'expected result' document, to avoid confusion ] Improvement patch |
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. Maybe something for Mohit to look into... David, How did you create the image you have attached here ? As in if you can write the steps for mypaint. Thanks :) To be more specific - how did you get the brush to smudge the background color ? 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 ? @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... 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 ? 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 ? 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. thanks :) So, even for the Smudge radius slider, if I implement a similar function like the color picker -- it should work right ? 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. 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 :( Hi Mohit, Sure, no hurry, no pressure, take all the time necessary to prepare your exams, it's the priority :-) Good luck ! Exams over :D Back to Work :D 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 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. @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); } } 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? 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. > 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 :(
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
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
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. David, I'll give it a try tonight after my distro is updated. 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 :)
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. Hm, that final patch doesn't work -- it re-adds al;ready existing files, so it cannot apply. 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. > 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 :(
Yes, it applied and built fine. I haven't had any crashes though! 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. > 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 Bollebib has reported a crash in the new code radius :( 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 :) 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. > 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 > 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 :)
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? 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 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.
I am going through it. Initally I thought I found the problem. I just realized I didn't :( 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. 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 :(
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.
Created attachment 85729 [details]
[ ^ Clean 'actual issue' VS 'expected result' document, to avoid confusion ]
Hi, Mohit! Looks like your samples loop is not uniform enough. Or it is just disabled for some accidental reason. 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. > 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 :( 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 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 :) Created attachment 85824 [details]
Improvement patch
Latest Patch for the above improvement
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 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 :) 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. 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 :) Mohit, Can you take another look at this one? 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 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 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. 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 !
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 |
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.