I am trying to use the C++ "Clipper Library" (http://www.angusj.com/delphi/clipper.php), but when I try to return one of the objects from the clipper library from a function, it seems to become null or is altered somehow
Here is the function I wrote. The only relevant lines should be the last 3.
ClipperLib::PolyTree MeshHandler::trianglesToPolyTreeUnion(std::vector<Triangle> triangles)
{
// Make all of the triangles CW
for (auto& triangle : triangles)
{
triangle.makeClockwise();
}
// Set up the Clipper
ClipperLib::Clipper clipper;
// To take a union, add all the paths as "subject" paths
for (auto& triangle : triangles)
{
ClipperLib::Path triContour(3);
triContour[0] = convertGLMToClipperPoint(triangle.getVertex(0));
triContour[1] = convertGLMToClipperPoint(triangle.getVertex(1));
triContour[2] = convertGLMToClipperPoint(triangle.getVertex(2));
clipper.AddPath(triContour, ClipperLib::PolyType::ptSubject, true);
}
// Now get the PolyTree representing the contours
ClipperLib::PolyTree tree;
clipper.Execute(ClipperLib::ClipType::ctUnion, tree);
return tree;
}
When I call clipper.execute, it writes into the tree structure some contour information. It writes the correct information, and I've tested that it's correct. However, when I return the tree, it doesn't seem to copy anything, and the PolyTree that results from this function is empty.
I'm sure that there's nothing wrong with the library and that I'm just making a beginner c++ mistake here. Hopefully someone has an idea of what it might be.
Thanks!
edit: For reference, here is a documentation page for the polytree (http://www.angusj.com/delphi/clipper/documentation/Docs/Units/ClipperLib/Classes/PolyTree/_Body.htm)
edit: I thought the clipper library wasn't open source, but it is. Here is the code
typedef std::vector< IntPoint > Path;
typedef std::vector< Path > Paths;
class PolyNode;
typedef std::vector< PolyNode* > PolyNodes;
class PolyNode
{
public:
PolyNode();
Path Contour;
PolyNodes Childs;
PolyNode* Parent;
PolyNode* GetNext() const;
bool IsHole() const;
bool IsOpen() const;
int ChildCount() const;
private:
unsigned Index; //node index in Parent.Childs
bool m_IsOpen;
JoinType m_jointype;
EndType m_endtype;
PolyNode* GetNextSiblingUp() const;
void AddChild(PolyNode& child);
friend class Clipper; //to access Index
friend class ClipperOffset;
};
class PolyTree: public PolyNode
{
public:
~PolyTree(){Clear();};
PolyNode* GetFirst() const;
void Clear();
int Total() const;
private:
PolyNodes AllNodes;
friend class Clipper; //to access AllNodes
};
Assuming you don't want to modify the (obviously badly designed) Clipper library, you can do it like I suggested in my comment:
// Make sure to have this at the top of your header file:
#include <memory>
std::unique_ptr<ClipperLib::PolyTree> MeshHandler::trianglesToPolyTreeUnion(std::vector<Triangle> triangles)
{
// Rest of your code...
std::unique_ptr<ClipperLib::PolyTree> tree(new ClipperLib::PolyTree);
clipper.Execute(ClipperLib::ClipType::ctUnion, *tree);
return tree;
}
Then, when calling your function:
std::unique_ptr<ClipperLib::PolyTree> tree(yourMeshHandler.trianglesToPolyTreeUnion(/*...*/);
// make use of tree...
Still, I would suggest opening a ticket (if there's a bug tracker) or contacting the library's author about this issue.
Before doing anything, make sure the following program works correctly:
int main()
{
PolyTree p1;
// fill PolyTree with some values that make sense (please add code to do this)
//...
PolyTree p2 = p1;
PolyTree p3;
p3 = p1;
}
That is basically what we want to test. If you can get this code to work (add the relevant headers and initializations necessary), then you can focus back on the function. If the code above doesn't work, then there is your answer.
You need to get the code above to produce the correct copy semantics, and even just important, when main() exits, no memory corruption occurs on the destruction of p1, p2, and p3.
So either you can fix the class to copy safely, or forget about it and live with a class that you have to handle very carefully and in limited situations (i.e. you can't reliably return copies of it as you're doing now).
For the record and combining all the responses in the lengthy discussion to the question.
Problems are:
- The value returned is a local variable that goes out of scope. This invokes the PolyTree destructor
- The PolyTree contains a vector of PolyNode * pointers. Those are allocated when clipper.Execute() is invoked.
- However PolyTree::Clear() does delete the nodes... and Clear() is invoked by the destructor.
- So within the function, the content is correct (allocated by Execute()), when passed outside, in the absence of copy constructors and
operator=
, the destructor of the local variable is invoked an the nodes are cleared, the result received outside of the function is empty.
The code for PolyTree::Clear()
void PolyTree::Clear()
{
for (PolyNodes::size_type i = 0; i < AllNodes.size(); ++i)
delete AllNodes[i];
AllNodes.resize(0);
Childs.resize(0);
}
Probably you should follow the pattern of Execute and define your function as:
void MeshHandler::trianglesToPolyTreeUnion(std::vector<Triangle> triangles,ClipperLib::PolyTree &tree)
Your problem is in the third line from the bottom of trianglesToPolyTreeUnion. The tree you are creating is created on the stack and is only in scope within the function.
You should dynamically allocate the memory and return a pointer to the tree or make your tree object a class member so it is still within scope once the function returns.