Updates based on comments from PullRequest in svalinn#660.

Author: pshriwise

src/make_watertight/CheckWatertight.cpp

@@ -81,16 +81,6 @@ moab::ErrorCode CheckWatertight::check_mesh_for_watertightness(moab::EntityHandl
   if (moab::MB_SUCCESS != result) { // failed to get edge data
     return result; // failed
   }
-//10/11/13
-//removed as a result of the change from the gen::find_skin function to the moab::Skinner:find_skin function
-  /*
-    result = MBI()->delete_entities( edges ); //otherwise delete all edge
-
-    if(moab::MB_SUCCESS != result) // failed to delete edge data
-      {
-        return result; // failed
-      }
-  */
   // loop over each volume meshset
   int vol_counter = 0;
   for (moab::Range::iterator i = vol_sets.begin(); i != vol_sets.end(); ++i) {
@@ -128,7 +118,6 @@ moab::ErrorCode CheckWatertight::check_mesh_for_watertightness(moab::EntityHandl
     }
 
     // save the edges in a vector that is large enough to avoid resizing
-    // presumably some kind of efficiency thing?? ad ??
     std::vector<coords_and_id> the_coords_and_id;
     the_coords_and_id.reserve(n_tris);
 
@@ -224,20 +213,6 @@ moab::ErrorCode CheckWatertight::check_mesh_for_watertightness(moab::EntityHandl
         temp.matched = false;
         the_coords_and_id.push_back(temp);
       }
-      //10/10/13
-      // Removed the following to avoid altering the data set at all
-      // -No need to delete skin_edges with the moab:Skinner find_skin function
-      // -skin_edges size will be reset to zero upon new moab::Range skin_edges; call
-      // clean up the edges for the next find_skin call
-      //result = MBI()->delete_entities( skin_edges );
-      //if(moab::MB_SUCCESS != result) return result;
-
-      //10/10/13
-      // - No ned to ensure edges aren't in the meshset with moab::Skinner find_skin function
-      //int n_edges;
-      //result = MBI()->get_number_entities_by_type(0, moab::MBEDGE, n_edges );
-      //if(moab::MB_SUCCESS != result) return result;
-      //if(0 != n_edges) MB_CHK_SET_ERR(moab::MB_MULTIPLE_ENTITIES_FOUND,"n_edges not equal to zero");
     }
 
     // sort the edges by the first vert. The first vert has a lower handle than the second.
@@ -271,7 +246,6 @@ moab::ErrorCode CheckWatertight::check_mesh_for_watertightness(moab::EntityHandl
               the_coords_and_id[j].vert2 == the_coords_and_id[k].vert2) {
             the_coords_and_id[j].matched = true;
             the_coords_and_id[k].matched = true;
-            //std::cout<< "matched by handle" << std::endl;
             break;
           }
         } else {
@@ -292,7 +266,6 @@ moab::ErrorCode CheckWatertight::check_mesh_for_watertightness(moab::EntityHandl
           if (d0 < tol * tol && d1 < tol * tol) {
             the_coords_and_id[j].matched = true;
             the_coords_and_id[k].matched = true;
-            //std::cout<< "matched by proximity" << std::endl;
             break;
           }
           // Due to the sort, once the x-coords are out of tolerance, a match

src/make_watertight/CheckWatertight.hpp

@@ -78,26 +78,23 @@ inline int compare_by_coords(const void* a, const void* b) {
             if (ia->z2 == ib->z2) {
               return ia->surf_id - ib->surf_id;
             } else {
-              return (ia->z2 > ib->z2) - (ia->z2 < ib->z2);
+              return (ia->z2 > ib->z2) ? 1 : -1;
             }
           } else {
-            return (ia->y2 > ib->y2) - (ia->y2 < ib->y2);
+            return (ia->y2 > ib->y2) ? 1 : -1;
           }
         } else {
-          return (ia->x2 > ib->x2) - (ia->x2 < ib->x2);
+          return (ia->x2 > ib->x2) ? 1 : -1;
         }
       } else {
-        return (ia->z1 > ib->z1) - (ia->z1 < ib->z1);
+        return (ia->z1 > ib->z1) ? 1 : -1;
       }
     } else {
-      return (ia->y1 > ib->y1) - (ia->y1 < ib->y1);;
+      return (ia->y1 > ib->y1) ? 1 : -1;
     }
   } else {
-    return (ia->x1 > ib->x1) - (ia->x1 < ib->x1);
+    return (ia->x1 > ib->x1) ? 1 : -1;
   }
-  /* float comparison: returns negative if b > a
-     and positive if a > b. We multiplied result by 100.0
-     to preserve decimal fraction */
 }
 
 #endif

src/make_watertight/Cleanup.cpp

@@ -115,7 +115,6 @@ moab::ErrorCode Cleanup::delete_small_edges(const moab::Range& surfaces, const d
 
     // get the skin first, because my find_skin does not check before creating edges.
     moab::Range skin_edges;
-    //result = gen->find_skin( tris, 1, skin_edges, false );
     moab::Skinner tool(MBI());
     result = tool.find_skin(0, tris, 1, skin_edges, false);
     assert(moab::MB_SUCCESS == result);
@@ -152,7 +151,7 @@ moab::ErrorCode Cleanup::delete_small_edges(const moab::Range& surfaces, const d
         MBI()->list_entities(duplicate_edges);
       assert(1 == duplicate_edges.size());
 
-      // if the edge length is less than MERGE_TOL do nothing
+      // if the edge length is less than FACET_TOL do nothing
       if (FACET_TOL < gen->dist_between_verts(endpts[0], endpts[1]))
         continue;
 
@@ -208,11 +207,6 @@ moab::ErrorCode Cleanup::delete_small_edges(const moab::Range& surfaces, const d
       }
       assert(3 <= adj_edges0.size());
       moab::Range adj_skin_edges0 = intersect(adj_edges0, skin_edges);
-      bool endpt0_is_skin;
-      if (adj_skin_edges0.empty())
-        endpt0_is_skin = false;
-      else
-        endpt0_is_skin = true;
 
       moab::Range adj_edges1;
       result = MBI()->get_adjacencies(&endpts[1], 1, 1, true, adj_edges1);
@@ -225,13 +219,11 @@ moab::ErrorCode Cleanup::delete_small_edges(const moab::Range& surfaces, const d
       }
       assert(3 <= adj_edges1.size());
       moab::Range adj_skin_edges1 = intersect(adj_edges1, skin_edges);
-      bool endpt1_is_skin;
-      if (adj_skin_edges1.empty())
-        endpt1_is_skin = false;
-      else
-        endpt1_is_skin = true;
-      if (endpt0_is_skin && endpt1_is_skin)
-        continue;
+
+      // check to see if the endpoints part of the skin
+      bool endpt0_is_skin = !adj_skin_edges0.empty();
+      bool endpt1_is_skin = !adj_skin_edges1.empty();
+      if (endpt0_is_skin && endpt1_is_skin) { continue; }
 
       // Keep the skin endpt, and delete the other endpt
       moab::EntityHandle keep_endpt, delete_endpt;

src/make_watertight/Gen.cpp

@@ -58,6 +58,9 @@ void Gen::moab_printer(moab::ErrorCode error_code) {
   if (error_code == moab::MB_FAILURE) {
     std::cerr << "ERROR: moab::MB_FAILURE" << std::endl;
   }
+  else {
+    std::cerr << "ERROR CODE NOT RECOGNIZED" << std::endl;
+  }
   return;
 }
 
@@ -69,7 +72,7 @@ void Gen::print_vertex_cubit(const moab::EntityHandle vertex) {
 
   moab::ErrorCode result;
   double coords[3];
-  int n_precision = 20;
+  const int n_precision = 20;
   result = MBI()->get_coords(&vertex, 1, coords);
   assert(moab::MB_SUCCESS == result);
   std::cout << "  create vertex "
@@ -214,7 +217,7 @@ void Gen::print_loop(const std::vector<moab::EntityHandle> loop_of_verts) {
 /// For efficiency: only get_coords on the reference vertex once
 ///                 if specified, limit search length along curve
 moab::ErrorCode Gen::find_closest_vert(const moab::EntityHandle reference_vert,
-                                       const std::vector<moab::EntityHandle> arc_of_verts,
+                                       const std::vector<moab::EntityHandle>& arc_of_verts,
                                        unsigned& position,
                                        const double dist_limit) {
   moab::ErrorCode rval;
@@ -264,7 +267,7 @@ moab::ErrorCode Gen::find_closest_vert(const moab::EntityHandle reference_vert,
 // This ensure that both are returned as candidates.
 moab::ErrorCode Gen::find_closest_vert(const double tol,
                                        const moab::EntityHandle reference_vert,
-                                       const std::vector<moab::EntityHandle> loop_of_verts,
+                                       const std::vector<moab::EntityHandle>& loop_of_verts,
                                        std::vector<unsigned>& positions,
                                        std::vector<double>& dists) {
 
@@ -305,7 +308,7 @@ moab::ErrorCode Gen::merge_vertices(moab::Range verts /* in */, const double tol
   const double SQR_TOL = tol * tol;
   // Clean up the created tree, and track verts so that if merged away they are
   // removed from the tree.
-  moab::AdaptiveKDTree kdtree(MBI()); //, true, 0, moab::MESHSET_TRACK_OWNER);
+  moab::AdaptiveKDTree kdtree(MBI());
   // initialize the KD Tree
   moab::EntityHandle root;
   const char settings[] = "MAX_PER_LEAF=6;MAX_DEPTH=50;SPLITS_PER_DIR=1;PLANE_SET=2;MESHSET_FLAGS=0x1;TAG_NAME=0";
@@ -327,8 +330,12 @@ moab::ErrorCode Gen::merge_vertices(moab::Range verts /* in */, const double tol
       std::vector<moab::EntityHandle> leaf_verts;
       result = MBI()->get_entities_by_type(leaves_out[j], moab::MBVERTEX, leaf_verts);
       assert(moab::MB_SUCCESS == result);
-      if (100 < leaf_verts.size())
+
+      // write number of leaf verts to screen if there is an abnormally high number of them
+      const int large_leaf_limit = 100;
+      if (leaf_verts.size() > large_leaf_limit)
         std::cout << "*i=" << *i << " leaf_verts.size()=" << leaf_verts.size() << std::endl;
+
       for (unsigned int k = 0; k < leaf_verts.size(); k++) {
         if (leaf_verts[k] == *i)
           continue;
@@ -357,7 +364,7 @@ moab::ErrorCode Gen::merge_vertices(moab::Range verts /* in */, const double tol
 
 moab::ErrorCode Gen::squared_dist_between_verts(const moab::EntityHandle v0,
                                                 const moab::EntityHandle v1,
-                                                double& d) {
+                                                double& squared_dist) {
   moab::ErrorCode result;
   moab::CartVect coords0, coords1;
   result = MBI()->get_coords(&v0, 1, coords0.array());
@@ -373,7 +380,7 @@ moab::ErrorCode Gen::squared_dist_between_verts(const moab::EntityHandle v0,
     return result;
   }
   const moab::CartVect diff = coords0 - coords1;
-  d = diff.length_squared();
+  squared_dist = diff.length_squared();
   return moab::MB_SUCCESS;
 }
 
@@ -427,15 +434,14 @@ double Gen::length(std::vector<moab::EntityHandle> edges) {
     return 0;
 
   moab::ErrorCode result;
-  std::vector<moab::EntityHandle>::iterator i;
   double dist = 0;
   moab::EntityType type = MBI()->type_from_handle(edges[0]);
 
   // if vector has both edges and verts, only use edges
   // NOTE: The curve sets from ReadCGM do not contain duplicate endpoints for loops!
   moab::EntityType end_type = MBI()->type_from_handle(edges.back());
   if (type != end_type) {
-    for (std::vector<moab::EntityHandle>::iterator i = edges.begin(); i != edges.end(); i++) {
+    for (auto i = edges.begin(); i != edges.end(); i++) {
       if (moab::MBVERTEX == MBI()->type_from_handle(*i)) {
         i = edges.erase(i) - 1;
       }
@@ -447,7 +453,7 @@ double Gen::length(std::vector<moab::EntityHandle> edges) {
   if (moab::MBEDGE == type) {
     if (edges.empty())
       return 0.0;
-    for (i = edges.begin(); i != edges.end(); i++) {
+    for (auto i = edges.begin(); i != edges.end(); i++) {
       int n_verts;
       const moab::EntityHandle* conn;
       result = MBI()->get_connectivity(*i, conn, n_verts);
@@ -463,7 +469,7 @@ double Gen::length(std::vector<moab::EntityHandle> edges) {
     if (2 > edges.size())
       return 0.0;
     moab::EntityHandle front_vert = edges.front();
-    for (i = edges.begin() + 1; i != edges.end(); i++) {
+    for (auto i = edges.begin() + 1; i != edges.end(); i++) {
       dist += dist_between_verts(front_vert, *i);
       front_vert = *i;
     }
@@ -510,7 +516,6 @@ bool Gen::edges_adjacent(moab::EntityHandle edge0, moab::EntityHandle edge1) {
 // get the direction unit vector from one vertex to another vertex
 moab::ErrorCode Gen::get_direction(const moab::EntityHandle from_vert, const moab::EntityHandle to_vert,
                                    moab::CartVect& dir) {
-  // double d[3];
   moab::ErrorCode result;
   moab::CartVect coords0, coords1;
   result = MBI()->get_coords(&from_vert, 1, coords0.array());
@@ -821,11 +826,6 @@ bool Gen::collinear(const moab::EntityHandle a, const moab::EntityHandle b,
     return false;
 }
 
-// Exclusive or: T iff exactly one argument is true
-bool logical_xor(const bool x, const bool y) {
-  return (x || y) && !(x && y);
-}
-
 bool Gen::intersect_prop(const moab::EntityHandle a, const moab::EntityHandle b,
                          const moab::EntityHandle c, const moab::EntityHandle d,
                          const moab::CartVect n) {
@@ -835,8 +835,8 @@ bool Gen::intersect_prop(const moab::EntityHandle a, const moab::EntityHandle b,
       collinear(c, d, b, n)) {
     return false;
   } else {
-    return logical_xor(left(a, b, c, n), left(a, b, d, n)) &&
-           logical_xor(left(c, d, a, n), left(c, d, b, n));
+    return (left(a, b, c, n) != left(a, b, d, n)) &&
+           (left(c, d, a, n) != left(c, d, b, n));
   }
 }
 
@@ -1246,18 +1246,11 @@ moab::ErrorCode Gen::dist_between_arcs(bool debug,
     }
   }
 
-  for (unsigned int i = 0; i < mgd_params.size(); i++) {
-
-  }
-
   // Insert new points to match the other arcs points, by parameter.
   for (unsigned int i = 0; i < 2; i++) {
     for (unsigned int j = 0; j < mgd_params.size(); j++) {
-      //std::cout << "params[" << i << "][" << j << "]=" << params[i][j]
-      //          << " mgd_params[" << j << "]=" << mgd_params[j] << std::endl;
       if (params[i][j] > mgd_params[j]) {
         double ratio = (mgd_params[j] - params[i][j - 1]) / (params[i][j] - params[i][j - 1]);
-        //std::cout << "j=" << j << " ratio=" << ratio << std::endl;
         moab::CartVect pt = coords[i][j - 1] + ratio * (coords[i][j] - coords[i][j - 1]);
         coords[i].insert(coords[i].begin() + j, pt);
         params[i].insert(params[i].begin() + j, mgd_params[j]);
@@ -1533,7 +1526,11 @@ moab::ErrorCode Gen::get_signed_volume(const moab::EntityHandle surf_set, double
   return moab::MB_SUCCESS;
 }
 
-moab::ErrorCode Gen::measure(const moab::EntityHandle set, const moab::Tag geom_tag, double& size, bool debug,  bool verbose) {
+moab::ErrorCode Gen::measure(const moab::EntityHandle set,
+                             const moab::Tag geom_tag,
+                             double& measurement,
+                             bool debug,
+                             bool verbose) {
   moab::ErrorCode result;
   int dim;
   result = MBI()->tag_get_data(geom_tag, &set, 1, &dim);
@@ -1546,17 +1543,17 @@ moab::ErrorCode Gen::measure(const moab::EntityHandle set, const moab::Tag geom_
     std::vector<moab::EntityHandle> vctr;
     result = get_meshset(set, vctr);
     assert(moab::MB_SUCCESS == result);
-    size = length(vctr);
+    measurement = length(vctr);
 
   } else if (2 == dim) {
     moab::Range tris;
     result = MBI()->get_entities_by_type(set, moab::MBTRI, tris);
     assert(moab::MB_SUCCESS == result);
-    size = triangle_area(tris);
+    measurement = triangle_area(tris);
 
   } else if (3 == dim) {
 
-    result = measure_volume(set, size, debug, verbose);
+    result = measure_volume(set, measurement, debug, verbose);
 
     if (moab::MB_SUCCESS != result) {
       std::cout << "result=" << result << " vol_id="
@@ -1758,10 +1755,8 @@ moab::ErrorCode Gen::remove_surf_sense_data(moab::EntityHandle del_surf, bool de
   moab::Range del_surf_curves;
   result = MBI() -> get_child_meshsets(del_surf, del_surf_curves);
   MB_CHK_SET_ERR(result, "could not get the curves of the surface to delete");
-  if (debug)
-    std::cout << "got the curves" << std::endl;
-
   if (debug) {
+    std::cout << "got the curves" << std::endl;
     std::cout << "number of curves to the deleted surface = " << del_surf_curves.size() << std::endl;
     for (unsigned int index = 0 ; index < del_surf_curves.size() ; index++) {
       std::cout << "deleted surface's child curve id " << index << " = " << geom_id_by_handle(del_surf_curves[index]) << std::endl;

src/make_watertight/Gen.hpp

@@ -54,13 +54,13 @@ class Gen {
   void print_loop(const std::vector<moab::EntityHandle> loop_of_verts);
 
   moab::ErrorCode find_closest_vert(const moab::EntityHandle reference_vert,
-                                    const std::vector<moab::EntityHandle> arc_of_verts,
+                                    const std::vector<moab::EntityHandle>& arc_of_verts,
                                     unsigned& position,
                                     const double dist_limit);
 
   moab::ErrorCode find_closest_vert(const double tol,
                                     const moab::EntityHandle reference_vert,
-                                    const std::vector<moab::EntityHandle> loop_of_verts,
+                                    const std::vector<moab::EntityHandle>& loop_of_verts,
                                     std::vector<unsigned>& positions,
                                     std::vector<double>&   dists);
 

src/make_watertight/MakeWatertight.cpp

@@ -724,7 +724,7 @@ moab::ErrorCode MakeWatertight::seal_arc_pair(const bool debug,
       std::cout << "skin:" << std::endl;
       gen->print_loop(skin);
     }
-    //return moab::MB_FAILURE;
+    return moab::MB_FAILURE;
   }
 
   if (debug) {
@@ -1520,7 +1520,7 @@ moab::ErrorCode MakeWatertight::delete_merged_curves(moab::Range& existing_curve
   result = MBI()->delete_entities(curves_to_delete);
   assert(moab::MB_SUCCESS == result);
   if (result != moab::MB_SUCCESS) {
-    std::cout << "Houston, we have a problem" << std::endl;
+    std::cout << "Failed to delete curves" << std::endl;
   }
   existing_curve_sets = subtract(existing_curve_sets, curves_to_delete);
   if (debug)