Wavelet decompose plugin
ClosedPublic

Authored by rempt on Aug 3 2016, 9:50 PM.

Details

Reviewers
dkazakov
miroslavtalasek
Group Reviewers
Krita
Summary

I use it in retouch of my photos , it enable change for example skin texture but keep color or change color but keep structure

here is original Gimp Plugin
http://registry.gimp.org/node/11742

mine is mathematicaly indentical, so the result is the same

Diff Detail

Lint
Lint Skipped
Unit
Unit Tests Skipped
miroslavtalasek retitled this revision from to Wavelet decompose plugin.
miroslavtalasek updated this object.
miroslavtalasek edited the test plan for this revision. (Show Details)
miroslavtalasek set the repository for this revision to R37 Krita.
rempt added a reviewer: Krita.Aug 3 2016, 9:52 PM
miroslavtalasek removed R37 Krita as the repository for this revision.

i miss some files ans i fix copyright

rempt added a subscriber: rempt.Aug 3 2016, 9:59 PM

I haven't built it yet: in general, it looks fine to me, I've just added a couple of nit-picking notes. I'll build the patch tomorrow and test it, but it looks fine in general!

libs/image/kis_wavelet_kernel.h
2 ↗(On Diff #5660)

Check your copyright :-)

Also, if this class is only used by your plugin, it would be better to move it into the plugin, instead of the already too-big libs/image library. If you intend to add other plugins that use this class, then we can always move it back.

plugins/extensions/waveletdecompose/dlg_waveletdecompose.cpp
23

No need to keep commented-out lines.

plugins/extensions/waveletdecompose/waveletdecompose.cpp
89

Maybe lose a few empty lines here?

93

I realize that there's a lot of places in Krita where we create dialogs this way, but "modern" thinking prefers to create dialogs on the stack, so you don't need to explicitly delete them -- that is helpful if later on there will be more than one exit location from this function.

97

I'm suddenly unsure, but if the dialog is accepted, it's hidden already, isn't it?

133

if "lev" is meant to mean "level", just write it out in full.

136

In general, don't forget to add a space after a comma, for readability.

  • fix dialog static instead pointer for beter safety
  • move file with kernel into plugin from library
  • delete comented lines of code
  • delete double empty line
  • add spaces after comma
  • rename lev to level

delete ugly obtain parent layer of image and add better way

I built it and it seems like it is working ok. The option is in the Layers menu which is getting really long. I think where you put it is ok for now. I started a discussion about organizing the layers menu, so it might get moved around at a later time.

https://forum.kde.org/viewtopic.php?f=137&t=135163

rempt accepted this revision.Aug 4 2016, 6:58 AM
rempt added a reviewer: rempt.

I'm fine with merging!

This revision is now accepted and ready to land.Aug 4 2016, 6:58 AM
rempt commandeered this revision.Aug 5 2016, 8:02 AM
rempt edited reviewers, added: miroslavtalasek; removed: rempt.

I'm fine with merging!

This revision now requires review to proceed.Aug 5 2016, 8:02 AM

There is a small safety comment, which can be fixed after the merge.

plugins/extensions/waveletdecompose/waveletdecompose.cpp
85

It is better to change this line into image->barrierLock(), otherwise your action can interfere with the suspended strokes.

to all:
probably, we should rename image->lock() into something image->lockNoBarrierNeverUseUnlessYouReallyKnowWhatYouAreDoing() ? :)

miroslavtalasek added inline comments.Aug 8 2016, 9:56 AM
plugins/extensions/waveletdecompose/waveletdecompose.cpp
85

I copy paste from extension split layers whis is my start example

dkazakov accepted this revision.Aug 17 2016, 7:08 AM
dkazakov added a reviewer: dkazakov.

The patch has been merged into master!

This revision is now accepted and ready to land.Aug 17 2016, 7:08 AM
dkazakov closed this revision.Aug 17 2016, 7:08 AM