From 0f7ea1ddc0d695726c9f66591cfedf50c5310561 Mon Sep 17 00:00:00 2001 From: Marius Kintel Date: Wed, 26 Oct 2011 16:59:26 +0200 Subject: Minor namespace fixes diff --git a/src/Tree.h b/src/Tree.h index aaa61d7..41ae613 100644 --- a/src/Tree.h +++ b/src/Tree.h @@ -3,8 +3,6 @@ #include "nodecache.h" -using std::string; - /*! For now, just an abstraction of the node tree which keeps a dump cache based on node indices around. @@ -20,8 +18,8 @@ public: void setRoot(const AbstractNode *root); const AbstractNode *root() const { return this->root_node; } - const string &getString(const AbstractNode &node) const; - const string &getIdString(const AbstractNode &node) const; + const std::string &getString(const AbstractNode &node) const; + const std::string &getIdString(const AbstractNode &node) const; private: const AbstractNode *root_node; diff --git a/src/color.cc b/src/color.cc index ee8f872..3c6942c 100644 --- a/src/color.cc +++ b/src/color.cc @@ -42,8 +42,6 @@ public: virtual AbstractNode *evaluate(const Context *ctx, const ModuleInstantiation *inst) const; }; -using std::string; - AbstractNode *ColorModule::evaluate(const Context *ctx, const ModuleInstantiation *inst) const { ColorNode *node = new ColorNode(inst); @@ -87,7 +85,7 @@ AbstractNode *ColorModule::evaluate(const Context *ctx, const ModuleInstantiatio return node; } -string ColorNode::toString() const +std::string ColorNode::toString() const { std::stringstream stream; @@ -96,7 +94,7 @@ string ColorNode::toString() const return stream.str(); } -string ColorNode::name() const +std::string ColorNode::name() const { return "color"; } diff --git a/src/nodedumper.h b/src/nodedumper.h index efaf4fa..aca17ed 100644 --- a/src/nodedumper.h +++ b/src/nodedumper.h @@ -7,10 +7,6 @@ #include "visitor.h" #include "nodecache.h" -using std::string; -using std::map; -using std::list; - class NodeDumper : public Visitor { public: @@ -26,15 +22,15 @@ private: void handleVisitedChildren(const State &state, const AbstractNode &node); bool isCached(const AbstractNode &node) const; void handleIndent(const State &state); - string dumpChildren(const AbstractNode &node); + std::string dumpChildren(const AbstractNode &node); NodeCache &cache; bool idprefix; - string currindent; + std::string currindent; const AbstractNode *root; - typedef list ChildList; - map visitedchildren; + typedef std::list ChildList; + std::map visitedchildren; }; #endif -- cgit v0.10.1 From ed54572c9b6d31abc7dddd14d0acc7a153db11a4 Mon Sep 17 00:00:00 2001 From: Marius Kintel Date: Wed, 26 Oct 2011 17:00:18 +0200 Subject: Make some code more readable and better error reporting on a reporter assert error diff --git a/src/CGALEvaluator.cc b/src/CGALEvaluator.cc index 10160ae..18b92f9 100644 --- a/src/CGALEvaluator.cc +++ b/src/CGALEvaluator.cc @@ -27,6 +27,8 @@ #include #include +#include + CGAL_Nef_polyhedron CGALEvaluator::evaluateCGALMesh(const AbstractNode &node) { if (!isCached(node)) { @@ -75,22 +77,25 @@ void CGALEvaluator::process(CGAL_Nef_polyhedron &target, const CGAL_Nef_polyhedr CGAL_Nef_polyhedron CGALEvaluator::applyToChildren(const AbstractNode &node, CGALEvaluator::CsgOp op) { CGAL_Nef_polyhedron N; - if (this->visitedchildren[node.index()].size() > 0) { - for (ChildList::const_iterator iter = this->visitedchildren[node.index()].begin(); - iter != this->visitedchildren[node.index()].end(); - iter++) { - const AbstractNode *chnode = iter->first; - const string &chcacheid = iter->second; - // FIXME: Don't use deep access to modinst members - if (chnode->modinst->tag_background) continue; - assert(isCached(*chnode)); - if (N.empty()) { - N = CGALCache::instance()->get(chcacheid).copy(); - } else { - process(N, CGALCache::instance()->get(chcacheid), op); - } - chnode->progress_report(); + BOOST_FOREACH(const ChildItem &item, this->visitedchildren[node.index()]) { + const AbstractNode *chnode = item.first; + const std::string &chcacheid = item.second; + // FIXME: Don't use deep access to modinst members + if (chnode->modinst->tag_background) continue; +// assert(isCached(*chnode)); + if (!isCached(*chnode)) { + PRINTF("Error - Not cached: Node %d", chnode->index()); + PRINTF(" chcacheid = %s", chcacheid.c_str()); + PRINTF(" getIdString() = %s", this->tree.getIdString(node).c_str()); + assert(false); } + + if (N.empty()) { + N = CGALCache::instance()->get(chcacheid).copy(); + } else { + process(N, CGALCache::instance()->get(chcacheid), op); + } + chnode->progress_report(); } return N; } @@ -107,7 +112,7 @@ CGAL_Nef_polyhedron CGALEvaluator::applyHull(const CgaladvNode &node) iter != this->visitedchildren[node.index()].end(); iter++) { const AbstractNode *chnode = iter->first; - const string &chcacheid = iter->second; + const std::string &chcacheid = iter->second; // FIXME: Don't use deep access to modinst members if (chnode->modinst->tag_background) continue; assert(isCached(*chnode)); @@ -520,9 +525,12 @@ CGAL_Nef_polyhedron CGALEvaluator::evaluateCGALMesh(const PolySet &ps) }; PolyReducer pr(ps); - PRINTF("Number of polygons before reduction: %d\n", pr.polygons.size()); + int numpolygons_before = pr.polygons.size(); pr.reduce(); - PRINTF("Number of polygons after reduction: %d\n", pr.polygons.size()); + int numpolygons_after = pr.polygons.size(); + if (numpolygons_after < numpolygons_before) { + PRINTF("reduce polygons: %d -> %d", numpolygons_before, numpolygons_after); + } return CGAL_Nef_polyhedron(pr.toNef()); #endif #if 0 diff --git a/src/CGALEvaluator.h b/src/CGALEvaluator.h index 0ac716c..6a26043 100644 --- a/src/CGALEvaluator.h +++ b/src/CGALEvaluator.h @@ -37,7 +37,8 @@ private: CGAL_Nef_polyhedron applyHull(const CgaladvNode &node); std::string currindent; - typedef std::list > ChildList; + typedef std::pair ChildItem; + typedef std::list ChildList; std::map visitedchildren; const Tree &tree; -- cgit v0.10.1 From 3080932440266b1f67ba106a536f3e6e6305fa80 Mon Sep 17 00:00:00 2001 From: Marius Kintel Date: Thu, 27 Oct 2011 00:49:55 +0200 Subject: Bugfix: Changed caching strategy to avoid the risk of sibling nodes being evicted from the cache before the parent node has evaluated. diff --git a/src/CGALEvaluator.cc b/src/CGALEvaluator.cc index 18b92f9..550f300 100644 --- a/src/CGALEvaluator.cc +++ b/src/CGALEvaluator.cc @@ -79,22 +79,20 @@ CGAL_Nef_polyhedron CGALEvaluator::applyToChildren(const AbstractNode &node, CGA CGAL_Nef_polyhedron N; BOOST_FOREACH(const ChildItem &item, this->visitedchildren[node.index()]) { const AbstractNode *chnode = item.first; - const std::string &chcacheid = item.second; + const CGAL_Nef_polyhedron &chN = item.second; // FIXME: Don't use deep access to modinst members if (chnode->modinst->tag_background) continue; -// assert(isCached(*chnode)); + + // NB! We insert into the cache here to ensure that all children of + // a node is a valid object. If we inserted as we created them, the + // cache could have been modified before we reach this point due to a large + // sibling object. if (!isCached(*chnode)) { - PRINTF("Error - Not cached: Node %d", chnode->index()); - PRINTF(" chcacheid = %s", chcacheid.c_str()); - PRINTF(" getIdString() = %s", this->tree.getIdString(node).c_str()); - assert(false); + CGALCache::instance()->insert(this->tree.getIdString(*chnode), chN); } + if (N.empty()) N = chN.copy(); + else process(N, chN, op); - if (N.empty()) { - N = CGALCache::instance()->get(chcacheid).copy(); - } else { - process(N, CGALCache::instance()->get(chcacheid), op); - } chnode->progress_report(); } return N; @@ -105,31 +103,25 @@ extern CGAL_Nef_polyhedron2 *convexhull2(std::list a); CGAL_Nef_polyhedron CGALEvaluator::applyHull(const CgaladvNode &node) { CGAL_Nef_polyhedron N; - if (this->visitedchildren[node.index()].size() > 0) { - std::list polys; - bool all2d = true; - for (ChildList::const_iterator iter = this->visitedchildren[node.index()].begin(); - iter != this->visitedchildren[node.index()].end(); - iter++) { - const AbstractNode *chnode = iter->first; - const std::string &chcacheid = iter->second; - // FIXME: Don't use deep access to modinst members - if (chnode->modinst->tag_background) continue; - assert(isCached(*chnode)); - const CGAL_Nef_polyhedron &ch = CGALCache::instance()->get(chcacheid); - if (ch.dim == 2) { - polys.push_back(ch.p2.get()); - } - else if (ch.dim == 3) { - PRINT("WARNING: hull() is not implemented yet for 3D objects!"); - all2d = false; - } - chnode->progress_report(); + std::list polys; + bool all2d = true; + BOOST_FOREACH(const ChildItem &item, this->visitedchildren[node.index()]) { + const AbstractNode *chnode = item.first; + const CGAL_Nef_polyhedron &chN = item.second; + // FIXME: Don't use deep access to modinst members + if (chnode->modinst->tag_background) continue; + if (chN.dim == 2) { + polys.push_back(chN.p2.get()); } - - if (all2d) { - N = CGAL_Nef_polyhedron(convexhull2(polys)); + else if (chN.dim == 3) { + PRINT("WARNING: hull() is not implemented yet for 3D objects!"); + all2d = false; } + chnode->progress_report(); + } + + if (all2d) { + N = CGAL_Nef_polyhedron(convexhull2(polys)); } return N; } @@ -145,11 +137,10 @@ Response CGALEvaluator::visit(State &state, const AbstractNode &node) { if (state.isPrefix() && isCached(node)) return PruneTraversal; if (state.isPostfix()) { - if (!isCached(node)) { - CGAL_Nef_polyhedron N = applyToChildren(node, CGE_UNION); - CGALCache::instance()->insert(this->tree.getIdString(node), N); - } - addToParent(state, node); + CGAL_Nef_polyhedron N; + if (!isCached(node)) N = applyToChildren(node, CGE_UNION); + else N = CGALCache::instance()->get(this->tree.getIdString(node)); + addToParent(state, node, N); } return ContinueTraversal; } @@ -158,11 +149,10 @@ Response CGALEvaluator::visit(State &state, const AbstractIntersectionNode &node { if (state.isPrefix() && isCached(node)) return PruneTraversal; if (state.isPostfix()) { - if (!isCached(node)) { - CGAL_Nef_polyhedron N = applyToChildren(node, CGE_INTERSECTION); - CGALCache::instance()->insert(this->tree.getIdString(node), N); - } - addToParent(state, node); + CGAL_Nef_polyhedron N; + if (!isCached(node)) N = applyToChildren(node, CGE_INTERSECTION); + else N = CGALCache::instance()->get(this->tree.getIdString(node)); + addToParent(state, node, N); } return ContinueTraversal; } @@ -171,6 +161,7 @@ Response CGALEvaluator::visit(State &state, const CsgNode &node) { if (state.isPrefix() && isCached(node)) return PruneTraversal; if (state.isPostfix()) { + CGAL_Nef_polyhedron N; if (!isCached(node)) { CGALEvaluator::CsgOp op; switch (node.type) { @@ -186,10 +177,12 @@ Response CGALEvaluator::visit(State &state, const CsgNode &node) default: assert(false); } - CGAL_Nef_polyhedron N = applyToChildren(node, op); - CGALCache::instance()->insert(this->tree.getIdString(node), N); + N = applyToChildren(node, op); + } + else { + N = CGALCache::instance()->get(this->tree.getIdString(node)); } - addToParent(state, node); + addToParent(state, node, N); } return ContinueTraversal; } @@ -198,9 +191,10 @@ Response CGALEvaluator::visit(State &state, const TransformNode &node) { if (state.isPrefix() && isCached(node)) return PruneTraversal; if (state.isPostfix()) { + CGAL_Nef_polyhedron N; if (!isCached(node)) { // First union all children - CGAL_Nef_polyhedron N = applyToChildren(node, CGE_UNION); + N = applyToChildren(node, CGE_UNION); // Then apply transform // If there is no geometry under the transform, N will be empty and of dim 0, @@ -236,9 +230,11 @@ Response CGALEvaluator::visit(State &state, const TransformNode &node) node.matrix(2,0), node.matrix(2,1), node.matrix(2,2), node.matrix(2,3), node.matrix(3,3)); N.p3->transform(t); } - CGALCache::instance()->insert(this->tree.getIdString(node), N); } - addToParent(state, node); + else { + N = CGALCache::instance()->get(this->tree.getIdString(node)); + } + addToParent(state, node, N); } return ContinueTraversal; } @@ -247,18 +243,20 @@ Response CGALEvaluator::visit(State &state, const AbstractPolyNode &node) { if (state.isPrefix() && isCached(node)) return PruneTraversal; if (state.isPostfix()) { + CGAL_Nef_polyhedron N; if (!isCached(node)) { // Apply polyset operation shared_ptr ps = this->psevaluator.getPolySet(node, false); - CGAL_Nef_polyhedron N; if (ps) { N = evaluateCGALMesh(*ps); // print_messages_pop(); node.progress_report(); } - CGALCache::instance()->insert(this->tree.getIdString(node), N); } - addToParent(state, node); + else { + N = CGALCache::instance()->get(this->tree.getIdString(node)); + } + addToParent(state, node, N); } return ContinueTraversal; } @@ -267,8 +265,8 @@ Response CGALEvaluator::visit(State &state, const CgaladvNode &node) { if (state.isPrefix() && isCached(node)) return PruneTraversal; if (state.isPostfix()) { + CGAL_Nef_polyhedron N; if (!isCached(node)) { - CGAL_Nef_polyhedron N; CGALEvaluator::CsgOp op; switch (node.type) { case MINKOWSKI: @@ -287,9 +285,11 @@ Response CGALEvaluator::visit(State &state, const CgaladvNode &node) N = applyHull(node); break; } - CGALCache::instance()->insert(this->tree.getIdString(node), N); } - addToParent(state, node); + else { + N = CGALCache::instance()->get(this->tree.getIdString(node)); + } + addToParent(state, node, N); } return ContinueTraversal; } @@ -298,12 +298,18 @@ Response CGALEvaluator::visit(State &state, const CgaladvNode &node) Adds ourself to out parent's list of traversed children. Call this for _every_ node which affects output during the postfix traversal. */ -void CGALEvaluator::addToParent(const State &state, const AbstractNode &node) +void CGALEvaluator::addToParent(const State &state, const AbstractNode &node, const CGAL_Nef_polyhedron &N) { assert(state.isPostfix()); this->visitedchildren.erase(node.index()); if (state.parent()) { - this->visitedchildren[state.parent()->index()].push_back(std::make_pair(&node, this->tree.getIdString(node))); + this->visitedchildren[state.parent()->index()].push_back(std::make_pair(&node, N)); + } + else { + // Root node, insert into cache + if (!isCached(node)) { + CGALCache::instance()->insert(this->tree.getIdString(node), N); + } } } diff --git a/src/CGALEvaluator.h b/src/CGALEvaluator.h index 6a26043..1dce4d9 100644 --- a/src/CGALEvaluator.h +++ b/src/CGALEvaluator.h @@ -30,14 +30,14 @@ public: const Tree &getTree() const { return this->tree; } private: - void addToParent(const State &state, const AbstractNode &node); + void addToParent(const State &state, const AbstractNode &node, const CGAL_Nef_polyhedron &N); bool isCached(const AbstractNode &node) const; void process(CGAL_Nef_polyhedron &target, const CGAL_Nef_polyhedron &src, CGALEvaluator::CsgOp op); CGAL_Nef_polyhedron applyToChildren(const AbstractNode &node, CGALEvaluator::CsgOp op); CGAL_Nef_polyhedron applyHull(const CgaladvNode &node); std::string currindent; - typedef std::pair ChildItem; + typedef std::pair ChildItem; typedef std::list ChildList; std::map visitedchildren; -- cgit v0.10.1