> On Oct. 16, 2011, 6:04 p.m., Dmitry Kazakov wrote:
> > Hi!
> >
> > It's a really cool idea to use uuids here! I think the patch is very good,
> > but i have one small concern. Is it possible to remove old code like
> > copyFromName(), setCopyFromName() and findNodeByName()? Probably, map the
> > name to the uuid right during the loading in KisKraLoader or
> > KisKraLoadVisitor. Is it possible? I think keeping this old code might play
> > a negative role in a long-term plan =)
>
> Torio Mlshi wrote:
> Hi!
>
> I guess it is impossible to do this good way because during loading clone
> layer in KisKraLoader all we know is a target name. We even don't know is it
> created or still only present in xml. I can imagine some hacks which would
> allow to know uuid of that node (search for existing node with the same name,
> if found get uuid from it; if not, write to a map that node with required
> name should have special uuid), but they would be definetly worse is
> long-term plan.
Well, then you could leave KisKraLoader as it is but encapsulate the clone
information into a separate object like:
CloneInfo {
public:
CloneInfo(const QUuid &uuid);
CloneInfo(const QString &name);
KisNodeSP findNode(KisNodeSP rootNode);
private:
QUuid m_uuid;
QString m_name;
};
And store this object in clone layers. This would remove all the duplications
from the layer's code and remove findNodeByName/ByUuid duplicated cycles from
the loader. The loader would create clone information from information it has
in xml. What do you think about this solution?
- Dmitry
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/102896/#review7401
-----------------------------------------------------------
On Oct. 16, 2011, 4:59 p.m., Torio Mlshi wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/102896/
> -----------------------------------------------------------
>
> (Updated Oct. 16, 2011, 4:59 p.m.)
>
>
> Review request for Calligra and Boudewijn Rempt.
>
>
> Description
> -------
>
> This patch fixes saving and loading of clone layers. It adds uuid to
> KisBaseNode so that clone layers use it to store information about their
> targets, making sure to target required layer.
>
> About compatibility:
> All nodes get uuid on creation. So node will have uuid even if no one is
> provided in file (which overwrites if present). Clone layers also use old
> code if no copyfromuuid field. So old files should work fine.
> And new files should also be loaded ok by old versions because no fields are
> removed and old fields (such as copyfrom) are still valid. Of course, old
> versions will open new files just as old ones and bug will still present
> there.
>
>
> This addresses bug 283610.
> http://bugs.kde.org/show_bug.cgi?id=283610
>
>
> Diffs
> -----
>
> krita/image/kis_base_node.h 798bb06
> krita/image/kis_base_node.cpp cccbee8
> krita/image/kis_clone_layer.h 21e766e
> krita/image/kis_clone_layer.cpp 050ef5f
> krita/ui/kra/kis_kra_load_visitor.h 89bf501
> krita/ui/kra/kis_kra_load_visitor.cpp d980ab0
> krita/ui/kra/kis_kra_loader.cpp 5d3e5d5
> krita/ui/kra/kis_kra_savexml_visitor.cpp ecee329
> krita/ui/kra/kis_kra_tags.h a737ae3
>
> Diff: http://git.reviewboard.kde.org/r/102896/diff/diff
>
>
> Testing
> -------
>
> Old files worked fine, as expected.
>
>
> Thanks,
>
> Torio Mlshi
>
>
_______________________________________________
calligra-devel mailing list
calligra-devel@kde.org
https://mail.kde.org/mailman/listinfo/calligra-devel
