Fix T49814: Collada Import Ignores Vertex Normals

We now import and apply custom normals using a similar strategy
to the STL importer. We store custom normal data for each loop
as we read each MPoly and then apply it to the mesh after
`BKE_mesh_calc_edges()` is called.

The new behavior is optional and may be disabled in the Collada import
UI. When disabled, we use the old behavior of only using normals
to determine whether or not to smooth shade an MPoly.

----

Patch as requested in {T49814}.

The Collada import UI now has an additional checkbox, similar to the glTF and FBX import UIs:

{F13428264}

Here is a test Collada file with a simple test cube with flipped custom normals:

{F13428260}

{F13428282}

And a sphere where the two halves are disconnected geometry, but has custom normals that make the halves appear to be connected:

{F13436363}

{F13436368}

I've tested it on a number of my own meshes, and the custom normals appear to be imported
correctly. I'm not too sure about how I've plumbed the option down, though, or whether this
is the most proper way to apply custom normals.

Reviewed By: gaiaclary

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

source/blender/editors/io/io_collada.c

@@ -693,6 +693,7 @@ static int wm_collada_import_exec(bContext *C, wmOperator *op)
   int min_chain_length;
 
   int keep_bind_info;
+  int custom_normals;
   ImportSettings import_settings;
 
   if (!RNA_struct_property_is_set_ex(op->ptr, "filepath", false)) {
@@ -702,6 +703,7 @@ static int wm_collada_import_exec(bContext *C, wmOperator *op)
 
   /* Options panel */
   import_units = RNA_boolean_get(op->ptr, "import_units");
+  custom_normals = RNA_boolean_get(op->ptr, "custom_normals");
   find_chains = RNA_boolean_get(op->ptr, "find_chains");
   auto_connect = RNA_boolean_get(op->ptr, "auto_connect");
   fix_orientation = RNA_boolean_get(op->ptr, "fix_orientation");
@@ -714,6 +716,7 @@ static int wm_collada_import_exec(bContext *C, wmOperator *op)
 
   import_settings.filepath = filename;
   import_settings.import_units = import_units != 0;
+  import_settings.custom_normals = custom_normals != 0;
   import_settings.auto_connect = auto_connect != 0;
   import_settings.find_chains = find_chains != 0;
   import_settings.fix_orientation = fix_orientation != 0;
@@ -741,6 +744,7 @@ static void uiCollada_importSettings(uiLayout *layout, PointerRNA *imfptr)
   uiItemL(box, IFACE_("Import Data Options"), ICON_MESH_DATA);
 
   uiItemR(box, imfptr, "import_units", 0, NULL, ICON_NONE);
+  uiItemR(box, imfptr, "custom_normals", 0, NULL, ICON_NONE);
 
   box = uiLayoutBox(layout);
   uiItemL(box, IFACE_("Armature Options"), ICON_ARMATURE_DATA);
@@ -791,6 +795,12 @@ void WM_OT_collada_import(wmOperatorType *ot)
                   "If disabled match import to Blender's current Unit settings, "
                   "otherwise use the settings from the Imported scene");
 
+  RNA_def_boolean(ot->srna,
+                  "custom_normals",
+                  1,
+                  "Custom Normals",
+                  "Import custom normals, if available (otherwise Blender will compute them)");
+
   RNA_def_boolean(ot->srna,
                   "fix_orientation",
                   0,

source/blender/io/collada/DocumentImporter.cpp

@@ -90,8 +90,12 @@ DocumentImporter::DocumentImporter(bContext *C, const ImportSettings *import_set
                         CTX_data_scene(C),
                         view_layer,
                         import_settings),
-      mesh_importer(
-          &unit_converter, &armature_importer, CTX_data_main(C), CTX_data_scene(C), view_layer),
+      mesh_importer(&unit_converter,
+                    import_settings->custom_normals,
+                    &armature_importer,
+                    CTX_data_main(C),
+                    CTX_data_scene(C),
+                    view_layer),
       anim_importer(C, &unit_converter, &armature_importer, CTX_data_scene(C))
 {
 }

source/blender/io/collada/ImportSettings.h

@@ -8,6 +8,7 @@
 
 typedef struct ImportSettings {
   bool import_units;
+  bool custom_normals;
   bool find_chains;
   bool auto_connect;
   bool fix_orientation;

source/blender/io/collada/MeshImporter.cpp

@@ -191,9 +191,14 @@ void VCOLDataWrapper::get_vcol(int v_index, MLoopCol *mloopcol)
   }
 }
 
-MeshImporter::MeshImporter(
-    UnitConverter *unitconv, ArmatureImporter *arm, Main *bmain, Scene *sce, ViewLayer *view_layer)
+MeshImporter::MeshImporter(UnitConverter *unitconv,
+                           bool use_custom_normals,
+                           ArmatureImporter *arm,
+                           Main *bmain,
+                           Scene *sce,
+                           ViewLayer *view_layer)
     : unitconverter(unitconv),
+      use_custom_normals(use_custom_normals),
       m_bmain(bmain),
       scene(sce),
       view_layer(view_layer),
@@ -597,7 +602,9 @@ void MeshImporter::read_lines(COLLADAFW::Mesh *mesh, Mesh *me)
   }
 }
 
-void MeshImporter::read_polys(COLLADAFW::Mesh *collada_mesh, Mesh *me)
+void MeshImporter::read_polys(COLLADAFW::Mesh *collada_mesh,
+                              Mesh *me,
+                              blender::Vector<blender::float3> &loop_normals)
 {
   unsigned int i;
 
@@ -640,7 +647,8 @@ void MeshImporter::read_polys(COLLADAFW::Mesh *collada_mesh, Mesh *me)
     /* If MeshPrimitive is TRIANGLE_FANS we split it into triangles
      * The first triangle-fan vertex will be the first vertex in every triangle
      * XXX The proper function of TRIANGLE_FANS is not tested!!!
-     * XXX In particular the handling of the normal_indices looks very wrong to me */
+     * XXX In particular the handling of the normal_indices is very wrong */
+    /* TODO: UV, vertex color and custom normal support */
     if (collada_meshtype == COLLADAFW::MeshPrimitive::TRIANGLE_FANS) {
       unsigned int grouped_vertex_count = mp->getGroupedVertexElementsCount();
       for (unsigned int group_index = 0; group_index < grouped_vertex_count; group_index++) {
@@ -727,9 +735,22 @@ void MeshImporter::read_polys(COLLADAFW::Mesh *collada_mesh, Mesh *me)
         }
 
         if (mp_has_normals) {
+          /* If it turns out that we have complete custom normals for each MPoly
+           * and we want to use custom normals, this will be overridden. */
           if (!is_flat_face(normal_indices, nor, vcount)) {
             mpoly->flag |= ME_SMOOTH;
           }
+
+          if (use_custom_normals) {
+            /* Store the custom normals for later application. */
+            float vert_normal[3];
+            unsigned int *cur_normal = normal_indices;
+            for (int k = 0; k < vcount; k++, cur_normal++) {
+              get_vector(vert_normal, nor, *cur_normal, 3);
+              normalize_v3(vert_normal);
+              loop_normals.append(vert_normal);
+            }
+          }
         }
 
         if (mp->hasColorIndices()) {
@@ -874,6 +895,16 @@ std::string *MeshImporter::get_geometry_name(const std::string &mesh_name)
   return nullptr;
 }
 
+static bool bc_has_out_of_bound_indices(Mesh *me)
+{
+  for (const MLoop &loop : me->loops()) {
+    if (loop.v >= me->totvert) {
+      return true;
+    }
+  }
+  return false;
+}
+
 /**
  * this function checks if both objects have the same
  * materials assigned to Object (in the same order)
@@ -1120,8 +1151,37 @@ bool MeshImporter::write_geometry(const COLLADAFW::Geometry *geom)
   this->mesh_geom_map[std::string(me->id.name)] = str_geom_id;
 
   read_vertices(mesh, me);
-  read_polys(mesh, me);
+
+  blender::Vector<blender::float3> loop_normals;
+  read_polys(mesh, me, loop_normals);
+
   BKE_mesh_calc_edges(me, false, false);
+
+  /* We must apply custom normals after edges have been calculated, because
+   * BKE_mesh_set_custom_normals()'s internals expect me->medge to be populated
+   * and for the MLoops to have correct edge indices. */
+  if (use_custom_normals && !loop_normals.is_empty()) {
+    /* BKE_mesh_set_custom_normals()'s internals also expect that each MLoop
+     * has a valid vertex index, which may not be the case due to the existing
+     * logic in read_polys(). This check isn't necessary in the no-custom-normals
+     * case because the invalid MLoops get stripped in a later step. */
+    if (bc_has_out_of_bound_indices(me)) {
+      fprintf(stderr, "Can't apply custom normals, encountered invalid loop vert indices!\n");
+    }
+    /* There may be a mismatch in lengths if one or more of the MeshPrimitives in
+     * the Geometry had missing or otherwise invalid normals. */
+    else if (me->totloop != loop_normals.size()) {
+      fprintf(stderr,
+              "Can't apply custom normals, me->totloop != loop_normals.size() (%d != %d)\n",
+              me->totloop,
+              (int)loop_normals.size());
+    }
+    else {
+      BKE_mesh_set_custom_normals(me, reinterpret_cast<float(*)[3]>(loop_normals.data()));
+      me->flag |= ME_AUTOSMOOTH;
+    }
+  }
+
   /* read_lines() must be called after the face edges have been generated.
    * Otherwise the loose edges will be silently deleted again. */
   read_lines(mesh, me);

source/blender/io/collada/MeshImporter.h

@@ -24,6 +24,7 @@
 #include "collada_utils.h"
 
 #include "BLI_edgehash.h"
+#include "BLI_math_vec_types.hh"
 
 #include "DNA_material_types.h"
 #include "DNA_mesh_types.h"
@@ -63,6 +64,7 @@ class VCOLDataWrapper {
 class MeshImporter : public MeshImporterBase {
  private:
   UnitConverter *unitconverter;
+  bool use_custom_normals;
 
   Main *m_bmain;
   Scene *scene;
@@ -156,7 +158,7 @@ class MeshImporter : public MeshImporterBase {
    *
    * TODO: import uv set names.
    */
-  void read_polys(COLLADAFW::Mesh *mesh, Mesh *me);
+  void read_polys(COLLADAFW::Mesh *mesh, Mesh *me, blender::Vector<blender::float3> &loop_normals);
   /**
    * Read all loose edges.
    * IMPORTANT: This function assumes that all edges from existing
@@ -179,6 +181,7 @@ class MeshImporter : public MeshImporterBase {
 
  public:
   MeshImporter(UnitConverter *unitconv,
+               bool use_custom_normals,
                ArmatureImporter *arm,
                Main *bmain,
                Scene *sce,

source/blender/io/collada/collada.cpp

@@ -29,6 +29,7 @@ static void print_import_header(ImportSettings &import_settings)
   fprintf(stderr, "+-- Collada Import parameters------\n");
   fprintf(stderr, "| input file      : %s\n", import_settings.filepath);
   fprintf(stderr, "| use units       : %s\n", (import_settings.import_units) ? "yes" : "no");
+  fprintf(stderr, "| custom normals  : %s\n", (import_settings.custom_normals) ? "yes" : "no");
   fprintf(stderr, "| autoconnect     : %s\n", (import_settings.auto_connect) ? "yes" : "no");
   fprintf(stderr, "+-- Armature Import parameters ----\n");
   fprintf(stderr, "| find bone chains: %s\n", (import_settings.find_chains) ? "yes" : "no");