Alembic: improved import/export of mesh normals

This commit implements the change in behaviour described in T71246.
In short:

For export, per mesh:
    - Custom loop normals are defined → loop normals are exported.
    - One or more polys are marked flat → loop normals are exported.
    - Otherwise, no normals are exported.

For import, when the Alembic mesh contains:
    - loop normals (kFacevaryingScope) → use as custom loop normals, and
      enble Auto Smooth to have Blender actually use them.
    - vertex normals (kVertexScope or kVaryingScope) → convert to loop
      normals, and handle as above.
    - no normals → mark mesh as smooth.
    - unsupported normal types (kConstantScope, kUniformScope,
      kUnknownScope) → handle as 'no normals'.

This also fixes T71130: Alembic split normal export issue

Previously the mesh flag `ME_AUTOSMOOTH` was used in conjunction with
the poly flag `ME_SMOOTH` to determine whether loop normals or vertex
normals were exported. This behaviour was hard to predict for artists,
and hard to describe in the manual. Instead, Blender now only exports
loop normals, computing them if necessary. This way, the mesh in Alembic
should always have the same loop normals as in Blender.

Maniphest Tasks: T71130

Differential Revision: https://developer.blender.org/D6197

2 files changed, 99 insertions, 135 deletions

diff --git a/source/blender/alembic/intern/abc_mesh.cc b/source/blender/alembic/intern/abc_mesh.cc
index 61e8771e845..89fafc65e5d 100644
--- a/source/blender/alembic/intern/abc_mesh.cc
+++ b/source/blender/alembic/intern/abc_mesh.cc
@@ -102,6 +102,7 @@ using Alembic::AbcGeom::kFacevaryingScope;
 using Alembic::AbcGeom::kVaryingScope;
 using Alembic::AbcGeom::kVertexScope;
 using Alembic::AbcGeom::kWrapExisting;
+using Alembic::AbcGeom::N3fArraySample;
 using Alembic::AbcGeom::N3fArraySamplePtr;
 using Alembic::AbcGeom::UInt32ArraySample;
 
@@ -124,12 +125,13 @@ static void get_vertices(struct Mesh *mesh, std::vector<Imath::V3f> &points)
 static void get_topology(struct Mesh *mesh,
                          std::vector<int32_t> &poly_verts,
                          std::vector<int32_t> &loop_counts,
-                         bool &r_export_loop_normals)
+                         bool &r_has_flat_shaded_poly)
 {
   const int num_poly = mesh->totpoly;
   const int num_loops = mesh->totloop;
   MLoop *mloop = mesh->mloop;
   MPoly *mpoly = mesh->mpoly;
+  r_has_flat_shaded_poly = false;
 
   poly_verts.clear();
   loop_counts.clear();
@@ -141,7 +143,7 @@ static void get_topology(struct Mesh *mesh,
     MPoly &poly = mpoly[i];
     loop_counts.push_back(poly.totloop);
 
-    r_export_loop_normals |= (poly.flag & ME_SMOOTH) != 0;
+    r_has_flat_shaded_poly |= (poly.flag & ME_SMOOTH) == 0;
 
     MLoop *loop = mloop + poly.loopstart + (poly.totloop - 1);
 
@@ -177,66 +179,31 @@ static void get_creases(struct Mesh *mesh,
   lengths.resize(sharpnesses.size(), 2);
 }
 
-static void get_vertex_normals(struct Mesh *mesh, std::vector<Imath::V3f> &normals)
+static void get_loop_normals(struct Mesh *mesh,
+                             std::vector<Imath::V3f> &normals,
+                             bool has_flat_shaded_poly)
 {
   normals.clear();
-  normals.resize(mesh->totvert);
 
-  MVert *verts = mesh->mvert;
-  float no[3];
-
-  for (int i = 0, e = mesh->totvert; i < e; i++) {
-    normal_short_to_float_v3(no, verts[i].no);
-    copy_yup_from_zup(normals[i].getValue(), no);
+  /* If all polygons are smooth shaded, and there are no custom normals, we don't need to export
+   * normals at all. This is also done by other software, see T71246. */
+  if (!has_flat_shaded_poly && !CustomData_has_layer(&mesh->ldata, CD_CUSTOMLOOPNORMAL)) {
+    return;
   }
-}
-
-static void get_loop_normals(struct Mesh *mesh, std::vector<Imath::V3f> &normals)
-{
-  MPoly *mp = mesh->mpoly;
-
-  MLoop *mloop = mesh->mloop;
-  MLoop *ml = mloop;
-
-  MVert *verts = mesh->mvert;
 
+  BKE_mesh_calc_normals_split(mesh);
   const float(*lnors)[3] = static_cast<float(*)[3]>(CustomData_get_layer(&mesh->ldata, CD_NORMAL));
+  BLI_assert(lnors != NULL || !"BKE_mesh_calc_normals_split() should have computed CD_NORMAL");
 
-  normals.clear();
   normals.resize(mesh->totloop);
 
   /* NOTE: data needs to be written in the reverse order. */
   int abc_index = 0;
-
-  if (lnors) {
-    for (int i = 0, e = mesh->totpoly; i < e; i++, mp++) {
-      for (int j = mp->totloop - 1; j >= 0; j--, abc_index++) {
-        int blender_index = mp->loopstart + j;
-        copy_yup_from_zup(normals[abc_index].getValue(), lnors[blender_index]);
-      }
-    }
-  }
-  else {
-    float no[3];
-
-    for (int i = 0, e = mesh->totpoly; i < e; i++, mp++) {
-      ml = mloop + mp->loopstart + (mp->totloop - 1);
-
-      /* Flat shaded, use common normal for all verts. */
-      if ((mp->flag & ME_SMOOTH) == 0) {
-        BKE_mesh_calc_poly_normal(mp, ml - (mp->totloop - 1), verts, no);
-
-        for (int j = 0; j < mp->totloop; ml--, j++, abc_index++) {
-          copy_yup_from_zup(normals[abc_index].getValue(), no);
-        }
-      }
-      else {
-        /* Smooth shaded, use individual vert normals. */
-        for (int j = 0; j < mp->totloop; ml--, j++, abc_index++) {
-          normal_short_to_float_v3(no, verts[ml->v].no);
-          copy_yup_from_zup(normals[abc_index].getValue(), no);
-        }
-      }
+  MPoly *mp = mesh->mpoly;
+  for (int i = 0, e = mesh->totpoly; i < e; i++, mp++) {
+    for (int j = mp->totloop - 1; j >= 0; j--, abc_index++) {
+      int blender_index = mp->loopstart + j;
+      copy_yup_from_zup(normals[abc_index].getValue(), lnors[blender_index]);
     }
   }
 }
@@ -409,11 +376,10 @@ void AbcGenericMeshWriter::writeMesh(struct Mesh *mesh)
   std::vector<Imath::V3f> points, normals;
   std::vector<int32_t> poly_verts, loop_counts;
   std::vector<Imath::V3f> velocities;
-
-  bool export_loop_normals = (mesh->flag & ME_AUTOSMOOTH) != 0;
+  bool has_flat_shaded_poly = false;
 
   get_vertices(mesh, points);
-  get_topology(mesh, poly_verts, loop_counts, export_loop_normals);
+  get_topology(mesh, poly_verts, loop_counts, has_flat_shaded_poly);
 
   if (m_first_frame && m_settings.export_face_sets) {
     writeFaceSets(mesh, m_mesh_schema);
@@ -441,16 +407,11 @@ void AbcGenericMeshWriter::writeMesh(struct Mesh *mesh)
   }
 
   if (m_settings.export_normals) {
-    if (export_loop_normals) {
-      get_loop_normals(mesh, normals);
-    }
-    else {
-      get_vertex_normals(mesh, normals);
-    }
+    get_loop_normals(mesh, normals, has_flat_shaded_poly);
 
     ON3fGeomParam::Sample normals_sample;
     if (!normals.empty()) {
-      normals_sample.setScope(export_loop_normals ? kFacevaryingScope : kVertexScope);
+      normals_sample.setScope(kFacevaryingScope);
       normals_sample.setVals(V3fArraySample(normals));
     }
 
@@ -475,11 +436,10 @@ void AbcGenericMeshWriter::writeSubD(struct Mesh *mesh)
   std::vector<Imath::V3f> points;
   std::vector<int32_t> poly_verts, loop_counts;
   std::vector<int32_t> crease_indices, crease_lengths;
-
-  bool export_loop_normals = false;
+  bool has_flat_poly = false;
 
   get_vertices(mesh, points);
-  get_topology(mesh, poly_verts, loop_counts, export_loop_normals);
+  get_topology(mesh, poly_verts, loop_counts, has_flat_poly);
   get_creases(mesh, crease_indices, crease_lengths, crease_sharpness);
 
   if (m_first_frame && m_settings.export_face_sets) {
@@ -756,10 +716,6 @@ struct AbcMeshData {
   P3fArraySamplePtr positions;
   P3fArraySamplePtr ceil_positions;
 
-  N3fArraySamplePtr vertex_normals;
-  N3fArraySamplePtr loop_normals;
-  bool poly_flag_smooth;
-
   V2fArraySamplePtr uvs;
   UInt32ArraySamplePtr uvs_indices;
 };
@@ -786,7 +742,6 @@ static void read_mverts(CDStreamConfig &config, const AbcMeshData &mesh_data)
 {
   MVert *mverts = config.mvert;
   const P3fArraySamplePtr &positions = mesh_data.positions;
-  const N3fArraySamplePtr &normals = mesh_data.vertex_normals;
 
   if (config.weight != 0.0f && mesh_data.ceil_positions != NULL &&
       mesh_data.ceil_positions->size() == positions->size()) {
@@ -794,12 +749,10 @@ static void read_mverts(CDStreamConfig &config, const AbcMeshData &mesh_data)
     return;
   }
 
-  read_mverts(mverts, positions, normals);
+  read_mverts(mverts, positions, nullptr);
 }
 
-void read_mverts(MVert *mverts,
-                 const P3fArraySamplePtr &positions,
-                 const N3fArraySamplePtr &normals)
+void read_mverts(MVert *mverts, const P3fArraySamplePtr positions, const N3fArraySamplePtr normals)
 {
   for (int i = 0; i < positions->size(); i++) {
     MVert &mvert = mverts[i];
@@ -846,12 +799,9 @@ static void read_mpolys(CDStreamConfig &config, const AbcMeshData &mesh_data)
     poly.loopstart = loop_index;
     poly.totloop = face_size;
 
-    if (mesh_data.poly_flag_smooth) {
-      poly.flag |= ME_SMOOTH;
-    }
-    else {
-      poly.flag &= ~ME_SMOOTH;
-    }
+    /* Polygons are always assumed to be smooth-shaded. If the Alembic mesh should be flat-shaded,
+     * this is encoded in custom loop normals. See T71246. */
+    poly.flag |= ME_SMOOTH;
 
     /* NOTE: Alembic data is stored in the reverse order. */
     rev_loop_index = loop_index + (face_size - 1);
@@ -879,26 +829,27 @@ static void read_mpolys(CDStreamConfig &config, const AbcMeshData &mesh_data)
   BKE_mesh_calc_edges(config.mesh, false, false);
 }
 
-static void process_normals(CDStreamConfig &config, const AbcMeshData &mesh_data)
+static void process_no_normals(CDStreamConfig &config)
 {
-  Mesh *mesh = config.mesh;
+  /* Absense of normals in the Alembic mesh is interpreted as 'smooth'. */
+  BKE_mesh_calc_normals(config.mesh);
+}
+
+static void process_loop_normals(CDStreamConfig &config, const N3fArraySamplePtr loop_normals_ptr)
+{
+  size_t loop_count = loop_normals_ptr->size();
 
-  if (!mesh_data.loop_normals) {
-    BKE_mesh_calc_normals(config.mesh);
-    /* Don't touch the ME_AUTOSMOOTH flag in this case. It can be used by artists to toggle between
-     * flat/smooth shaded when the Alembic mesh doesn't contain loop normals. */
+  if (loop_count == 0) {
+    process_no_normals(config);
     return;
   }
 
-  config.mesh->flag |= ME_AUTOSMOOTH;
-
-  const Alembic::AbcGeom::N3fArraySample &loop_normals = *mesh_data.loop_normals;
-  long int loop_count = loop_normals.size();
-
   float(*lnors)[3] = static_cast<float(*)[3]>(
       MEM_malloc_arrayN(loop_count, sizeof(float[3]), "ABC::FaceNormals"));
 
+  Mesh *mesh = config.mesh;
   MPoly *mpoly = mesh->mpoly;
+  const N3fArraySample &loop_normals = *loop_normals_ptr;
   int abc_index = 0;
   for (int i = 0, e = mesh->totpoly; i < e; i++, mpoly++) {
     /* As usual, ABC orders the loops in reverse. */
@@ -907,11 +858,63 @@ static void process_normals(CDStreamConfig &config, const AbcMeshData &mesh_data
       copy_zup_from_yup(lnors[blender_index], loop_normals[abc_index].getValue());
     }
   }
-  BKE_mesh_set_custom_normals(config.mesh, lnors);
+
+  mesh->flag |= ME_AUTOSMOOTH;
+  BKE_mesh_set_custom_normals(mesh, lnors);
 
   MEM_freeN(lnors);
 }
 
+static void process_vertex_normals(CDStreamConfig &config,
+                                   const N3fArraySamplePtr vertex_normals_ptr)
+{
+  size_t normals_count = vertex_normals_ptr->size();
+  if (normals_count == 0) {
+    process_no_normals(config);
+    return;
+  }
+
+  float(*vnors)[3] = static_cast<float(*)[3]>(
+      MEM_malloc_arrayN(normals_count, sizeof(float[3]), "ABC::VertexNormals"));
+
+  const N3fArraySample &vertex_normals = *vertex_normals_ptr;
+  for (int index = 0; index < normals_count; index++) {
+    copy_zup_from_yup(vnors[index], vertex_normals[index].getValue());
+  }
+
+  config.mesh->flag |= ME_AUTOSMOOTH;
+  BKE_mesh_set_custom_normals_from_vertices(config.mesh, vnors);
+  MEM_freeN(vnors);
+}
+
+static void process_normals(CDStreamConfig &config,
+                            const IN3fGeomParam &normals,
+                            const ISampleSelector &selector)
+{
+  if (!normals.valid()) {
+    process_no_normals(config);
+    return;
+  }
+
+  IN3fGeomParam::Sample normsamp = normals.getExpandedValue(selector);
+  Alembic::AbcGeom::GeometryScope scope = normals.getScope();
+
+  switch (scope) {
+    case Alembic::AbcGeom::kFacevaryingScope:  // 'Vertex Normals' in Houdini.
+      process_loop_normals(config, normsamp.getVals());
+      break;
+    case Alembic::AbcGeom::kVertexScope:
+    case Alembic::AbcGeom::kVaryingScope:  // 'Point Normals' in Houdini.
+      process_vertex_normals(config, normsamp.getVals());
+      break;
+    case Alembic::AbcGeom::kConstantScope:
+    case Alembic::AbcGeom::kUniformScope:
+    case Alembic::AbcGeom::kUnknownScope:
+      process_no_normals(config);
+      break;
+  }
+}
+
 ABC_INLINE void read_uvs_params(CDStreamConfig &config,
                                 AbcMeshData &abc_data,
                                 const IV2fGeomParam &uv,
@@ -942,34 +945,6 @@ ABC_INLINE void read_uvs_params(CDStreamConfig &config,
   }
 }
 
-ABC_INLINE void read_normals_params(AbcMeshData &abc_data,
-                                    const IN3fGeomParam &normals,
-                                    const ISampleSelector &selector)
-{
-  if (!normals.valid()) {
-    return;
-  }
-
-  IN3fGeomParam::Sample normsamp = normals.getExpandedValue(selector);
-
-  Alembic::AbcGeom::GeometryScope scope = normals.getScope();
-  switch (scope) {
-    case Alembic::AbcGeom::kFacevaryingScope:
-      abc_data.loop_normals = normsamp.getVals();
-      break;
-    case Alembic::AbcGeom::kVertexScope:
-    case Alembic::AbcGeom::kVaryingScope:
-      /* Vertex normals from ABC aren't handled for now. */
-      abc_data.poly_flag_smooth = true;
-      abc_data.vertex_normals = N3fArraySamplePtr();
-      break;
-    case Alembic::AbcGeom::kConstantScope:
-    case Alembic::AbcGeom::kUniformScope:
-    case Alembic::AbcGeom::kUnknownScope:
-      break;
-  }
-}
-
 static void *add_customdata_cb(Mesh *mesh, const char *name, int data_type)
 {
   CustomDataType cd_data_type = static_cast<CustomDataType>(data_type);
@@ -1020,12 +995,6 @@ static void read_mesh_sample(const std::string &iobject_full_name,
   abc_mesh_data.face_indices = sample.getFaceIndices();
   abc_mesh_data.positions = sample.getPositions();
 
-  /* The auto-smoothing flag can be used by artists when the Alembic file does not contain custom
-   * loop normals. Auto-smoothing only works when polys are marked as smooth. */
-  abc_mesh_data.poly_flag_smooth = (config.mesh->flag & ME_AUTOSMOOTH);
-
-  read_normals_params(abc_mesh_data, schema.getNormalsParam(), selector);
-
   get_weight_and_index(config, schema.getTimeSampling(), schema.getNumSamples());
 
   if (config.weight != 0.0f) {
@@ -1044,7 +1013,7 @@ static void read_mesh_sample(const std::string &iobject_full_name,
 
   if ((settings->read_flag & MOD_MESHSEQ_READ_POLY) != 0) {
     read_mpolys(config, abc_mesh_data);
-    process_normals(config, abc_mesh_data);
+    process_normals(config, schema.getNormalsParam(), selector);
   }
 
   if ((settings->read_flag & (MOD_MESHSEQ_READ_UV | MOD_MESHSEQ_READ_COLOR)) != 0) {
@@ -1316,8 +1285,6 @@ static void read_subd_sample(const std::string &iobject_full_name,
   AbcMeshData abc_mesh_data;
   abc_mesh_data.face_counts = sample.getFaceCounts();
   abc_mesh_data.face_indices = sample.getFaceIndices();
-  abc_mesh_data.vertex_normals = N3fArraySamplePtr();
-  abc_mesh_data.loop_normals = N3fArraySamplePtr();
   abc_mesh_data.positions = sample.getPositions();
 
   get_weight_and_index(config, schema.getTimeSampling(), schema.getNumSamples());
@@ -1339,12 +1306,9 @@ static void read_subd_sample(const std::string &iobject_full_name,
   if ((settings->read_flag & MOD_MESHSEQ_READ_POLY) != 0) {
     /* Alembic's 'SubD' scheme is used to store subdivision surfaces, i.e. the pre-subdivision
      * mesh. Currently we don't add a subdivision modifier when we load such data. This code is
-     * assuming that the subdivided surface should be smooth, and sets a flag that will eventually
-     * mark all polygons as such. */
-    abc_mesh_data.poly_flag_smooth = true;
-
+     * assuming that the subdivided surface should be smooth. */
     read_mpolys(config, abc_mesh_data);
-    process_normals(config, abc_mesh_data);
+    process_no_normals(config);
   }
 
   if ((settings->read_flag & (MOD_MESHSEQ_READ_UV | MOD_MESHSEQ_READ_COLOR)) != 0) {
diff --git a/source/blender/alembic/intern/abc_mesh.h b/source/blender/alembic/intern/abc_mesh.h
index c15fb948e03..38c589bbc32 100644
--- a/source/blender/alembic/intern/abc_mesh.h
+++ b/source/blender/alembic/intern/abc_mesh.h
@@ -148,8 +148,8 @@ class AbcSubDReader : public AbcObjectReader {
 /* ************************************************************************** */
 
 void read_mverts(MVert *mverts,
-                 const Alembic::AbcGeom::P3fArraySamplePtr &positions,
-                 const Alembic::AbcGeom::N3fArraySamplePtr &normals);
+                 const Alembic::AbcGeom::P3fArraySamplePtr positions,
+                 const Alembic::AbcGeom::N3fArraySamplePtr normals);
 
 CDStreamConfig get_config(struct Mesh *mesh);