Deep copy of an Element

[JaimeIvanCervantes]

I'm working on a class that takes an Element as input and is doing constant modifications to such Element. I would like to make a deep copy of this Element in order to keep the original intact though, but the nature of unique_ptrs doesn't allow it though. Is there any way of doing a deep copy of an Element? Maybe a cloning option?

[igagis]

I think it should be possible to write some kind of CloneVisitor which will go over the SVG tree and create a clone of each element. The idea is something like this:

class CloneVisitor : public svgdom::ConstVisitor{
    Container root;
    Container* curParent = &root;

    void cloneChildren(const Container& e, Container& clone){
        auto oldParent = this->curParent;
        this->curParent = &clone;
        this->relayAccept(e);
        this->curParent = oldParent;
    }
public:

    void visit(const GElement& e){
        auto clone = utki::makeUnique<GElement>();
        //copy different properties from e to clone (like transformations, styles, etc.)
        ....
        this->cloneChildren(e, *clone); //clone children
        this->curParent->children.push_back(std::move(clone));
    }

    .....

    std::unique_ptr<Element> getClone(){
        auto ret = std::move(this->root.children.back());
        this->root.children.clear();
        return ret;
    }
};

std::unique_ptr<Element> someElementToClone = ....;

CloneVisitor visitor;

someElementToClone->accept(visitor);

auto clonedElement = visitor.getClone();

[igagis]

I have just added an empty copy constructor to Container, so now I think it is possible to use copy constructor for all elements to copy all stuff except children when doing e.g. utki::makeUnique<GElement>(e) and children should be copied with cloneChildren() as I described in the post above.

[JaimeIvanCervantes]

I wrote the CloneVisitor as you described, but the function cloneChildren() is not working as expected since it's returning a unique_ptr<Element>. Here's my visitor code:

void CloneVisitor::cloneChildren(const Container& e, Container& clone){
	auto oldParent = curParent;
	curParent = &clone;
	relayAccept(e);
	curParent = oldParent;
}

std::unique_ptr<Element> CloneVisitor::getClone() {
	auto ret = std::move(root.children.back());
	root.children.clear();
	return ret;
}

void CloneVisitor::visit(const GElement& e) {
	auto clone = utki::makeUnique<GElement>(e);
	cloneChildren(e, *clone); 
	curParent->children.push_back(std::move(clone));
}

...

And when I try to do the following:

std::unique_ptr<SvgElement> domOriginal = utki::makeUnique<SvgElement>();
	
CloneVisitor cvisitor;
	
domOriginal->accept(cvisitor);
	
std::unique_ptr<SvgElement> domClone = cvisitor.getClone();

I get the following error:

error: no viable conversion from 'unique_ptr<svgdom::Element>' to 'unique_ptr<svgdom::SvgElement>

domOriginal and domClone should be exactly the same and the getClone() function should return the same type as the Element that it is cloning right?

[igagis]

Well, getClone actually returns the right object, it only is cast to its base (i.e. Element). Do you actually need to know that you are working with SvgElement?
I think you can modify the getClone function to something like this:

template <class T> std::unique_ptr<T> CloneVisitor::getCloneAs() {
        assert(root.children.size() == 1);
	auto ret = std::unique_ptr<T>(dynamic_cast<T*>(root.children.back().get()));
        if(ret){//if cast was successful
            root.children.back().release();
        }
	root.children.clear();
	return ret;
}

Then when using it you will have to make sure that cast was successful, in case you know that you are cloning SvgElement the assertion would be a good practice:

std::unique_ptr<SvgElement> domOriginal = utki::makeUnique<SvgElement>();
	
CloneVisitor cvisitor;
	
domOriginal->accept(cvisitor);
	
std::unique_ptr<SvgElement> domClone = cvisitor.getCloneAs<SvgElement>();
assert(domClone);
//do something with domClone
...

[JaimeIvanCervantes]

getClone actually returns the right object, it only is cast to its base (i.e. Element)

I get that, it's just that it doesn't feel right, a clone() should return an object of the same type.

I am doing something similar now, but I'm not very happy with it, so I'm thinking of alternatives:

unique_ptr<Element> e = cvisitor.getClone();
unique_ptr<SvgElement> domClone = std::unique_ptr<SvgElement>(dynamic_cast<SvgElement*>(e.get()));
e.release();

At least is working :).

Thanks for your help again!

[igagis]

I would make a template function:

template <class T> std::unique_ptr<T> cloneElement(const T& e){
     CloneVisitor cvisitor;
     e.accept(cvisitor);
     unique_ptr<Element> e = cvisitor.getClone();
     unique_ptr<T> domClone = std::unique_ptr<T>(dynamic_cast<T*>(e.get()));
     assert(domClone);
     e.release();
     return domClone;
}

std::unique_ptr<SvgElement> domOriginal = utki::makeUnique<SvgElement>();
	
std::unique_ptr<SvgElement> domClone = cloneElement(*domClone);

so you can use it to clone any element type and it's return type is deduced from argument type.

[JaimeIvanCervantes]

Here's my latest version:

template <class T> std::unique_ptr<T> clone(const std::unique_ptr<T>& element){
     CloneVisitor visitor;
     element->accept(visitor);
     unique_ptr<Element> buffer = visitor.getClone();
     unique_ptr<T> clone = std::unique_ptr<T>(dynamic_cast<T*>(buffer.get()));
     assert(clone);
     buffer.release();
     return clone;
}

Would you be interested in a PR to add a clone() method to svgdom? Maybe to dom.hpp? (seems like the right place to do it)

I would have to create the files CloneVisitor.hpp and CloneVisitor.cpp, and just add the function from above.

[igagis]

Well, it is ok to include CloneVisitor to svgdom, but I would prefer to have that getCloneAs template function as I wrote above in the CloneVisitor. Because it will make it more extensible to custom elements.
so, if you do a PR, please also add some test for CloneVisitor, at least sanity test.

[igagis]

can this one be closed?

[JaimeIvanCervantes]

yeah, go ahead