[Bf-blender-cvs] [dcd909122f9] master: Scew Modifier: Remove eager normal calculation

Hans Goudey noreply at git.blender.org
Tue Sep 27 01:06:29 CEST 2022


Commit: dcd909122f9ad644dcc11e49f565e58dabe276e2
Author: Hans Goudey
Date:   Mon Sep 26 17:59:44 2022 -0500
Branches: master
https://developer.blender.org/rBdcd909122f9ad644dcc11e49f565e58dabe276e2

Scew Modifier: Remove eager normal calculation

The screw modifier calculated normals eagerly (whether or not the
next modifier actually used them). However, this was incorrect and
set invalid normals. It isn't necessary because they can be calculated
later anyway. The potential performance improvement isn't worth the
complexity or maintenance burden.

Fixes T101075

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

===================================================================

M	source/blender/modifiers/intern/MOD_screw.c

===================================================================

diff --git a/source/blender/modifiers/intern/MOD_screw.c b/source/blender/modifiers/intern/MOD_screw.c
index 71ffe91f364..91f8947bbc3 100644
--- a/source/blender/modifiers/intern/MOD_screw.c
+++ b/source/blender/modifiers/intern/MOD_screw.c
@@ -58,8 +58,6 @@ typedef struct ScrewVertConnect {
   float dist_sq;
   /** Location relative to the transformed axis. */
   float co[3];
-  /** Calc normal of the vertex. */
-  float no[3];
   /** 2 verts on either side of this one. */
   uint v[2];
   /** Edges on either side, a bit of a waste since each edge ref's 2 edges. */
@@ -381,9 +379,6 @@ static Mesh *modifyMesh(ModifierData *md, const ModifierEvalContext *ctx, Mesh *
 
   /* The `screw_ofs` cannot change from now on. */
   const bool do_remove_doubles = (ltmd->flag & MOD_SCREW_MERGE) && (screw_ofs == 0.0f);
-  /* Only calculate normals if `do_remove_doubles` since removing doubles frees the normals. */
-  const bool do_normal_create = (ltmd->flag & MOD_SCREW_NORMAL_CALC) &&
-                                (do_remove_doubles == false);
 
   result = BKE_mesh_new_nomain_from_template(
       mesh, (int)maxVerts, (int)maxEdges, 0, (int)maxPolys * 4, (int)maxPolys);
@@ -479,9 +474,6 @@ static Mesh *modifyMesh(ModifierData *md, const ModifierEvalContext *ctx, Mesh *
     }
   }
 
-  float(*vert_normals_new)[3] = do_normal_create ? BKE_mesh_vertex_normals_for_write(result) :
-                                                   NULL;
-
   if (ltmd->flag & MOD_SCREW_NORMAL_CALC) {
 
     /* Normal Calculation (for face flipping)
@@ -510,15 +502,7 @@ static Mesh *modifyMesh(ModifierData *md, const ModifierEvalContext *ctx, Mesh *
     vc = vert_connect;
 
     /* Copy Vert Locations */
-    /* - We can do this in a later loop - only do here if no normal calc */
-    if (!totedge) {
-      for (i = 0; i < totvert; i++, mv_orig++, mv_new++) {
-        copy_v3_v3(mv_new->co, mv_orig->co);
-        /* No edges: this is really a dummy normal. */
-        normalize_v3_v3(vc->no, mv_new->co);
-      }
-    }
-    else {
+    if (totedge != 0) {
       // printf("\n\n\n\n\nStarting Modifier\n");
       /* set edge users */
       med_new = medge_new;
@@ -770,77 +754,6 @@ static Mesh *modifyMesh(ModifierData *md, const ModifierEvalContext *ctx, Mesh *
             }
           }
         }
-
-        /* *VERTEX NORMALS*
-         * we know the surrounding edges are ordered correctly now
-         * so its safe to create vertex normals.
-         *
-         * calculate vertex normals that can be propagated on lathing
-         * use edge connectivity work this out */
-        if (do_normal_create) {
-          if (SV_IS_VALID(vc->v[0])) {
-            if (SV_IS_VALID(vc->v[1])) {
-              /* 2 edges connected. */
-              /* make 2 connecting vert locations relative to the middle vert */
-              sub_v3_v3v3(tmp_vec1, mvert_new[vc->v[0]].co, mvert_new[i].co);
-              sub_v3_v3v3(tmp_vec2, mvert_new[vc->v[1]].co, mvert_new[i].co);
-              /* normalize so both edges have the same influence, no matter their length */
-              normalize_v3(tmp_vec1);
-              normalize_v3(tmp_vec2);
-
-              /* vc_no_tmp1 - this line is the average direction of both connecting edges
-               *
-               * Use the edge order to make the subtraction, flip the normal the right way
-               * edge should be there but check just in case... */
-              if (vc->e[0]->v1 == i) {
-                sub_v3_v3(tmp_vec1, tmp_vec2);
-              }
-              else {
-                sub_v3_v3v3(tmp_vec1, tmp_vec2, tmp_vec1);
-              }
-            }
-            else {
-              /* only 1 edge connected - same as above except
-               * don't need to average edge direction */
-              if (vc->e[0]->v2 == i) {
-                sub_v3_v3v3(tmp_vec1, mvert_new[i].co, mvert_new[vc->v[0]].co);
-              }
-              else {
-                sub_v3_v3v3(tmp_vec1, mvert_new[vc->v[0]].co, mvert_new[i].co);
-              }
-            }
-
-            /* tmp_vec2 - is a line 90d from the pivot to the vec
-             * This is used so the resulting normal points directly away from the middle */
-            cross_v3_v3v3(tmp_vec2, axis_vec, vc->co);
-
-            if (UNLIKELY(is_zero_v3(tmp_vec2))) {
-              /* we're _on_ the axis, so copy it based on our winding */
-              if (vc->e[0]->v2 == i) {
-                negate_v3_v3(vc->no, axis_vec);
-              }
-              else {
-                copy_v3_v3(vc->no, axis_vec);
-              }
-            }
-            else {
-              /* edge average vector and right angle to the pivot make the normal */
-              cross_v3_v3v3(vc->no, tmp_vec1, tmp_vec2);
-            }
-          }
-          else {
-            copy_v3_v3(vc->no, vc->co);
-          }
-
-          /* we won't be looping on this data again so copy normals here */
-          if ((angle < 0.0f) != do_flip) {
-            negate_v3(vc->no);
-          }
-
-          normalize_v3(vc->no);
-          copy_v3_v3(vert_normals_new[i], vc->no);
-        }
-        /* Done with normals */
       }
     }
   }
@@ -881,14 +794,6 @@ static Mesh *modifyMesh(ModifierData *md, const ModifierEvalContext *ctx, Mesh *
     mv_new = &mvert_new[varray_stride]; /* advance to the next slice */
 
     for (j = 0; j < totvert; j++, mv_new_base++, mv_new++) {
-      /* set normal */
-      if (vert_connect) {
-        if (do_normal_create) {
-          /* Set the normal now its transformed. */
-          mul_v3_m3v3(vert_normals_new[mv_new - mvert_new], mat3, vert_connect[j].no);
-        }
-      }
-
       /* set location */
       copy_v3_v3(mv_new->co, mv_new_base->co);
 
@@ -1130,10 +1035,6 @@ static Mesh *modifyMesh(ModifierData *md, const ModifierEvalContext *ctx, Mesh *
     MEM_freeN(vert_loop_map);
   }
 
-  if (do_normal_create) {
-    BKE_mesh_vertex_normals_clear_dirty(result);
-  }
-
   if (do_remove_doubles) {
     result = mesh_remove_doubles_on_axis(result,
                                          mvert_new,



More information about the Bf-blender-cvs mailing list