[Bf-blender-cvs] [e6ad4ec3fc7] master: Fix T58994: Subdivision modifier generates artifacts with crease=1

Sergey Sharybin noreply at git.blender.org
Wed Jan 23 12:19:32 CET 2019


Commit: e6ad4ec3fc7db82062e93d49395275ac326f2a37
Author: Sergey Sharybin
Date:   Wed Jan 23 12:14:34 2019 +0100
Branches: master
https://developer.blender.org/rBe6ad4ec3fc7db82062e93d49395275ac326f2a37

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.

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

M	source/blender/blenkernel/intern/subdiv_mesh.c

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

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(s

@@ Diff output truncated at 10240 characters. @@



More information about the Bf-blender-cvs mailing list