Processing framework and color space conversion

[Stefano Bonicatti]

2015-01-23 10:26 GMT+01:00 Dmitry Kazakov <dimula73 at gmail.com>:

> I don't think splitting the layer into rects and doing additional
> multithreading for a single-layered case is worth the effort. Converting a
> single layer takes a couple of seconds only and it is not so regular
> operation for painters, so they would not notice if we make it even twice
> faster :)
>

It's not only for the single layered case but for any case, but the single
layer case highlights the poor usage of the threads.
Converting a 4268x5690 image from 8bit to 16bit, single layer, takes 13
seconds on my machine (AMD Athlon II X4 640, 3Ghz), while the same image
from 8 bit to 16bit float 100 seconds.
Then remember that if you do this conversion (and you don't convert the
canvas), it keeps doing the conversion for every layer you move in the
layer stack, so it slows down things quite a bit.
Not everyone has a processor that can overclock one core to speed up
applications with few threads, moreover threads are quite underutilized in
Krita (or basicly there are very few if none operations that uses all the
cores), and this will become worse when using even bigger images.

Also the idea is not to split the image whatever dimension it has, clearly
a certain minimum rect dimension has to be chosen to be fed to a thread and
also the number of cores will dictate how much parallelization has to be
done (this if the multithreaded way needs any special code that may slow
the single/few core way).


> And for multilayered case we already have multithreading provided by the
> processings framework. Adding more threads may easily saturate cores and
> memory bus, so the benefit might drop to zero. Some time ago I was trying
> to make bitBlt() calls multithreaded (in addition to what we have already)
> and it gave only 5-20% better performance for rendering. Obviously it
> wasn't worth the effort to continue those experiments.
>

Here we agree, in fact i said that i didn't wanted to spawn threads from
the thread dealing with processing and the current node visited, but it
would make sense something like TBB or C# TPL does: you have a "generic"
scheduler (in Krita we have it already, and it doesn't have to be that
generic) where you can enqueue tasks/jobs from a job thread that is
executed and those jobs are marked to be run as childs of the job thread
they are launched in, so that they are actually part of the work the parent
job has to do and the parent also waits on them (a parallel for on the
rects to convert, and some could be processed by the caller so that the
thread doesn't get unused).


> Short answer: for your case just emit ColorSpaceChangedSignal and
> ModifiedSignal and don't think about the rest :)
>
> Long answer:
>
> startResizeImage() is called from the image worker's thread. So all the
> signals it emits get queued. It forms the queue in a special way: first
> sigContinueResizeImage() to resize the openGL textures and then
> startUpdateInPatches() to initiated their re-fetching data from the image.
> This fetching must happen form image worker threads only. After the data is
> fetched, it gets queued to be pushed further to openGL.
>
> In your case of colorspace change, all pixel data updates are handled by
> KisProcessingApplicator. You should just emit ColorSpaceChangedSignal,
> which is just "a metadata", it doesn't start any updates.
>

ColorSpaceChangedSignal is not handled by KisImage or any other class (a
part from the signal rerouter), why shouldn't i handle it and do something
with it?
If you recall we talked about color conversion and there's a problem with
the OpenGL texture bit format that is not updated when you do a conversion.
Sometimes may mean doing one less conversion, sometimes you do the
conversion anyway but at least the texture is in the correct format.
Also ModifiedSignal it's only handled by KisDocument so that it knows the
document has changed and can be saved, but KisImage doesn't handle it.
Right now, in the current single threaded conversion, when the conversion
has finished, updateCache is called. In this case if i don't handle
ColorSpaceChangedSignal, no updateCache will be called, am i wrong?
Moreover looking at the shear impl, that calls the update by itself, so why
shouldn't i?
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kimageshop/attachments/20150123/280447bc/attachment.html>