diff --git a/Source/test/tmModelTester/tmModelTester.cpp b/Source/test/tmModelTester/tmModelTester.cpp index 31ab059..5fd4bde 100644 --- a/Source/test/tmModelTester/tmModelTester.cpp +++ b/Source/test/tmModelTester/tmModelTester.cpp @@ -80,7 +80,7 @@ void DoReadFile(tmTree* theTree, const string& filename) try { theTree->GetSelf(fin); } - catch(...) { + catch (...) { cout << "Unexpected exception reading file " << fullname << endl; exit(1); } @@ -106,11 +106,11 @@ void DoScaleOptimization(char* name) try { theOptimizer->Optimize(); } - catch (tmNLCO::EX_BAD_CONVERGENCE ex) { + catch (const tmNLCO::EX_BAD_CONVERGENCE& ex) { cout << "Scale optimization failed with result code " << ex.GetReason() << endl; } - catch(tmScaleOptimizer::EX_BAD_SCALE) { + catch (const tmScaleOptimizer::EX_BAD_SCALE&) { cout << "Scale optimization failed with scale too small. " << endl; } stopTime = clock(); @@ -135,7 +135,7 @@ Read in a file and perform an edge optimization on it. template void DoEdgeOptimization(char* name) { - if(!name)return; + if(!name)return; const string filename = name; tmTree* theTree = new tmTree(); DoReadFile(theTree, filename); @@ -150,26 +150,31 @@ void DoEdgeOptimization(char* name) try { theOptimizer->Optimize(); } - catch (tmNLCO::EX_BAD_CONVERGENCE ex) { + catch (const tmNLCO::EX_BAD_CONVERGENCE& ex) { cout << "Edge optimization failed with result code " << ex.GetReason() << endl; + stopTime = clock(); + } + + if (stopTime < 0) { stopTime = clock(); + } + ReportCalls((filename + " objective").c_str(), - theOptimizer->GetNLCO()->GetObjective()); + theOptimizer->GetNLCO()->GetObjective()); ReportCalls((filename + " constraint").c_str(), - theOptimizer->GetNLCO()->GetConstraints()); - delete theOptimizer; - delete theNLCO; + theOptimizer->GetNLCO()->GetConstraints()); + cout << "Optimized over " << movingNodes.size() << " nodes and " << - stretchyEdges.size() << " edges. " << endl; + stretchyEdges.size() << " edges. " << endl; cout << "Edge strain is " << stretchyEdges.front()->GetStrain() << endl; - } - if (stopTime < 0) - stopTime = clock(); cout << "Feasibility after optimization is " << - (theTree->IsFeasible() ? "true " : "false") << endl; + (theTree->IsFeasible() ? "true " : "false") << endl; cout << "Elapsed time = " << (stopTime - startTime) << " ticks" << endl; cout << endl; + + delete theOptimizer; + delete theNLCO; delete theTree; } @@ -195,7 +200,7 @@ void DoStrainOptimization(char* name) try { theOptimizer->Optimize(); } - catch (tmNLCO::EX_BAD_CONVERGENCE ex) { + catch (const tmNLCO::EX_BAD_CONVERGENCE& ex) { cout << "Strain optimization failed with result code " << ex.GetReason() << endl; } diff --git a/Source/tmModel/tmPtrClasses/tmDpptrTarget.cpp b/Source/tmModel/tmPtrClasses/tmDpptrTarget.cpp index 8efa231..dca9fbb 100644 --- a/Source/tmModel/tmPtrClasses/tmDpptrTarget.cpp +++ b/Source/tmModel/tmPtrClasses/tmDpptrTarget.cpp @@ -12,6 +12,7 @@ Copyright: ©2003 Robert J. Lang. All Rights Reserved. #include "tmDpptrSrc.h" #include +#include using namespace std; @@ -68,16 +69,19 @@ Notify any DpptrSrcs that point to me to clear their pointers to me. *****/ tmDpptrTarget::~tmDpptrTarget() { - // Note: a tmDpptrArray can hold multiple references to the same object, in which - // case mDpptrSrcs will hold multiple pointers to the same tmDpptrArray. - vector theDpptrSrcs(mDpptrSrcs); - for (size_t i = 0; i < theDpptrSrcs.size(); ++i) { - tmDpptrSrc* theDpptrSrc = theDpptrSrcs[i]; - theDpptrSrc->RemoveDpptrTarget(this); + try { + // Note: a tmDpptrArray can hold multiple references to the same object, in which + // case mDpptrSrcs will hold multiple pointers to the same tmDpptrArray. + vector theDpptrSrcs(mDpptrSrcs); + for (size_t i = 0; i < theDpptrSrcs.size(); ++i) { + tmDpptrSrc* theDpptrSrc = theDpptrSrcs[i]; + theDpptrSrc->RemoveDpptrTarget(this); + } + } catch (...) { + std::cout << "Exception caught while cleaning up tmDpptrTarget" << std::endl; } } - /***** void tmDpptrTarget::AddDpptrSrc(tmDpptrSrc* r) Add a pointer-to-me diff --git a/Source/tmModel/tmSolvers/tmStubFinder.cpp b/Source/tmModel/tmSolvers/tmStubFinder.cpp index e2210d6..89ac16a 100644 --- a/Source/tmModel/tmSolvers/tmStubFinder.cpp +++ b/Source/tmModel/tmSolvers/tmStubFinder.cpp @@ -222,11 +222,11 @@ void tmStubFinder::TestOneCombo(tmArray& sInfoList) try { SolveEqns(10, u, 1.0e-8, 1.0e-8); } - catch(EX_TOO_MANY_ITERATIONS) { + catch(const EX_TOO_MANY_ITERATIONS&) { // no solution, so go on to the next case return; } - catch(EX_SINGULAR_MATRIX) { + catch(const EX_SINGULAR_MATRIX&) { // no solution, so go on to the next case return; } diff --git a/Source/tmModel/tmTreeClasses/tmFacetOwner.cpp b/Source/tmModel/tmTreeClasses/tmFacetOwner.cpp index 1a822b1..cde923f 100644 --- a/Source/tmModel/tmTreeClasses/tmFacetOwner.cpp +++ b/Source/tmModel/tmTreeClasses/tmFacetOwner.cpp @@ -64,7 +64,10 @@ inside of the poly. *****/ bool tmFacetOwner::CanStartFacetFwd(tmCrease* aCrease) { - if (aCrease->mFwdFacet) return false; + if (!aCrease || !aCrease->mVertices.front() || !aCrease->mVertices.back() || + !FacetOwnerAsPoly() || aCrease->mFwdFacet) { + return false; + } if (!aCrease->IsAxialCrease()) return true; return AreCCW(aCrease->mVertices.front()->mLoc, aCrease->mVertices.back()->mLoc, FacetOwnerAsPoly()->mCentroid); @@ -77,7 +80,10 @@ CanStartFacetBkd(), we will only start facets on non-axial creases. *****/ bool tmFacetOwner::CanStartFacetBkd(tmCrease* aCrease) { - if (aCrease->mBkdFacet) return false; + if (!aCrease || !aCrease->mVertices.front() || !aCrease->mVertices.back() || + !FacetOwnerAsPoly() || aCrease->mBkdFacet) { + return false; + } if (!aCrease->IsAxialCrease()) return true; return AreCW(aCrease->mVertices.front()->mLoc, aCrease->mVertices.back()->mLoc, FacetOwnerAsPoly()->mCentroid); @@ -133,40 +139,50 @@ counterclockwise. Also return the tmVertex (nextVertex) at the opposite end of the crease. *****/ void tmFacetOwner::GetNextCreaseAndVertex(tmCrease* thisCrease, - tmVertex* thisVertex, tmCrease*& nextCrease, tmVertex*& nextVertex) + tmVertex* thisVertex, tmCrease*& nextCrease, tmVertex*& nextVertex) { + // Validate input pointers + if (!thisCrease || !thisVertex || !thisCrease->mVertices.front() || !thisCrease->mVertices.back()) { + nextCrease = nullptr; + nextVertex = nullptr; + return; + } + // Get the angle of thisCrease. tmVertex* thatVertex = thisCrease->mVertices.front(); - if (thatVertex == thisVertex) thatVertex = thisCrease->mVertices.back(); + if (thatVertex == thisVertex) { + thatVertex = thisCrease->mVertices.back(); + } + + // Validate vertices + if (!thatVertex || !thisVertex) { + nextCrease = nullptr; + nextVertex = nullptr; + return; + } + tmFloat thisAngle = Angle(thatVertex->mLoc - thisVertex->mLoc); - // delta is the increment in angle from thisCrease to nextCrease. Start with - // the largest possible value. Then go through each of the Creases emanating - // from thisVertex and compare its angle to the angle of thisCrease. We'll - // keep the smallest positive angle we find. tmFloat delta = TWO_PI; nextCrease = thisCrease; - nextVertex = 0; + nextVertex = nullptr; + for (size_t i = 0; i < thisVertex->mCreases.size(); ++i) { tmCrease* thatCrease = thisVertex->mCreases[i]; - if (thatCrease == thisCrease) continue; + if (!thatCrease || thatCrease == thisCrease) continue; - // Get the angle of thatCrease and the tmVertex at the other end of - // thatCrease (thatVertex). + // Get vertices and validate thatVertex = thatCrease->mVertices.front(); - if (thatVertex == thisVertex) thatVertex = thatCrease->mVertices.back(); - tmFloat nextAngle = Angle(thatVertex->mLoc - thisVertex->mLoc); + if (!thatVertex || thatVertex == thisVertex) { + thatVertex = thatCrease->mVertices.back(); + } + if (!thatVertex) continue; - // Find the angular increment to the new crease. Constrain it to lie - // between 0 and 2 PI. + tmFloat nextAngle = Angle(thatVertex->mLoc - thisVertex->mLoc); tmFloat newDelta = thisAngle - nextAngle; while (newDelta < 0) newDelta += TWO_PI; while (newDelta >= TWO_PI) newDelta -= TWO_PI; - // If the angular increment is less than our current champion, we'll - // replace the current values with those of the new tmCrease. When we're - // done, we'll be left with the tmCrease that makes the smallest angle with - // the current tmCrease. if (newDelta < delta) { delta = newDelta; nextCrease = thatCrease; @@ -180,6 +196,7 @@ void tmFacetOwner::GetNextCreaseAndVertex(tmCrease* thisCrease, } + /***** Starting with a list of creases, construct a complete network of counterclockwise facets with their creases and vertices. diff --git a/Source/tmModel/tmTreeClasses/tmPath.cpp b/Source/tmModel/tmTreeClasses/tmPath.cpp index cf7c640..37e039a 100644 --- a/Source/tmModel/tmTreeClasses/tmPath.cpp +++ b/Source/tmModel/tmTreeClasses/tmPath.cpp @@ -5,7 +5,7 @@ Purpose: Implementation file for class tmPath Author: Robert J. Lang Modified by: Created: 2003-11-25 -Copyright: ©2003 Robert J. Lang. All Rights Reserved. +Copyright: 2003 Robert J. Lang. All Rights Reserved. *******************************************************************************/ #include "tmPath.h" @@ -417,20 +417,20 @@ vertex creation before we create any creases owned by the path. tmVertex* tmPath::MakeVertex(const tmPoint& p, tmNode* aTreeNode) { // Create the new vertex and insert it into the list at the appropriate spot. - tmPoint& p1 = mNodes.front()->mLoc; - tmPoint& p2 = mNodes.back()->mLoc; + const tmPoint& p1 = mNodes.front()->mLoc; + const tmPoint& p2 = mNodes.back()->mLoc; tmFloat dist_p = Mag(p - p1); tmFloat x = dist_p / Mag(p2 - p1); tmFloat elevation = (1 - x) * mNodes.front()->mElevation + x * mNodes.back()->mElevation; - // Create vertex and store it in a vector to track ownership - std::vector newVertices; - std::unique_ptr theVertex = std::make_unique(mTree, this, p, elevation, mIsBorderPath, aTreeNode); - newVertices.push_back(theVertex); - + // Create vertex and transfer ownership to mOwnedVertices + auto vertex = std::make_unique(mTree, this, p, elevation, mIsBorderPath, aTreeNode); + auto theVertex = vertex.get(); // Keep raw pointer for return value + mOwnedVertices.push_back(vertex.release()); // Transfer ownership to mOwnedVertices + // Insert vertex at correct position for (size_t i = 0; i < mOwnedVertices.size() - 1; ++i) { - tmVertex* testVertex = mOwnedVertices[i]; + const tmVertex* testVertex = mOwnedVertices[i]; tmFloat dist_t = Mag(testVertex->mLoc - p1); if (dist_p < dist_t) { mOwnedVertices.MoveItem(mOwnedVertices.size(), (i + 1)); @@ -439,8 +439,7 @@ tmVertex* tmPath::MakeVertex(const tmPoint& p, tmNode* aTreeNode) } // Handle crease splitting - for (size_t i = 0; i < mOwnedCreases.size(); ++i) { - tmCrease* theCrease = mOwnedCreases[i]; + for (auto* theCrease : mOwnedCreases) { tmVertex* frontVertex = theCrease->mVertices.front(); tmVertex* backVertex = theCrease->mVertices.back(); @@ -451,8 +450,12 @@ tmVertex* tmPath::MakeVertex(const tmPoint& p, tmNode* aTreeNode) if (x > 0 && x < 1) { TMLOG("tmPath::MakeVertex(..) -- crease split during vertex creation"); - std::make_unique(mTree, this, frontVertex, theVertex, theCrease->mKind); + // Create new creases and ensure they're properly owned + auto newCrease1 = std::make_unique(mTree, this, frontVertex, theVertex, theCrease->mKind); auto newCrease2 = std::make_unique(mTree, this, theVertex, backVertex, theCrease->mKind); + mOwnedCreases.push_back(newCrease1.release()); + mOwnedCreases.push_back(newCrease2.release()); + // Now that new creases are created and owned, we can safely delete the old one delete theCrease; break; } @@ -461,6 +464,7 @@ tmVertex* tmPath::MakeVertex(const tmPoint& p, tmNode* aTreeNode) return theVertex; } + /***** Return front vertex in the path, i.e., the vertex corresponding to the front node. Note: this is NOT the front vertex in mOwnedVertices. Return 0 if the @@ -513,13 +517,15 @@ void tmPath::BuildSelfVertices() tmPath* maxOutsetPath; tmFloat maxFrontReduction, maxBackReduction; GetMaxOutsetPath(maxOutsetPath, maxFrontReduction, maxBackReduction); + // maxOutsetPath is a view into an existing path, no ownership transfer tmFloat curPos = -maxFrontReduction; // Step through the nodes and edges of the path. Only create a vertex if // the position falls within the path. TMASSERT(maxOutsetPath->mEdges.not_empty()); - for (size_t i = 0; i < maxOutsetPath->mEdges.size(); ++i) { - tmNode* curNode = maxOutsetPath->mNodes[i + 1]; - curPos += maxOutsetPath->mEdges[i]->GetStrainedScaledLength(); + size_t nodeIndex = 1; + for (const auto& edge : maxOutsetPath->mEdges) { + tmNode* curNode = maxOutsetPath->mNodes[nodeIndex++]; + curPos += edge->GetStrainedScaledLength(); if (curPos <= 0.0) continue; if (curPos >= mActPaperLength) break; GetOrMakeVertex(q1 + qu * curPos, curNode);