Saturday, October 2, 2010

Krita: panning, race conditions and other bug fixes

Hi, All!

I haven't been blogging for about three weeks. They were quite tough. Like everyone in Krita team I was working on fixing bugs and making Krita user ready. And now I'm going to describe what I've done.

The first week was spent on making Panning and Color-picking gestures work for all the tools. I achieved this by moving gestures code to base classes KisTool and KisToolPaint. It was quite non-trivial task, because I had to fix classes of all the tools in Krita as there is no common strategy how to handle mouse events.

I think, we should elaborate some good system for gestures after 2.3 release, but I can't really imagine how. This new system must handle all the mouse and tablet events itself, and provide some callbacks(?) and interfaces for tools. The callbacks should look like: startStroke(), cancelStroke(), endStroke(). The problem is, our tools use different interaction types. I mean not all of them use "strokes" strategy. For example, Paint Bucket tool uses single clicks, Gradient tool uses dragging, Path tool has a very complicated system with various types of clicks and modifiers. I think, we need to incorporate "strategy" pattern here, but I really can't get it how.

The next week I was fixing various bugs. I fixed selections of masks and filters, so now they are applied to the entire layer, not just to a small rectangle in the bounds of the image. Then I fixed a race condition in KisUpdaterContext. It used to rely on KisSharedPtr in concurrent environment heavily. But the shared pointer do not have many guarantees for multithreading. So I created a very convenient environment for the shared pointer with only one producer and one consumer. It can easily handle it. After that I fixed a race between KisGroupLayer and KisAsyncMerger. Then Color Conversion Curves Filter was fixed. The problem was really tricky here. It was due to the use of temporarily copied QVector, returned form the function. When we performed data() call, Qt made a deep copy of the vector and deleted the temporary object very soon. So the code used to read free'd memory. The problem was solved by using constData() instead to avoid copying.

And the last week was spent on fixing race conditions inside the data manager. They were really difficult races to find. You can read very detailed report about them in my commit message [0].

[0] -