IO: enable mesh validation by default for STL, PLY & OBJ importers
When moving importers from Python to C++ validation was removed (from PLY) & disabled by default (STL & OBJ) which re-introduces crashes reported by users such as #31835. Enable validation by default because crashes caused by imported data should be avoided, especially since the crashes may happen later when users enter edit-mode or run certain editing operations. This does slow down importing, from testing a 236MB .OBJ it takes around twice as long to import (~1.5 to ~3 seconds). Although validation can be optimized to reduce the overhead ad well as run in parallel for importers that load multiple objects. The defaults for USD and Alembic remain unchanged since this was never enabled by default, although we could consider enabling these as well.
This commit is contained in:
@@ -697,11 +697,13 @@ void WM_OT_alembic_import(wmOperatorType *ot)
|
||||
"Set Frame Range",
|
||||
"If checked, update scene's start and end frame to match those of the Alembic archive");
|
||||
|
||||
RNA_def_boolean(ot->srna,
|
||||
"validate_meshes",
|
||||
false,
|
||||
"Validate Meshes",
|
||||
"Check imported mesh objects for invalid data (slow)");
|
||||
RNA_def_boolean(
|
||||
ot->srna,
|
||||
"validate_meshes",
|
||||
false,
|
||||
"Validate Meshes",
|
||||
"Ensure the data is valid "
|
||||
"(when disabled, data may be imported which causes crashes displaying or editing)");
|
||||
|
||||
RNA_def_boolean(ot->srna,
|
||||
"always_add_cache_reader",
|
||||
|
||||
@@ -537,11 +537,13 @@ void WM_OT_obj_import(wmOperatorType *ot)
|
||||
false,
|
||||
"Vertex Groups",
|
||||
"Import OBJ groups as vertex groups");
|
||||
RNA_def_boolean(ot->srna,
|
||||
"validate_meshes",
|
||||
false,
|
||||
"Validate Meshes",
|
||||
"Check imported mesh objects for invalid data (slow)");
|
||||
RNA_def_boolean(
|
||||
ot->srna,
|
||||
"validate_meshes",
|
||||
true,
|
||||
"Validate Meshes",
|
||||
"Ensure the data is valid "
|
||||
"(when disabled, data may be imported which causes crashes displaying or editing)");
|
||||
|
||||
RNA_def_string(ot->srna,
|
||||
"collection_separator",
|
||||
|
||||
@@ -291,11 +291,14 @@ void WM_OT_stl_import(wmOperatorType *ot)
|
||||
"Use (import) facet normals (note that this will still give flat shading)");
|
||||
RNA_def_enum(ot->srna, "forward_axis", io_transform_axis, IO_AXIS_Y, "Forward Axis", "");
|
||||
RNA_def_enum(ot->srna, "up_axis", io_transform_axis, IO_AXIS_Z, "Up Axis", "");
|
||||
RNA_def_boolean(ot->srna,
|
||||
"use_mesh_validate",
|
||||
false,
|
||||
"Validate Mesh",
|
||||
"Validate and correct imported mesh (slow)");
|
||||
|
||||
RNA_def_boolean(
|
||||
ot->srna,
|
||||
"use_mesh_validate",
|
||||
true,
|
||||
"Validate Mesh",
|
||||
"Ensure the data is valid "
|
||||
"(when disabled, data may be imported which causes crashes displaying or editing)");
|
||||
|
||||
/* Only show .stl files by default. */
|
||||
prop = RNA_def_string(ot->srna, "filter_glob", "*.stl", 0, "Extension Filter", "");
|
||||
|
||||
@@ -884,11 +884,13 @@ void WM_OT_usd_import(wmOperatorType *ot)
|
||||
"Custom Properties",
|
||||
"Behavior when importing USD attributes as Blender custom properties");
|
||||
|
||||
RNA_def_boolean(ot->srna,
|
||||
"validate_meshes",
|
||||
false,
|
||||
"Validate Meshes",
|
||||
"Check imported mesh objects for invalid data (slow)");
|
||||
RNA_def_boolean(
|
||||
ot->srna,
|
||||
"validate_meshes",
|
||||
false,
|
||||
"Validate Meshes",
|
||||
"Ensure the data is valid "
|
||||
"(when disabled, data may be imported which causes crashes displaying or editing)");
|
||||
}
|
||||
|
||||
namespace blender::ed::io {
|
||||
|
||||
@@ -99,16 +99,14 @@ Mesh *convert_ply_to_mesh(PlyData &data, const PLYImportParams ¶ms)
|
||||
uv_map.finish();
|
||||
}
|
||||
|
||||
/* Calculate edges from the rest of the mesh. */
|
||||
bke::mesh_calc_edges(*mesh, true, false);
|
||||
|
||||
/* If we have custom vertex normals, set them (note: important to do this
|
||||
* after initializing the loops). */
|
||||
bool set_custom_normals_for_verts = false;
|
||||
if (!data.vertex_normals.is_empty()) {
|
||||
if (!data.face_sizes.is_empty()) {
|
||||
/* For a non-point-cloud mesh, set custom normals. */
|
||||
BKE_mesh_set_custom_normals_from_verts(
|
||||
mesh, reinterpret_cast<float(*)[3]>(data.vertex_normals.data()));
|
||||
/* Deferred because this relies on valid mesh data. */
|
||||
set_custom_normals_for_verts = true;
|
||||
}
|
||||
else if (params.import_attributes) {
|
||||
/* If we have no faces, add vertex normals as custom attribute. */
|
||||
@@ -132,6 +130,23 @@ Mesh *convert_ply_to_mesh(PlyData &data, const PLYImportParams ¶ms)
|
||||
}
|
||||
}
|
||||
|
||||
/* It's important to validate the mesh before using it's geometry to calculate derived data. */
|
||||
{
|
||||
/* Calculate edges from the rest of the mesh (this could be merged with validate). */
|
||||
bke::mesh_calc_edges(*mesh, true, false);
|
||||
|
||||
bool verbose_validate = false;
|
||||
#ifndef NDEBUG
|
||||
verbose_validate = true;
|
||||
#endif
|
||||
BKE_mesh_validate(mesh, verbose_validate, false);
|
||||
}
|
||||
|
||||
if (set_custom_normals_for_verts) {
|
||||
BKE_mesh_set_custom_normals_from_verts(
|
||||
mesh, reinterpret_cast<float(*)[3]>(data.vertex_normals.data()));
|
||||
}
|
||||
|
||||
/* Merge all vertices on the same location. */
|
||||
if (params.merge_verts) {
|
||||
std::optional<Mesh *> merged_mesh = blender::geometry::mesh_merge_by_distance_all(
|
||||
|
||||
@@ -77,7 +77,7 @@ struct OBJImportParams {
|
||||
bool use_split_objects = true;
|
||||
bool use_split_groups = false;
|
||||
bool import_vertex_groups = false;
|
||||
bool validate_meshes = false;
|
||||
bool validate_meshes = true;
|
||||
bool relative_paths = true;
|
||||
bool clear_selection = true;
|
||||
|
||||
|
||||
Reference in New Issue
Block a user