Filter's changeRect() and merging problems

Dmitry Kazakov dimula73 at gmail.com
Mon Aug 24 13:52:43 CEST 2009


> It used to work like this:
>
> on painting:
> * call layer->setDirty: add the rect to a QRegion that represents the total dirty area on the layer
> * emit a signal to KisProjection to add the rect to the total dirty area of the image
> * KisProjection would, starting with the visible area, update in a number of subthreads parts of the
> dirty area
> * as soon as the bottom-up update strategy had recomputed an area of a layer, that rect would be
> subtracted from the QRegion
>
> So, every layer at any point in time knew which area was dirty. But QRegion isn't threadsafe,
> and putting mutexes around the dirty area gave us lots of deadlocks.

Aha! That's why we get this large cycle though all code on merging =)

I mean, KisImage::updateProjection is called three times on every
change. This causes prescaled projection to update a rect three times
instead of one! I've prepared a patch for that, but haven't committed
it yet [2]. Shall do it now. It's a temporary fix.

I think it's better to brake this cycle. An update for the layer is
reported to KisImage once only. And it schedules it as it wishes.
This means that locks would be not per-layer-rect, but per-image-rect.

I think that old scheduler could have yet another trouble. I mean that
updateStrategy has a shared structure - filthyNode.

[2] - http://reviewboard.kde.org/r/1347/


> All this code is still around, for instance in http://websvn.kde.org/tags/koffice/1.9.95.1/krita/image/.
>
>> Current top-down approach has an issue too - it's not stateless.
>> FilthyNode is global. I want to make merging with recursion. In such a
>> case all the data will be stored in a local stack, so it'll be a bit
>> more thread-safe. But that is not my aim, itself. First i want to add
>> needRects to merging. This'll fix adj. layers bug. [1]
>>
>> Secondly, i want to make this stuff thread-safe, by splitting update
>> regions into non-overlapping areas. This, coupled with synchronization
>
> That's something I simply never managed to do correctly.

I want to try not to use QRegion. Just a list of rects. But i've not
thought it over properly. For now i have to fix adj. layers.


>> with KisCanvas, would eliminate flickering while painting, that is too
>> heavy now due to race conditions between threads.
>
> As long as the recomposition of the image's projection stays in a separate
> thread and doesn't go back to the gui thread,

BUT! The problem is - gui is a separate thread and it reads projection
while KisGroupLayer writes to it. I think sigImageUpdated should be
split to two parts: first is synchronous - it reads a projection, the
second is asynchronous - prescales, draws and etc.

But this is a subject for future investigation, cause Qt doesn't have
synchronous signals (has it?).

All i want to say, these three bugs (merging adj. layers, flickering
and threading KisProjection) should be solved together, as a whole.


> I'm okay with what you propose
> to do, but it should be easy to back out of in case we don't get it stable for
> the release. It is a bug fix -- but the fix you propose is really invasive.

i think merging and flickering will be fixed before release.


-- 
Dmitry Kazakov


More information about the kimageshop mailing list