Page MenuHome

Fix T76514: Invalid geometry in Alembic crashes Blender
ClosedPublic

Authored by Sybren A. Stüvel (sybren) on May 12 2020, 1:24 PM.

Details

Summary

Even though T76514: Invalid geometry in Alembic crashes Blender is caused by invalid geometry, and thus a bug in SpeedTree, the software that created the Alembic file, I would like Blender not to crash on importing such a file. The error in the Alembic file consists of invalid mesh loops, where consecutive loops refer to the same vertex.

The BKE_mesh_validate() can actually correct these errors, so this patch focuses on two things:

  • Letting Blender survive the situation until the mesh is loaded, and
  • Detecting the error so that BKE_mesh_validate() can be called only when necessary. This ensures there is only a minimal impact on performance when loading actually valid data.

An alternative approach would be to detect the invalid loops while loading the Alembic data, and skipping the invalid faces. The current mesh reading code is already not the simplest, though, and adding the required conditions to it would also make things like the mesh datastructure allocation more complex. Adding one extra safety, one simple check, and then deferring to existing error checking+fixing code seemed like a better approach.

Diff Detail

Repository
rB Blender

Event Timeline

Sybren A. Stüvel (sybren) requested review of this revision.May 12 2020, 1:24 PM
Sybren A. Stüvel (sybren) created this revision.
source/blender/blenlib/intern/edgehash.c
199 ↗(On Diff #24669)

I'm not really a fan of adding this check into this core code path. This function is quite hot / executed often. While branch prediction should make this branch very cheap, unnecessary branches should be avoided anyway.

How many API calls depend on this behavior? If it's just one during mesh validation, I'd prefer to move this check there. Different users of the API might have to handle this case differently anyway.

We don't allow v0 == v1 in init_edge to detect invalid API usage. I think it's bad to allow v0 == v1 in some API functions, but not in others.

  • Moved edge validity check to BKE_mesh_calc_edges()
Sybren A. Stüvel (sybren) marked an inline comment as done.May 12 2020, 1:41 PM
This revision is now accepted and ready to land.May 12 2020, 1:45 PM