[digikam] [Bug 322789] New: Enabling "color managed view" in editor slows down tool startup

[Peter Albrecht-2]

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

--- Comment #21 from Peter Albrecht <[hidden email]> ---
Gilles, thanks for your reply.

Your bugfix (git commit aa796e8a) is not the reason for my bug:
I took digikam 3.5.0 source code, reverted your bugfix-changes in
"editortool.cpp" and "editortool.h" and compiled digikam from this source. The
long waiting time caused by many "IccTransform::apply()" calls is still there.

Another fact for "your bugfix not causing my bug":
You commit your bugfix 2013-07-23 for a bugreport in digikam 3.3.0.
I encounted "my bug" with digikam 3.2.0 on 2013-07-18 and reported my bug on
2013-07-24.
So "my digikam" was already broken as you commited this bugfix.

But nevertheless, thanks for the hint. It could have saved a few hours of
searching. ;)

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

[Peter Albrecht-2]

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

--- Comment #22 from Peter Albrecht <[hidden email]> ---
Created attachment 84472
  --> https://bugs.kde.org/attachment.cgi?id=84472&action=edit
Valgrind dump with callgrind of digikams main thread during editor tool opening

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

[Peter Albrecht-2]

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

--- Comment #23 from Peter Albrecht <[hidden email]> ---
Update:
I found the reason, why there are 70 calls to "IccTransform::apply()": Tiling!
 - the PreviewWidget is based on Q3ScrollView.
 - during opening of an image editor tool, this Q3ScrollView recieves a
QEvent::Paint
 - in Q3ScrollView::eventFilter(), there is a call to
PreviewWidget::viewportPaintEvent()
 - and in this function, there are two nested for-loops which seem to split the
image in tiles of 128x128 pixels
 - in case of my screen resolution (image size on screen: width ~= 1280, height
~= 896) there are 10 * 7 for-loop iterations => 70 iterations
 - and every iteration calls paintPreview(), which itself calls
"IccTransform::apply()"

=> one Paint-Event causes 70 "IccTransform::apply()".

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

[Peter Albrecht-2]

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

--- Comment #24 from Peter Albrecht <[hidden email]> ---
In sequence to comment #23:
On the call stack from "ImageRegionWidget::paintPreview()" to
"IccTransform::apply()", in "EditorCore::convertToPixmap()" you can find a
fresh reinitialisation of IccManager and IccTransform.

This may be the reason for those many syscalls of
  open("/usr/share/apps/libkdcraw/profiles/srgb-d65.icm", O_RDONLY|O_CLOEXEC) =
65
in the strace snipped of comment #16.

Those many reinitialisation look bad, but according to my valgrind/callgrind
dump (see  attachment #84472) (very nice to look at with KCachegrind) relative
cost of "IccManager::displayTransform()" is 0.00% (391.390 of 74.381.518.896).
The total cost is the process from "opening the tool Brightness/Contrast/Gamma"
to "tool ready to be used".

The main consumer according to the valgrind-dump is lcms'
"cmsReverseToneCurveEx()", which eats up 76.62%!

So one important question comes to my mind: What does this function do?
A) Is this part of the initialisation process of lcms? (performance loss, cause
of tiling)
B) Or is it part of the pixel-crunching, which processes my image pixels? (no
effect, cause of tiling)
In case of question A == Yes, the amount of 70 lcms initialisations per
Paint-Event would be fatal!

The API (http://www.littlecms.com/LittleCMS2.5%20API.pdf) I found, reads like A
== Yes.

But I know almost nothing about color management and lcms. So please, Gilles,
redirect this question to one of digikam's colormanagement experts.

--
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=322789

Gilles Caulier <[hidden email]> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[hidden email]

--- Comment #25 from Gilles Caulier <[hidden email]> ---
Very good investigations Peter.

I CC Francesco Riosa who has written digiKam lcms1/2 wrapper (look
core/libs/dklcms/ for details)

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

[Peter Albrecht-2]

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

--- Comment #26 from Peter Albrecht <[hidden email]> ---
I did some more testing: Tiling is definitely a problem with colormanagement on
my machine.

I changed
"digikam-3.5.0/core/utilities/imageeditor/q3support/previewwidget.cpp" line 65
from
     tileSize(128), zoomMultiplier(1.2)
to
     tileSize(1280), zoomMultiplier(1.2)
This effectively disables tiling, since all image fits in one tile.

I compiled this source and now tool starting in Image Editor is as fast as
before (about 1 second).

So I have found a first personal workaround. ;)
But, of course we/I should investigate further. It must have a reason, why
tiling was introduced first.

What bothers me, too, is that this bug can't be reproduced by you, Gilles and
Andreas. Are your CPUs that more powerful than mine? I use a "Intel(R)
Core(TM)2 Duo CPU, E8400 @ 3.00GHz".

--
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=322789

--- Comment #27 from Gilles Caulier <[hidden email]> ---
Tiling in image editor canvas exist still a very long time, before 1.0.0
release. So tiling is not the problem really...

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

[Peter Albrecht-2]

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

--- Comment #28 from Peter Albrecht <[hidden email]> ---
I guess, the problem is, that lcms' "cmsReverseToneCurveEx()" is called for
every single tile.
I'm curious about, what Francesco Riosa is saying.

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

[Marcel Wiesweg]

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

--- Comment #29 from Marcel Wiesweg <[hidden email]> ---
Why do we not encounter these problems with other places where images are
drawn, mainly the preview view?
1) color correction is applied as part of the loading process on the whole
picture.
2) there is no tiling, but of course painting region optimization, and there is
caching of scaled pixmap content
3) All is redrawn when CM settings change.

Apparently point 1) cannot be fully transferred to the editor, where we must
edit the original data. Color correction could yet be done on the preview,
before displaying it.
Pixmap caching should come for free if the new branch should be successfully
integrated at some time.

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

[Marcel Wiesweg]

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

--- Comment #30 from Marcel Wiesweg <[hidden email]> ---
Not to forget, color correction for viewing is a computing intensive task.
Letting the GPU do this may be a possibility that opens for us on KDE when
frameworks 5 arrives and if the work on CM in KWin is continued.

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

[Francesco Riosa-2]

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

--- Comment #31 from Francesco Riosa <[hidden email]> ---
Francesco Riosa has to say he forgot almost everything about the matter.
should I try to recap, there is also the lcms1 ban to commit, possibly from
every component in the digikam universe

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

[Peter Albrecht-2]

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

--- Comment #32 from Peter Albrecht <[hidden email]> ---
(In reply to comment #31)
> Francesco Riosa has to say he forgot almost everything about the matter.
> should I try to recap, there is also the lcms1 ban to commit, possibly from
> every component in the digikam universe
It's a pity. But thanks for your reply. And thanks for implementing it at all.
:)

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

[Peter Albrecht-2]

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

--- Comment #33 from Peter Albrecht <[hidden email]> ---
(In reply to comment #29)
> Why do we not encounter these problems with other places where images are
> drawn, mainly the preview view?

That's a good point! As I posted in comment #2: The Image Editor itself starts
very fast (~1 second), although its image is also color corrected. So I
compared the "editor loading" process with the "tool loading" process.

EDITOR:
      #0 Digikam::IccTransform::apply() at
/srv/local/debug/media-gfx/digikam-3.5.0/work/digikam-3.5.0/core/libs/dimg/filters/icc/icctransform.cpp:615
      #1 Digikam::DImg::convertToPixmap() at
/srv/local/debug/media-gfx/digikam-3.5.0/work/digikam-3.5.0/core/libs/dimg/dimg.cpp:2038
      #2 Digikam::EditorCore::paintOnDevice() at
/srv/local/debug/media-gfx/digikam-3.5.0/work/digikam-3.5.0/core/utilities/imageeditor/core/editorcore.cpp:666
      #3 Digikam::Canvas::paintViewport() at
/srv/local/debug/media-gfx/digikam-3.5.0/work/digikam-3.5.0/core/utilities/imageeditor/q3support/canvas.cpp:622
      #4 Digikam::Canvas::viewportPaintEvent() at
/srv/local/debug/media-gfx/digikam-3.5.0/work/digikam-3.5.0/core/utilities/imageeditor/q3support/canvas.cpp:514

PREVIEW WIDGET:
      #0 Digikam::IccTransform::apply() at
/srv/local/debug/media-gfx/digikam-3.5.0/work/digikam-3.5.0/core/libs/dimg/filters/icc/icctransform.cpp:615
      #1 Digikam::DImg::convertToPixmap() at
/srv/local/debug/media-gfx/digikam-3.5.0/work/digikam-3.5.0/core/libs/dimg/dimg.cpp:2038
      #2 Digikam::EditorCore::convertToPixmap() at
/srv/local/debug/media-gfx/digikam-3.5.0/work/digikam-3.5.0/core/utilities/imageeditor/core/editorcore.cpp:999
      #3 Digikam::ImageIface::convertToPixmap() at
/srv/local/debug/media-gfx/digikam-3.5.0/work/digikam-3.5.0/core/utilities/imageeditor/plugin/imageiface.cpp:309
      #4 Digikam::ImageRegionWidget::paintPreview() at
/srv/local/debug/media-gfx/digikam-3.5.0/work/digikam-3.5.0/core/utilities/imageeditor/q3support/imageregionwidget.cpp:141
      #5 Digikam::PreviewWidget::viewportPaintEvent() at
/srv/local/debug/media-gfx/digikam-3.5.0/work/digikam-3.5.0/core/utilities/imageeditor/q3support/previewwidget.cpp:591

Both processes call "IccTransform::apply()". And both use tiling. So both call
"IccTransform::apply()" 70 times (for my screen resolution). So why is "editor
loading" faster than "tool loading"?

Further debugging shows:
Every call to "IccTransform::apply()" in "tool loading" (PreviewWidget) causes
6 calls to "cmsReverseToneCurveEx()". (Two times three color channels.)
For "editor loading", only the first call to "IccTransform::apply()" causes 6
calls to "cmsReverseToneCurveEx()". The other 69 calls to
"IccTransform::apply()" do not cause calls to "cmsReverseToneCurveEx()"!?!?

apply() calls open():
#0 Digikam::IccTransform::open() at
/srv/local/debug/media-gfx/digikam-3.5.0/work/digikam-3.5.0/core/libs/dimg/filters/icc/icctransform.cpp:550
#1 Digikam::IccTransform::apply() at
/srv/local/debug/media-gfx/digikam-3.5.0/work/digikam-3.5.0/core/libs/dimg/filters/icc/icctransform.cpp:636
and open()'s first lines are:
bool IccTransform::open(TransformDescription& description) {
    if (d->handle)
    {
        if (d->currentDescription == description)
        {
            return true;
        }
     ...

The big difference is:
In case of "tool loading", d->handle is always "0x0", so the IccTransformation
is created via dkCmsCreateTransform() every time of the 70 calls of
IccTransform::apply(). Leading to 70 x 6 = 420 calls to
cmsReverseToneCurveEx().
In case of "editor loading", digikam realizes that an IccTransformation has
already been created and leaves "IccTransform::open()" with the "return true"
mentioned in the code snipped above. So "editor loading" produces 1 x 6 = 6
calls to cmsReverseToneCurveEx().

So, the next question seems to be: Why does the "tool loading" process lose
"d->handle"?
It is set in IccTransform::open(), in line 543: d->handle =
dkCmsCreateTransform(...);

--
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=322789

--- Comment #34 from Gilles Caulier <[hidden email]> ---
>So why is "editor loading" faster than "tool loading"?

Because, as i explained before, canvas from editor tool (which is not the same
widget than editor main canvas) is redraw more than one time due to
signal/slots problem at init. I suspect something wrong with widget style which
send a lots of signal to widget at init (i don't know why).

This is true with CM on or off. As CM is time consuming, this affect tool time
loading.

Just to be sure, change your KDE widget style. Go to digiKam setup misc. Here i
use Oxygen widget.

Note : in all case, i suspect something wrong in EditorTool.cpp implementation
about tool initialization.

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

[Peter Albrecht-2]

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

--- Comment #35 from Peter Albrecht <[hidden email]> ---
(In reply to comment #34)
> >So why is "editor loading" faster than "tool loading"?
> Because, as i explained before, ...

This was meant as a "rhetorical question" to show my chain of thoughts. ;)
Sorry, I won't use such sentence stile any more, since it can easily be
misunderstood.

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

[Peter Albrecht-2]

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

--- Comment #36 from Peter Albrecht <[hidden email]> ---
(In reply to comment #34)
> I suspect something wrong with widget style which send a lots of signal to widget at init (i don't know why).
> ...
> Just to be sure, change your KDE widget style. Go to digiKam setup misc.
> Here i use Oxygen widget.

Gilles, you are right! I use widget style "QtCurve" (gentoo ebuilds:
x11-themes/gtk-engines-qtcurve-1.8.16 and x11-themes/qtcurve-1.8.14). And there
is definitely a problem with this style and digikam: It adds one redundant call
to PreviewWidget::viewportPaintEvent() to the "editor tool opening" process!

Using digikam with "Oxygen" widget style, PreviewWidget::viewportPaintEvent()
is called ONCE, while editor tool is beeing opened.
Using digikam with "QtCurve" widget style, PreviewWidget::viewportPaintEvent()
is called TWICE, while editor tool is beeing opened.

But there is also another, more important bug in
PreviewWidget::viewportPaintEvent() and its called functions: See
> d->handle is always "0x0"
in comment #33.

This "viewportPaintEvent()" bug makes me wait 6 seconds for one call to
PreviewWidget::viewportPaintEvent().

So fixing the "QtCurve widget bug" will reduce my waiting time from 12 to 6
seconds, while fixing the "viewportPaintEvent() bug" will reduce waiting time
to 1 second. That's the time the editor needs to process its
Canvas::viewportPaintEvent(), so I guess viewportPaintEvent() can get as fast
as that, if d->handle is not always "0x0".

So I'll concentrate on viewportPaintEvent() first. Also the "QtCurve widget
bug" seems to be more complicated to solve, since you need good KDE knowledge,
which I don't have (yet ;).
Should I open a separate bug: "QtCurve widget: calling
PreviewWidget::viewportPaintEvent() twice on editor tool opening"  ?

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

[Peter Albrecht-2]

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

--- Comment #37 from Peter Albrecht <[hidden email]> ---
I guess, I finally found the problem:

*** Short version (if you know the whole comment history) ***
The IccTransform object is created on the stack in
 #2 Digikam::EditorCore::convertToPixmap() at
/srv/local/debug/media-gfx/digikam-3.5.0/work/digikam-3.5.0/core/utilities/imageeditor/core/editorcore.cpp:999
See stacktrace "PREVIEW WIDGET:" in comment #33
So it gets created and deleted for every tile of the preview beeing processed.


*** Long version (if you don't want to read the whole comment history) ***
cmsReverseToneCurveEx() is a quite time consuming function.

Problematic stacktrace:
#0 cmsReverseToneCurveEx() at
/srv/local/debug/media-libs/lcms-2.5-r1/work/lcms2-2.5/src/cmsgamma.c:867
#1 BuildRGBOutputMatrixShaper() at
/srv/local/debug/media-libs/lcms-2.5-r1/work/lcms2-2.5/src/cmsio1.c:476
#2 _cmsReadOutputLUT() at
/srv/local/debug/media-libs/lcms-2.5-r1/work/lcms2-2.5/src/cmsio1.c:644
#3 DefaultICCintents() at
/srv/local/debug/media-libs/lcms-2.5-r1/work/lcms2-2.5/src/cmscnvrt.c:561
#4 cmsCreateExtendedTransform() at
/srv/local/debug/media-libs/lcms-2.5-r1/work/lcms2-2.5/src/cmsxform.c:692
#5 cmsCreateMultiprofileTransformTHR() at
/srv/local/debug/media-libs/lcms-2.5-r1/work/lcms2-2.5/src/cmsxform.c:807
#6 cmsCreateTransformTHR() at
/srv/local/debug/media-libs/lcms-2.5-r1/work/lcms2-2.5/src/cmsxform.c:848
#7 cmsCreateTransform() at
/srv/local/debug/media-libs/lcms-2.5-r1/work/lcms2-2.5/src/cmsxform.c:858
#8 Digikam::IccTransform::open() at
/srv/local/debug/media-gfx/digikam-3.5.0/work/digikam-3.5.0/core/libs/dimg/filters/icc/icctransform.cpp:541
#9 Digikam::IccTransform::apply() at
/srv/local/debug/media-gfx/digikam-3.5.0/work/digikam-3.5.0/core/libs/dimg/filters/icc/icctransform.cpp:636
#10 Digikam::DImg::convertToPixmap() at
/srv/local/debug/media-gfx/digikam-3.5.0/work/digikam-3.5.0/core/libs/dimg/dimg.cpp:2038
#11 Digikam::EditorCore::convertToPixmap() at
/srv/local/debug/media-gfx/digikam-3.5.0/work/digikam-3.5.0/core/utilities/imageeditor/core/editorcore.cpp:999
#12 Digikam::ImageIface::convertToPixmap() at
/srv/local/debug/media-gfx/digikam-3.5.0/work/digikam-3.5.0/core/utilities/imageeditor/plugin/imageiface.cpp:309
#13 Digikam::ImageRegionWidget::paintPreview() at
/srv/local/debug/media-gfx/digikam-3.5.0/work/digikam-3.5.0/core/utilities/imageeditor/q3support/imageregionwidget.cpp:141
#14 Digikam::PreviewWidget::viewportPaintEvent() at
/srv/local/debug/media-gfx/digikam-3.5.0/work/digikam-3.5.0/core/utilities/imageeditor/q3support/previewwidget.cpp:591

Opening an editor tool in digikam's image editor calls
PreviewWidget::viewportPaintEvent().
This function uses tiling, so the subsequent called functions are all called 70
times in my case. (see comment #23)
In EditorCore::convertToPixmap() an IccTransform object is created and placed
on the stack.
In IccTransform::open() this IccTransform object realizes, that no color
profile has been loaded yet.
So the color profile gets loaded which leads to calls to
cmsReverseToneCurveEx().
This color profile then is applied to the current tile.
When done with this tile, the execution pointer climbs up the stacktrace again
to PreviewWidget::viewportPaintEvent(), where the next tile will be processed
this way.

The problem is: When climbing up the stacktrace and returning from
EditorCore::convertToPixmap(), the IccTransform object gets
deleted from the stack. And together with this object, the loaded color profile
gets deleted. So for the next tile to be processed,
an new IccTransform object gets created, which again loads the color profile.
Again leading to time consuming calls to
cmsReverseToneCurveEx().


*** Solution ***
The IccTransform object someway needs to be kept alive for the whole
tile-processing of PreviewWidget::viewportPaintEvent().
Since I am not very familiar with KDE programming itself nor do I know the
digikam architecture very well, I need
your help Gilles, Marcel or whoever wants to help.

--
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=322789

--- Comment #38 from Gilles Caulier <[hidden email]> ---
Peter,

With 4.0.0, we have ported image editor to Qt4 model/view. A lots of code have
changed here.

Can you reproduce the problem with this release ?

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

[Peter Albrecht-2]

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

--- Comment #39 from Peter Albrecht <[hidden email]> ---
Sorry for the late reply. I'll test it as sone as I have enough spare time
again.

--
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=322789

--- Comment #40 from Gilles Caulier <[hidden email]> ---
Peter,

digiKam 4.1.0 is just out. Please test with this release if you can...

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