Review Request: Fix clone layers updating

[Dmitry Kazakov]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/103021/#review7845
-----------------------------------------------------------


The bug is a really good catch! I didn't even think about it. =)

But the problem is, this very solution will not work, I'm afraid. There is a couple of problems.
1) setDirty() and refreshGraphAsync() do different things actually. setDirty() updates the node and all its parent nodes (that is the nodes above the dirty one). refreshGraphAsync first updates the subgraph of nodes below the dirty node, after that all the parent nodes (the work of setDirty()). Specifically for this solution, it won't work for moving group layers with children.
2) every setDirty() call adds a separate job to the updater, so calling setDirty() for all the parent nodes will add too much excessive work.


As for working solution, atm I cannot tell exactly we can do here, but I have the following thoughts:
1) There are already updates for clone layers in KisLayers::setDirty() so there is no need for additional method in KisNode.
2) The real problem here is that refreshGraphAsync() does not (and should not) call to setDirty(), so the clones are not updated.
3) There is one more problem possible: when the node above the dirty node is updated, its setDirty() is not called as well, so the clones of such node (it can be an issue for a group and filter layer only) will not be updated.
4) The most sane solution, i think, would be to move the call to 'm_d->clonesList.setDirty(rect);' from setDirty() into something like updateProjection() call. But there are several problems possible like race conditions and deadlocks as the latter method is called asynchronously from several non-UI threads. I will check whether it is possible, but not earlier than Thursday.

- Dmitry Kazakov


On Nov. 1, 2011, 7:25 p.m., Torio Mlshi wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/103021/
> -----------------------------------------------------------
> 
> (Updated Nov. 1, 2011, 7:25 p.m.)
> 
> 
> Review request for Calligra.
> 
> 
> Description
> -------
> 
> This patch fixes issues, when clone layer wasn't updating despite its original was changed.
> 
> KisNode::setDirty now calls setDirty of parent to let its clones know about update. In MoveStrokeStrategy updating is now implemented via setDirty, so clone layers will know about update.
> 
> 
> Diffs
> -----
> 
>   krita/image/kis_node.cpp 52881e7 
>   krita/plugins/tools/defaulttools/strokes/move_stroke_strategy.cpp df8b6da 
> 
> Diff: http://git.reviewboard.kde.org/r/103021/diff/diff
> 
> 
> Testing
> -------
> 
> Normal moving (when no clones) seems to have the same performance. Of course, performance becomes low when many clones are involved.
> 
> Unit tests have same results for me.
> 
> 
> Thanks,
> 
> Torio Mlshi
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/calligra-devel/attachments/20111101/bbbef2ea/attachment.htm>