Krita useable for Blender movies

Hello,

you may know, if you are regularly on IRC, that we would like to start  
project which makes Krita useable for movies, that Blender Foundation is
producing these years (Durian this year).

I would be hired to work on Krita for 3-4 months if we manage to get donations
from people. I was chosen not because I'm special, but because I have time
(I'm finishing my studies this year). If any more experienced Krita hacker
would go for it, I don't have problem to be replaced ;) Krita will remain
hobby for me anyway. I would really like to see the whole Krita community be
involved as this collaboration with Blender people can help Krita become more
professional. Without you I can't and will not do anything.

So..we all know that our performance is not good enough. But I want to know
what is slow and what can be done for fixing stuff.

We also have some UI problems in Krita. There are some controls for GUI that
are not very clever, they need to be fixed. shicmap is working on some stuff..

I mailed few e-mails to David Revoy, very talented digital painter and he
mailed me back about Krita. He supports us but so far Krita is not useable for
him as digital painter. I wrote down few stuff in wiki [1].

So far it is performance, gui and openexr compatiblity. I'm still waiting for
more emails for David but he is reasonable busy these days.

Then I noticed some notes about slowness here [2]
There are some bug reports in bugzilla also e.g. [3]

Also enki wrote use-cases which are very usable [5]

So I'm starting this discussion to point out what can be done, what is slow,
how you see the problem?

What I, as the paintop creator, see, is that many things are slow with CPU,
even with dual cores. I created small OpenGL app that uses QtOpenGl to paint
and that is just real-time painting with big brushes that can be scaled
realtime etc. (Just with RGB8 colorspace of course) but I see this as big
opportunity for Krita. Use GLSL where it is possible and here we could really
speed-up Krita.

Regarding GUI I'm offering help to shicmap, she can design, I can code.

Lukas

[1] http://wiki.koffice.org/index.php?title=Krita/Artists_Requirements
[2] http://wiki.koffice.org/index.php?title=Krita/Optimization#Hot_Spots
[3] https://bugs.kde.org/show_bug.cgi?id=187344
[4] http://www.opengl.org/documentation/glsl/
[5] http://wiki.koffice.org/index.php?title=Krita/Use_Cases
_______________________________________________
kimageshop mailing list
kimageshop@...
https://mail.kde.org/mailman/listinfo/kimageshop

So I'm starting this discussion to point out what can be done, what is slow,
how you see the problem?

How i see it:

1) I wouldn't say it's a tile engine internal fault that Krita is slow. I'd even say that it's not tile engine's fault at all (mostly). =)

Most problems come from misusing the engine. And the biggest problem here is bitBlt'ing at higher level of abstraction (colorspaces). We used to discuss it on irc, as i remember. Tile engine (at least new one) supports implicit tile sharing and lazy copying, BUT higher layers of abstractions do not use it at all! They must use it when source and destination colorspaces coincide, it'll be much faster than direct copying of data stride by stride.

But to solve this issue we need to design a good programming interface for it.
The implementation will not be that difficult, i can do this as soon we discuss the interface. We could discuss it on sprint?


2) To make bitBlt'ing more faster we should solve another one issue: compositeOps constants in pigment.

I don't know how it happened, but all the compositeOp constants (like COMPOSITE_OVER) are strings! And for every stride of the image (there might be more than 1000 strides per image) we do a string comparison! More than that, we do many string comparisons on every composition! See [1].

I don't know where COMPOSITE_OVER and COMPOSITE_ALPHA_DARKEN are implemented, maybe their application is optimized a bit, but most of the others should suffer from this comparison.

Of course, Qt may smooth this comparison procedure a bit with shared internal data, but nevertheless integer switch should be much faster than that.

What do i suggest?
Change all these constants' type to qint32 and create separate function that would show their name using integers. This chain of if() constrictions in [1] could be replaced with a simple 'switch' construction. Theoretically, a compiler can optimize this switch with a jump table at asm level.

3) Slow work on dual-core processors is connected with the fact that there almost no threading in Krita at the moment. =) Yes, we have KisImageUpdater but it does almost nothing for speed, because while it works other parts of Krita (e.g. KisView) are waiting for his work finished. It means that Krita is almost single-threaded: while one thread is working, others - are waiting. I saw it when i started a system monitor in parallel with Krita, it showed that only one core is working at the same moment (i tested it about a month ago).

I'm working on a layers merging parallelization right now. The same algorithm can (and should) be used in KisView for pre-scaling.


4) openGL is good, but i think we could use processor's capabilities like SSE and friends first.


[1] - libs/pigment/colorspaces/KoRgbU8CompositeOp.cpp:65

PS:
I don't want to say the tile engine shouldn't be optimized itself, i mean it's API should be optimized first.

--
Dmitry Kazakov

_______________________________________________
kimageshop mailing list
kimageshop@...
https://mail.kde.org/mailman/listinfo/kimageshop

[Cyrille Berger]

On Friday 23 October 2009, Dmitry Kazakov wrote:
> [1] - libs/pigment/colorspaces/KoRgbU8CompositeOp.cpp:65
We don't use that code path for most composite ops. For the remaining one,
which still use that path, someone should take the time to move them to their
own classes (see libs/pigment/colorspaces/KoRgbU8ColorSpace.h l83 to l94)

I really insist on the need to use valgrind or sysprof when profiling to find
issues, reading the code isn't enough, because in a complex program it is
extremely difficult to know wether a code path or not is used.

That said some operation, like COPY would need a better architecture, and
probably delegation to the tile manager instead of doing individual memcpy in
a composite op.
--
Cyrille Berger
_______________________________________________
kimageshop mailing list
kimageshop@...
https://mail.kde.org/mailman/listinfo/kimageshop

[Thomas Zander-7]

On Friday 23. October 2009 13.47.54 Dmitry Kazakov wrote:

> 2) To make bitBlt'ing more faster we should solve another one issue:
> compositeOps constants in pigment.
>
> I don't know how it happened, but all the compositeOp constants (like
> COMPOSITE_OVER) are strings! And for every stride of the image (there might
> be more than 1000 strides per image) we do a string comparison! More than
> that, we do many string comparisons on every composition! See [1].
>
> I don't know where COMPOSITE_OVER and COMPOSITE_ALPHA_DARKEN are
> implemented, maybe their application is optimized a bit, but most of the
> others should suffer from this comparison.
>
> Of course, Qt may smooth this comparison procedure a bit with shared
> internal data, but nevertheless integer switch should be much faster than
> that.
>
> What do i suggest?
> Change all these constants' type to qint32 and create separate function
> that would show their name using integers. This chain of if() constrictions
> in [1] could be replaced with a simple 'switch' construction.
> Theoretically, a compiler can optimize this switch with a jump table at asm
> level.

My suggestion would be to avoid this lookup altogether; use the strategy
design pattern and instead of having an Id (either string or integer) you can
just store a pointer to the method/class that implements your code.

http://en.wikipedia.org/wiki/Strategy_pattern

> [1] libs/pigment/colorspaces/KoRgbU8CompositeOp.cpp:65

A quick optimization for this would be to cache the qstring at the start of
the method and not call id() every time.
Sorting on most used composite op to be at top helps too, I'd guess that
  if (id() == COMPOSITE_UNDEF)
is called a lot more than needed...

--
Thomas Zander
_______________________________________________
kimageshop mailing list
kimageshop@...
https://mail.kde.org/mailman/listinfo/kimageshop

On Friday 23 October 2009, Thomas Zander wrote:

> My suggestion would be to avoid this lookup altogether; use the strategy
> design pattern and instead of having an Id (either string or integer) you
>  can just store a pointer to the method/class that implements your code.

That's already done. This file is really old and the implementation dates back
to when composite ops were identified by const ints. And, of course,
comparison of const QString's isn't as expensive as one would expect, since
it's all implicitly shared.

> http://en.wikipedia.org/wiki/Strategy_pattern
>
> > [1] libs/pigment/colorspaces/KoRgbU8CompositeOp.cpp:65
>
> A quick optimization for this would be to cache the qstring at the start of
> the method and not call id() every time.
> Sorting on most used composite op to be at top helps too, I'd guess that
>   if (id() == COMPOSITE_UNDEF)
> is called a lot more than needed...

I suspect it's hardly called at all. As Cyrille said, talking about
performance optimization without running valgrind is worse than useless. We
have done a lot of profiling already and we know where the hotspots currently
are, and it's not in this legacy code that only is used for a relatively small
number of less used composit ops.

--
Boudewijn Rempt | http://www.valdyas.org
_______________________________________________
kimageshop mailing list
kimageshop@...
https://mail.kde.org/mailman/listinfo/kimageshop

On Friday 23 October 2009, Cyrille Berger wrote:

> We don't use that code path for most composite ops. For the remaining one,
> which still use that path, someone should take the time to move them to
>  their own classes (see libs/pigment/colorspaces/KoRgbU8ColorSpace.h l83 to
>  l94)

That would have the nice side effect of making them available for other
colorspaces as well, if implemented in a similar way to the other composite op
classes :-).

What is really needed, as well, is a good set of unittests for our composite
ops. We have at least four bugs related to buggy composite ops:

176536: unexpected behavior of eraser, copy, add and substract modes
194476: stylus doesn't work in rgb 16
209532: no paintop transparency with sRGB & SRGB64 profiles (16 bit float and
32 bit modes)
211185: rgb8 out composite op broken

--
Boudewijn Rempt | http://www.valdyas.org
_______________________________________________
kimageshop mailing list
kimageshop@...
https://mail.kde.org/mailman/listinfo/kimageshop

Well, i agree with you! =) If you say this code is rarely used, it won't help us boost performance =)

PS:
/me still thinks that using strings for internally used constants is a bad practice =)
And /me doesn't think implicit sharing of QString will excuse it.
But i wont argue about it this time =)


--
Dmitry Kazakov

_______________________________________________
kimageshop mailing list
kimageshop@...
https://mail.kde.org/mailman/listinfo/kimageshop

On Friday 23 October 2009, Dmitry Kazakov wrote:

> 1) I wouldn't say it's a tile engine internal fault that Krita is slow. I'd
> even say that it's not tile engine's fault at all (mostly). =)

Depends a bit on which tile engine you're talking about -- the currently
active one has that nasty lock that kicks in whenever a tile is accessed. This
effectively serializes everything in Krita -- see the mutrace output I posted
a while ago. The second one doesn't support swapping, so it's not comparable.

And I am sure there is a lot left to improve for both tile engines, if even
the Gimp people found some serious issues in their decades old tile engine
this year.

Right now, the easiest win we can make in the tile engine is to make the
iterators cache the tiles they are accessing in a row or column. That's
something that has been borne out by proper profiling.

> Most problems come from *misusing* the engine. And the biggest problem here
> is bitBlt'ing at higher level of abstraction (colorspaces). We used to
> discuss it on irc, as i remember. Tile engine (at least new one) supports
> implicit tile sharing and lazy copying, BUT higher layers of abstractions
>  do not use it at all! They must use it when source and destination
>  colorspaces coincide, it'll be much faster than direct copying of data
>  stride by stride.
>
> But to solve this issue we need to design a good programming interface for
> it.
> The implementation will not be that difficult, i can do this as soon we
> discuss the interface. We could discuss it on sprint?
>
>
> 2) To make bitBlt'ing more faster we should solve another one issue:
> compositeOps constants in pigment.
>
> I don't know how it happened, but all the compositeOp constants (like
> COMPOSITE_OVER) are strings! And for every stride of the image (there might
> be more than 1000 strides per image) we do a string comparison! More than
> that, we do many string comparisons on every composition! See [1].

No, we don't. We get a KoCompositeOp subclass instance and apply that. It's
easy enough to see that we don't do all those string comparisons by running
valgrind.

> I don't know where COMPOSITE_OVER and COMPOSITE_ALPHA_DARKEN are
> implemented, maybe their application is optimized a bit, but most of the
> others should suffer from this comparison.

For rgbu8, ADD, ALPHA_DARKEN, BURN, DIVIDE, DODGE, ERASE, MULTIPLY, OVER,
OVERLAY, SCREEN, SUBTRACT are implemented as separate, templated classes.

COMPOSITE_DARKEN, COMPOSITE_LIGHTEN,COMPOSITE_HUE,  
COMPOSITE_SATURATIONCOMPOSITE_VALUE, COMPOSITE_COLOR, COMPOSITE_IN
COMPOSITE_OUT,  COMPOSITE_ADD,   COMPOSITE_DIFF COMPOSITE_BUMPMAP,  
COMPOSITE_CLEAR, COMPOSITE_DISSOLVE are implemented in this legacy code.

Other colorspaces do not have the legacy code path.

> Of course, Qt may smooth this comparison procedure a bit with shared
> internal data, but nevertheless integer switch should be much faster than
> that.

It is optimized a lot in Qt -- and don't forget that these are const's. But,
as Cyrille has said, it's old code.

>
> What do i suggest?
> Change all these constants' type to qint32 and create separate function
>  that would show their name using integers. This chain of if()
>  constrictions in [1] could be replaced with a simple 'switch'
>  construction. Theoretically, a compiler can optimize this switch with a
>  jump table at asm level.

That's not necessary because we've already implemented something like Thomas
Zander suggested three years ago. We just haven't had the time to port all
code.

> 3) Slow work on dual-core processors is connected with the fact that there
> almost no threading in Krita at the moment. =) Yes, we have KisImageUpdater
> but it does almost nothing for speed, because while it works other parts of
> Krita (e.g. KisView) are waiting for his work finished.

KisView is not waiting in the sense that it blocks until the image is
refreshed. The gui is kept responsive at all times because the recomputation
isn't in done in the gui thread. Note that actually painting, i.e., making
strokes, also involves running a thread, which means there are 3 threads
running already while painting.

> It means that Krita
> is almost single-threaded: while one thread is working, others - are
> waiting. I saw it when i started a system monitor in parallel with Krita,
>  it showed that only one core is working at the same moment (i tested it
>  about a month ago).

When painting with a big brush (~500) pixels, kysguard shows that both cores
are actually saturated. One is recompositing, the other is computing the 500
pixel mask for every dab (which is the biggest measurable performance issue we
have).

> I'm working on a layers merging parallelization right now. The same
> algorithm can (and should) be used in KisView for pre-scaling.

That sounds cool! But please -- no enormous code drops, make sure we have a
manageable set of small patches for review!

> 4) openGL is good, but i think we could use processor's capabilities like
> SSE and friends first.

I'm vacillating on this... sse etc. are nice, but most distributors do not
enable them by default, except for the 64 bits systems. OpenGL + glsl has
the potential to give much more performance, but I haven't got a system where
I can use it. And if we can move more and more work to opengtl, we can profit
from the automatic optimization it gives us.


--
Boudewijn Rempt | http://www.valdyas.org
_______________________________________________
kimageshop mailing list
kimageshop@...
https://mail.kde.org/mailman/listinfo/kimageshop

On Friday 23 October 2009, Dmitry Kazakov wrote:
> Well, i agree with you! =) If you say this code is rarely used, it won't
> help us boost performance =)
>
> PS:
> /me still thinks that using strings for internally used constants is a bad
> practice =)
> And /me doesn't think implicit sharing of QString will excuse it.
> But i wont argue about it this time =)

Indeed it doesn't, I just read the Qt code: it first compares the size, then
does a very optimized memory comparison. I would have thought it would have
done a comparison of the shared pointer, but no doubt the Qt hackers know
better.


--
Boudewijn Rempt | http://www.valdyas.org
_______________________________________________
kimageshop mailing list
kimageshop@...
https://mail.kde.org/mailman/listinfo/kimageshop

[Cyrille Berger]

On Friday 23 October 2009, Dmitry Kazakov wrote:
> PS:
> /me still thinks that using strings for internally used constants is a bad
> practice =)
> And /me doesn't think implicit sharing of QString will excuse it.
> But i wont argue about it this time =)
The problem is that those strings are not internal, and we want the set of
composite op to be extensible. That's why we moved from int to qstring. As as
side note, since QString (even with implicite sharing) comparison can be
extensive, there is a class in pigment that gives you an integer for a string
that can be stored to get fast comparison, we do that for comparing to
colorspace. Here it would be an improvement, but the real fix is to migrate the
remaining composite op.

--
Cyrille Berger
_______________________________________________
kimageshop mailing list
kimageshop@...
https://mail.kde.org/mailman/listinfo/kimageshop

[Cyrille Berger]

On Friday 23 October 2009, Boudewijn Rempt wrote:
> On Friday 23 October 2009, Cyrille Berger wrote:
> > We don't use that code path for most composite ops. For the remaining
> > one, which still use that path, someone should take the time to move them
> > to their own classes (see libs/pigment/colorspaces/KoRgbU8ColorSpace.h
> > l83 to l94)
>
> That would have the nice side effect of making them available for other
> colorspaces as well, if implemented in a similar way to the other composite
>  op classes :-).
To be honest, most of the remaining composite op in that file where very RGB
specific. Could still be usefull for higher bit depth ;)

> What is really needed, as well, is a good set of unittests for our
>  composite ops. We have at least four bugs related to buggy composite ops:
yes.
 
> 176536: unexpected behavior of eraser, copy, add and substract modes
> 194476: stylus doesn't work in rgb 16
> 209532: no paintop transparency with sRGB & SRGB64 profiles (16 bit float
>  and 32 bit modes)
The funny thing... there is a test for that one :D

> 211185: rgb8 out composite op broken


--
Cyrille Berger
_______________________________________________
kimageshop mailing list
kimageshop@...
https://mail.kde.org/mailman/listinfo/kimageshop

> > 4) openGL is good, but i think we could use processor's capabilities like
> > SSE and friends first.
> I'm vacillating on this... sse etc. are nice, but most distributors do not
> enable them by default, except for the 64 bits systems. OpenGL + glsl has
> the potential to give much more performance, but I haven't got a system
>  where I can use it. And

Yep, for Blender people, they can always enable these features. And for
anybody seriously interested in Krita is not a problem to get optimized
version I think.

>  if we can move more and more work to opengtl, we
>  can profit from the automatic optimization it gives us.
>

How the OpenGTL can give us optimizations? Cyrille?
_______________________________________________
kimageshop mailing list
kimageshop@...
https://mail.kde.org/mailman/listinfo/kimageshop

On Friday 23 October 2009, LukasT.dev@... wrote:

> So I'm starting this discussion to point out what can be done, what is
>  slow, how you see the problem?

We should first fix the hot spots in the links you mention. That's for after
2.1. Cyrille hinted that he knew why the opengtl-based colorspaces are pretty
slow, and those are important since they deliver the linear, hdr colorspace
that Ton asked about.

For the rest, it's profiling, profiling, profiling, with oprofile, valgrind
and mutrace.

> What I, as the paintop creator, see, is that many things are slow with CPU,
> even with dual cores. I created small OpenGL app that uses QtOpenGl to
>  paint and that is just real-time painting with big brushes that can be
>  scaled realtime etc. (Just with RGB8 colorspace of course) but I see this
>  as big opportunity for Krita. Use GLSL where it is possible and here we
>  could really speed-up Krita.

Is that in svn or on gitorious somewhere? I'm not opposed on principle to
using opengl in more places -- we might even have an opengl textures backed
paint device, if that is useful. As long as Krita remains usable for me :-)

> Regarding GUI I'm offering help to shicmap, she can design, I can code.

Have you already seen the original design on the wiki? Shicmap told me she
will be very busy for some time.

--
Boudewijn Rempt | http://www.valdyas.org
_______________________________________________
kimageshop mailing list
kimageshop@...
https://mail.kde.org/mailman/listinfo/kimageshop

On Friday 23 October 2009 14:49:07 Boudewijn Rempt wrote:

> On Friday 23 October 2009, Dmitry Kazakov wrote:
> > Well, i agree with you! =) If you say this code is rarely used, it won't
> > help us boost performance =)
> >
> > PS:
> > /me still thinks that using strings for internally used constants is a
> > bad practice =)
> > And /me doesn't think implicit sharing of QString will excuse it.
> > But i wont argue about it this time =)
>
> Indeed it doesn't, I just read the Qt code: it first compares the size,
>  then does a very optimized memory comparison. I would have thought it
>  would have done a comparison of the shared pointer, but no doubt the Qt
>  hackers know better.
>

I'm student of image processing and we are taught that Strings, no matter how
they are implemented, are in the end expensive in image processing. I never
seen them in code which aims to be fast (OpenGL, OpenCV,...).

But I don't say we should rewrite Krita and throw away QString, I just would
not use them for comparison in loops etc. And the loops can be hidden pretty
much, you can't say that this or that code will not be in a loop in the end.
The reason? Because of complexity of Krita and image processing in general.
_______________________________________________
kimageshop mailing list
kimageshop@...
https://mail.kde.org/mailman/listinfo/kimageshop

On Friday 23 October 2009 14:55:50 Boudewijn Rempt wrote:

> On Friday 23 October 2009, LukasT.dev@... wrote:
> > So I'm starting this discussion to point out what can be done, what is
> >  slow, how you see the problem?
>
> We should first fix the hot spots in the links you mention. That's for
>  after 2.1. Cyrille hinted that he knew why the opengtl-based colorspaces
>  are pretty slow, and those are important since they deliver the linear,
>  hdr colorspace that Ton asked about.
>
> For the rest, it's profiling, profiling, profiling, with oprofile, valgrind
> and mutrace.

Clear to me.
 

> > What I, as the paintop creator, see, is that many things are slow with
> > CPU, even with dual cores. I created small OpenGL app that uses QtOpenGl
> > to paint and that is just real-time painting with big brushes that can be
> > scaled realtime etc. (Just with RGB8 colorspace of course) but I see this
> > as big opportunity for Krita. Use GLSL where it is possible and here we
> > could really speed-up Krita.
>
> Is that in svn or on gitorious somewhere? I'm not opposed on principle to
> using opengl in more places -- we might even have an opengl textures backed
> paint device, if that is useful. As long as Krita remains usable for me :-)

It is just custom app that suffers from some serious bugs now and I would be
ashamed to share it :) I will show it to you in Oslo.
 
> > Regarding GUI I'm offering help to shicmap, she can design, I can code.
>
> Have you already seen the original design on the wiki? Shicmap told me she
> will be very busy for some time.
>

No, I don't. I just found this
http://wiki.koffice.org/index.php?title=Krita/Brushes

_______________________________________________
kimageshop mailing list
kimageshop@...
https://mail.kde.org/mailman/listinfo/kimageshop

On Fri, Oct 23, 2009 at 2:59 PM, LukasT.dev@... <lukast.dev@...> wrote:

On Friday 23 October 2009 14:49:07 Boudewijn Rempt wrote:
> On Friday 23 October 2009, Dmitry Kazakov wrote:
> > Well, i agree with you! =) If you say this code is rarely used, it won't
> > help us boost performance =)
> >
> > PS:
> > /me still thinks that using strings for internally used constants is a
> > bad practice =)
> > And /me doesn't think implicit sharing of QString will excuse it.
> > But i wont argue about it this time =)
>
> Indeed it doesn't, I just read the Qt code: it first compares the size,
>  then does a very optimized memory comparison. I would have thought it
>  would have done a comparison of the shared pointer, but no doubt the Qt
>  hackers know better.
>

I'm student of image processing and we are taught that Strings, no matter how
they are implemented, are in the end expensive in image processing. I never
seen them in code which aims to be fast (OpenGL, OpenCV,...).

But I don't say we should rewrite Krita and throw away QString, I just would
not use them for comparison in loops etc. And the loops can be hidden pretty
much, you can't say that this or that code will not be in a loop in the end.
The reason? Because of complexity of Krita and image processing in general.

It's not like we are doing a string compare on every pixel. At the moment string operations take only a small fraction of the whole processing.
What I learned was to make the common case fast. First we should concentrate on the big performance problem and when strings make a bigger impact in the end we can still optimize that.

_______________________________________________
kimageshop mailing list
kimageshop@...
https://mail.kde.org/mailman/listinfo/kimageshop

[Cyrille Berger]

On Friday 23 October 2009, Boudewijn Rempt wrote:
> Cyrille hinted that he knew why the opengtl-based colorspaces are pretty
> slow, and those are important since they deliver the linear, hdr
>  colorspace  that Ton asked about.
For the record, the opengtl-based color spaces recompile themself at each
cloning, for no good reason. And since we clone a lot the color spaces... The
solution is to share the compilation result, like we share part of the lcms
color space.
And of course, to investigate colorspace recycling.
--
Cyrille Berger
_______________________________________________
kimageshop mailing list
kimageshop@...
https://mail.kde.org/mailman/listinfo/kimageshop

On Fri, Oct 23, 2009 at 4:45 PM, Boudewijn Rempt <boud@...> wrote:

On Friday 23 October 2009, Dmitry Kazakov wrote:

> 1) I wouldn't say it's a tile engine internal fault that Krita is slow. I'd
> even say that it's not tile engine's fault at all (mostly). =)

Depends a bit on which tile engine you're talking about -- the currently
active one has that nasty lock that kicks in whenever a tile is accessed. This
effectively serializes everything in Krita

I wouldn't say so =)

 

-- see the mutrace output I posted
a while ago. The second one doesn't support swapping, so it's not comparable.

And I am sure there is a lot left to improve for both tile engines, if even
the Gimp people found some serious issues in their decades old tile engine
this year.

Of course we should optimize it! I mean that we should design an "optimal" API first! ;)
 

Right now, the easiest win we can make in the tile engine is to make the
iterators cache the tiles they are accessing in a row or column. That's
something that has been borne out by proper profiling.

Yes, it could be done.

 

 

> Most problems come from *misusing* the engine. And the biggest problem here
> is bitBlt'ing at higher level of abstraction (colorspaces). We used to
> discuss it on irc, as i remember. Tile engine (at least new one) supports
> implicit tile sharing and lazy copying, BUT higher layers of abstractions
>  do not use it at all! They must use it when source and destination
>  colorspaces coincide, it'll be much faster than direct copying of data
>  stride by stride.
>
> But to solve this issue we need to design a good programming interface for
> it.
> The implementation will not be that difficult, i can do this as soon we
> discuss the interface. We could discuss it on sprint?
>
>
> 2) To make bitBlt'ing more faster we should solve another one issue:
> compositeOps constants in pigment.
>
> I don't know how it happened, but all the compositeOp constants (like
> COMPOSITE_OVER) are strings! And for every stride of the image (there might
> be more than 1000 strides per image) we do a string comparison! More than
> that, we do many string comparisons on every composition! See [1].

No, we don't. We get a KoCompositeOp subclass instance and apply that. It's
easy enough to see that we don't do all those string comparisons by running
valgrind.

> I don't know where COMPOSITE_OVER and COMPOSITE_ALPHA_DARKEN are
> implemented, maybe their application is optimized a bit, but most of the
> others should suffer from this comparison.

For rgbu8, ADD, ALPHA_DARKEN, BURN, DIVIDE, DODGE, ERASE, MULTIPLY, OVER,
OVERLAY, SCREEN, SUBTRACT are implemented as separate, templated classes.

COMPOSITE_DARKEN, COMPOSITE_LIGHTEN,COMPOSITE_HUE,
COMPOSITE_SATURATIONCOMPOSITE_VALUE, COMPOSITE_COLOR, COMPOSITE_IN
COMPOSITE_OUT,  COMPOSITE_ADD,   COMPOSITE_DIFF COMPOSITE_BUMPMAP,
COMPOSITE_CLEAR, COMPOSITE_DISSOLVE are implemented in this legacy code.

Other colorspaces do not have the legacy code path.

> Of course, Qt may smooth this comparison procedure a bit with shared
> internal data, but nevertheless integer switch should be much faster than
> that.

It is optimized a lot in Qt -- and don't forget that these are const's. But,
as Cyrille has said, it's old code.

>
> What do i suggest?
> Change all these constants' type to qint32 and create separate function
>  that would show their name using integers. This chain of if()
>  constrictions in [1] could be replaced with a simple 'switch'
>  construction. Theoretically, a compiler can optimize this switch with a
>  jump table at asm level.

That's not necessary because we've already implemented something like Thomas
Zander suggested three years ago. We just haven't had the time to port all
code.

> 3) Slow work on dual-core processors is connected with the fact that there
> almost no threading in Krita at the moment. =) Yes, we have KisImageUpdater
> but it does almost nothing for speed, because while it works other parts of
> Krita (e.g. KisView) are waiting for his work finished.

KisView is not waiting in the sense that it blocks until the image is
refreshed. The gui is kept responsive at all times because the recomputation
isn't in done in the gui thread. Note that actually painting, i.e., making
strokes, also involves running a thread, which means there are 3 threads
running already while painting.

> It means that Krita
> is almost single-threaded: while one thread is working, others - are
> waiting. I saw it when i started a system monitor in parallel with Krita,
>  it showed that only one core is working at the same moment (i tested it
>  about a month ago).

When painting with a big brush (~500) pixels, kysguard shows that both cores
are actually saturated. One is recompositing, the other is computing the 500
pixel mask for every dab (which is the biggest measurable performance issue we
have).

> I'm working on a layers merging parallelization right now. The same
> algorithm can (and should) be used in KisView for pre-scaling.

That sounds cool! But please -- no enormous code drops, make sure we have a
manageable set of small patches for review!

> 4) openGL is good, but i think we could use processor's capabilities like
> SSE and friends first.

I'm vacillating on this... sse etc. are nice, but most distributors do not
enable them by default, except for the 64 bits systems.

But not all systems support hardware opengl, do they?

 

OpenGL + glsl has
the potential to give much more performance, but I haven't got a system where
I can use it. And if we can move more and more work to opengtl, we can profit
from the automatic optimization it gives us.

--
Boudewijn Rempt | http://www.valdyas.org
_______________________________________________

kimageshop mailing list
kimageshop@...
https://mail.kde.org/mailman/listinfo/kimageshop

--
Dmitry Kazakov

_______________________________________________
kimageshop mailing list
kimageshop@...
https://mail.kde.org/mailman/listinfo/kimageshop

On Fri, Oct 23, 2009 at 4:52 PM, Cyrille Berger <cberger@...> wrote:

On Friday 23 October 2009, Dmitry Kazakov wrote:
> PS:
> /me still thinks that using strings for internally used constants is a bad
> practice =)
> And /me doesn't think implicit sharing of QString will excuse it.
> But i wont argue about it this time =)

The problem is that those strings are not internal, and we want the set of
composite op to be extensible. That's why we moved from int to qstring. As as
side note, since QString (even with implicite sharing) comparison can be
extensive, there is a class in pigment that gives you an integer for a string
that can be stored to get fast comparison, we do that for comparing to
colorspace. Here it would be an improvement, but the real fix is to migrate the
remaining composite op.

Why ints are not extensible?

--
Dmitry Kazakov

_______________________________________________
kimageshop mailing list
kimageshop@...
https://mail.kde.org/mailman/listinfo/kimageshop

On Fri, Oct 23, 2009 at 4:59 PM, LukasT.dev@... <lukast.dev@...> wrote:

On Friday 23 October 2009 14:49:07 Boudewijn Rempt wrote:
> On Friday 23 October 2009, Dmitry Kazakov wrote:
> > Well, i agree with you! =) If you say this code is rarely used, it won't
> > help us boost performance =)
> >
> > PS:
> > /me still thinks that using strings for internally used constants is a
> > bad practice =)
> > And /me doesn't think implicit sharing of QString will excuse it.
> > But i wont argue about it this time =)
>
> Indeed it doesn't, I just read the Qt code: it first compares the size,
>  then does a very optimized memory comparison. I would have thought it
>  would have done a comparison of the shared pointer, but no doubt the Qt
>  hackers know better.
>

I'm student of image processing and we are taught that Strings, no matter how
they are implemented, are in the end expensive in image processing. I never
seen them in code which aims to be fast (OpenGL, OpenCV,...).

+1000
Though i'm not a student of computer science =)
 

--
Dmitry Kazakov

_______________________________________________
kimageshop mailing list
kimageshop@...
https://mail.kde.org/mailman/listinfo/kimageshop