Bug 342513

Created attachment 90227 [details]
Blur filter enhancement

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.

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

Maik,

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

Gilles

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

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

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

Created attachment 90530 [details]
Blur filter enhancement (v.2)

> 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

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

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.

> 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

Pochini,

Did you seen my previous message ?

Gilles Caulier

(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?

Bump the version number.

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

Gilles Caulier

Created attachment 91223 [details]
Blur filter enhancement (v.4)

Same patch, with filter version = 2.

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

Created attachment 102990 [details]
[patch] Blur filter enhancement (v.5)

Same patch. This version can be applied against digikam 5.3.0.

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

I considerate this file as obsolete. If you want a more powerful blur/sharpen filters, with more adjustments,considerate to use GmicQt tool available in digiKam image editor

https://i.imgur.com/txAhZbR.png

Gilles Caulier