[digikam] [Bug 318726] New: Some BQM tools in a workflow don't restore (all) parameters properly

[Jan Wolter-2]

https://bugs.kde.org/show_bug.cgi?id=318726

--- Comment #18 from Jan Wolter <[hidden email]> ---
Created attachment 89843
  --> https://bugs.kde.org/attachment.cgi?id=89843&action=edit
Solves problem with Unsharp Mask tool in BQM

With this patch the radius parameter in unsharp mask tool in BQM is not castet
to int anymore.

--
You are receiving this mail because:
You are the assignee for the bug.
_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

[Gilles Caulier-4]

https://bugs.kde.org/show_bug.cgi?id=318726

Gilles Caulier <[hidden email]> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #89831|0                           |1
        is obsolete|                            |

--
You are receiving this mail because:
You are the assignee for the bug.
_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

[Gilles Caulier-4]

https://bugs.kde.org/show_bug.cgi?id=318726

--- Comment #19 from Gilles Caulier <[hidden email]> ---
Git commit d690771168ad5496e56ecc85769ba7d6c98ee577 by Gilles Caulier.
Committed on 06/12/2014 at 17:03.
Pushed by cgilles into branch 'master'.

apply patch #89843 from Jan Wolter to fix BQM sharp tool settings conversion to
XML

M  +1    -1    libs/dimg/filters/sharp/unsharpmaskfilter.cpp
M  +1    -1    utilities/queuemanager/basetools/enhance/sharpen.cpp

http://commits.kde.org/digikam/d690771168ad5496e56ecc85769ba7d6c98ee577

--
You are receiving this mail because:
You are the assignee for the bug.
_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

[Jan Wolter-2]

https://bugs.kde.org/show_bug.cgi?id=318726

--- Comment #20 from Jan Wolter <[hidden email]> ---
I started to fix the problem with the resize tool, I mentiond in comment #4. I
noticed that there is a similar problem with the watermark tool: the slider
displays default values instead of the values defined by the user. I guess,
there is a signal slot problem, but I wasn't able to solve it. I recorded an
video to show the problem: https://dl.dropboxusercontent.com/u/91057768/out.ogv
The "watermark" worflow, I created before has the following value: size: 33,
xMargin: 44, yMargin: 55. Before recording the video, I restarted digiKam,
since the problem occurs only when digiKam is just started.

--
You are receiving this mail because:
You are the assignee for the bug.
_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

[Gilles Caulier-4]

https://bugs.kde.org/show_bug.cgi?id=318726

--- Comment #21 from Gilles Caulier <[hidden email]> ---
Yes, it's sound like a race condition with signals/slots.

- The first stage is to identify which king of widget do not handle settings
passed by workflow manager

- In second identify which methods are used in this widget to set value in
widget.

- Look if this method is not call by another part which can reset value passed
by workflow manager.

Using kDebug() instance much be enough to see values passed to widget.

Using debuger can be suitable to identify who call values to set it in widget.
Turn on breakpoint and look backtrace (bt gdb command)

Gilles Caulier

--
You are receiving this mail because:
You are the assignee for the bug.
_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

[DrSlony-4]

https://bugs.kde.org/show_bug.cgi?id=318726

--- Comment #22 from DrSlony <[hidden email]> ---
The watermark tool's settings are not loaded correctly.

--
You are receiving this mail because:
You are the assignee for the bug.
_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

[Jan Wolter-2]

https://bugs.kde.org/show_bug.cgi?id=318726

Jan Wolter <[hidden email]> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #89843|0                           |1
        is obsolete|                            |

--- Comment #23 from Jan Wolter <[hidden email]> ---
Created attachment 89859
  --> https://bugs.kde.org/attachment.cgi?id=89859&action=edit
Solves problem with local contrast tool

Solves the problem that all "Power" and "Blur" parameters are set to 0 after
loading. The local contrast BQ tool expects double values for power and blur,
but the container stores them as float. With this patch, it works as expected.

P.S. Thanks for your explanation regarded race condition with signals/slots. I
will try to fix it.

--
You are receiving this mail because:
You are the assignee for the bug.
_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

[Gilles Caulier-4]

https://bugs.kde.org/show_bug.cgi?id=318726

--- Comment #24 from Gilles Caulier <[hidden email]> ---
Thanks Jan to help us.

We are very busy to port digiKam to Qt5, and it's very important to maintain
KDE4 code until next major release will be published.

More details can be seen here :

https://techbase.kde.org/Projects/Digikam/CodingSprint2014#KDE_Framework_Port

Gilles Caulier

--
You are receiving this mail because:
You are the assignee for the bug.
_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

[Gilles Caulier-4]

https://bugs.kde.org/show_bug.cgi?id=318726

--- Comment #25 from Gilles Caulier <[hidden email]> ---
Git commit c23d85f02365b3c27d1d38c914d5e2aeb13e7e98 by Gilles Caulier.
Committed on 07/12/2014 at 15:28.
Pushed by cgilles into branch 'master'.

Apply patch #89859 from JAn Wolter to fix Local Contrast tool settings
conversion from/to XML for BQM workflow

M  +2    -2    libs/dimg/filters/lc/localcontrastcontainer.cpp
M  +4    -4    libs/dimg/filters/lc/localcontrastcontainer.h

http://commits.kde.org/digikam/c23d85f02365b3c27d1d38c914d5e2aeb13e7e98

--
You are receiving this mail because:
You are the assignee for the bug.
_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

[Gilles Caulier-4]

https://bugs.kde.org/show_bug.cgi?id=318726

Gilles Caulier <[hidden email]> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #89859|0                           |1
        is obsolete|                            |

--
You are receiving this mail because:
You are the assignee for the bug.
_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

[Gilles Caulier-4]

https://bugs.kde.org/show_bug.cgi?id=318726

--- Comment #26 from Gilles Caulier <[hidden email]> ---
yes, exactly, digiKam 5.0.0.

We will continue to release 4.7.0, 4.8.0, etc... each week as bugfixes release.
If we branch digiKam core after 4.6.0 to start Qt5 port, this want mean to sync
both branches (master and frameworks). If patch applied to master are not too
intrusive, this will be easy, else... This is why only bugfixes must be apply
to 4.x series, no news features.

Porting all shared libs is done. Kipi-plugins will be long, but at least few
plugins are there to test.

digiKam core will be long, due to large regression tests to do.

Gilles

--
You are receiving this mail because:
You are the assignee for the bug.
_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

[Jan Wolter-2]

https://bugs.kde.org/show_bug.cgi?id=318726

--- Comment #27 from Jan Wolter <[hidden email]> ---
Created attachment 89911
  --> https://bugs.kde.org/attachment.cgi?id=89911&action=edit
Patch solves problem with iccConvert tool

This patch solves the problem mentioned in comment #7 by pascalio.
The problem occured because slotAssignSettings2Widget was not registered which
happens in BatchTool::registerSettingsWidget().
Furthermore, there was a small issue with the default profile. Now, the profile
on index 0 (instead of undefined index -1) in comboBox is selected.

--
You are receiving this mail because:
You are the assignee for the bug.
_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

[Gilles Caulier-4]

https://bugs.kde.org/show_bug.cgi?id=318726

--- Comment #28 from Gilles Caulier <[hidden email]> ---
Git commit 93fc881698d36a70eba1fd1749ef851574d0d2df by Gilles Caulier.
Committed on 10/12/2014 at 21:43.
Pushed by cgilles into branch 'master'.

apply patch #89911 from Jan Wolter to solves problem with BQM IccConvert tool
about settings management with workflow
Related: bug 333929
FIXED-IN: 4.6.0

M  +2    -1    NEWS
M  +1    -1    libs/dimg/filters/icc/iccprofilesettings.cpp
M  +2    -0    utilities/queuemanager/basetools/color/iccconvert.cpp

http://commits.kde.org/digikam/93fc881698d36a70eba1fd1749ef851574d0d2df

--
You are receiving this mail because:
You are the assignee for the bug.
_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

[Gilles Caulier-4]

https://bugs.kde.org/show_bug.cgi?id=318726

Gilles Caulier <[hidden email]> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #89911|0                           |1
        is obsolete|                            |

--
You are receiving this mail because:
You are the assignee for the bug.
_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

[DrSlony-4]

https://bugs.kde.org/show_bug.cgi?id=318726

--- Comment #29 from DrSlony <[hidden email]> ---
Thank you for fixing this, hope it fixes the watermark position and offset and
JPEG chroma subsampling values too.

--
You are receiving this mail because:
You are the assignee for the bug.
_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

[Jan Wolter-2]

https://bugs.kde.org/show_bug.cgi?id=318726

--- Comment #30 from Jan Wolter <[hidden email]> ---
(In reply to Gilles Caulier from comment #15)

> A filter define 3 parts :
>
> - an engine to process image, as you pass an image, it processed, and a
> target image is provided. ex :
> - a settings container, to pass to engine to tune filter effect. This
> container is used by BQM to get and set from XML to settings and vis-versa.
> Conversion is done through BatchToolSet, BatchToolSettings, and
> WorkflowManager classes from BQM. To simplify, filter settings stored in
> container, are converted as a QMap of (QString, QVariant) pair and converted
> by Qt automatically to XML in Workflow manager loops. Note that XML
> conversion is only used in BQM, not Editor. Editor use VersionManager engine
> which is mostly complicated and we don't need it in BQM.
> - a setttings widget which permit to BQM and Editor to show all settings in
> GUI.

I took a more global view on the implementation of the image filters. I noticed
that the stratefy based on a settings and container implementation (in
libs/dimg/filters/) is not used in the same manner for all filters. In the
settings code the GUI is implemented, which is used by batch queue tools (in
utilities/queuemanager/basetools/) and from the editor tools (in
imageplugins/). The container implementations are only used to communicate with
BQM.

But I noticed that there are only containers for 4 tools: local contrast, white
balance, curves, and icc. But more surprising was for me that not all tools
share a common settings implementation. For example, the resize tool has a
completely different implementation in the image editor
(imageplugins/transform/resizetool.cpp) than in the batch queue manager
(utilities/queuemanager/basetools/transform/resize.cpp). Gilles, could you
please explain these differences?

By the way, first I will fix the remaining problems mentioned in this bug
report. But I tooks this look ti see how much work it would be to port the
remaing tools (imageplugins/TODO) to BQM.

--
You are receiving this mail because:
You are the assignee for the bug.
_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

[Gilles Caulier-4]

https://bugs.kde.org/show_bug.cgi?id=318726

--- Comment #31 from Gilles Caulier <[hidden email]> ---
Jan,

The difference about filters shared between IE and BQM are due to missing time
to factoring code.

BQM is come  later than IE. Filters have been implemented for IE first. Now
step by step, BQM will use same filter than IE, if it's possible.

I resume all difference in this file :

https://projects.kde.org/projects/extragear/graphics/digikam/repository/revisions/master/entry/imageplugins/TODO

Look "Port to BQM" column.

But file is incomplete. If you look into resizetool, BQM port implement just
few features, not whole Editor tool. Look into bugzilla, there are reports
about this problem.

Also, some filters have settings container or not. This depend of complexity of
filters (especially when settings structure is huge).

Ideally, all filters must use the same framework base, but it's not the case...
due to time missing to implement it, and too much work to follow with students.

Gilles

--
You are receiving this mail because:
You are the assignee for the bug.
_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

[Jan Wolter-2]

https://bugs.kde.org/show_bug.cgi?id=318726

--- Comment #32 from Jan Wolter <[hidden email]> ---
Created attachment 89917
  --> https://bugs.kde.org/attachment.cgi?id=89917&action=edit
I forgot to fix one line related to icc default profile

--
You are receiving this mail because:
You are the assignee for the bug.
_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

[Gilles Caulier-4]

https://bugs.kde.org/show_bug.cgi?id=318726

--- Comment #33 from Gilles Caulier <[hidden email]> ---
Git commit be04a8d3befdd99fbabad00b9f4e2b1dc51a8d00 by Gilles Caulier.
Committed on 11/12/2014 at 13:48.
Pushed by cgilles into branch 'master'.

apply patch #89917

M  +1    -1    libs/dimg/filters/icc/iccprofilesettings.cpp

http://commits.kde.org/digikam/be04a8d3befdd99fbabad00b9f4e2b1dc51a8d00

--
You are receiving this mail because:
You are the assignee for the bug.
_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

[Gilles Caulier-4]

https://bugs.kde.org/show_bug.cgi?id=318726

Gilles Caulier <[hidden email]> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #89917|0                           |1
        is obsolete|                            |

--
You are receiving this mail because:
You are the assignee for the bug.
_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel