setForeGroundColor() execution speed

Develop Developer Questions
1

Hi,

I was playing with setForeGroundColor() method, and I faced to performance issue.

Maybe it’s just I missed something, I don’t know, but I’ve executed the following code:

from krita import *
import time

av=Krita.instance().activeWindow().activeView()
color=QColor(Qt.black)

ts=time.time()
for i in range(255):
    color.setRed(i)
    mc=ManagedColor.fromQColor(color, av.canvas())
    av.setForeGroundColor(mc)
print(time.time()-ts)

Tested on Krita 4.4.2 stable + last 5.0.0prealpha, Linux Appimage

  • Running on my AMD FX-8350 / 32GB RAM ==> 11s
  • Running on my Intel i7-6600U 2.6GHz / 16GB RAM ==> 0.5s

If I put in comment instruction av.setForeGroundColor(mc) execution time (AMD computer) is around 0.01s
So problem is not to convert a QColor to ManagedColor as I was thinking at the beginning, but on method used to set foreground color.
(same problem occurs with setBackGroundColor() method)

Problem seems to be relative to my configuration (as it run x20 faster on my laptop)
But I’m not able to understand why.
I’ve deactivated all plugins, hidden all color dockers, … but no change in final execution time.
Both executions made on a default RGBA 8bits document.

Anyone have an idea about why this is so slow one on my computer and not the other one?
Am I the only one facing this problem?

Thanks.

Grum999

2

I have the same problem.
Important: one needs to open a document to see the slowdown.
On master I get:

==== (No document open) ====
0.005090951919555664
==== (A document open) ====
89.17738342285156

On krita 4.4.2 appimage I get:

1.958707332611084

However on Krita 5.0.0-prealpha appimage from today I also get:

2.010404109954834

So it looks like either a matter of Qt or something… my Qt is 5.11.1, while the appimage is on 5.12.9.

Do you also get the terrible lag on dragging the mouse on the Advanced Color Selector? I was wondering if the reason might be that it tries to paint the color selector too often; another option would be that it tries to find the appropriate color in the palette?

3

Yes

But, I think i found why it occurs on my desktop computer.

Just let me time to made some additional tests before providing what Ifound

Grum999

1 Like
4

Ok that’s really weird…

I tried different configurations.

Krita’s window size Advanced color selector size Execution time
454x352 None ~0.50s
454x352 100x40 ~0.65s
1920x1080 None ~0.73s
1920x1080 100x40 ~0.90s
1920x1080 1570x875 ~5.90s
3840x2094 None ~10.90s
3840x2094 100x40 ~12.00s
3840x2094 3494x1885 ~164.00s
Note: by None, I mean deactivated in settings + removed from docker

It’s seems problem is related to Krita’s window and Advanced color selector size:

  • Problem is here when advanced color selector is deactivated
    image
    image791×354 50.1 KB
  • Problem is bigger when advanced color is bigger

My drawing tablet connected to laptop is a 1920x1080 size screen.
And maybe processor (single thread I think) is a little bit faster on laptop
=> That’s why I don’t feel the problem on it

But on a 4K monitor execution time seems to be exponential, just to change the color :thinking:

I also tried to play with configuration settings:

  • Colour management options (no impact)
  • Colour Selector Settings options (no impact

The impact of main window size, independently of advanced color selector size/visibility is really disturbing…
Starting to try to understand source code behind this function, but with my current skills I’m not sure to be able to find something easily…

Grum999

5

Another very interesting thing I found: removing the Dual Color Button (KoDualColorButton.cpp?) from toolbar, execution time is normal, even in the worst tested configuration.

And in this case, Advanced Color Selector is not lgagy anymore.

Tried to understand code, but :sweat_smile:
My though is, there might be kind of recursive call when signals are emitted when color is changed…

Grum999

6

That’s really weird…sure, the code of KoDualColorButton really is not the nicest, under certain conditions it re-emits signals, but so far I thought it’s mainly a problem for the color dialogs it opens.

I don’t know if it yields anything useful with an appimage, but you could try to run “perf top” (probably have to install some kernel tools package) during your script that takes 10+ seconds to complete, and see which functions eat the most CPU time.

7

This is more the association of all files and their connection on which I’m lost…
I discover for example, the color is managed from a resource manager and then, start to try to follow The Ariadne’s thread but that’s very difficult, signals are naturally difficult to analyze because many object can be connected to them, themselves sometimes emitting another signals or calling function that emit signal…
And I don’t know Krita’s internal architecture, a grep return me many things I’m not sure if they’re concerned or pertinent to follow…

The KisDlgInternalColorSelector identified by WdgDlgInternalColorSelector?
I’m not sure about that, testman made another test :slight_smile:

  1. Open this dialog box
    – run the script ==> slow!
  2. Let the dialog box opened
    – Remove DualColorButton from toolbar
    – run the script ==> fast! (and dialog box color is updated)

So maybe the dialog box is involved in the problem, but it’s also related to DualColorButton visible in toolbar.
And the weird thing is even if it’s related to some signals that made a ping-pong, I’m not able to understand the relationship with Krita’s main window size…

the module kritacolorselectorng.so comes in the top…

  • 3.47% KisColorSelectorRing::paintCache
  • 1.47% KisColorSelectorSimple::colorAt

Don’t know if it’s really helpful?

Grum999

8

I did make the advanced color selector be nicer on 4k screen that made it 4x slower there. But when I tried to find where the slowness came from, I could easily go past that commit and it was all super slow… so I’m not sure if that’s the reason. And the DualButton and the fact that appimage is fast are two other clues that makes it less possible that it is the reason for that. And my calculation time isn’t 4x bigger but 40x bigger, so it doesn’t fit.

9

Yes those are the classes that handle the foreground/background color dialog. Interesting overservations…

Hm…not the clear hint I was hoping for, but if that gets called even if the Advanced Color Selector is not visible, that’s definitely a design issue, it should not recreate paint caches all the time if it’s not even on screen.

But it doesn’t answer where the extreme slowdown comes from, 4.something times more pixels should not take 27 times longer.
Nor does it explain why the screen resolution affects it.

10

Also tested an old 4.4.0 compiled by myself, running with Qt 5.11

Results are the same on my side than with appimage:

  • slow when dualbutton is visible
  • fast when dualbutton is hidden

So not sure that Qt version is involved into the problem… :thinking:

Grum999

11

Didn’t provided all informations…
I didn’t know the perf tool until now, but it permit to disassemble code and provides other information.
I was just not able to understand which information is relevant or not and just provided the most basic information returned by tool :sweat_smile:
But I found one instruction in KisColorSelectorRing::paintCache() method that is time consumming

image
image1248×683 68.4 KB

But if this method is normally used to refresh a widget that is not visible…

That’s not the screen resolution, but more the main window geometry…

Maybe I can open a new entry in bug.kde.org?

Grum999

12

Damn I am so happy someone noticed this bottle neck!

I have code especially made for example for linux users where I only apply color when you mouse release while in window I can keep the apply color in real time to krita as mouse move in the edit. This because issuing to many color commands drowns linux because it has to go by the numbers. Windows somehow frees itself from the amount of commands no problem and solves it.

Also depending of the color picker you have you will have more or less lag. The small color selector is the heavier one.

1 Like
13

So far I had no luck reproducing the issue, the script always executes in <0.5s for me when the Advanced Color Selector is not visible.
I don’t have a 4k screen unfortunately, the max docker size I can have with 1440p is ~1300, that still takes at most 3s, no matter if 4.4.2 appimage or my own master build.

What I don’t get is why having the KoDualColorButton on the toolbar makes such a difference…its paint event should be cheap, and it does not really do anything strange there either. Only debatable thing is that it uses repaint() instead of update(), so it cannot compress multiple updates into one paint event.
And not having it on the toolbar doesn’t even stop it from being around and receiving color updates…

1 Like
14

Please do report it on bugs.kde.org, you can still investigate here but it’s good to have a bug report on bugzilla just to make sure it won’t get forgotten (although if there are more people than just me with the issue, I do hope it will get picked and fixed fast :slight_smile: I don’t think it will fit into 4.4.3 beta though :frowning: ).

15

Can’t really tell that’s an issue related with “too many color commands”, to be able to determinate origin of the problem, a code analysis should be made.
And looking my tests, with the same number of color commands sent to Krita, the result differs according to the mainwindow layout configuration (size, dual color button visible or not)

And on my side, I can reproduce it in windows.
Just the performance test I made on windows can’t really be used because my Windows 10 is running in a virtual machine with only 1 CPU=>OS is naturally slow, even without any application started…
The only thing I can test in this situation is execution with ou without dual color button (with: 87s on FullHD screen / without: 1.70s)

Maybe @tiar you can confirm the problem is visible or not in Windows 10, and compare execution result with execution time you got on Linux (OS are running on same computer/configuration?)

My chance, @tiar is able to reproduce it, I’m not insane :sweat_smile:

Here’s a tip that might work to reproduce case without a 4K monitor (tested on my side with a FullHD monitor plugged instead of 4K monitor)

qw=Krita.instance().activeWindow().qwindow()
qw.showNormal()
qw.move(10,10)
qw.resize(1980*2,1080*2)

:man_shrugging:
Absolutely no idea, like no idea why mainwindow size have an impact too…

I’ll do it today
And I’ll try to do some other tests.

One another interesting thing is, in Krita 4.1.7 I don’t have any lag on Advanced Color Selector like I have on 4.4.2
Also tested Krita 4.2.8, the lag is present.

But unfortunately, PyKrita API in 4.1.7 and 4.2.8 doesn’t allow to convert a QColor to a ManagedColor and then I can’t execute the script to get result more precise than current feeling is user interface usage.

Grum999

16

Ok, I’ve made a simple test…

void KoDualColorButton::setForegroundColor(const KoColor &color)
{
    d->foregroundColor = color;
    {
        /**
       * The internal color selector might emit the color of a different profile, so
       * we should break this cycling dependency somehow.
       */
        KisSignalsBlocker b(d->colorSelectorDialog);
        d->colorSelectorDialog->slotColorUpdated(color);
    }
    repaint();
}

I replaced repaint() with update()
==> problem is fixed, performances are normal in all cases ! :tada:

I’ll provide this information in bug report

@tiar, I think it something you can test easily on your side too.
Do you confirm that on your side, replacing repaint() with update() fixed the problem?

Maybe if it’s only this, it can be in 4.4.3?

Grum999

1 Like
17

If it fixes the issue, you can open a Merge request :slight_smile: I mean instead of a bug report.

18

I’m currently really uncomfortable with git.

Last time I tried a MR on invent.kde.org, it take me more time to try to understand how to fix the mess on my git repo than coding stuff… And finally my repo is still in a mess and @halla did the MR for me :sweat_smile:

Grum999

19

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

Grum999

3 Likes
20

So it was really the repaint()?
I wonder if desktop compositor and/or OpenGL drivers have some influence on the performance penalty here, it’s really strange that painting an icon sized widget would be so expensive…like, VSync behavior etc.

Looks like halla already committed the change.