Krita/Centralized Queue For Tool Jobs

From KDE Community Wiki
Revision as of 00:23, 19 January 2011 by Nalvarez (talk | contribs) (Remove redundant top-level header)
(diff) ← Older revision | Latest revision (diff) | Newer revision → (diff)

Motivation

Several bugs [1],[2] have been reported recently, showing that several tools are trying to do they work on image simultaneously. Everything would have be ok with it, but they can't do that. It is impossible to track the undo information when two transactions are opened simultaneously. That is why KisMementoManager asserts showing he's been asked to do impossible mission.

[1] - http://bugs.kde.org/248374 "Krita crashed when using the gradient tool"

[2] - http://bugs.kde.org/251389 "crashes in transform tool due to background work"

Mechanism of the problem

I'll try to explain the problem on the Gradient Tool example. KisGradientPainter::paintGradient calls progressUpdater()->setProgress(). The latter one asks UI thread to process events. After that UI thread delivers KisToolGradient::mouseReleaseEvent for the next gradient "stroke" requested by the user. It opens nested transaction... Crash.

Roots of the problem

  • Some of our tools do their job in a context of the UI thread. It means that two processes of modifying the image and delivering signals are mixed together. More than that, the moment of time of opening a transaction is defined by the time of delivering a signal to the UI thread, that can be quite random.
  • Some of the tools have their local queues (e.g. KisToolFreehand) and they do not use progressUpdater. That is why their event are not mixed with image processing, so they do not crash. But this approach is vulnerable as well, because queues of any two tools are not synchronized.

Possible solution

As a solution we could encapsulate all the tools into separate jobs (like KisGradientJob) and queue them into a separate thread. This will allow us to fix the first cause: interleaving events and paint operations. To fix the second cause we will have to make this queue global.