diff --git a/.github/workflows/download_test_data.sh b/.github/workflows/download_test_data.sh index 7cace146b80..81d9adec375 100755 --- a/.github/workflows/download_test_data.sh +++ b/.github/workflows/download_test_data.sh @@ -18,17 +18,19 @@ if [ "$PLATFORM" == "windows" ]; then choco install wget --no-progress fi -# Tiny test data downloads +# Tiny test data. A failed attempt leaves a partial file behind, so retry on whether the +# archive is complete rather than on whether it exists, and let wget resume it. +cd /tmp for run in {1..5}; do - if [ ! -f /tmp/tiny_test_cache.tar.gz ] ; then - cd /tmp - wget https://www.sci.utah.edu/~shapeworks/doc-resources/zips/tiny_test_cache.tar.gz || true - sleep 1 + if tar -tzf /tmp/tiny_test_cache.tar.gz > /dev/null 2>&1 ; then + break fi + wget --continue --tries=3 --timeout=60 https://www.sci.utah.edu/~shapeworks/doc-resources/zips/tiny_test_cache.tar.gz || true + sleep 1 done -# Decompress if successful -if [ -f /tmp/tiny_test_cache.tar.gz ] ; then +# Only extract a complete archive: a half-extracted cache sends the use cases to the web. +if tar -tzf /tmp/tiny_test_cache.tar.gz > /dev/null 2>&1 ; then cd $BASE/Examples/Python tar -xzvf /tmp/tiny_test_cache.tar.gz rm /tmp/tiny_test_cache.tar.gz diff --git a/Applications/shapeworks/shapeworks.cpp b/Applications/shapeworks/shapeworks.cpp index 16acbba8f43..ea12ab45297 100644 --- a/Applications/shapeworks/shapeworks.cpp +++ b/Applications/shapeworks/shapeworks.cpp @@ -6,11 +6,14 @@ #include "Executable.h" #include "Commands.h" #include +#include using namespace shapeworks; int main(int argc, char *argv[]) { + ShapeWorksUtils::initialize_path_handling(); + Executable shapeworks; // Image Commands diff --git a/Libs/Analyze/Shape.cpp b/Libs/Analyze/Shape.cpp index 2fa8ef3fda4..4f9a74670c3 100644 --- a/Libs/Analyze/Shape.cpp +++ b/Libs/Analyze/Shape.cpp @@ -5,6 +5,7 @@ #include #include #include +#include #include #include #include @@ -912,7 +913,7 @@ void Shape::set_reconstruction_transforms(std::vector #include #include #include +#include #include +#include +#include namespace shapeworks { @@ -19,16 +27,33 @@ void ShapeWorksUtils::set_rng_seed(const unsigned seed) { mt_.seed(rng_seed_); } +//----------------------------------------------------------------------------- +void ShapeWorksUtils::initialize_path_handling() { + // boost::filesystem stores a path in the character type the OS API uses: char on POSIX, but + // wchar_t on Windows. There every narrow string is converted using the codecvt facet imbued + // into boost::filesystem::path, which by default comes from the global locale rather than being + // UTF-8. The rest of ShapeWorks (Qt, VTK, ITK) speaks UTF-8, so without this a path holding + // non-ASCII characters is mangled on the way in and no longer resolves. On POSIX no conversion + // takes place and this only affects the wide-string accessors. + boost::filesystem::path::imbue(std::locale(std::locale(), new boost::nowide::utf8_codecvt)); +} + //----------------------------------------------------------------------------- bool ShapeWorksUtils::is_directory(const std::string& pathname) { - boost::system::error_code ec; - return boost::filesystem::is_directory(pathname, ec); + return vtksys::SystemTools::FileIsDirectory(pathname); } //----------------------------------------------------------------------------- bool ShapeWorksUtils::file_exists(const std::string& filename) { - boost::system::error_code ec; - return boost::filesystem::is_regular_file(filename, ec); + // vtksys rather than boost::filesystem: on Windows it decodes UTF-8 and applies the \\?\ prefix, + // so it agrees with the VTK and ITK readers about which paths exist. boost::filesystem does + // neither, and rejects long or non-ASCII paths that those readers open without complaint. + return vtksys::SystemTools::FileExists(filename, true); +} + +//----------------------------------------------------------------------------- +bool ShapeWorksUtils::path_exists(const std::string& pathname) { + return vtksys::SystemTools::PathExists(pathname); } //----------------------------------------------------------------------------- diff --git a/Libs/Common/ShapeworksUtils.h b/Libs/Common/ShapeworksUtils.h index 84e0414e917..d4f5cf3a72f 100644 --- a/Libs/Common/ShapeworksUtils.h +++ b/Libs/Common/ShapeworksUtils.h @@ -31,14 +31,19 @@ class ShapeWorksUtils { /// generates random number static unsigned get_random_number() { return mt_(); } + //! Configure process-wide filesystem path handling. Call this once at startup, before any path + //! is constructed from a narrow string. + static void initialize_path_handling(); + /// returns true if pathname is a directory - // TODO: in C++17 this is a standard function static bool is_directory(const std::string& pathname); - /// returns true if filename exists - // TODO: in C++17 this is a standard function + /// returns true if filename exists and can be read static bool file_exists(const std::string& filename); + /// returns true if a file or a directory exists at pathname + static bool path_exists(const std::string& pathname); + //! Set up the console logging options static void setup_console_logging(bool show_progress, bool xml_status); diff --git a/Libs/Image/Image.cpp b/Libs/Image/Image.cpp index 63c66bb089f..235777a5def 100644 --- a/Libs/Image/Image.cpp +++ b/Libs/Image/Image.cpp @@ -117,7 +117,7 @@ Image::ImageType::Pointer Image::read(const std::string& pathname) { } // check if it exists - if (!boost::filesystem::exists(pathname)) { + if (!ShapeWorksUtils::file_exists(pathname)) { throw std::runtime_error("File does not exist: " + pathname); } diff --git a/Libs/Mesh/Mesh.cpp b/Libs/Mesh/Mesh.cpp index be83154b804..598cdfcc351 100644 --- a/Libs/Mesh/Mesh.cpp +++ b/Libs/Mesh/Mesh.cpp @@ -5,7 +5,9 @@ #include #include +#include #include +#include #include // libigl @@ -20,6 +22,7 @@ #include #include #include +#include #include #include #include @@ -29,6 +32,7 @@ #include #include #include +#include #include #include #include @@ -57,6 +61,7 @@ #include #include #include +#include #include #include #include @@ -113,6 +118,78 @@ vtkSmartPointer update_and_check(vtkAlgorithm* reader, const std::s } return vtkPolyData::SafeDownCast(reader->GetOutputDataObject(0)); } + +//! Remove polygons that appear more than once (same point ids, in any order or winding). Duplicated +//! faces typically form zero-thickness fins hanging off the surface, which create non-manifold edges +//! and corrupt normals. All copies are removed unless that would open a hole in the surface, in which +//! case one copy is kept. Returns the number of faces removed. +vtkIdType remove_duplicate_faces(vtkPolyData* poly_data) { + auto polys = poly_data->GetPolys(); + const vtkIdType num_polys = polys->GetNumberOfCells(); + if (num_polys == 0) { + return 0; + } + + auto edge_key = [](vtkIdType a, vtkIdType b) { + return (static_cast(std::min(a, b)) << 32) | static_cast(std::max(a, b)); + }; + + std::vector> faces(num_polys); + std::unordered_map edge_count; + auto iter = vtk::TakeSmartPointer(polys->NewIterator()); + vtkIdType index = 0; + for (iter->GoToFirstCell(); !iter->IsDoneWithTraversal(); iter->GoToNextCell(), index++) { + vtkIdType num_points; + const vtkIdType* points; + iter->GetCurrentCell(num_points, points); + faces[index].assign(points, points + num_points); + for (vtkIdType i = 0; i < num_points; i++) { + edge_count[edge_key(points[i], points[(i + 1) % num_points])]++; + } + std::sort(faces[index].begin(), faces[index].end()); + } + + std::vector order(num_polys); + std::iota(order.begin(), order.end(), 0); + std::sort(order.begin(), order.end(), [&](vtkIdType a, vtkIdType b) { return faces[a] < faces[b]; }); + + // polys follow verts and lines in the cell id ordering + const vtkIdType poly_offset = poly_data->GetNumberOfVerts() + poly_data->GetNumberOfLines(); + std::vector to_delete; + for (vtkIdType start = 0; start < num_polys;) { + vtkIdType end = start + 1; + while (end < num_polys && faces[order[end]] == faces[order[start]]) { + end++; + } + const int copies = end - start; + if (copies > 1) { + // if removing every copy would leave an edge with a single face, this duplicates a real surface face + const auto& face = faces[order[start]]; + bool opens_hole = false; + for (size_t i = 0; i < face.size(); i++) { + for (size_t j = i + 1; j < face.size(); j++) { + auto it = edge_count.find(edge_key(face[i], face[j])); + if (it != edge_count.end() && it->second - copies == 1) { + opens_hole = true; + } + } + } + for (vtkIdType k = opens_hole ? start + 1 : start; k < end; k++) { + to_delete.push_back(poly_offset + order[k]); + } + } + start = end; + } + + if (!to_delete.empty()) { + poly_data->BuildCells(); + for (auto id : to_delete) { + poly_data->DeleteCell(id); + } + poly_data->RemoveDeletedCells(); + } + return to_delete.size(); +} } // namespace vtkSmartPointer MeshReader::read(const std::string& pathname) { @@ -125,6 +202,17 @@ vtkSmartPointer MeshReader::read(const std::string& pathname) { try { if (StringUtils::hasSuffix(pathname, ".vtk")) { + // legacy VTK files may store a surface as an unstructured grid; extract its surface as polydata + auto ug_reader = vtkSmartPointer::New(); + ug_reader->SetFileName(pathname.c_str()); + if (ug_reader->IsFileUnstructuredGrid()) { + ug_reader->SetReadAllScalars(1); + update_and_check(ug_reader, pathname); + auto geometry = vtkSmartPointer::New(); + geometry->SetInputConnection(ug_reader->GetOutputPort()); + return update_and_check(geometry, pathname); + } + auto reader = vtkSmartPointer::New(); reader->SetFileName(pathname.c_str()); reader->SetReadAllScalars(1); @@ -390,7 +478,24 @@ Mesh& Mesh::remesh(int numVertices, double adaptivity) { numVertices = std::max(numVertices, 1); - remesh->SetForceManifold(true); + // ACVD's ForceManifold treats clusters on an open boundary as non-manifold and keeps splitting + // them, piling extra vertices and sliver triangles along the boundary (e.g. a mesh clipped by a + // cutting plane ends up with more vertices than requested). Only force manifold on closed meshes. + // geometry only: vtkFeatureEdges copies point data, which reads out of bounds on a field that + // doesn't have one value per point + auto geometry = vtkSmartPointer::New(); + geometry->SetPoints(this->poly_data_->GetPoints()); + geometry->SetPolys(this->poly_data_->GetPolys()); + auto boundary_edges = vtkSmartPointer::New(); + boundary_edges->SetInputData(geometry); + boundary_edges->BoundaryEdgesOn(); + boundary_edges->FeatureEdgesOff(); + boundary_edges->NonManifoldEdgesOff(); + boundary_edges->ManifoldEdgesOff(); + boundary_edges->Update(); + bool is_closed = boundary_edges->GetOutput()->GetNumberOfCells() == 0; + + remesh->SetForceManifold(is_closed); remesh->SetInput(surf); remesh->SetFileLoadSaveOption(0); remesh->SetConsoleOutput(0); @@ -499,14 +604,22 @@ Mesh& Mesh::fillHoles(double hole_size) { Mesh& Mesh::clean() { TIME_SCOPE("Mesh::clean"); - auto clean = vtkSmartPointer::New(); - clean->ConvertPolysToLinesOff(); - clean->ConvertLinesToPointsOff(); - clean->ConvertStripsToPolysOff(); - clean->PointMergingOn(); - clean->SetInputData(poly_data_); - clean->Update(); - poly_data_ = clean->GetOutput(); + auto run_clean = [](vtkPolyData* input) { + auto clean = vtkSmartPointer::New(); + clean->ConvertPolysToLinesOff(); + clean->ConvertLinesToPointsOff(); + clean->ConvertStripsToPolysOff(); + clean->PointMergingOn(); + clean->SetInputData(input); + clean->Update(); + return vtkSmartPointer(clean->GetOutput()); + }; + + poly_data_ = run_clean(poly_data_); + if (remove_duplicate_faces(poly_data_) > 0) { + // drop points left unused by the removed faces + poly_data_ = run_clean(poly_data_); + } invalidateLocators(); return *this; } diff --git a/Libs/Mesh/Mesh.h b/Libs/Mesh/Mesh.h index c4f4bdef2ef..f584f9ac33f 100644 --- a/Libs/Mesh/Mesh.h +++ b/Libs/Mesh/Mesh.h @@ -104,7 +104,7 @@ class Mesh { /// finds holes in a mesh and closes them Mesh& fillHoles(double hole_size = 1000.0); - /// clean mesh + /// clean mesh: merges duplicate points, removes unused points, and removes duplicated faces Mesh& clean(); /// samples image data values at point locations specified by image diff --git a/Libs/Mesh/MeshUtils.cpp b/Libs/Mesh/MeshUtils.cpp index ce4a2453a89..f0edf8af841 100644 --- a/Libs/Mesh/MeshUtils.cpp +++ b/Libs/Mesh/MeshUtils.cpp @@ -192,23 +192,26 @@ int MeshUtils::findReferenceMesh(std::vector& meshes, int random_subset_si random_subset_size = 30; } bool use_random_subset = random_subset_size > 0 && random_subset_size < meshes.size(); - int num_meshes = use_random_subset ? random_subset_size : meshes.size(); - std::vector subset; + // indices of the meshes that are compared, only these are candidates for the reference + std::vector candidates; if (use_random_subset) { - subset = get_random_subset(random_subset_size, meshes.size()); + candidates = get_random_subset(random_subset_size, meshes.size()); + } else { + candidates.resize(meshes.size()); + std::iota(candidates.begin(), candidates.end(), 0); + } + + if (candidates.empty()) { + return 0; } std::vector > pairs; - // enumerate all pairs of meshes - for (size_t i = 0; i < num_meshes; i++) { - for (size_t j = i + 1; j < num_meshes; j++) { - if (use_random_subset) { - pairs.push_back(std::make_pair(subset[i], subset[j])); - } else { - pairs.push_back(std::make_pair(i, j)); - } + // enumerate all pairs of candidate meshes + for (size_t i = 0; i < candidates.size(); i++) { + for (size_t j = i + 1; j < candidates.size(); j++) { + pairs.push_back(std::make_pair(candidates[i], candidates[j])); } } @@ -246,7 +249,7 @@ int MeshUtils::findReferenceMesh(std::vector& meshes, int random_subset_si std::vector means(meshes.size(), 0); - double count = meshes.size() - 1; + double count = candidates.size() - 1; for (size_t i = 0; i < pairs.size(); i++) { auto pair = pairs[i]; double result = results[i]; @@ -254,9 +257,11 @@ int MeshUtils::findReferenceMesh(std::vector& meshes, int random_subset_si means[pair.second] += result / count; } - auto smallest = std::min_element(means.begin(), means.end()); + // only the candidates have a mean, the rest were never compared + auto smallest = std::min_element(candidates.begin(), candidates.end(), + [&](int a, int b) { return means[a] < means[b]; }); - return std::distance(means.begin(), smallest); + return *smallest; } /* @@ -305,7 +310,25 @@ Mesh MeshUtils::extract_boundary_loop(Mesh mesh) { throw std::runtime_error("Expected at least one boundary loop in the mesh"); } - auto loop = loops[0]; // copy so we can rotate it to a canonical start vertex + // A mesh may have more than one boundary loop (e.g. a hole in the shared surface). igl::boundary_loop + // returns them in arbitrary order, so use the longest one. + size_t longest = 0; + if (loops.size() > 1) { + double longest_length = -1.0; + for (size_t i = 0; i < loops.size(); i++) { + double length = 0.0; + for (size_t j = 0; j < loops[i].size(); j++) { + length += (V.row(loops[i][j]) - V.row(loops[i][(j + 1) % loops[i].size()])).norm(); + } + if (length > longest_length) { + longest_length = length; + longest = i; + } + } + SW_WARN("Mesh has {} boundary loops, extracting the longest", loops.size()); + } + + auto loop = loops[longest]; // copy so we can rotate it to a canonical start vertex // Rotate the loop so it always starts at a canonical vertex (the one with the // largest Y coordinate; lexicographic tiebreakers on Z then X). igl::boundary_loop @@ -369,9 +392,9 @@ Mesh MeshUtils::extract_boundary_loop(Mesh mesh) { */ //--------------------------------------------------------------------------- -static std::tuple rem_into_eigen_mesh(const std::vector& faces, - const Eigen::MatrixXd& src_V, - const Eigen::MatrixXi& src_F) { +// Also returns the list mapping each output vertex to its index in src_V +static std::tuple rem_into_eigen_mesh( + const std::vector& faces, const Eigen::MatrixXd& src_V, const Eigen::MatrixXi& src_F) { Eigen::MatrixXd V; Eigen::MatrixXi F; const std::unordered_set faces_set(faces.begin(), faces.end()); @@ -385,9 +408,10 @@ static std::tuple rem_into_eigen_mesh(const st } Eigen::VectorXi mapping; - igl::remove_unreferenced(src_V, F2, V, F, mapping); + Eigen::VectorXi new_to_old; + igl::remove_unreferenced(src_V, F2, V, F, mapping, new_to_old); - return std::make_tuple(V, F); + return std::make_tuple(V, F, new_to_old); } //--------------------------------------------------------------------------- @@ -417,18 +441,20 @@ static bool is_empty(const Eigen::MatrixXd& V, const Eigen::MatrixXi& F) { retur //--------------------------------------------------------------------------- // Identifies faces from the source mesh that lie within tolerance distance of the other mesh surface. // Uses spatial queries to determine which faces are "shared" between the two meshes. -// Returns both the shared faces as one mesh and the remaining non-shared faces as another mesh. -static std::tuple find_shared_surface( - const Eigen::MatrixXd& src_V, const Eigen::MatrixXi& src_F, const Eigen::MatrixXd& other_V, - const Eigen::MatrixXi& other_F, double tol = 1e-3) { +// Returns both the shared faces as one mesh and the remaining non-shared faces as another mesh, along with the +// seam vertices: the vertices of the remaining mesh that were also used by a shared face. +static std::tuple> +find_shared_surface(const Eigen::MatrixXd& src_V, const Eigen::MatrixXi& src_F, const Eigen::MatrixXd& other_V, + const Eigen::MatrixXi& other_F, double tol = 1e-3) { Eigen::MatrixXd out_V; Eigen::MatrixXi out_F; Eigen::MatrixXd rem_V; Eigen::MatrixXi rem_F; + std::vector seam; // If either mesh is empty, there can be no shared surface if (is_empty(src_V, src_F) || is_empty(other_V, other_F)) { - return std::make_tuple(out_V, out_F, src_V, src_F); + return std::make_tuple(out_V, out_F, src_V, src_F, seam); } igl::AABB tree; @@ -462,36 +488,56 @@ static std::tuple in_shared(src_V.rows(), false); + for (const auto& f : shared_faces) { + for (int i = 0; i < 3; i++) { + in_shared[src_F(f, i)] = true; + } + } + for (int i = 0; i < rem_to_src.size(); i++) { + if (in_shared[rem_to_src(i)]) { + seam.push_back(i); + } + } + + return std::make_tuple(out_V, out_F, rem_V, rem_F, seam); } //--------------------------------------------------------------------------- -// Snaps boundary vertices of the source mesh to the closest points on the shared mesh boundary. +// Snaps the seam vertices of the source mesh to the closest points on the shared mesh boundary. // This ensures the boundaries are geometrically aligned, which is typically needed for mesh // stitching or boolean operations. -// Returns a copy of the source mesh with modified boundary vertex positions. +// Only the seam vertices (those left on the boundary by removing the shared surface) are moved, so any other +// boundary loops of the source mesh (e.g. a cropped bone shaft) are left untouched. All boundary loops of the +// shared mesh are considered, since the shared surface may have holes or consist of several patches. +// Returns a copy of the source mesh with modified seam vertex positions. static void snap_boundary_to_mesh(const Eigen::MatrixXd& src_V, const Eigen::MatrixXi& src_F, - const Eigen::MatrixXd& shared_V, const Eigen::MatrixXi& shared_F, - Eigen::MatrixXd& out_V, Eigen::MatrixXi& out_F) { - std::vector > src_loops, shared_loops; - igl::boundary_loop(src_F, src_loops); + const std::vector& seam, const Eigen::MatrixXd& shared_V, + const Eigen::MatrixXi& shared_F, Eigen::MatrixXd& out_V, Eigen::MatrixXi& out_F) { + std::vector > shared_loops; igl::boundary_loop(shared_F, shared_loops); - if (src_loops.size() < 1 || shared_loops.size() < 1) { - SW_DEBUG("src_loops size: {}, shared_loops size: {}", src_loops.size(), shared_loops.size()); - throw std::runtime_error("Expected at least one boundary loop in each mesh"); + if (shared_loops.size() < 1) { + throw std::runtime_error("Expected at least one boundary loop in the shared surface"); } - const auto& src_loop = src_loops[0]; - const auto& shared_loop = shared_loops[0]; + size_t num_edges = 0; + for (const auto& shared_loop : shared_loops) { + num_edges += shared_loop.size(); + } Eigen::MatrixXi shared_F_boundary; - shared_F_boundary.resize(shared_loop.size(), 3); - for (int i = 0; i < shared_loop.size(); i++) { - const int v0 = shared_loop[i]; - const int v1 = shared_loop[(i + 1) % shared_loop.size()]; - shared_F_boundary.row(i) = Eigen::Vector3i{v0, v1, v1}; + shared_F_boundary.resize(num_edges, 3); + int next_idx = 0; + for (const auto& shared_loop : shared_loops) { + for (int i = 0; i < shared_loop.size(); i++) { + const int v0 = shared_loop[i]; + const int v1 = shared_loop[(i + 1) % shared_loop.size()]; + shared_F_boundary.row(next_idx++) = Eigen::Vector3i{v0, v1, v1}; + } } out_V = src_V; @@ -500,15 +546,15 @@ static void snap_boundary_to_mesh(const Eigen::MatrixXd& src_V, const Eigen::Mat igl::AABB tree; tree.init(shared_V, shared_F_boundary); - for (int i = 0; i < src_loop.size(); i++) { + for (const auto& v : seam) { Eigen::VectorXd sqrD; Eigen::VectorXi I; Eigen::MatrixXd C; Eigen::MatrixXd P(1, 3); - P.row(0) = src_V.row(src_loop[i]); + P.row(0) = src_V.row(v); tree.squared_distance(shared_V, shared_F_boundary, P, sqrD, I, C); - out_V.row(src_loop[i]) = C.row(0); + out_V.row(v) = C.row(0); } } @@ -530,8 +576,9 @@ std::array MeshUtils::shared_boundary_extractor(const Mesh& mesh_l, con Eigen::MatrixXd shared_V_l, shared_V_r, rem_V_l, rem_V_r; Eigen::MatrixXi shared_F_l, shared_F_r, rem_F_l, rem_F_r; - std::tie(shared_V_l, shared_F_l, rem_V_l, rem_F_l) = find_shared_surface(V_l, F_l, V_r, F_r, tol); - std::tie(shared_V_r, shared_F_r, rem_V_r, rem_F_r) = find_shared_surface(V_r, F_r, V_l, F_l, tol); + std::vector seam_l; + std::tie(shared_V_l, shared_F_l, rem_V_l, rem_F_l, seam_l) = find_shared_surface(V_l, F_l, V_r, F_r, tol); + std::tie(shared_V_r, shared_F_r, rem_V_r, rem_F_r, std::ignore) = find_shared_surface(V_r, F_r, V_l, F_l, tol); if (is_empty(shared_V_l, shared_F_l) || is_empty(shared_V_r, shared_F_r) || is_empty(rem_V_l, rem_F_l) || is_empty(rem_V_r, rem_F_r)) { @@ -543,7 +590,7 @@ std::array MeshUtils::shared_boundary_extractor(const Mesh& mesh_l, con Eigen::MatrixXd bridge_V; Eigen::MatrixXi bridge_F; - snap_boundary_to_mesh(rem_V_l, rem_F_l, shared_V_r, shared_F_r, bridge_V, bridge_F); + snap_boundary_to_mesh(rem_V_l, rem_F_l, seam_l, shared_V_r, shared_F_r, bridge_V, bridge_F); Mesh out_l(bridge_V, bridge_F); Mesh out_r(rem_V_r, rem_F_r); diff --git a/Libs/Project/Project.cpp b/Libs/Project/Project.cpp index 289e5638749..db3d28f4f21 100644 --- a/Libs/Project/Project.cpp +++ b/Libs/Project/Project.cpp @@ -1,6 +1,7 @@ #include #include #include +#include #include #include @@ -91,17 +92,27 @@ void Project::set_project_path(const std::string& new_pathname) { auto old_path = fs::path(project_path_); auto new_path = fs::path(new_pathname); + // Re-anchor one path to the new project location. Returns the path unchanged if it can't be, + // so that a single awkward path doesn't fail the whole save. + auto rebase = [&](std::string path) { + if (!ShapeWorksUtils::path_exists(path)) { + return path; + } + // replace \ with / in path + path = StringUtils::replace_string(path, "\\", "/"); + try { + auto canonical = fs::canonical(path, old_path); + return fs::relative(canonical, new_path).string(); + } catch (std::exception& e) { + SW_WARN("Unable to update path \"{}\" for new project location: {}", path, e.what()); + return path; + } + }; + auto fixup = [&](StringList paths) { StringList new_paths; - for (auto path : paths) { - if (fs::exists(path)) { - // replace \ with / in path - path = StringUtils::replace_string(path, "\\", "/"); - auto canonical = fs::canonical(path, old_path); - new_paths.push_back(fs::relative(canonical, new_path).string()); - } else { - new_paths.push_back(path); - } + for (auto& path : paths) { + new_paths.push_back(rebase(path)); } return new_paths; }; @@ -117,11 +128,7 @@ void Project::set_project_path(const std::string& new_pathname) { auto features = subject->get_feature_filenames(); project::types::StringMap new_features; for (auto const& x : features) { - auto path = x.second; - // replace \ with / in path - path = StringUtils::replace_string(path, "\\", "/"); - auto canonical = fs::canonical(path, old_path); - new_features[x.first] = fs::relative(canonical, new_path).string(); + new_features[x.first] = rebase(x.second); } subject->set_feature_filenames(new_features); } @@ -350,7 +357,7 @@ void Project::determine_feature_names() { if (get_original_domain_types()[d] == DomainType::Mesh) { if (subject->get_original_filenames().size() > d) { auto filename = subject->get_original_filenames()[d]; - if (fs::exists(filename)) { + if (ShapeWorksUtils::file_exists(filename)) { try { auto poly_data = MeshUtils::threadSafeReadMesh(filename).getVTKMesh(); if (poly_data) { diff --git a/Libs/Python/ShapeworksPython.cpp b/Libs/Python/ShapeworksPython.cpp index e656ad8bdf8..dc48fce866e 100644 --- a/Libs/Python/ShapeworksPython.cpp +++ b/Libs/Python/ShapeworksPython.cpp @@ -59,6 +59,8 @@ namespace fs = boost::filesystem; using namespace shapeworks; PYBIND11_MODULE(shapeworks_py, m) { + ShapeWorksUtils::initialize_path_handling(); + m.doc() = "ShapeWorks Python API"; m.attr("Pi") = std::atan(1.0) * 4.0; @@ -993,6 +995,8 @@ PYBIND11_MODULE(shapeworks_py, m) { .def("fillHoles", &Mesh::fillHoles, "finds holes in a mesh and closes them", "hole_size"_a = 1000) + .def("clean", &Mesh::clean, "merges duplicate points, removes unused points, and removes duplicated faces") + .def("probeVolume", &Mesh::probeVolume, "samples image data values at point locations specified by image", "image"_a) diff --git a/Python/shapeworks/shapeworks/datasets.py b/Python/shapeworks/shapeworks/datasets.py index 4dd68637f58..41adf1de8c9 100644 --- a/Python/shapeworks/shapeworks/datasets.py +++ b/Python/shapeworks/shapeworks/datasets.py @@ -18,28 +18,46 @@ makes verification worth doing, since a bad re-upload cannot rewrite the hash it is checked against. +Downloads resume, within a run and across runs, from the bytes already on disk. + Set SW_DATA_URL to point at a different server or a local mirror when testing. Archives are built by Support/build_dataset_archives.py. """ import hashlib +import http.client import json import os +import socket import ssl import sys -import tempfile import time import urllib.error import urllib.parse import urllib.request import zipfile +try: + import certifi +except ImportError: + certifi = None + INDEX_FILE = os.path.join(os.path.dirname(os.path.abspath(__file__)), "datasets.json") _CHUNK = 1024 * 256 -_RETRIES = 3 -_TIMEOUT = 60 +_TIMEOUT = 60 # seconds one read may stall +_STALLED_RETRIES = 6 # consecutive attempts moving no bytes before giving up +_MAX_ATTEMPTS = 40 +_MAX_BACKOFF = 30 +_REPORT_EVERY = 30 # seconds between progress lines in a log + +# socket.timeout only aliases TimeoutError from Python 3.10 on. +_TRANSIENT = (urllib.error.URLError, http.client.HTTPException, ssl.SSLError, + socket.timeout, TimeoutError, ConnectionError, EOFError) +_RETRY_STATUS = (408, 425, 429, 500, 502, 503, 504) _index_cache = None +_ssl_context = None +_reported = [0.0] def get_index(): @@ -62,67 +80,162 @@ def base_url(): return url if url.endswith("/") else url + "/" -def _open(url): - """Open a URL, retrying transient failures.""" - last = None - for attempt in range(_RETRIES): - try: - return urllib.request.urlopen(url, timeout=_TIMEOUT) - except (urllib.error.URLError, ssl.SSLError, TimeoutError) as e: - last = e - if attempt < _RETRIES - 1: - time.sleep(2 ** attempt) - raise RuntimeError(f"Could not reach {url}\n {last}\n" - f"Check your network connection, or set $SW_DATA_URL to a mirror.") - - def list_datasets(): """Names of every dataset this release knows how to download.""" return sorted(get_index().get("datasets", {})) def _progress(name, done, total): - if not sys.stdout.isatty(): + """Report progress: continuously on a terminal, every _REPORT_EVERY seconds in a log.""" + now = time.monotonic() + if sys.stdout.isatty(): + line_end = "\r" + elif now - _reported[0] >= _REPORT_EVERY: + line_end = "\n" + else: return + _reported[0] = now if total: - pct = 100.0 * done / total - sys.stdout.write(f"\r {name}: {pct:5.1f}% " - f"({done / 1e6:.1f} / {total / 1e6:.1f} MB)") + sys.stdout.write(f" {name}: {100.0 * done / total:5.1f}% " + f"({done / 1e6:.1f} / {total / 1e6:.1f} MB){line_end}") else: - sys.stdout.write(f"\r {name}: {done / 1e6:.1f} MB") + sys.stdout.write(f" {name}: {done / 1e6:.1f} MB{line_end}") sys.stdout.flush() -def _download(url, destination, name, expected_size): - """Stream a URL to disk, returning its sha256.""" +def _context(): + """TLS context trusting both the system roots and certifi's. + + A fresh Windows install holds only a few roots and fetches the rest on demand, + which happens only when Windows itself verifies a connection. OpenSSL reads the + store as it stands, so it can miss the root our server chains to. certifi + supplies it. + """ + global _ssl_context + if _ssl_context is None: + _ssl_context = ssl.create_default_context() + if certifi is not None: + _ssl_context.load_verify_locations(cafile=certifi.where()) + return _ssl_context + + +def _size(path): + return os.path.getsize(path) if os.path.exists(path) else 0 + + +def _sha256(path): digest = hashlib.sha256() - downloaded = 0 - with _open(url) as response: - total = expected_size or int(response.headers.get("Content-Length") or 0) - with open(destination, "wb") as out: + with open(path, "rb") as fh: + for block in iter(lambda: fh.read(_CHUNK), b""): + digest.update(block) + return digest.hexdigest() + + +def _transient(error): + """Whether another attempt could get past this failure.""" + if isinstance(error, urllib.error.HTTPError): + return error.code in _RETRY_STATUS + return isinstance(error, _TRANSIENT) + + +def _stream(url, destination, name, total, tag): + """One attempt at fetching url, appending to whatever destination already holds. + + total and tag (length and ETag) carry over between attempts. Returns the bytes on + disk, short of total if the connection dropped. + """ + have = _size(destination) + request = urllib.request.Request(url) + if have: + request.add_header("Range", f"bytes={have}-") + if tag: + # A changed archive answers 200, not 206, so we start over rather than + # splice two different downloads together. + request.add_header("If-Range", tag) + + with urllib.request.urlopen(request, timeout=_TIMEOUT, context=_context()) as response: + tag = response.headers.get("ETag") or tag + length = int(response.headers.get("Content-Length") or 0) + if response.getcode() != 206: + have = 0 # not partial content: start over + total = total or (have + length) + with open(destination, "ab" if have else "wb") as out: while True: chunk = response.read(_CHUNK) if not chunk: break out.write(chunk) - digest.update(chunk) - downloaded += len(chunk) - _progress(name, downloaded, total) + have += len(chunk) + _progress(name, have, total) + return have, total, tag + + +def _download(url, destination, name, expected_size): + """Fetch a URL to disk and return its sha256. + + Each attempt resumes from the bytes already on disk, and the partial file is left + behind when we give up. + """ + total = expected_size or 0 + tag = None + have = _size(destination) + stalled = 0 + problem = None + + for attempt in range(_MAX_ATTEMPTS): + before = have + try: + have, total, tag = _stream(url, destination, name, total, tag) + if have >= total: + break + problem = f"connection closed after {have} of {total} bytes" + except _TRANSIENT as e: + if isinstance(e, urllib.error.HTTPError) and e.code == 416: + # A partial longer than the archive cannot belong to it. + os.remove(destination) + total = expected_size or 0 + tag = None + elif not _transient(e): + raise RuntimeError(f"Could not download {url}\n {e}\n" + f"Set $SW_DATA_URL to a mirror if the server has " + f"moved.") from None + have = _size(destination) + problem = str(e) or e.__class__.__name__ + + stalled = 0 if have > before else stalled + 1 + if stalled >= _STALLED_RETRIES: + problem = f"{problem} (nothing transferred in {stalled} attempts)" + break + if attempt + 1 >= _MAX_ATTEMPTS: + problem = f"{problem} (gave up after {_MAX_ATTEMPTS} attempts)" + break + delay = min(_MAX_BACKOFF, 2 ** stalled) + if sys.stdout.isatty(): + sys.stdout.write("\n") + print(f" {name}: {problem}; resuming in {delay}s " + f"(attempt {attempt + 2} of {_MAX_ATTEMPTS})", flush=True) + time.sleep(delay) + if sys.stdout.isatty(): sys.stdout.write("\n") - if expected_size and downloaded != expected_size: + if have < total or (expected_size and have != expected_size): raise RuntimeError( - f"{name}: download truncated, got {downloaded} bytes, expected {expected_size}") - return digest.hexdigest() + f"Could not download {name} from {url}\n {problem}\n" + f"{_size(destination) / 1e6:.1f} MB were kept in {destination}, so running " + f"this again resumes rather than starting over.\n" + f"Check your network connection, or set $SW_DATA_URL to a mirror.") + return _sha256(destination) def _read_marker(path): - """Return what a previous download recorded, or None.""" + """What a previous download recorded, or {} for the empty markers older releases and + the CI tiny test cache wrote.""" try: with open(path) as fh: return json.load(fh) - except (OSError, json.JSONDecodeError): - # Pre-existing marker files were empty, so treat them as unknown content. + except json.JSONDecodeError: + return {} + except OSError: return None @@ -136,6 +249,8 @@ def download_dataset(datasetName, outputDirectory, force=False): The marker records the checksum that was installed, so pointing this release at a newer version of a dataset makes the next run replace it instead of quietly reusing whatever is already on disk. + + A download that gives up leaves its partial archive behind for the next run. """ marker = os.path.join("Output", datasetName + ".downloaded") installed = _read_marker(marker) if os.path.exists(marker) else None @@ -147,14 +262,16 @@ def download_dataset(datasetName, outputDirectory, force=False): f"Available datasets:\n " + "\n ".join(sorted(datasets))) entry = datasets[datasetName] + recorded = installed.get("sha256") if installed is not None else None - # Nothing here touches the network, so re-running a use case works offline. - if installed and not force and installed.get("sha256") == entry.get("sha256"): + # Nothing here touches the network, so re-running a use case works offline. A marker + # naming no checksum is taken at its word: CI pre-populates the tiny test data. + if installed is not None and not force and recorded in (None, entry.get("sha256")): print(f"Dataset {datasetName} already downloaded ({marker} exists)") if os.environ.get("SW_PORTAL_DOWNLOAD_ONLY") == "1": sys.exit(0) return - if installed and installed.get("sha256") != entry.get("sha256"): + if recorded and recorded != entry.get("sha256"): print(f"Dataset {datasetName} is pinned to {entry['file']}, downloading it") url = urllib.parse.urljoin(base_url(), entry["file"]) @@ -162,22 +279,25 @@ def download_dataset(datasetName, outputDirectory, force=False): os.makedirs(os.path.dirname(marker) or ".", exist_ok=True) print(f"Downloading {datasetName} from {url}") - handle, temporary = tempfile.mkstemp(suffix=".zip", prefix=datasetName + "-", - dir=outputDirectory) - os.close(handle) + partial = os.path.join(outputDirectory, entry["file"] + ".part") + if os.path.exists(partial): + print(f"Resuming from {_size(partial) / 1e6:.1f} MB already in {partial}") + + # Kept on a failed download so the next run resumes, removed once judged. + actual = _download(url, partial, datasetName, entry.get("size")) try: - actual = _download(url, temporary, datasetName, entry.get("size")) expected = entry.get("sha256") if expected and actual != expected: raise RuntimeError( f"{datasetName}: checksum mismatch for {entry['file']}\n" f" expected {expected}\n got {actual}\n" - f"The copy on the server does not match what this release of " - f"ShapeWorks was built against. Published archives are immutable, " - f"so {entry['file']} was either overwritten or is corrupt.") + f"The bytes we ended up with do not match what this release of " + f"ShapeWorks was built against. Published archives are immutable, so " + f"{entry['file']} was either overwritten on the server or arrived " + f"corrupt. It has been discarded, so running this again starts fresh.") print(f"Extracting {datasetName} to {outputDirectory}") - with zipfile.ZipFile(temporary) as archive: + with zipfile.ZipFile(partial) as archive: members = [m for m in archive.namelist() if not m.endswith("/")] archive.extractall(outputDirectory) @@ -187,8 +307,8 @@ def download_dataset(datasetName, outputDirectory, force=False): f"{datasetName}: archive holds {len(members)} files, " f"datasets.json expects {expected_files}. The archive needs rebuilding.") finally: - if os.path.exists(temporary): - os.remove(temporary) + if os.path.exists(partial): + os.remove(partial) with open(marker, "w") as fh: json.dump({"dataset": datasetName, "sha256": entry.get("sha256"), diff --git a/Studio/Data/Session.cpp b/Studio/Data/Session.cpp index a4bd48e1a25..e54bf5c2328 100644 --- a/Studio/Data/Session.cpp +++ b/Studio/Data/Session.cpp @@ -901,13 +901,17 @@ std::string Session::get_default_feature_map() { auto subject = get_project()->get_subjects()[0]; if (!subject->get_original_filenames().empty()) { if (project_->get_original_domain_types()[0] == DomainType::Mesh) { - Mesh m = MeshUtils::threadSafeReadMesh(subject->get_original_filenames()[0]); - auto poly_data = m.getVTKMesh(); - if (poly_data) { - auto scalars = poly_data->GetPointData()->GetScalars(); - if (scalars) { - return scalars->GetName(); + try { + Mesh m = MeshUtils::threadSafeReadMesh(subject->get_original_filenames()[0]); + auto poly_data = m.getVTKMesh(); + if (poly_data) { + auto scalars = poly_data->GetPointData()->GetScalars(); + if (scalars) { + return scalars->GetName(); + } } + } catch (std::exception& e) { + SW_ERROR("{}", e.what()); } } } diff --git a/Studio/Interface/ExportImageDialog.cpp b/Studio/Interface/ExportImageDialog.cpp index 2bb74d3d233..09278012323 100644 --- a/Studio/Interface/ExportImageDialog.cpp +++ b/Studio/Interface/ExportImageDialog.cpp @@ -1,12 +1,17 @@ // std +#include +#include #include // qt includes +#include #include #include #include #include #include +#include +#include // studio #include @@ -20,6 +25,29 @@ namespace shapeworks { +namespace { +//! Bounds memory, not the GPU: tiling lifted the hardware limit. 32768 x 32768, 4 GB as ARGB. +const qint64 MAX_EXPORT_PIXELS = 1024LL * 1024 * 1024; + +//! Sanity bound, well inside the width at which QImage's int bytes-per-line would overflow +const int MAX_EXPORT_DIMENSION = 65536; + +//! The preview only fills a label; rendering it full size would touch gigabytes per keystroke +const int MAX_PREVIEW_DIMENSION = 2048; + +//--------------------------------------------------------------------------- +QSize clamp_export_size(QSize size) { + QSize result(clamp(size.width(), 1, MAX_EXPORT_DIMENSION), clamp(size.height(), 1, MAX_EXPORT_DIMENSION)); + qint64 pixels = static_cast(result.width()) * result.height(); + if (pixels > MAX_EXPORT_PIXELS) { + double factor = std::sqrt(static_cast(MAX_EXPORT_PIXELS) / static_cast(pixels)); + result = QSize(std::max(1, static_cast(result.width() * factor)), + std::max(1, static_cast(result.height() * factor))); + } + return result; +} +} // namespace + //--------------------------------------------------------------------------- ExportImageDialog::ExportImageDialog(QWidget* parent, Preferences& prefs, QSharedPointer analysis_tool, QSharedPointer visualizer, bool pca_mode) @@ -39,12 +67,15 @@ ExportImageDialog::ExportImageDialog(QWidget* parent, Preferences& prefs, QShare setGeometry(rect); // load state from prefs - QIntValidator* size_validator = new QIntValidator(1, 65535, this); + QIntValidator* size_validator = new QIntValidator(1, MAX_EXPORT_DIMENSION, this); ui_->override_width->setValidator(size_validator); ui_->override_height->setValidator(size_validator); - ui_->override_width->setText(QString::number(prefs_.get_export_override_size().width())); - ui_->override_height->setText(QString::number(prefs_.get_export_override_size().height())); + // a size stored by an older version, or stored while it was still being typed, may be unrenderable + QSize export_size = clamp_export_size(prefs_.get_export_override_size()); + ui_->override_width->setText(QString::number(export_size.width())); + ui_->override_height->setText(QString::number(export_size.height())); ui_->override_window_size->setChecked(prefs_.get_export_override_size_enabled()); + ui_->window_size_widget->setEnabled(ui_->override_window_size->isChecked()); ui_->show_corner_widget->setChecked(prefs_.get_export_show_orientation_marker()); ui_->show_color_scale->setChecked(prefs_.get_export_show_color_scale()); ui_->pca_num_images->setValue(prefs_.get_export_num_pca_images()); @@ -64,7 +95,15 @@ ExportImageDialog::ExportImageDialog(QWidget* parent, Preferences& prefs, QShare }; connect(ui_->override_width, &QLineEdit::textChanged, start_timer); connect(ui_->override_height, &QLineEdit::textChanged, start_timer); + auto clear_size_message = [=]() { ui_->size_message->clear(); }; + connect(ui_->override_width, &QLineEdit::textEdited, clear_size_message); + connect(ui_->override_height, &QLineEdit::textEdited, clear_size_message); connect(ui_->override_window_size, &QCheckBox::toggled, this, &ExportImageDialog::update_preview); + connect(ui_->override_window_size, &QCheckBox::toggled, ui_->window_size_widget, &QWidget::setEnabled); + connect(ui_->preset_1x, &QPushButton::clicked, this, [=]() { apply_size_preset(1); }); + connect(ui_->preset_2x, &QPushButton::clicked, this, [=]() { apply_size_preset(2); }); + connect(ui_->preset_4x, &QPushButton::clicked, this, [=]() { apply_size_preset(4); }); + connect(ui_->preset_8x, &QPushButton::clicked, this, [=]() { apply_size_preset(8); }); connect(ui_->transparent_background, &QCheckBox::toggled, this, &ExportImageDialog::update_preview); connect(ui_->show_corner_widget, &QCheckBox::toggled, this, &ExportImageDialog::update_preview); connect(ui_->show_color_scale, &QCheckBox::toggled, this, &ExportImageDialog::update_preview); @@ -93,7 +132,16 @@ void ExportImageDialog::export_clicked() { filename = filename + ".png"; // default PNG } prefs_.set_last_directory(QFileInfo(filename).absolutePath()); - if (pixmap_.save(filename)) { + + // the preview renders small, so render again here at the full requested size + QScopedValueRollback guard(updating_preview_, true); + bool all_ready = true; + QPixmap image = render_image(get_export_size(), all_ready); + if (image.isNull()) { + SW_ERROR("Unable to generate the image to export"); + return; + } + if (image.save(filename)) { SW_LOG("Saved: " + filename.toStdString()); } else { SW_ERROR("Error saving: " + filename.toStdString()); @@ -106,103 +154,206 @@ void ExportImageDialog::export_clicked() { } //--------------------------------------------------------------------------- -void ExportImageDialog::update_preview() { - QSize size(ui_->override_width->text().toInt(), ui_->override_height->text().toInt()); - if (size.width() < 1 || size.height() < 1) { - ui_->preview->setPixmap(QPixmap{}); - return; - } - prefs_.set_export_override_size(size); - prefs_.set_export_override_size_enabled(ui_->override_window_size->isChecked()); - prefs_.set_export_show_orientation_marker(ui_->show_corner_widget->isChecked()); - prefs_.set_export_show_color_scale(ui_->show_color_scale->isChecked()); - prefs_.set_export_num_pca_images(ui_->pca_num_images->value()); - prefs_.set_export_pca_range(ui_->pca_range->value()); - prefs_.set_export_pca_modes(ui_->pca_modes->text()); - int num_pca_steps = ui_->pca_num_images->value(); - double pca_range = ui_->pca_range->value(); +void ExportImageDialog::begin_progress(int maximum, QString message) { + ui_->progress_label->setText(message); + ui_->progress->setMaximum(maximum); + ui_->progress->setValue(0); + ui_->progress_widget->show(); + QApplication::setOverrideCursor(Qt::WaitCursor); + // last chance to paint before the render blocks the GUI thread; input stays excluded so a click + // cannot re-enter the render or close the dialog under it + QApplication::processEvents(QEventLoop::ExcludeUserInputEvents); +} - auto mode_list = get_modes(ui_->pca_modes->text()); +//--------------------------------------------------------------------------- +void ExportImageDialog::step_progress(int value) { + ui_->progress->setValue(value); + QApplication::processEvents(QEventLoop::ExcludeUserInputEvents); +} + +//--------------------------------------------------------------------------- +void ExportImageDialog::end_progress() { + QApplication::restoreOverrideCursor(); + ui_->progress_widget->hide(); +} - if (!prefs_.get_export_override_size_enabled()) { - size = visualizer_->get_render_size(); +//--------------------------------------------------------------------------- +QSize ExportImageDialog::get_export_size() { + if (!ui_->override_window_size->isChecked()) { + return visualizer_->get_render_size(); } + return clamp_export_size(QSize(ui_->override_width->text().toInt(), ui_->override_height->text().toInt())); +} - bool transparent = ui_->transparent_background->isChecked(); +//--------------------------------------------------------------------------- +void ExportImageDialog::apply_size_preset(int multiplier) { + QSize view_size = visualizer_->get_render_size(); + QSize size = clamp_export_size(QSize(view_size.width() * multiplier, view_size.height() * multiplier)); + + // set every field without signals, then render once, rather than once per field + QSignalBlocker block_override(ui_->override_window_size); + QSignalBlocker block_width(ui_->override_width); + QSignalBlocker block_height(ui_->override_height); + ui_->override_window_size->setChecked(true); + ui_->window_size_widget->setEnabled(true); + ui_->override_width->setText(QString::number(size.width())); + ui_->override_height->setText(QString::number(size.height())); + ui_->size_message->clear(); - bool all_ready = true; + update_preview(); +} + +//--------------------------------------------------------------------------- +QPixmap ExportImageDialog::render_image(QSize size, bool& all_ready) { + all_ready = true; + bool transparent = ui_->transparent_background->isChecked(); ColorSchemes color_schemes; ColorScheme colors = color_schemes[prefs_.get_color_scheme()]; - if (pca_mode_) { - int num_columns = 2 * num_pca_steps + 1; - int num_rows = mode_list.size(); - double increment = pca_range / num_pca_steps; - double margin = size.height() * 0.2; - double side_margin = size.width() * 0.2; - auto canvas = - QPixmap(size.width() * num_columns + side_margin, size.height() * num_rows + margin); // extra 20% for labels - - canvas.fill(colors.background_qcolor(transparent ? 0 : 255)); - - int y = 0; - for (int mode_idx = 0; mode_idx < mode_list.size(); mode_idx++) { - int mode = mode_list[mode_idx]; - int x = side_margin; - for (int step = -num_pca_steps; step <= num_pca_steps; step++) { - double pca_value = step * increment; - visualizer_->display_shape(analysis_tool_->get_mode_shape(mode, pca_value)); - bool ready = false; - - bool orientation_marker = ui_->show_corner_widget->isChecked() && step == num_pca_steps && mode_idx == 0; - bool color_scale = ui_->show_color_scale->isChecked() && step == num_pca_steps; - auto pixmap = visualizer_->export_to_pixmap(size, transparent, orientation_marker, color_scale, ready); - if (!ready) { - all_ready = false; - } + if (!pca_mode_) { + // a single render has no intermediate steps to report, so this is a busy indicator + begin_progress(0, "Generating image"); + auto pixmap = visualizer_->export_to_pixmap(size, transparent, ui_->show_corner_widget->isChecked(), + ui_->show_color_scale->isChecked(), all_ready); + end_progress(); + return pixmap; + } - QString text = QString::number(pca_value, 'g', 3) + " SD"; - if (step == 0) { - text = "Mean Shape"; - } + int num_pca_steps = ui_->pca_num_images->value(); + double pca_range = ui_->pca_range->value(); + auto mode_list = get_modes(ui_->pca_modes->text()); - QPainter painter(&canvas); - painter.drawPixmap(x, y, pixmap); - painter.setPen(colors.get_text_color()); - QFont font = painter.font(); - font.setPixelSize(margin * 0.75); - painter.setFont(font); + int num_columns = 2 * num_pca_steps + 1; + int num_rows = mode_list.size(); + double increment = pca_range / num_pca_steps; + double margin = size.height() * 0.2; + double side_margin = size.width() * 0.2; + + // extra 20% for labels + qint64 canvas_width = static_cast(size.width()) * num_columns + side_margin; + qint64 canvas_height = static_cast(size.height()) * num_rows + margin; + if (canvas_width * canvas_height > MAX_EXPORT_PIXELS) { + SW_ERROR("Image too large: reduce the export size, the number of modes, or the number of images"); + return QPixmap{}; + } - if (mode_idx == mode_list.size() - 1) { - QRect rect = QRect(QPoint(x, y + pixmap.height()), QPoint(x + pixmap.width(), y + pixmap.height() + margin)); - painter.drawText(rect, Qt::AlignCenter, text); - } + auto canvas = QPixmap(static_cast(canvas_width), static_cast(canvas_height)); + if (canvas.isNull()) { + SW_ERROR("Unable to allocate a {}x{} image", canvas_width, canvas_height); + return QPixmap{}; + } + canvas.fill(colors.background_qcolor(transparent ? 0 : 255)); + + begin_progress(num_rows * num_columns, "Generating images"); + int completed = 0; + + int y = 0; + for (int mode_idx = 0; mode_idx < mode_list.size(); mode_idx++) { + int mode = mode_list[mode_idx]; + int x = side_margin; + for (int step = -num_pca_steps; step <= num_pca_steps; step++) { + double pca_value = step * increment; + visualizer_->display_shape(analysis_tool_->get_mode_shape(mode, pca_value)); + bool ready = false; + + bool orientation_marker = ui_->show_corner_widget->isChecked() && step == num_pca_steps && mode_idx == 0; + bool color_scale = ui_->show_color_scale->isChecked() && step == num_pca_steps; + auto pixmap = visualizer_->export_to_pixmap(size, transparent, orientation_marker, color_scale, ready); + if (!ready) { + all_ready = false; + } + step_progress(++completed); - // draw rotated "mode x" string - QString mode_string = "Mode " + QString::number(mode + 1); - QPointF anchor(0, y + size.height()); - QRect rect = QRect(0, 0, pixmap.height(), side_margin); - drawRotatedText(painter, mode_string, anchor, -90, rect); + QString text = QString::number(pca_value, 'g', 3) + " SD"; + if (step == 0) { + text = "Mean Shape"; + } + + QPainter painter(&canvas); + painter.drawPixmap(x, y, pixmap); + painter.setPen(colors.get_text_color()); + QFont font = painter.font(); + font.setPixelSize(margin * 0.75); + painter.setFont(font); - x += size.width(); + if (mode_idx == mode_list.size() - 1) { + QRect rect = QRect(QPoint(x, y + pixmap.height()), QPoint(x + pixmap.width(), y + pixmap.height() + margin)); + painter.drawText(rect, Qt::AlignCenter, text); } - y += size.height(); + + // draw rotated "mode x" string + QString mode_string = "Mode " + QString::number(mode + 1); + QPointF anchor(0, y + size.height()); + QRect rect = QRect(0, 0, pixmap.height(), side_margin); + drawRotatedText(painter, mode_string, anchor, -90, rect); + + x += size.width(); } - pixmap_ = canvas; + y += size.height(); + } + end_progress(); + return canvas; +} - } else { - pixmap_ = visualizer_->export_to_pixmap(size, ui_->transparent_background->isChecked(), - ui_->show_corner_widget->isChecked(), ui_->show_color_scale->isChecked(), - all_ready); +//--------------------------------------------------------------------------- +void ExportImageDialog::update_preview() { + // processEvents() still delivers timer events, so the update timer can land back in here + if (updating_preview_) { + return; } + QScopedValueRollback guard(updating_preview_, true); + QSize typed_size(ui_->override_width->text().toInt(), ui_->override_height->text().toInt()); + if (typed_size.width() < 1 || typed_size.height() < 1) { + ui_->preview->setPixmap(QPixmap{}); + return; + } + QSize requested_size = clamp_export_size(typed_size); + if (requested_size != typed_size) { + // QIntValidator calls an over-range number Intermediate, so the fields accept it as you type + QSignalBlocker block_width(ui_->override_width); + QSignalBlocker block_height(ui_->override_height); + ui_->override_width->setText(QString::number(requested_size.width())); + ui_->override_height->setText(QString::number(requested_size.height())); + // shown here rather than logged: this dialog is modal, so a warning dialog would open behind it + ui_->size_message->setText( + QString("Limited to %1 x %2 (maximum 1 gigapixel)").arg(requested_size.width()).arg(requested_size.height())); + } + + prefs_.set_export_override_size_enabled(ui_->override_window_size->isChecked()); + prefs_.set_export_show_orientation_marker(ui_->show_corner_widget->isChecked()); + prefs_.set_export_show_color_scale(ui_->show_color_scale->isChecked()); + prefs_.set_export_num_pca_images(ui_->pca_num_images->value()); + prefs_.set_export_pca_range(ui_->pca_range->value()); + prefs_.set_export_pca_modes(ui_->pca_modes->text()); + + // The export renders at the full size; the preview only has to fill a label. + QSize preview_size = get_export_size(); + if (preview_size.width() > MAX_PREVIEW_DIMENSION || preview_size.height() > MAX_PREVIEW_DIMENSION) { + preview_size.scale(MAX_PREVIEW_DIMENSION, MAX_PREVIEW_DIMENSION, Qt::KeepAspectRatio); + } + + bool all_ready = true; + pixmap_ = render_image(preview_size, all_ready); + if (pixmap_.isNull()) { + end_progress(); + ui_->preview->setPixmap(QPixmap{}); + ui_->preview->setText("Unable to generate the preview. See the log for details."); + return; + } + + // a shape was still loading, so keep an indicator up and come back for another pass ui_->progress_widget->setVisible(!all_ready); if (!all_ready) { + ui_->progress_label->setText("Waiting for shapes"); + ui_->progress->setMaximum(0); update_preview_timer_.start(2000); } ui_->preview->setPixmap(pixmap_); + + prefs_.set_export_override_size(requested_size); } //--------------------------------------------------------------------------- diff --git a/Studio/Interface/ExportImageDialog.h b/Studio/Interface/ExportImageDialog.h index b9827a31053..8a2551910a3 100644 --- a/Studio/Interface/ExportImageDialog.h +++ b/Studio/Interface/ExportImageDialog.h @@ -33,6 +33,23 @@ class ExportImageDialog : public QDialog { private: void update_preview(); + //! Render the composed image; reduced size for the preview, full requested size for the export + QPixmap render_image(QSize size, bool& all_ready); + + //! The size the export will actually be rendered at + QSize get_export_size(); + + //! Set the export size to a multiple of the size the view is currently rendered at + void apply_size_preset(int multiplier); + + //! Show the progress widget and paint it before a render blocks the GUI thread; maximum 0 is busy + void begin_progress(int maximum, QString message); + + //! Advance the progress bar and repaint between renders + void step_progress(int value); + + void end_progress(); + void drawRotatedText(QPainter& painter, QString text, QPointF point, qreal angle, QRect rect); QVector get_modes(QString string); @@ -44,6 +61,8 @@ class ExportImageDialog : public QDialog { bool pca_mode_ = false; QTimer update_preview_timer_; QSharedPointer analysis_tool_; + //! guards update_preview() against re-entering itself via the repaint it processes events for + bool updating_preview_ = false; }; } // namespace shapeworks diff --git a/Studio/Interface/ExportImageDialog.ui b/Studio/Interface/ExportImageDialog.ui index af1ac000411..cd344a222fd 100644 --- a/Studio/Interface/ExportImageDialog.ui +++ b/Studio/Interface/ExportImageDialog.ui @@ -141,14 +141,20 @@ + + 0 + - + 0 0 + + 80 + 2048 @@ -167,19 +173,114 @@ - + 0 0 + + 80 + 2048 + + + + Qt::Horizontal + + + + 0 + 0 + + + + + + + + + 0 + + + + + Multiple of view: + + + + + + + 1x + + + false + + + + + + + 2x + + + false + + + + + + + 4x + + + false + + + + + + + 8x + + + false + + + + + + + Qt::Horizontal + + + + 0 + 0 + + + + + + + + + + + true + + + + + + @@ -258,7 +359,7 @@ ranges separated by commas - + Qt::Vertical @@ -345,6 +446,9 @@ ranges separated by commas Cancel + + false + @@ -352,6 +456,9 @@ ranges separated by commas Export + + true + diff --git a/Studio/Interface/ShapeWorksStudioApp.cpp b/Studio/Interface/ShapeWorksStudioApp.cpp index 3be4716e68d..5dab4ca472b 100644 --- a/Studio/Interface/ShapeWorksStudioApp.cpp +++ b/Studio/Interface/ShapeWorksStudioApp.cpp @@ -500,7 +500,7 @@ void ShapeWorksStudioApp::import_files(QStringList file_names) { // reload groom parameters because iso spacing depends on the images loaded groom_tool_->load_params(); } - } catch (std::runtime_error& e) { + } catch (std::exception& e) { handle_error(e.what()); } diff --git a/Studio/Utils/StudioUtils.cpp b/Studio/Utils/StudioUtils.cpp index a3707d77097..e31c42fde9f 100644 --- a/Studio/Utils/StudioUtils.cpp +++ b/Studio/Utils/StudioUtils.cpp @@ -55,7 +55,14 @@ QImage StudioUtils::vtk_image_to_qimage(vtkSmartPointer image_data int num_components = image_data->GetNumberOfScalarComponents(); QImage image(width, height, QImage::Format_ARGB32); - QRgb* rgb_ptr = reinterpret_cast(image.bits()) + width * (height - 1); + if (image.isNull()) { // the allocation can fail outright for a very large export + SW_ERROR("Unable to allocate a {}x{} image", width, height); + return QImage(); + } + + // qsizetype, not int: a tiled export can exceed the two gigapixels at which this index overflows + const qsizetype row_length = width; + QRgb* rgb_ptr = reinterpret_cast(image.bits()) + row_length * (height - 1); unsigned char* colors_ptr = reinterpret_cast(image_data->GetScalarPointer()); // Loop over the vtkImageData contents. @@ -70,7 +77,7 @@ QImage StudioUtils::vtk_image_to_qimage(vtkSmartPointer image_data colors_ptr += num_components; } - rgb_ptr -= width * 2; + rgb_ptr -= row_length * 2; } return image; diff --git a/Studio/Visualization/Visualizer.cpp b/Studio/Visualization/Visualizer.cpp index 24f66ef7f9a..ee646daa0f5 100644 --- a/Studio/Visualization/Visualizer.cpp +++ b/Studio/Visualization/Visualizer.cpp @@ -8,15 +8,19 @@ #include #include #include +#include #include #include #include +#include #include #include +// std #include #include #include +#include namespace shapeworks { @@ -616,7 +620,22 @@ QPixmap Visualizer::export_to_pixmap(QSize size, bool transparent_background, bo } }; + // Render in tiles within the GPU's maximum dimension: past it macOS aborts inside the Metal + // layer, uncatchably. vtkWindowToImageFilter stitches them, which lifts the export size limit. + int max_tile = vtkTextureObject::GetMaximumTextureSize(vtkOpenGLRenderWindow::SafeDownCast(render_window)); + if (max_tile < 1024) { // the context could not be queried; every GL 3.2 implementation clears this + max_tile = 2048; + } + max_tile = std::min(max_tile, 16384); + + int tile_scale = 1; + while (size.width() > max_tile * tile_scale || size.height() > max_tile * tile_scale) { + tile_scale++; + } + QSize tile_size((size.width() + tile_scale - 1) / tile_scale, (size.height() + tile_scale - 1) / tile_scale); + auto window_to_image_filter = vtkSmartPointer::New(); + window_to_image_filter->SetScale(tile_scale, tile_scale); int original_size[2]; original_size[0] = render_window->GetSize()[0]; @@ -657,13 +676,18 @@ QPixmap Visualizer::export_to_pixmap(QSize size, bool transparent_background, bo renderer = collection->GetNextItem(); } - off_render_window->SetSize(size.width(), size.height()); + off_render_window->SetSize(tile_size.width(), tile_size.height()); off_render_window->Modified(); off_render_window->Render(); window_to_image_filter->Update(); auto qimage = StudioUtils::vtk_image_to_qimage(window_to_image_filter->GetOutput()); + // tiles cover at least the requested size; trim the rounding when it does not divide evenly + if (!qimage.isNull() && qimage.size() != size) { + qimage = qimage.copy(0, 0, size.width(), size.height()); + } + // set back to the original render window collection->InitTraversal(); renderer = collection->GetNextItem(); diff --git a/Studio/main.cpp b/Studio/main.cpp index 5947ebb94c8..50175efd46d 100644 --- a/Studio/main.cpp +++ b/Studio/main.cpp @@ -19,6 +19,7 @@ // vtk #include #include +#include #include #include #include @@ -83,6 +84,8 @@ int main(int argc, char** argv) { TIME_SCOPE("ShapeWorksStudio"); try { + ShapeWorksUtils::initialize_path_handling(); + #ifdef Q_OS_MACOS // Prevent cursor crashes on Apple Silicon qputenv("QT_MAC_DISABLE_NATIVE_CURSORS", "1"); diff --git a/Testing/CMakeLists.txt b/Testing/CMakeLists.txt index 58dfb2fe165..7581bc6361a 100644 --- a/Testing/CMakeLists.txt +++ b/Testing/CMakeLists.txt @@ -55,6 +55,7 @@ target_include_directories(Testing PUBLIC target_link_libraries(Testing gtest gtest_main + Common ${Boost_LIBRARIES} ) set_target_properties(Testing PROPERTIES PUBLIC_HEADER diff --git a/Testing/MeshTests/MeshTests.cpp b/Testing/MeshTests/MeshTests.cpp index ca2b730a667..e06dae9f761 100644 --- a/Testing/MeshTests/MeshTests.cpp +++ b/Testing/MeshTests/MeshTests.cpp @@ -1,6 +1,9 @@ +#include #include #include #include +#include +#include #include #include @@ -11,6 +14,7 @@ #include "MeshWarper.h" #include "ParticleSystem.h" #include "ParticleSystemEvaluation.h" +#include "ShapeworksUtils.h" #include "Testing.h" using namespace shapeworks; @@ -352,6 +356,16 @@ TEST(MeshTests, remeshPercentTest2) { ASSERT_TRUE(femur == ground_truth); } +// Remeshing an open (clipped) mesh should hit the requested vertex count instead of piling extra +// vertices along the open boundary (#2672) +TEST(MeshTests, remeshPercentOpenMeshTest) { + Mesh femur(std::string(TEST_DATA_DIR) + "/femur.vtk"); + femur.clip(makePlane(Point3({-91.0, 0.0, 1230.0}), makeVector({0.0, 0.0, 1.0}))); + auto num_points = femur.numPoints(); + femur.remeshPercent(0.40, 1.0); + ASSERT_NEAR(femur.numPoints(), num_points * 0.40, num_points * 0.02); +} + TEST(MeshTests, smoothTest1) { Mesh femur(std::string(TEST_DATA_DIR) + "/femur.vtk"); femur.smooth(10, 0.01); @@ -408,6 +422,31 @@ TEST(MeshTests, fillHolesTest) { ASSERT_TRUE(femur == ground_truth); } +TEST(MeshTests, cleanDuplicateFacesTest) { + // closed tetrahedron + Eigen::MatrixXd points(5, 3); + points << 0, 0, 0, 1, 0, 0, 0, 1, 0, 0, 0, 1, 0.5, -0.5, 0.2; + Eigen::MatrixXi tet(4, 3); + tet << 0, 2, 1, 0, 1, 3, 1, 2, 3, 0, 3, 2; + + // a doubled fin triangle hanging off edge (0,1) is removed entirely, along with its tip point + Eigen::MatrixXi fin(6, 3); + fin << tet, Eigen::RowVector3i(0, 1, 4), Eigen::RowVector3i(1, 4, 0); + Mesh fin_mesh(points, fin); + fin_mesh.clean(); + ASSERT_EQ(fin_mesh.numFaces(), 4); + ASSERT_EQ(fin_mesh.numPoints(), 4); + ASSERT_FALSE(fin_mesh.detectNonManifold()); + + // a duplicate of a real surface face keeps one copy rather than opening a hole + Eigen::MatrixXi dup(5, 3); + dup << tet, Eigen::RowVector3i(1, 0, 2); + Mesh dup_mesh(points.topRows(4), dup); + dup_mesh.clean(); + ASSERT_EQ(dup_mesh.numFaces(), 4); + ASSERT_EQ(dup_mesh.numPoints(), 4); +} + TEST(MeshTests, probeVolumeTest) { Mesh femur(std::string(TEST_DATA_DIR) + "/femur.vtk"); femur.probeVolume(Image(std::string(TEST_DATA_DIR) + "/femurVtkDT.nrrd")); @@ -997,6 +1036,28 @@ TEST(MeshTests, findReferenceMeshTest) { ASSERT_EQ(ref, 2); } +TEST(MeshTests, findReferenceMeshSubsetTest) { + // more meshes than the default subset size, with outliers at the lowest indices + const int num_meshes = 100; + const int num_outliers = 10; + std::vector meshes; + for (int i = 0; i < num_meshes; i++) { + auto sphere = vtkSmartPointer::New(); + sphere->SetRadius(i < num_outliers ? 100.0 + 10.0 * i : 10.0); + sphere->SetThetaResolution(16); + sphere->SetPhiResolution(16); + sphere->Update(); + meshes.push_back(Mesh(sphere->GetOutput())); + } + + // the reference must come from the compared subset, never an outlier + int ref = MeshUtils::findReferenceMesh(meshes); + ASSERT_GE(ref, num_outliers); + + ref = MeshUtils::findReferenceMesh(meshes, 20); + ASSERT_GE(ref, num_outliers); +} + TEST(MeshTests, addMesh) { Mesh mesh1(std::string(TEST_DATA_DIR) + "/sphere_00.ply"); Mesh mesh2(std::string(TEST_DATA_DIR) + "/sphere_00_translated.ply"); @@ -1039,6 +1100,121 @@ TEST(MeshTests, boundaryLoopExtractor) { ASSERT_TRUE(loop == ground_truth); } +// Returns a copy of the mesh with the faces around the given vertex removed, leaving a hole +static Mesh punch_hole(const Mesh& mesh, int center) { + Eigen::MatrixXd V = mesh.points(); + Eigen::MatrixXi F = mesh.faces(); + Eigen::MatrixXi F2(F.rows(), 3); + int count = 0; + for (int f = 0; f < F.rows(); f++) { + if (F(f, 0) != center && F(f, 1) != center && F(f, 2) != center) { + F2.row(count++) = F.row(f); + } + } + F2.conservativeResize(count, 3); + return Mesh(V, F2); +} + +static double max_edge_length(const Mesh& mesh) { + Eigen::MatrixXd V = mesh.points(); + Eigen::MatrixXi F = mesh.faces(); + double max_length = 0.0; + for (int f = 0; f < F.rows(); f++) { + for (int i = 0; i < 3; i++) { + max_length = std::max(max_length, (V.row(F(f, i)) - V.row(F(f, (i + 1) % 3))).norm()); + } + } + return max_length; +} + +// A mesh that already has a boundary loop (e.g. a bone cropped at the shaft) must not have that loop dragged +// onto the shared boundary (issue #2610) +TEST(MeshTests, sharedBoundaryExtractorMultipleLoops) { + Mesh left(std::string(TEST_DATA_DIR) + "/shared_boundary/00_l.vtk"); + Mesh right(std::string(TEST_DATA_DIR) + "/shared_boundary/00_r.vtk"); + double tol = 0.001; + + Eigen::MatrixXd V = left.points(); + Eigen::MatrixXi F = left.faces(); + Eigen::VectorXd sqrD; + Eigen::VectorXi I; + Eigen::MatrixXd C; + igl::point_mesh_squared_distance(V, right.points(), right.faces(), sqrD, I, C); + + // The lowest indexed vertex anywhere near the shared surface. A hole whose rim has a lower indexed vertex + // is returned by igl::boundary_loop ahead of the loop left by removing the shared surface. + int first_near = V.rows(); + for (int i = 0; i < V.rows(); i++) { + if (sqrD(i) <= tol) { + first_near = i; + break; + } + } + + // Find a face far from the shared surface with a vertex below first_near, and punch a hole next to it + int center = -1; + for (int f = 0; f < F.rows() && center < 0; f++) { + for (int i = 0; i < 3; i++) { + int rim = F(f, i); + int candidate = F(f, (i + 1) % 3); + if (rim < first_near && sqrD(rim) > 0.25 * sqrD.maxCoeff() && sqrD(candidate) > 0.25 * sqrD.maxCoeff()) { + center = candidate; + break; + } + } + } + ASSERT_GE(center, 0); + + Mesh left_open = punch_hole(left, center); + std::array output = MeshUtils::shared_boundary_extractor(left_open, right, tol); + + // snapping only moves vertices a short distance, so no edge should end up spanning the mesh + ASSERT_LT(max_edge_length(output[0]), 3.0 * max_edge_length(left_open)); +} + +// With more than one boundary loop, the longest should be extracted rather than an arbitrary one +TEST(MeshTests, boundaryLoopExtractorMultipleLoops) { + Mesh ground_truth(std::string(TEST_DATA_DIR) + "/shared_boundary/00_out_c.vtp"); + Mesh mesh(std::string(TEST_DATA_DIR) + "/shared_boundary/00_out_s.vtk"); + + // punch a hole in the interior of the mesh + Eigen::MatrixXd V; + Eigen::MatrixXi F; + std::vector> loops; + for (int center = 0; center < mesh.numPoints(); center++) { + Mesh holed = punch_hole(mesh, center); + V = holed.points(); + F = holed.faces(); + loops.clear(); + igl::boundary_loop(F, loops); + if (loops.size() == 2) { + break; + } + } + ASSERT_EQ(loops.size(), 2); + + // igl::boundary_loop returns the loop with the lowest indexed vertex first, so swap vertex 0 with a vertex on + // the rim of the hole to make sure the hole is returned ahead of the outer loop + const int rim = loops[0].size() < loops[1].size() ? loops[0][0] : loops[1][0]; + V.row(0).swap(V.row(rim)); + for (int f = 0; f < F.rows(); f++) { + for (int i = 0; i < 3; i++) { + if (F(f, i) == 0) { + F(f, i) = rim; + } else if (F(f, i) == rim) { + F(f, i) = 0; + } + } + } + loops.clear(); + igl::boundary_loop(F, loops); + ASSERT_EQ(loops.size(), 2); + ASSERT_LT(loops[0].size(), loops[1].size()); + + Mesh loop = MeshUtils::extract_boundary_loop(Mesh(V, F)); + ASSERT_TRUE(loop == ground_truth); +} + TEST(MeshTests, thicknessTest) { Mesh sphere(std::string(TEST_DATA_DIR) + "/thickness/sphere.vtk"); Image ct(std::string(TEST_DATA_DIR) + "/thickness/ct.nrrd"); @@ -1163,3 +1339,38 @@ TEST(MeshTests, extractLargestComponentSingleComponentTest) { ASSERT_EQ(femur.numPoints(), original_points); ASSERT_EQ(femur.numFaces(), original_faces); } + +TEST(MeshTests, nonAsciiPathTest) { + // Mesh's existence check has to agree with the reader underneath it about which paths are + // readable. On Windows boost::filesystem did not, and meshes that VTK opened without trouble + // were rejected as missing before the reader ever saw them. (#2648) + auto dir = TestUtils::Instance().get_output_dir("non_ascii_path_test"); + std::string unicode_dir = dir + "/Ünïcode Dätä"; + ASSERT_TRUE(vtksys::SystemTools::MakeDirectory(unicode_dir)); + + std::string path = unicode_dir + "/mesh.vtk"; + Mesh(std::string(TEST_DATA_DIR) + "/femur.vtk").write(path); + + ASSERT_TRUE(ShapeWorksUtils::file_exists(path)); + ASSERT_NO_THROW(Mesh mesh(path)); +} + +TEST(MeshTests, longPathTest) { + // Paths beyond MAX_PATH need the \\?\ prefix on Windows. The STL reader applies it; the + // existence check in front of it did not. (#2648) This uses .stl deliberately: VTK's legacy .vtk + // reader stats the file without the prefix first, so it can't open such a path on Windows at all. + auto dir = TestUtils::Instance().get_output_dir("long_path_test"); + + std::string deep = dir; + while (deep.size() < 300) { + deep += "/a_directory_with_a_deliberately_long_name"; + } + ASSERT_TRUE(vtksys::SystemTools::MakeDirectory(deep)); + + std::string path = deep + "/mesh.stl"; + ASSERT_GT(path.size(), 260u); + Mesh(std::string(TEST_DATA_DIR) + "/femur.vtk").write(path); + + ASSERT_TRUE(ShapeWorksUtils::file_exists(path)); + ASSERT_NO_THROW(Mesh mesh(path)); +} diff --git a/Testing/PythonTests/PythonTests.cpp b/Testing/PythonTests/PythonTests.cpp index b40bf85c865..0ed99cc41ea 100644 --- a/Testing/PythonTests/PythonTests.cpp +++ b/Testing/PythonTests/PythonTests.cpp @@ -258,6 +258,10 @@ TEST(pythonTests, fillholesTest) { run_test("fillHoles.py"); } +TEST(pythonTests, cleanTest) { + run_test("clean.py"); +} + TEST(pythonTests, probevolumeTest) { run_test("probeVolume.py"); } diff --git a/Testing/PythonTests/clean.py b/Testing/PythonTests/clean.py new file mode 100644 index 00000000000..5fe60fc266f --- /dev/null +++ b/Testing/PythonTests/clean.py @@ -0,0 +1,18 @@ +import sys +import numpy as np +from shapeworks import * + +success = True + +def cleanTest(): + # closed tetrahedron plus a doubled fin triangle hanging off edge (0,1) + points = np.array([[0, 0, 0], [1, 0, 0], [0, 1, 0], [0, 0, 1], [0.5, -0.5, 0.2]], dtype=np.float64) + faces = np.array([[0, 2, 1], [0, 1, 3], [1, 2, 3], [0, 3, 2], [0, 1, 4], [1, 4, 0]], dtype=np.int32) + mesh = Mesh(points, faces) + mesh.clean() + + return mesh.numFaces() == 4 and mesh.numPoints() == 4 + +success &= utils.test(cleanTest) + +sys.exit(not success) diff --git a/Testing/Testing.cpp b/Testing/Testing.cpp index 2e890c7906d..58437793eec 100644 --- a/Testing/Testing.cpp +++ b/Testing/Testing.cpp @@ -1,5 +1,7 @@ #include "Testing.h" +#include + #include #include @@ -31,7 +33,7 @@ TestUtils& TestUtils::Instance() { } //----------------------------------------------------------------------------- -TestUtils::TestUtils() {} +TestUtils::TestUtils() { ShapeWorksUtils::initialize_path_handling(); } //----------------------------------------------------------------------------- TestUtils::~TestUtils() { diff --git a/Testing/data/remeshPercent1.vtk b/Testing/data/remeshPercent1.vtk index e93b2c5d2c8..5a97443feb2 100644 --- a/Testing/data/remeshPercent1.vtk +++ b/Testing/data/remeshPercent1.vtk @@ -1,3 +1,3 @@ version https://git-lfs.github.com/spec/v1 -oid sha256:780e4de73c8b478a83d6a443be5358b600378618fb811b7be791a89e7887e5ca -size 231379 +oid sha256:8fdd1416339b792e7e6e4c1d9b95f31d19108a9c7e8e56bb6097f8be3fad0ebc +size 253904 diff --git a/Testing/data/remeshPercent2.vtk b/Testing/data/remeshPercent2.vtk index 66328557279..b055bdaf0cd 100644 --- a/Testing/data/remeshPercent2.vtk +++ b/Testing/data/remeshPercent2.vtk @@ -1,3 +1,3 @@ version https://git-lfs.github.com/spec/v1 -oid sha256:1f05141126a75d16e30c9281fdcc66d2dd6988fb9d4cede9b257681bdd285737 -size 95218 +oid sha256:4841cc2611eafbb6ec7445636b5f6725ef0d6b86a5208dabf372300b8fdf6780 +size 98795 diff --git a/docs/dev/release-process.md b/docs/dev/release-process.md index 282da35dfac..03fee2662a4 100644 --- a/docs/dev/release-process.md +++ b/docs/dev/release-process.md @@ -74,6 +74,16 @@ SW_MAJOR_VERSION=6.5 `master`'s `SHAPEWORKS_VERSION_STRING` (the next `-dev` version) over the release branch's when resolving +* Bump `CMakeLists.txt` on `master` to the next `-dev` version **before** the next + push to `master` + +This is not cosmetic. The docs version comes from `CMakeLists.txt`, so as long as `master` still +carries the version that was just released, every `master` build republishes its own docs on top of +the released version's — which is how the 6.7 docs ended up serving 6.8 release notes and +bundled-Python install instructions. `deploy_docs.sh` now refuses to publish `master` over +whichever version holds `latest`, so getting this wrong fails the docs deploy instead of corrupting +a shipped version. + * Point the documentation's `latest` version at the new release The docs site publishes one version per minor release, taken from `CMakeLists.txt`. Builds of @@ -82,7 +92,13 @@ The docs site publishes one version per minor release, taken from `CMakeLists.tx version dropdown. The `latest` alias, which is what the site redirects to, has to be moved to the released version by hand: run the **Mac Arm64 Build** workflow manually with the release tag selected as the ref, `deploy_docs` checked, and `docs_alias` set to `latest`. That rebuilds the -release's docs from its tag and moves `latest` onto it. +release's docs from its tag, moves `latest` onto it, titles it ` (latest)` in the version +dropdown, and drops the `(latest)` suffix from the version that previously held the alias. + +Everything published for a version — the release notes and guides, but also the API reference and +`ShapeWorksCommands.md`, which are generated by running the binaries that were just built — comes +from the ref the workflow runs on. Rebuild a version from its own tag or release branch, never +from `master`, or the version gets another release's API docs. * Update `version.json` on the web server diff --git a/python_requirements.txt b/python_requirements.txt index 53e5c03b7df..61e919d24bc 100644 --- a/python_requirements.txt +++ b/python_requirements.txt @@ -8,7 +8,7 @@ annotated-types==0.7.0 # via # -r requirements.in # pydantic -anyio==4.10.0 +anyio==4.14.2 # via # -r requirements.in # httpx @@ -367,7 +367,7 @@ jupyter-server-terminals==0.5.3 # via # -r requirements.in # jupyter-server -jupyterlab==4.5.10 +jupyterlab==4.5.11 # via notebook jupyterlab-pygments==0.3.0 # via @@ -455,7 +455,7 @@ mergedeep==1.3.4 # mkdocs-get-deps mike==2.1.3 # via -r requirements.in -mistune==3.3.0 +mistune==3.3.3 # via # -r requirements.in # nbconvert @@ -613,7 +613,7 @@ pexpect==4.9.0 # ipython pickleshare==0.7.5 # via -r requirements.in -pillow==12.3.0 +pillow==13.0.0 # via # -r requirements.in # bokeh @@ -806,7 +806,7 @@ sniffio==1.3.1 # via # -r requirements.in # anyio -soupsieve==2.8.4 +soupsieve==2.9 # via # -r requirements.in # beautifulsoup4 @@ -839,7 +839,7 @@ toml==0.10.2 # via # -r requirements.in # swcc -tornado==6.5.7 +tornado==6.5.9 # via # -r requirements.in # bokeh @@ -899,7 +899,7 @@ uri-template==1.3.0 # via # -r requirements.in # jsonschema -urllib3==2.7.0 +urllib3==2.8.0 # via # -r requirements.in # requests @@ -932,7 +932,7 @@ websocket-client==1.8.0 # via # -r requirements.in # jupyter-server -werkzeug==3.1.6 +werkzeug==3.1.9 # via # -r requirements.in # flask diff --git a/python_requirements_bundled.txt b/python_requirements_bundled.txt index a0edb22cadf..1c3d6695a92 100644 --- a/python_requirements_bundled.txt +++ b/python_requirements_bundled.txt @@ -65,7 +65,7 @@ lxml==6.0.1 markupsafe==3.0.2 matplotlib==3.10.6 mbpls==1.0.4 -mistune==3.3.0 +mistune==3.3.3 narwhals==2.5.0 numpy==2.3.3 openpyxl==3.1.5 @@ -75,7 +75,7 @@ pandas==2.3.2 pandocfilters==1.5.1 param==2.2.1 patsy==1.0.1 -pillow==12.3.0 +pillow==13.0.0 platformdirs==4.4.0 pooch==1.8.2 prometheus-client==0.22.1 @@ -83,7 +83,7 @@ pycparser==2.23 pyct==0.5.0 pydantic==2.11.9 pydantic-core==2.33.2 -pygments==2.19.2 +pygments==2.20.0 pyparsing==3.2.4 pyrsistent==0.20.0 python-dateutil==2.9.0.post0 @@ -93,7 +93,7 @@ pyvista==0.46.3 pyxdg==0.28 pyyaml==6.0.2 referencing==0.36.2 -requests==2.32.5 +requests==2.33.0 requests-toolbelt==1.0.0 rfc3339-validator==0.1.4 rfc3986-validator==0.1.1 @@ -108,7 +108,7 @@ shapely==2.1.1 simpleitk==2.5.2 six==1.17.0 sniffio==1.3.1 -soupsieve==2.8.4 +soupsieve==2.9 spm1d==0.4.53 statsmodels==0.14.5 swcc==1.1.0 @@ -123,7 +123,7 @@ typing-extensions==4.15.0 typing-inspection==0.4.1 tzdata==2025.2 uri-template==1.3.0 -urllib3==2.6.3 +urllib3==2.8.0 vtk==9.5.1 webcolors==24.11.1 webencodings==0.5.1