Fix T58994: Subdivision modifier generates artifacts with crease=1

The issue was caused by the lack of averaging of normals for
vertices which are on the ptex face boundaries.

1 files changed, 132 insertions, 74 deletions

diff --git a/source/blender/blenkernel/intern/subdiv_mesh.c b/source/blender/blenkernel/intern/subdiv_mesh.c
index 9c4793a9efc..13ade957839 100644
--- a/source/blender/blenkernel/intern/subdiv_mesh.c
+++ b/source/blender/blenkernel/intern/subdiv_mesh.c
@@ -65,6 +65,24 @@ typedef struct SubdivMeshContext {
 	/* UV layers interpolation. */
 	int num_uv_layers;
 	MLoopUV *uv_layers[MAX_MTFACE];
+	/* Accumulated values.
+	 *
+	 * Averaging is happening for vertices along the coarse edges and corners.
+	 * This is needed for both displacement and normals.
+	 *
+	 * Displacement is being accumulated to a verticies coordinates, since those
+	 * are not needed during traversal of edge/corner vertices.
+	 *
+	 * For normals we are using dedicated array, since we can not use same
+	 * vertices (normals are `short`, which will cause a lot of precision
+	 * issues). */
+	float (*accumulated_normals)[3];
+	/* Per-subdivided vertex counter of averaged values. */
+	int *accumulated_counters;
+	/* Denotes whether normals can be evaluated from a limit surface. One case
+	 * when it's not possible is when displacement is used. */
+	bool can_evaluate_normals;
+	bool have_displacement;
 } SubdivMeshContext;
 
 static void subdiv_mesh_ctx_cache_uv_layers(SubdivMeshContext *ctx)
@@ -94,6 +112,30 @@ static void subdiv_mesh_ctx_cache_custom_data_layers(SubdivMeshContext *ctx)
 	subdiv_mesh_ctx_cache_uv_layers(ctx);
 }
 
+static void subdiv_mesh_prepare_accumulator(
+        SubdivMeshContext *ctx, int num_vertices)
+{
+	if (!ctx->can_evaluate_normals && !ctx->have_displacement) {
+		return;
+	}
+	/* TODO(sergey): Technically, this is overallocating, we don't need memory
+	 * for an inner subdivision vertices. */
+	ctx->accumulated_normals = MEM_calloc_arrayN(
+	        sizeof(*ctx->accumulated_normals),
+	        num_vertices,
+	        "subdiv accumulated normals");
+	ctx->accumulated_counters = MEM_calloc_arrayN(
+	        sizeof(*ctx->accumulated_counters),
+	        num_vertices,
+	        "subdiv accumulated counters");
+}
+
+static void subdiv_mesh_context_free(SubdivMeshContext *ctx)
+{
+	MEM_SAFE_FREE(ctx->accumulated_normals);
+	MEM_SAFE_FREE(ctx->accumulated_counters);
+}
+
 /* =============================================================================
  * Loop custom data copy helpers.
  */
@@ -120,8 +162,7 @@ static void loops_of_ptex_get(
 	/* Loop which look in the (opposite) V direction of the current
 	 * ptex face.
 	 *
-	 * TODO(sergey): Get rid of using module on every iteration.
-	 */
+	 * TODO(sergey): Get rid of using module on every iteration. */
 	const int last_ptex_loop_index =
 	        coarse_poly->loopstart +
 	        (ptex_of_poly_index + coarse_poly->totloop - 1) %
@@ -143,14 +184,12 @@ static void loops_of_ptex_get(
  */
 
 /* TODO(sergey): Somehow de-duplicate with loops storage, without too much
- * exception cases all over the code.
- */
+ * exception cases all over the code. */
 
 typedef struct VerticesForInterpolation {
 	/* This field points to a vertex data which is to be used for interpolation.
 	 * The idea is to avoid unnecessary allocations for regular faces, where
-	 * we can simply
-	 */
+	 * we can simply use corner verticies. */
 	const CustomData *vertex_data;
 	/* Vertices data calculated for ptex corners. There are always 4 elements
 	 * in this custom data, aligned the following way:
@@ -160,13 +199,11 @@ typedef struct VerticesForInterpolation {
 	 *   index 2 -> uv (1, 1)
 	 *   index 3 -> uv (1, 0)
 	 *
-	 * Is allocated for non-regular faces (triangles and n-gons).
-	 */
+	 * Is allocated for non-regular faces (triangles and n-gons). */
 	CustomData vertex_data_storage;
 	bool vertex_data_storage_allocated;
 	/* Infices within vertex_data to interpolate for. The indices are aligned
-	 * with uv coordinates in a similar way as indices in loop_data_storage.
-	 */
+	 * with uv coordinates in a similar way as indices in loop_data_storage. */
 	int vertex_indices[4];
 } VerticesForInterpolation;
 
@@ -205,8 +242,7 @@ static void vertex_interpolation_init(
 		vertex_interpolation->vertex_indices[3] = 3;
 		vertex_interpolation->vertex_data_storage_allocated = true;
 		/* Interpolate center of poly right away, it stays unchanged for all
-		 * ptex faces.
-		 */
+		 * ptex faces. */
 		const float weight = 1.0f / (float)coarse_poly->totloop;
 		float *weights = BLI_array_alloca(weights, coarse_poly->totloop);
 		int *indices = BLI_array_alloca(indices, coarse_poly->totloop);
@@ -249,8 +285,7 @@ static void vertex_interpolation_from_corner(
 		 * middle points.
 		 *
 		 * TODO(sergey): Re-use one of interpolation results from previous
-		 * iteration.
-		 */
+		 * iteration. */
 		const float weights[2] = {0.5f, 0.5f};
 		const int first_loop_index = loops_of_ptex.first_loop - coarse_mloop;
 		const int last_loop_index = loops_of_ptex.last_loop - coarse_mloop;
@@ -291,8 +326,7 @@ static void vertex_interpolation_end(
 typedef struct LoopsForInterpolation {
  /* This field points to a loop data which is to be used for interpolation.
 	 * The idea is to avoid unnecessary allocations for regular faces, where
-	 * we can simply
-	 */
+	 * we can simply interpolate corner verticies. */
 	const CustomData *loop_data;
 	/* Loops data calculated for ptex corners. There are always 4 elements
 	 * in this custom data, aligned the following way:
@@ -302,13 +336,11 @@ typedef struct LoopsForInterpolation {
 	 *   index 2 -> uv (1, 1)
 	 *   index 3 -> uv (1, 0)
 	 *
-	 * Is allocated for non-regular faces (triangles and n-gons).
-	 */
+	 * Is allocated for non-regular faces (triangles and n-gons). */
 	CustomData loop_data_storage;
 	bool loop_data_storage_allocated;
 	/* Infices within loop_data to interpolate for. The indices are aligned with
-	 * uv coordinates in a similar way as indices in loop_data_storage.
-	 */
+	 * uv coordinates in a similar way as indices in loop_data_storage. */
 	int loop_indices[4];
 } LoopsForInterpolation;
 
@@ -341,8 +373,7 @@ static void loop_interpolation_init(
 		loop_interpolation->loop_indices[3] = 3;
 		loop_interpolation->loop_data_storage_allocated = true;
 		/* Interpolate center of poly right away, it stays unchanged for all
-		 * ptex faces.
-		 */
+		 * ptex faces. */
 		const float weight = 1.0f / (float)coarse_poly->totloop;
 		float *weights = BLI_array_alloca(weights, coarse_poly->totloop);
 		int *indices = BLI_array_alloca(indices, coarse_poly->totloop);
@@ -466,27 +497,34 @@ static void eval_final_point_and_vertex_normal(
 }
 
 /* =============================================================================
- * Displacement helpers
+ * Accumulation helpers.
  */
 
-static void subdiv_accumulate_vertex_displacement(
-        Subdiv *subdiv,
+static void subdiv_accumulate_vertex_normal_and_displacement(
+        SubdivMeshContext *ctx,
         const int ptex_face_index,
         const float u, const float v,
         MVert *subdiv_vert)
 {
+	Subdiv *subdiv = ctx->subdiv;
+	const int subdiv_vertex_index = subdiv_vert - ctx->subdiv_mesh->mvert;
 	float dummy_P[3], dPdu[3], dPdv[3], D[3];
 	BKE_subdiv_eval_limit_point_and_derivatives(
 	        subdiv, ptex_face_index, u, v, dummy_P, dPdu, dPdv);
-	BKE_subdiv_eval_displacement(subdiv,
-	                             ptex_face_index, u, v,
-	                             dPdu, dPdv,
-	                             D);
-	add_v3_v3(subdiv_vert->co, D);
-	if (subdiv_vert->flag & ME_VERT_TMP_TAG) {
-		mul_v3_fl(subdiv_vert->co, 0.5f);
+	/* Accumulate normal. */
+	if (ctx->can_evaluate_normals) {
+		float N[3];
+		cross_v3_v3v3(N, dPdu, dPdv);
+		normalize_v3(N);
+		add_v3_v3(ctx->accumulated_normals[subdiv_vertex_index], N);
 	}
-	subdiv_vert->flag |= ME_VERT_TMP_TAG;
+	/* Accumulate displacement if needed. */
+	if (ctx->have_displacement) {
+		BKE_subdiv_eval_displacement(
+		        subdiv, ptex_face_index, u, v, dPdu, dPdv, D);
+		add_v3_v3(subdiv_vert->co, D);
+	}
+	++ctx->accumulated_counters[subdiv_vertex_index];
 }
 
 /* =============================================================================
@@ -509,6 +547,7 @@ static bool subdiv_mesh_topology_info(
 	        num_loops,
 	        num_polygons);
 	subdiv_mesh_ctx_cache_custom_data_layers(subdiv_context);
+	subdiv_mesh_prepare_accumulator(subdiv_context, num_vertices);
 	return true;
 }
 
@@ -525,7 +564,6 @@ static void subdiv_vertex_data_copy(
 	Mesh *subdiv_mesh = ctx->subdiv_mesh;
 	const int coarse_vertex_index = coarse_vertex - coarse_mesh->mvert;
 	const int subdiv_vertex_index = subdiv_vertex - subdiv_mesh->mvert;
-	subdiv_vertex->flag &= ~ME_VERT_TMP_TAG;
 	CustomData_copy_data(&coarse_mesh->vdata,
 	                     &ctx->subdiv_mesh->vdata,
 	                     coarse_vertex_index,
@@ -544,7 +582,6 @@ static void subdiv_vertex_data_interpolate(
 	                          u * (1.0f - v),
 	                          u * v,
 	                          (1.0f - u) * v};
-	subdiv_vertex->flag &= ~ME_VERT_TMP_TAG;
 	CustomData_interp(vertex_interpolation->vertex_data,
 	                  &ctx->subdiv_mesh->vdata,
 	                  vertex_interpolation->vertex_indices,
@@ -563,19 +600,29 @@ static void evaluate_vertex_and_apply_displacement_copy(
         const MVert *coarse_vert,
         MVert *subdiv_vert)
 {
+	const int subdiv_vertex_index = subdiv_vert - ctx->subdiv_mesh->mvert;
+	const float inv_num_accumulated =
+	        1.0f / ctx->accumulated_counters[subdiv_vertex_index];
 	/* Displacement is accumulated in subdiv vertex position.
-	 * need to back it up before copying data from original vertex.
-	 */
-	float D[3];
-	copy_v3_v3(D, subdiv_vert->co);
+	 * Needs to to be backed up before copying data from original vertex. */
+	float D[3] = {0.0f, 0.0f, 0.0f};
+	if (ctx->have_displacement) {
+		copy_v3_v3(D, subdiv_vert->co);
+		mul_v3_fl(D, inv_num_accumulated);
+	}
+	/* Copy custom data and evaluate position. */
 	subdiv_vertex_data_copy(ctx, coarse_vert, subdiv_vert);
-	BKE_subdiv_eval_limit_point_and_short_normal(
-	        ctx->subdiv,
-	        ptex_face_index,
-	        u, v,
-	        subdiv_vert->co, subdiv_vert->no);
+	BKE_subdiv_eval_limit_point(
+	        ctx->subdiv, ptex_face_index, u, v, subdiv_vert->co);
 	/* Apply displacement. */
 	add_v3_v3(subdiv_vert->co, D);
+	/* Copy normal from accumulated storage. */
+	if (ctx->can_evaluate_normals) {
+		float N[3];
+		copy_v3_v3(N, ctx->accumulated_normals[subdiv_vertex_index]);
+		normalize_v3(N);
+		normal_float_to_short_v3(subdiv_vert->no, N);
+	}
 }
 
 static void evaluate_vertex_and_apply_displacement_interpolate(
@@ -585,22 +632,31 @@ static void evaluate_vertex_and_apply_displacement_interpolate(
         VerticesForInterpolation *vertex_interpolation,
         MVert *subdiv_vert)
 {
+	const int subdiv_vertex_index = subdiv_vert - ctx->subdiv_mesh->mvert;
+	const float inv_num_accumulated =
+	        1.0f / ctx->accumulated_counters[subdiv_vertex_index];
 	/* Displacement is accumulated in subdiv vertex position.
-	 * need to back it up before copying data from original vertex.
-	 */
-	float D[3];
-	copy_v3_v3(D, subdiv_vert->co);
-	subdiv_vertex_data_interpolate(ctx,
-	                               subdiv_vert,
-	                               vertex_interpolation,
-	                               u, v);
-	BKE_subdiv_eval_limit_point_and_short_normal(
-	        ctx->subdiv,
-	        ptex_face_index,
-	        u, v,
-	        subdiv_vert->co, subdiv_vert->no);
+	 * Needs to to be backed up before copying data from original vertex. */
+	float D[3] = {0.0f, 0.0f, 0.0f};
+	if (ctx->have_displacement) {
+		copy_v3_v3(D, subdiv_vert->co);
+		mul_v3_fl(D, inv_num_accumulated);
+	}
+	/* Interpolate custom data and evaluate position. */
+	subdiv_vertex_data_interpolate(
+	        ctx, subdiv_vert, vertex_interpolation, u, v);
+	BKE_subdiv_eval_limit_point(
+	        ctx->subdiv, ptex_face_index, u, v, subdiv_vert->co);
 	/* Apply displacement. */
 	add_v3_v3(subdiv_vert->co, D);
+	/* Copy normal from accumulated storage. */
+	if (ctx->can_evaluate_normals) {
+		float N[3];
+		copy_v3_v3(N, ctx->accumulated_normals[subdiv_vertex_index]);
+		mul_v3_fl(N, inv_num_accumulated);
+		normalize_v3(N);
+		normal_float_to_short_v3(subdiv_vert->no, N);
+	}
 }
 
 static void subdiv_mesh_vertex_every_corner_or_edge(
@@ -611,12 +667,11 @@ static void subdiv_mesh_vertex_every_corner_or_edge(
         const int subdiv_vertex_index)
 {
 	SubdivMeshContext *ctx = foreach_context->user_data;
-	Subdiv *subdiv = ctx->subdiv;
 	Mesh *subdiv_mesh = ctx->subdiv_mesh;
 	MVert *subdiv_mvert = subdiv_mesh->mvert;
 	MVert *subdiv_vert = &subdiv_mvert[subdiv_vertex_index];
-	subdiv_accumulate_vertex_displacement(
-	        subdiv, ptex_face_index, u, v, subdiv_vert);
+	subdiv_accumulate_vertex_normal_and_displacement(
+	        ctx, ptex_face_index, u, v, subdiv_vert);
 }
 
 static void subdiv_mesh_vertex_every_corner(
@@ -970,8 +1025,7 @@ static void subdiv_mesh_vertex_loose(
 
 /* Get neighbor edges of the given one.
  * - neighbors[0] is an edge adjacent to edge->v1.
- * - neighbors[1] is an edge adjacent to edge->v1.
- */
+ * - neighbors[1] is an edge adjacent to edge->v2. */
 static void find_edge_neighbors(const SubdivMeshContext *ctx,
                                 const MEdge *edge,
                                 const MEdge *neighbors[2])
@@ -1116,14 +1170,16 @@ static void subdiv_mesh_vertex_of_loose_edge(
  * Initialization.
  */
 
-static void setup_foreach_callbacks(SubdivForeachContext *foreach_context,
-                                    const Subdiv *subdiv)
+static void setup_foreach_callbacks(const SubdivMeshContext *subdiv_context,
+                                    SubdivForeachContext *foreach_context)
 {
 	memset(foreach_context, 0, sizeof(*foreach_context));
 	/* General information. */
 	foreach_context->topology_info = subdiv_mesh_topology_info;
-	/* Every boundary geometry. Used for dispalcement averaging. */
-	if (subdiv->displacement_evaluator != NULL) {
+	/* Every boundary geometry. Used for dispalcement and normals averaging. */
+	if (subdiv_context->can_evaluate_normals ||
+	    subdiv_context->have_displacement)
+	{
 		foreach_context->vertex_every_corner = subdiv_mesh_vertex_every_corner;
 		foreach_context->vertex_every_edge = subdiv_mesh_vertex_every_edge;
 	}
@@ -1160,8 +1216,7 @@ Mesh *BKE_subdiv_to_mesh(
 		 * - OpenSubdiv is disabled.
 		 * - Something totally bad happened, and OpenSubdiv rejected our
 		 *   topology.
-		 * In either way, we can't safely continue.
-		 */
+		 * In either way, we can't safely continue. */
 		if (coarse_mesh->totpoly) {
 			BKE_subdiv_stats_end(&subdiv->stats, SUBDIV_STATS_SUBDIV_TO_MESH);
 			return NULL;
@@ -1172,25 +1227,28 @@ Mesh *BKE_subdiv_to_mesh(
 	subdiv_context.settings = settings;
 	subdiv_context.coarse_mesh = coarse_mesh;
 	subdiv_context.subdiv = subdiv;
+	subdiv_context.have_displacement =
+	        (subdiv->displacement_evaluator != NULL);
+	subdiv_context.can_evaluate_normals = !subdiv_context.have_displacement;
 	/* Multi-threaded traversal/evaluation. */
 	BKE_subdiv_stats_begin(&subdiv->stats,
 	                       SUBDIV_STATS_SUBDIV_TO_MESH_GEOMETRY);
 	SubdivForeachContext foreach_context;
-	setup_foreach_callbacks(&foreach_context, subdiv);
+	setup_foreach_callbacks(&subdiv_context, &foreach_context);
 	SubdivMeshTLS tls = {0};
 	foreach_context.user_data = &subdiv_context;
 	foreach_context.user_data_tls_size = sizeof(SubdivMeshTLS);
 	foreach_context.user_data_tls = &tls;
-	BKE_subdiv_foreach_subdiv_geometry(subdiv,
-	                                   &foreach_context,
-	                                   settings,
-	                                   coarse_mesh);
+	BKE_subdiv_foreach_subdiv_geometry(
+	        subdiv, &foreach_context, settings, coarse_mesh);
 	BKE_subdiv_stats_end(&subdiv->stats, SUBDIV_STATS_SUBDIV_TO_MESH_GEOMETRY);
 	Mesh *result = subdiv_context.subdiv_mesh;
 	// BKE_mesh_validate(result, true, true);
 	BKE_subdiv_stats_end(&subdiv->stats, SUBDIV_STATS_SUBDIV_TO_MESH);
-	if (subdiv->displacement_evaluator != NULL) {
+	if (!subdiv_context.can_evaluate_normals) {
 		result->runtime.cd_dirty_vert |= CD_MASK_NORMAL;
 	}
+	/* Free used memoty. */
+	subdiv_mesh_context_free(&subdiv_context);
 	return result;
 }