From 04bc3f155e6400f04b117523d21dad97b9bb5b37 Mon Sep 17 00:00:00 2001 From: Jesse Yurkovich Date: Tue, 3 Jun 2025 19:26:34 +0200 Subject: [PATCH] Fix #137662: Auto-validate meshes during USD import if we detect bad faces A small number of USD files in the wild contain invalid face index data for some of their meshes. This leads to asserts in debug builds and crashes for users in retail builds(sometimes). There is already an import option to Validate Meshes but it turns out that we, and most other importers, perform validation too late. We crash before getting to that validate option (see notes). This PR implements a cheap detection mechanism and will auto-fix if we detect broken data. The detection may not find all types of bad data but it will detect what is known to fail today for duplicate vertex indices. We immediately validate/fix before loading in the rest of the data. The downside is that this will mean no additional data will be loaded. Normals, edge creases, velocities, UVs, and all other attributes will be lost because the incoming data arrays will no longer align. It should be noted also that Alembic has also chosen this approach. It's check is significantly weaker though and can be improved separately if needed. If auto-fix is triggered, it will typically appear as one trace on the terminal. ``` WARN (io.usd): \io\usd\intern\usd_reader_mesh.cc:684 read_mesh_sample: Invalid face data detected for mesh '/degenerate/m_degenerate'. Automatic correction will be used. ``` A more general downside of these fixes is that this applies to each frame of animated mesh data. The mesh will be fixed, and re-fixed, on every frame update when the frame in question contains bad data. For well-behaved USD scenes, the penalty for this check is between 2-4%. For broken USD scenes, it depends on how many meshes need the fixup. In the case of the Intel 4004 Moore Lane scene, the penalty is a 2.7x slowdown in import time (4.5 s to 12.5 s). Pull Request: https://projects.blender.org/blender/blender/pulls/138633 --- .../blender/io/usd/intern/usd_reader_mesh.cc | 49 ++++++++++++++++++- .../blender/io/usd/intern/usd_reader_mesh.hh | 2 +- tests/python/bl_usd_import_test.py | 4 +- 3 files changed, 50 insertions(+), 5 deletions(-) diff --git a/source/blender/io/usd/intern/usd_reader_mesh.cc b/source/blender/io/usd/intern/usd_reader_mesh.cc index 38028c1ff71..3984457fc3b 100644 --- a/source/blender/io/usd/intern/usd_reader_mesh.cc +++ b/source/blender/io/usd/intern/usd_reader_mesh.cc @@ -28,9 +28,13 @@ #include "BLI_map.hh" #include "BLI_math_vector_types.hh" #include "BLI_ordered_edge.hh" +#include "BLI_set.hh" #include "BLI_span.hh" +#include "BLI_task.hh" #include "BLI_vector_set.hh" +#include "BLT_translation.hh" + #include "DNA_customdata_types.h" #include "DNA_material_types.h" #include "DNA_modifier_types.h" @@ -47,6 +51,8 @@ #include #include +#include + #include #include "CLG_log.h" @@ -253,7 +259,7 @@ bool USDMeshReader::topology_changed(const Mesh *existing_mesh, const double mot face_indices_.size() != existing_mesh->corners_num; } -void USDMeshReader::read_mpolys(Mesh *mesh) const +bool USDMeshReader::read_faces(Mesh *mesh) const { MutableSpan face_offsets = mesh->face_offsets_for_write(); MutableSpan corner_verts = mesh->corner_verts_for_write(); @@ -281,7 +287,44 @@ void USDMeshReader::read_mpolys(Mesh *mesh) const } } + /* Check for faces with duplicate vertex indices. These will require a mesh validate to fix. */ + const OffsetIndices faces = mesh->faces(); + const bool all_faces_ok = threading::parallel_reduce( + faces.index_range(), + 1024, + true, + [&](const IndexRange part, const bool ok_so_far) { + bool current_faces_ok = ok_so_far; + if (ok_so_far) { + for (const int i : part) { + const IndexRange face_range = faces[i]; + const Set used_verts(corner_verts.slice(face_range)); + current_faces_ok = current_faces_ok && used_verts.size() == face_range.size(); + } + } + return current_faces_ok; + }, + std::logical_and<>()); + + /* If we detect bad faces it would be unsafe to continue beyond this point without first + * performing a destructive validate. Any operation requiring mesh connectivity information can + * assert or crash if the problem isn't addressed. Performing the check here, before most of the + * data has been loaded, unfortunately means any remaining data will be lost. */ + if (!all_faces_ok) { + if (is_initial_load_) { + const char *message = N_( + "Invalid face data detected for mesh '%s'. Automatic correction will be used, but some " + "data will most likely be lost"); + const std::string prim_path = this->prim_path().GetAsString(); + BKE_reportf(this->reports(), RPT_WARNING, message, prim_path.c_str()); + CLOG_WARN(&LOG, message, prim_path.c_str()); + } + BKE_mesh_validate(mesh, false, false); + } + bke::mesh_calc_edges(*mesh, false, false); + + return all_faces_ok; } void USDMeshReader::read_uv_data_primvar(Mesh *mesh, @@ -646,7 +689,9 @@ void USDMeshReader::read_mesh_sample(ImportSettings *settings, } if (new_mesh || (settings->read_flag & MOD_MESHSEQ_READ_POLY) != 0) { - read_mpolys(mesh); + if (!read_faces(mesh)) { + return; + } read_edge_creases(mesh, motionSampleTime); if (normal_interpolation_ == pxr::UsdGeomTokens->faceVarying) { diff --git a/source/blender/io/usd/intern/usd_reader_mesh.hh b/source/blender/io/usd/intern/usd_reader_mesh.hh index fbdffb8f508..df6d4673fe9 100644 --- a/source/blender/io/usd/intern/usd_reader_mesh.hh +++ b/source/blender/io/usd/intern/usd_reader_mesh.hh @@ -81,7 +81,7 @@ class USDMeshReader : public USDGeomReader { MutableSpan material_indices, blender::Map *r_mat_map); - void read_mpolys(Mesh *mesh) const; + bool read_faces(Mesh *mesh) const; void read_subdiv(); void read_vertex_creases(Mesh *mesh, double motionSampleTime); void read_edge_creases(Mesh *mesh, double motionSampleTime); diff --git a/tests/python/bl_usd_import_test.py b/tests/python/bl_usd_import_test.py index 2e53a0c1aef..3969f54e984 100644 --- a/tests/python/bl_usd_import_test.py +++ b/tests/python/bl_usd_import_test.py @@ -128,8 +128,8 @@ class USDImportTest(AbstractUSDTest): # Test topology counts. self.assertIn("m_degenerate", objects, "Scene does not contain object m_degenerate") mesh = objects["m_degenerate"].data - self.assertEqual(len(mesh.polygons), 2) - self.assertEqual(len(mesh.edges), 7) + self.assertEqual(len(mesh.polygons), 0) + self.assertEqual(len(mesh.edges), 0) self.assertEqual(len(mesh.vertices), 6) self.assertIn("m_triangles", objects, "Scene does not contain object m_triangles")