> On Nov. 1, 2011, 9:41 p.m., Dmitry Kazakov wrote:
> > 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.

I didn't think about this either, I just tried to use clones of group layers =)

1) What is the reason for updating nodes below the moving one? I've tried 
moving group layers and it worked.. And even if it doesn't work in some cases, 
the solution could be either to call setDirty from refreshGraphAsync (if it 
does this work anyway, just replace that part with setDirty call) or to call 
them both (this probably would lead to slowness) or to call it after moving 
stroke ends (this is worse because clone layers would not be updated while 
moving, but better from performace point of view).

2) Ok, then we could add some call like updateClones() and call it from 
setDirty to parents. The point for calling from KisNode (not KisLayer) is that 
it fixes not only problem of group layers' clones, but also problem of clones 
of layers with masks.

About moving clones updating to updateProjection -- may be it is better 
solution if it will work. But I'm not sure if it will be easier than fixing my 
solution (I've done some testing and it worked, so only problem that I see is 
potential performance problems; if it it does not work in some case, please 
write).


- Torio


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


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
> 
>

_______________________________________________
calligra-devel mailing list
calligra-devel@kde.org
https://mail.kde.org/mailman/listinfo/calligra-devel

Reply via email to