Bug 342513 - Add support for fractional radius in blur and unsharp mask filters [patch]
Summary: Add support for fractional radius in blur and unsharp mask filters [patch]
Status: REPORTED
Alias: None
Product: digikam
Classification: Applications
Component: Plugin-Editor-Sharpen (show other bugs)
Version: 5.3.0
Platform: Other Linux
: NOR wishlist
Target Milestone: ---
Assignee: Digikam Developers
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-01-05 14:13 UTC by pochini
Modified: 2019-02-24 10:43 UTC (History)
2 users (show)

See Also:
Latest Commit:
Version Fixed In:


Attachments
Blur filter enhancement (6.26 KB, patch)
2015-01-05 14:14 UTC, pochini
Details
blurtest.png (71.09 KB, image/png)
2015-01-10 19:43 UTC, Maik Qualmann
Details
Blur filter enhancement (v.2) (10.37 KB, patch)
2015-01-19 22:00 UTC, pochini
Details
Blur filter enhancement (v.3) (10.89 KB, patch)
2015-02-14 11:50 UTC, pochini
Details
Blur filter enhancement (v.4) (11.21 KB, patch)
2015-02-22 23:07 UTC, pochini
Details
[patch] Blur filter enhancement (v.5) (10.29 KB, patch)
2016-12-25 20:32 UTC, pochini
Details

Note You need to log in before you can comment on or make changes to this bug.
Description pochini 2015-01-05 14:13:35 UTC
Currently the blur filter accepts and interger value for the radius parameter. This unfortunate because it does not allow the user to blur an image slightly. The blur filter is also used by the unsharp mask filter, so it has the same problem, too.
The attached patch adds support for fractional radius to the blur filter and, consequently, to the unsharp mask filter.
Just like the current version the blur kernel is a square. I was about to write a generic convolution filter, but it would have resulted in a much slower code for (probably) only a small advantage in quality.

* NOTES:
- Beta and RFC: please ignore debug code.
- It's slower than the current version.
- The effective radius is d->radius/10, which I don't like a lot. I'd prefer to make d->radius a float and to make the code pass the actual radius in place of 10*r. Both the blur and umask GUI code have to be changed.
- 8-bits images are converted to 16 bits in order to avoid rounding issues.

* Known bugs:
- It does not take into account sRGB colorspace non-linearity (e.g.: averaging a black (0) and white (255) pixel should result in a middle gray (187) value, but it returns 127 instead).
- The approximation bug of the radius value has yet to be addressed (sometimes 1.0 is passed as 9). It's actually a GUI bug and I'll file another report later.


Reproducible: Always
Comment 1 pochini 2015-01-05 14:14:44 UTC
Created attachment 90227 [details]
Blur filter enhancement
Comment 2 Maik Qualmann 2015-01-10 19:43:43 UTC
Created attachment 90332 [details]
blurtest.png

Hi pochini,

I also think this should be a double value of the radius. You seem to understand a lot of the blur kernel, maybe you could enhance it further? I once created a test image. Left image is from digiKam and right image is from Gimp or Krita. Look on the mouth.
Comment 3 pochini 2015-01-10 21:07:01 UTC
(In reply to Maik Qualmann from comment #2)
> Created attachment 90332 [details]
> blurtest.png
> 
> Hi pochini,
> 
> I also think this should be a double value of the radius. You seem to
> understand a lot of the blur kernel

Not really...


> maybe you could enhance it further? I
> once created a test image. Left image is from digiKam and right image is
> from Gimp or Krita. Look on the mouth.

Yes, most likely it is due the square kernel. Gimp and other programs feature a gaussian blur filter instead that produces a much more pleasing "bokeh" (and less artifacts that are not visible in your example). I can write a generic convolution filter that can approximate the gaussian filter. It will be a lot slower, though.
I think the blur filter in Digikam is useless. I never had the need to blur a photo... quite the opposite. I enhanced it because it is used internally by the unsharp mask filter and the integer-only radius does not allow the user (me :)) ) to set the radius small enough. IMO radius>=1 is too large in most cases because it creates halo artifacts around sharp edges.
If you think a good blur filter is useful, then I'll try to write something better. Otherwise I think it is already good enough for the unsharp mask filter, which is my primary goal.
Comment 4 caulier.gilles 2015-01-19 16:46:25 UTC
Maik,

So, we can considerate that patch is enough to be included in 4.7.0, planed next week end ?

Gilles
Comment 5 caulier.gilles 2015-01-19 16:47:57 UTC
Note : i don't yet checked the patch, but blur algorithm is used in a lots of places. For ex, in B&W converter.

We must take a care that blur loop must be very fast everywhere.

Gilles
Comment 6 Maik Qualmann 2015-01-19 18:33:49 UTC
We need yet to changes to the GUI, the radius value must be done in double value. The patch is now a blur value of 10, which corresponded to previously 1.

Maik
Comment 7 pochini 2015-01-19 21:58:59 UTC
Yes, it is not ready yet. Attached is a more complete patch (against 4.6.0) involving four files:

dimg/filters/fx/blurfilter.cpp: About the same of the previous patch, but without debug stuff. I also moved the check for too-small-radius outside the loop and reindented.

dimg/filters/fx/blurfilter.h: changed radius type to float.

dimg/filters/fx/charcoalfilter.cpp: Do not cast smooth/10 to integer.

dimg/filters/sharp/unsharpmaskfilter.cpp: Pass m_radius as-is, instead of (int)r/10.

Concerning the speed, it is about 25% slower. For example on my pc the time to blur with radius=3 an 8 bits 15MP image was 650ms and now it is 800ms.

TODO:

- Some users have not been checked yes (watermark, redeyetool, blurfxfilter, filtermanager?, infraredfilter)
- Am I missing something ?

(ok, don't laugh now, but I can't find those filters in the menus... where are they ??)
Comment 8 pochini 2015-01-19 22:00:51 UTC
Created attachment 90530 [details]
Blur filter enhancement (v.2)
Comment 9 caulier.gilles 2015-01-20 08:35:52 UTC
> Some filters that have not been checked yet (watermark, redeyetool, blurfxfilter, >filtermanager?, infraredfilter). Am I missing something ?

Grep "BlurFilter" class name to identify where algorithm is used in digiKam core :

[gilles@localhost core]$ pwd                                                                                                                                                                                       
/home/gilles/Devel/KF5/dk-sc/core 
[gilles@localhost core]$ grep -r "BlurFilter" .

==> output filtered :

UnsharpedMask filter : ./libs/dimg/filters/sharp/unsharpmaskfilter.cpp:    BlurFilter(this, m_orgImage, m_destImage, 0, 10, (int)(m_radius*10.0));
Infrared filter : ./libs/dimg/filters/bw/infraredfilter.cpp:    BlurFilter(this, BWImage, BWBlurImage, 10, 20, blurRadius);

Filter Manager for versionning feature : This much by patched to be compatible, especially if filter argumer change. There is a version ID in BlurFilter implementation dedicated for that :
./libs/dimg/filters/dimgfiltermanager.cpp:            << ImgFilterPtr(new BasicDImgFilterGenerator<BlurFilter>())
./libs/dimg/filters/dimgfiltermanager.cpp:    filterIcons.insert("digikam:BlurFilter",           "blurimage");

BlurFx filter :

./libs/dimg/filters/fx/blurfxfilter.cpp:        BlurFilter(this, *orgImage, *destImage, 10, 75, BlurRadius);
./libs/dimg/filters/fx/blurfxfilter.cpp:        BlurFilter(this, *orgImage, *destImage, 10, 80, BlurRadius);

Charcoal Filter :

./libs/dimg/filters/fx/charcoalfilter.cpp:    BlurFilter(this, m_destImage, m_destImage, 80, 85, (int)(d->smooth / 10.0));

Versionning test code :

./tests/abstractdimagehistorytest.cpp:    BlurFilter filter(iface.original(), this);

BQM Watermark tool :

./utilities/queuemanager/basetools/decorate/watermark.cpp:        BlurFilter blur(&backgroundLayer, 0L, radius);

BQM Blur Tool :

./utilities/queuemanager/basetools/enhance/blur.cpp:    BlurFilter blur(&image(), 0L, radius);

Editor Red Eyes removal tool :

./imageplugins/enhance/redeyetool.cpp:    BlurFilter blur(&mask2, 0L, d->smoothLevel->value());

Editor Blur tool :

./imageplugins/enhance/blurtool.cpp:    setFilter(new BlurFilter(&img, this, d->radiusInput->value()));
./imageplugins/enhance/blurtool.cpp:    setFilter(new BlurFilter(iface.original(), this, d->radiusInput->value()));

This grep is done on code from KF5 frameworks branch but this must be very similar on KDE4 code (git/master branch)

Voilà

Gilles Caulier
Comment 10 caulier.gilles 2015-01-20 08:41:22 UTC
Another point : Blur filter use muti-threading to compute loop.

It's based on QtConcurrent API. Most of filters in digiKam core which require computation loop use the same mechanism. It's simple : you look how CPU core are available, and you divide image to process by CPU core. The main loop is coded in a re-entrant method, and you call in parallel this method on separated thread running on each CPU core.

If you change computation loop, you must preserve multithreading support to have the best performances.

Gilles Caulier
Comment 11 pochini 2015-02-14 11:50:06 UTC
Created attachment 91070 [details]
Blur filter enhancement (v.3)

(In reply to Gilles Caulier from comment #9)
> > Some filters that have not been checked yet (watermark, redeyetool, blurfxfilter, >filtermanager?, infraredfilter). Am I missing something ?
> 
> Grep "BlurFilter" class name to identify where algorithm is used in digiKam
> core :
> 
> [gilles@localhost core]$ pwd                                                
> 
> /home/gilles/Devel/KF5/dk-sc/core 
> [gilles@localhost core]$ grep -r "BlurFilter" .

Yes, I know how to use grep and I wrote a list of the things to to yet... The point is I could not test all those tools. I didn't figure out that some of them are batch tools.


> ==> output filtered :
> 
> UnsharpedMask filter : ./libs/dimg/filters/sharp/unsharpmaskfilter.cpp:   
> BlurFilter(this, m_orgImage, m_destImage, 0, 10, (int)(m_radius*10.0));

Already fixed in the first patch.


> Infrared filter : ./libs/dimg/filters/bw/infraredfilter.cpp:   
> BlurFilter(this, BWImage, BWBlurImage, 10, 20, blurRadius);

I cannot find this one. It was a tool its own reachable from the menu. Now it is only part of the B/W tool, right?
No changes are needed as the images compare identical with and without the patch (I tried a couple of IR films).


> Filter Manager for versionning feature : This much by patched to be
> compatible, especially if filter argumer change. There is a version ID in
> BlurFilter implementation dedicated for that :
> ./libs/dimg/filters/dimgfiltermanager.cpp:            << ImgFilterPtr(new
> BasicDImgFilterGenerator<BlurFilter>())
> ./libs/dimg/filters/dimgfiltermanager.cpp:   
> filterIcons.insert("digikam:BlurFilter",           "blurimage");

Uhmm... I don't know what do do here.


> BlurFx filter :
> 
> ./libs/dimg/filters/fx/blurfxfilter.cpp:        BlurFilter(this, *orgImage,
> *destImage, 10, 75, BlurRadius);
> ./libs/dimg/filters/fx/blurfxfilter.cpp:        BlurFilter(this, *orgImage,
> *destImage, 10, 80, BlurRadius);

As above, no changes are needed: since the radius is an integer value the result is identical to the old filter.


> Charcoal Filter :
> 
> ./libs/dimg/filters/fx/charcoalfilter.cpp:    BlurFilter(this, m_destImage,
> m_destImage, 80, 85, (int)(d->smooth / 10.0));

Fixed.


> Versionning test code :
> 
> ./tests/abstractdimagehistorytest.cpp:    BlurFilter
> filter(iface.original(), this);

Looks ok as-is, but I'm not sure.


> BQM Watermark tool :
> 
> ./utilities/queuemanager/basetools/decorate/watermark.cpp:        BlurFilter
> blur(&backgroundLayer, 0L, radius);
> 
> BQM Blur Tool :
> 
> ./utilities/queuemanager/basetools/enhance/blur.cpp:    BlurFilter
> blur(&image(), 0L, radius);
> 
> Editor Red Eyes removal tool :
> 
> ./imageplugins/enhance/redeyetool.cpp:    BlurFilter blur(&mask2, 0L,
> d->smoothLevel->value());
> 
> Editor Blur tool :
> 
> ./imageplugins/enhance/blurtool.cpp:    setFilter(new BlurFilter(&img, this,
> d->radiusInput->value()));
> ./imageplugins/enhance/blurtool.cpp:    setFilter(new
> BlurFilter(iface.original(), this, d->radiusInput->value()));

No changes are needed for those tools.


> This grep is done on code from KF5 frameworks branch but this must be very
> similar on KDE4 code (git/master branch)

I attached another patch. It's against digikam-4.7.0.
Comment 12 caulier.gilles 2015-02-14 14:34:52 UTC
> Infrared filter : ./libs/dimg/filters/bw/infraredfilter.cpp:   
> BlurFilter(this, BWImage, BWBlurImage, 10, 20, blurRadius);

>>I cannot find this one. It was a tool its own reachable from the menu. Now it is only part of the B/W tool, right?

yes it it

>>No changes are needed as the images compare identical with and without the patch (I tried a couple of IR films).

ok

> Filter Manager for versionning feature : This much by patched to be
> compatible, especially if filter argumer change. There is a version ID in
> BlurFilter implementation dedicated for that :
> ./libs/dimg/filters/dimgfiltermanager.cpp:            << ImgFilterPtr(new
> BasicDImgFilterGenerator<BlurFilter>())
> ./libs/dimg/filters/dimgfiltermanager.cpp:   
> filterIcons.insert("digikam:BlurFilter",           "blurimage");

>>Uhmm... I don't know what do do here.

Nothing special here.

> BlurFx filter :
> 
> ./libs/dimg/filters/fx/blurfxfilter.cpp:        BlurFilter(this, *orgImage,
> *destImage, 10, 75, BlurRadius);
> ./libs/dimg/filters/fx/blurfxfilter.cpp:        BlurFilter(this, *orgImage,
> *destImage, 10, 80, BlurRadius);

>>As above, no changes are needed: since the radius is an integer value the result is identical to the old filter.

ok.

> Charcoal Filter :
> 
> ./libs/dimg/filters/fx/charcoalfilter.cpp:    BlurFilter(this, m_destImage,
> m_destImage, 80, 85, (int)(d->smooth / 10.0));

>>Fixed.

ok

> Versionning test code :
> 
> ./tests/abstractdimagehistorytest.cpp:    BlurFilter
> filter(iface.original(), this);

>>Looks ok as-is, but I'm not sure.

ok

> BQM Watermark tool :
> 
> ./utilities/queuemanager/basetools/decorate/watermark.cpp:        BlurFilter
> blur(&backgroundLayer, 0L, radius);
> 
> BQM Blur Tool :
> 
> ./utilities/queuemanager/basetools/enhance/blur.cpp:    BlurFilter
> blur(&image(), 0L, radius);
> 
> Editor Red Eyes removal tool :
> 
> ./imageplugins/enhance/redeyetool.cpp:    BlurFilter blur(&mask2, 0L,
> d->smoothLevel->value());
> 
> Editor Blur tool :
> 
> ./imageplugins/enhance/blurtool.cpp:    setFilter(new BlurFilter(&img, this,
> d->radiusInput->value()));
> ./imageplugins/enhance/blurtool.cpp:    setFilter(new
> BlurFilter(iface.original(), this, d->radiusInput->value()));

>>No changes are needed for those tools.

ok

I think you forgot somthing important in your patch : Blur filter settings version ID with compatibility with older version. It's for versionning feature.

This ID are registered in this header :

https://projects.kde.org/projects/extragear/graphics/digikam/repository/revisions/master/entry/libs/dimg/filters/fx/blurfilter.h#L62

Both methods :

CurrentVersion()
SupportedVersions()

return respectively the version ID of filter (with the current settings supported), and the list of version where current settings still compatible.

As you have changed settings, ID must be updated and supported version ID patched.

Gilles Caulier
Comment 13 caulier.gilles 2015-02-21 17:42:02 UTC
Pochini,

Did you seen my previous message ?

Gilles Caulier
Comment 14 pochini 2015-02-22 14:04:55 UTC
(In reply to Gilles Caulier from comment #13)
> Pochini,
> 
> Did you seen my previous message ?
> 
> Gilles Caulier

Yes, sorry, I don't have a lot of time outside the weekends...
I had a look at supportedVersions() methods. All filters are version 1, so there are no examples. I don't know how the versioning system is supposed to work. I have to (correct mw if I'm wrong):

- write a wrapper or an overloaded version of the blur filter method to provide compatibility with external (all internal users have been adapted) tools/plugins.
- call setFilterVersion() to allow caller undo/redo/repeat an operation using the correct version.
- Increment version to 2.

Since the caller invokes the methos directly, do I have to get the version the caller wants to use (how?), translate the arguments and finally call the desired filter?

Or can I just bump the version number?
Comment 15 caulier.gilles 2015-02-22 15:08:51 UTC
Bump the version number.

I plan to introduce the patch with next Qt5 based version 5.0.0, not 4.x.

Gilles Caulier
Comment 16 pochini 2015-02-22 23:07:10 UTC
Created attachment 91223 [details]
Blur filter enhancement (v.4)

Same patch, with filter version = 2.
Comment 17 caulier.gilles 2015-03-08 10:17:58 UTC
Maik,

Patch from this entry must be applied against frameworks branch, not git/master. There are to much changed and a broken compatibility about blur filter settings. Planing to fix this entry for KF5 sound the best way.

Gilles
Comment 18 pochini 2016-12-25 20:32:56 UTC
Created attachment 102990 [details]
[patch] Blur filter enhancement (v.5)

Same patch. This version can be applied against digikam 5.3.0.
Comment 19 caulier.gilles 2019-02-24 10:43:10 UTC
Pochini,

Can you adjust your patch to git/dplugins branch which will be used to release next digiKam 6.1.0 ?


Thanks in advance

Gilles Caulier