Fix different shading between CPU and GPU subdivision

Reuse the same vertex normals calculation as for the GPU code, by
weighing each vertex normals by the angle of the edges incident to the
vertex on the face.

Additionally, remove limit normals, as the CPU code does not use them
either, and would also cause different shading issues when limit surface
is used.

Fixes T95242: shade smooth artifacts with edge crease and limit surface
Fixes T94919: subdivision, different shading between CPU and GPU

7 files changed, 79 insertions, 48 deletions

diff --git a/source/blender/draw/intern/draw_cache_impl_subdivision.cc b/source/blender/draw/intern/draw_cache_impl_subdivision.cc
index e8ff9a508a2..8c42eea1689 100644
--- a/source/blender/draw/intern/draw_cache_impl_subdivision.cc
+++ b/source/blender/draw/intern/draw_cache_impl_subdivision.cc
@@ -82,7 +82,6 @@ enum {
   SHADER_BUFFER_NORMALS_ACCUMULATE,
   SHADER_BUFFER_NORMALS_FINALIZE,
   SHADER_PATCH_EVALUATION,
-  SHADER_PATCH_EVALUATION_LIMIT_NORMALS,
   SHADER_PATCH_EVALUATION_FVAR,
   SHADER_PATCH_EVALUATION_FACE_DOTS,
   SHADER_COMP_CUSTOM_DATA_INTERP_1D,
@@ -122,7 +121,6 @@ static const char *get_shader_code(int shader_type)
       return datatoc_common_subdiv_normals_finalize_comp_glsl;
     }
     case SHADER_PATCH_EVALUATION:
-    case SHADER_PATCH_EVALUATION_LIMIT_NORMALS:
     case SHADER_PATCH_EVALUATION_FVAR:
     case SHADER_PATCH_EVALUATION_FACE_DOTS: {
       return datatoc_common_subdiv_patch_evaluation_comp_glsl;
@@ -174,9 +172,6 @@ static const char *get_shader_name(int shader_type)
     case SHADER_PATCH_EVALUATION: {
       return "subdiv patch evaluation";
     }
-    case SHADER_PATCH_EVALUATION_LIMIT_NORMALS: {
-      return "subdiv patch evaluation limit normals";
-    }
     case SHADER_PATCH_EVALUATION_FVAR: {
       return "subdiv patch evaluation face-varying";
     }
@@ -214,13 +209,7 @@ static GPUShader *get_patch_evaluation_shader(int shader_type)
     const char *compute_code = get_shader_code(shader_type);
 
     const char *defines = nullptr;
-    if (shader_type == SHADER_PATCH_EVALUATION_LIMIT_NORMALS) {
-      defines =
-          "#define OSD_PATCH_BASIS_GLSL\n"
-          "#define OPENSUBDIV_GLSL_COMPUTE_USE_1ST_DERIVATIVES\n"
-          "#define LIMIT_NORMALS\n";
-    }
-    else if (shader_type == SHADER_PATCH_EVALUATION_FVAR) {
+    if (shader_type == SHADER_PATCH_EVALUATION_FVAR) {
       defines =
           "#define OSD_PATCH_BASIS_GLSL\n"
           "#define OPENSUBDIV_GLSL_COMPUTE_USE_1ST_DERIVATIVES\n"
@@ -261,7 +250,6 @@ static GPUShader *get_subdiv_shader(int shader_type, const char *defines)
 {
   if (ELEM(shader_type,
            SHADER_PATCH_EVALUATION,
-           SHADER_PATCH_EVALUATION_LIMIT_NORMALS,
            SHADER_PATCH_EVALUATION_FVAR,
            SHADER_PATCH_EVALUATION_FACE_DOTS)) {
     return get_patch_evaluation_shader(shader_type);
@@ -1209,9 +1197,7 @@ static void drw_subdiv_compute_dispatch(const DRWSubdivCache *cache,
   GPU_compute_dispatch(shader, dispatch_rx, dispatch_ry, 1);
 }
 
-void draw_subdiv_extract_pos_nor(const DRWSubdivCache *cache,
-                                 GPUVertBuf *pos_nor,
-                                 const bool do_limit_normals)
+void draw_subdiv_extract_pos_nor(const DRWSubdivCache *cache, GPUVertBuf *pos_nor)
 {
   Subdiv *subdiv = cache->subdiv;
   OpenSubdiv_Evaluator *evaluator = subdiv->evaluator;
@@ -1236,8 +1222,7 @@ void draw_subdiv_extract_pos_nor(const DRWSubdivCache *cache,
                                                                get_patch_param_format());
   evaluator->wrapPatchParamBuffer(evaluator, &patch_param_buffer_interface);
 
-  GPUShader *shader = get_patch_evaluation_shader(
-      do_limit_normals ? SHADER_PATCH_EVALUATION_LIMIT_NORMALS : SHADER_PATCH_EVALUATION);
+  GPUShader *shader = get_patch_evaluation_shader(SHADER_PATCH_EVALUATION);
   GPU_shader_bind(shader);
 
   GPU_vertbuf_bind_as_ssbo(src_buffer, 0);
@@ -1409,6 +1394,7 @@ void draw_subdiv_accumulate_normals(const DRWSubdivCache *cache,
                                     GPUVertBuf *pos_nor,
                                     GPUVertBuf *face_adjacency_offsets,
                                     GPUVertBuf *face_adjacency_lists,
+                                    GPUVertBuf *vertex_loop_map,
                                     GPUVertBuf *vertex_normals)
 {
   GPUShader *shader = get_subdiv_shader(SHADER_BUFFER_NORMALS_ACCUMULATE, nullptr);
@@ -1419,6 +1405,7 @@ void draw_subdiv_accumulate_normals(const DRWSubdivCache *cache,
   GPU_vertbuf_bind_as_ssbo(pos_nor, binding_point++);
   GPU_vertbuf_bind_as_ssbo(face_adjacency_offsets, binding_point++);
   GPU_vertbuf_bind_as_ssbo(face_adjacency_lists, binding_point++);
+  GPU_vertbuf_bind_as_ssbo(vertex_loop_map, binding_point++);
   GPU_vertbuf_bind_as_ssbo(vertex_normals, binding_point++);
 
   drw_subdiv_compute_dispatch(cache, shader, 0, 0, cache->num_subdiv_verts);
@@ -1866,8 +1853,6 @@ static bool draw_subdiv_create_requested_buffers(const Scene *scene,
   draw_cache->subdiv = subdiv;
   draw_cache->optimal_display = optimal_display;
   draw_cache->num_subdiv_triangles = tris_count_from_number_of_loops(draw_cache->num_subdiv_loops);
-  /* We can only evaluate limit normals if the patches are adaptive. */
-  draw_cache->do_limit_normals = settings.is_adaptive;
 
   draw_cache->use_custom_loop_normals = (smd->flags & eSubsurfModifierFlag_UseCustomNormals) &&
                                         (mesh_eval->flag & ME_AUTOSMOOTH) &&
diff --git a/source/blender/draw/intern/draw_subdivision.h b/source/blender/draw/intern/draw_subdivision.h
index 78313441a70..2f339cf18f8 100644
--- a/source/blender/draw/intern/draw_subdivision.h
+++ b/source/blender/draw/intern/draw_subdivision.h
@@ -66,7 +66,6 @@ typedef struct DRWSubdivCache {
   struct BMesh *bm;
   struct Subdiv *subdiv;
   bool optimal_display;
-  bool do_limit_normals;
   bool use_custom_loop_normals;
 
   /* Coordinates used to evaluate patches for UVs, positions, and normals. */
@@ -180,6 +179,7 @@ void draw_subdiv_accumulate_normals(const DRWSubdivCache *cache,
                                     struct GPUVertBuf *pos_nor,
                                     struct GPUVertBuf *face_adjacency_offsets,
                                     struct GPUVertBuf *face_adjacency_lists,
+                                    struct GPUVertBuf *vertex_loop_map,
                                     struct GPUVertBuf *vertex_normals);
 
 void draw_subdiv_finalize_normals(const DRWSubdivCache *cache,
@@ -191,9 +191,7 @@ void draw_subdiv_finalize_custom_normals(const DRWSubdivCache *cache,
                                          GPUVertBuf *src_custom_normals,
                                          GPUVertBuf *pos_nor);
 
-void draw_subdiv_extract_pos_nor(const DRWSubdivCache *cache,
-                                 struct GPUVertBuf *pos_nor,
-                                 bool do_limit_normals);
+void draw_subdiv_extract_pos_nor(const DRWSubdivCache *cache, struct GPUVertBuf *pos_nor);
 
 void draw_subdiv_interp_custom_data(const DRWSubdivCache *cache,
                                     struct GPUVertBuf *src_data,
diff --git a/source/blender/draw/intern/mesh_extractors/extract_mesh_vbo_pos_nor.cc b/source/blender/draw/intern/mesh_extractors/extract_mesh_vbo_pos_nor.cc
index ef88a34021e..0645683d58b 100644
--- a/source/blender/draw/intern/mesh_extractors/extract_mesh_vbo_pos_nor.cc
+++ b/source/blender/draw/intern/mesh_extractors/extract_mesh_vbo_pos_nor.cc
@@ -233,14 +233,12 @@ static void extract_pos_nor_init_subdiv(const DRWSubdivCache *subdiv_cache,
                                         void *UNUSED(data))
 {
   GPUVertBuf *vbo = static_cast<GPUVertBuf *>(buffer);
-  const bool do_limit_normals = subdiv_cache->do_limit_normals &&
-                                !subdiv_cache->use_custom_loop_normals;
 
   /* Initialize the vertex buffer, it was already allocated. */
   GPU_vertbuf_init_build_on_device(
       vbo, get_pos_nor_format(), subdiv_cache->num_subdiv_loops + mr->loop_loose_len);
 
-  draw_subdiv_extract_pos_nor(subdiv_cache, vbo, do_limit_normals);
+  draw_subdiv_extract_pos_nor(subdiv_cache, vbo);
 
   if (subdiv_cache->use_custom_loop_normals) {
     Mesh *coarse_mesh = subdiv_cache->mesh;
@@ -266,7 +264,7 @@ static void extract_pos_nor_init_subdiv(const DRWSubdivCache *subdiv_cache,
     GPU_vertbuf_discard(src_custom_normals);
     GPU_vertbuf_discard(dst_custom_normals);
   }
-  else if (!do_limit_normals) {
+  else {
     /* We cannot evaluate vertex normals using the limit surface, so compute them manually. */
     GPUVertBuf *subdiv_loop_subdiv_vert_index = draw_subdiv_build_origindex_buffer(
         subdiv_cache->subdiv_loop_subdiv_vert_index, subdiv_cache->num_subdiv_loops);
@@ -279,6 +277,7 @@ static void extract_pos_nor_init_subdiv(const DRWSubdivCache *subdiv_cache,
                                    vbo,
                                    subdiv_cache->subdiv_vertex_face_adjacency_offsets,
                                    subdiv_cache->subdiv_vertex_face_adjacency,
+                                   subdiv_loop_subdiv_vert_index,
                                    vertex_normals);
 
     draw_subdiv_finalize_normals(subdiv_cache, vertex_normals, subdiv_loop_subdiv_vert_index, vbo);
diff --git a/source/blender/draw/intern/shaders/common_subdiv_lib.glsl b/source/blender/draw/intern/shaders/common_subdiv_lib.glsl
index 9dd86c35ee4..a620905622a 100644
--- a/source/blender/draw/intern/shaders/common_subdiv_lib.glsl
+++ b/source/blender/draw/intern/shaders/common_subdiv_lib.glsl
@@ -140,6 +140,13 @@ void set_vertex_nor(inout PosNorLoop vertex_data, vec3 nor)
   set_vertex_nor(vertex_data, nor, 0);
 }
 
+void add_newell_cross_v3_v3v3(inout vec3 n, vec3 v_prev, vec3 v_curr)
+{
+  n[0] += (v_prev[1] - v_curr[1]) * (v_prev[2] + v_curr[2]);
+  n[1] += (v_prev[2] - v_curr[2]) * (v_prev[0] + v_curr[0]);
+  n[2] += (v_prev[0] - v_curr[0]) * (v_prev[1] + v_curr[1]);
+}
+
 #define ORIGINDEX_NONE -1
 
 #ifdef SUBDIV_POLYGON_OFFSET
diff --git a/source/blender/draw/intern/shaders/common_subdiv_normals_accumulate_comp.glsl b/source/blender/draw/intern/shaders/common_subdiv_normals_accumulate_comp.glsl
index 575090472b1..a6d6c205615 100644
--- a/source/blender/draw/intern/shaders/common_subdiv_normals_accumulate_comp.glsl
+++ b/source/blender/draw/intern/shaders/common_subdiv_normals_accumulate_comp.glsl
@@ -16,11 +16,33 @@ layout(std430, binding = 2) readonly buffer faceAdjacencyLists
   uint face_adjacency_lists[];
 };
 
-layout(std430, binding = 3) writeonly buffer vertexNormals
+layout(std430, binding = 3) readonly buffer vertexLoopMap
+{
+  uint vert_loop_map[];
+};
+
+layout(std430, binding = 4) writeonly buffer vertexNormals
 {
   vec3 normals[];
 };
 
+void find_prev_and_next_vertex_on_face(
+    uint face_index, uint vertex_index, out uint curr, out uint next, out uint prev)
+{
+  uint start_loop_index = face_index * 4;
+
+  for (uint i = 0; i < 4; i++) {
+    uint subdiv_vert_index = vert_loop_map[start_loop_index + i];
+
+    if (subdiv_vert_index == vertex_index) {
+      curr = i;
+      next = (i + 1) % 4;
+      prev = (i + 4 - 1) % 4;
+      break;
+    }
+  }
+}
+
 void main()
 {
   uint vertex_index = get_global_invocation_index();
@@ -39,18 +61,37 @@ void main()
     uint adjacent_face = face_adjacency_lists[first_adjacent_face_offset + i];
     uint start_loop_index = adjacent_face * 4;
 
-    /* Compute face normal. */
-    vec3 adjacent_verts[3];
-    for (uint j = 0; j < 3; j++) {
-      adjacent_verts[j] = get_vertex_pos(pos_nor[start_loop_index + j]);
+    /* Compute the face normal using Newell's method. */
+    vec3 verts[4];
+    for (uint j = 0; j < 4; j++) {
+      verts[j] = get_vertex_pos(pos_nor[start_loop_index + j]);
     }
 
-    vec3 face_normal = normalize(
-        cross(adjacent_verts[1] - adjacent_verts[0], adjacent_verts[2] - adjacent_verts[0]));
-    accumulated_normal += face_normal;
+    vec3 face_normal = vec3(0.0);
+    add_newell_cross_v3_v3v3(face_normal, verts[0], verts[1]);
+    add_newell_cross_v3_v3v3(face_normal, verts[1], verts[2]);
+    add_newell_cross_v3_v3v3(face_normal, verts[2], verts[3]);
+    add_newell_cross_v3_v3v3(face_normal, verts[3], verts[0]);
+
+    /* Accumulate angle weighted normal. */
+    uint curr_vert = 0;
+    uint next_vert = 0;
+    uint prev_vert = 0;
+    find_prev_and_next_vertex_on_face(
+        adjacent_face, vertex_index, curr_vert, next_vert, prev_vert);
+
+    vec3 curr_co = verts[curr_vert];
+    vec3 prev_co = verts[next_vert];
+    vec3 next_co = verts[prev_vert];
+
+    vec3 edvec_prev = normalize(prev_co - curr_co);
+    vec3 edvec_next = normalize(curr_co - next_co);
+
+    float fac = acos(-dot(edvec_prev, edvec_next));
+
+    accumulated_normal += face_normal * fac;
   }
 
-  float weight = 1.0 / float(number_of_adjacent_faces);
   vec3 normal = normalize(accumulated_normal);
   normals[vertex_index] = normal;
 }
diff --git a/source/blender/draw/intern/shaders/common_subdiv_patch_evaluation_comp.glsl b/source/blender/draw/intern/shaders/common_subdiv_patch_evaluation_comp.glsl
index 5dd7decf663..26410eb2d7f 100644
--- a/source/blender/draw/intern/shaders/common_subdiv_patch_evaluation_comp.glsl
+++ b/source/blender/draw/intern/shaders/common_subdiv_patch_evaluation_comp.glsl
@@ -394,12 +394,8 @@ void main()
 
     evaluate_patches_limits(patch_co.patch_index, uv.x, uv.y, pos, du, dv);
 
-#  if defined(LIMIT_NORMALS)
-    vec3 nor = normalize(cross(du, dv));
-#  else
     /* This will be computed later. */
     vec3 nor = vec3(0.0);
-#  endif
 
     int origindex = input_vert_origindex[loop_index];
     uint flag = 0;
diff --git a/source/blender/draw/intern/shaders/common_subdiv_vbo_lnor_comp.glsl b/source/blender/draw/intern/shaders/common_subdiv_vbo_lnor_comp.glsl
index 41a8df3cf82..8b4a5dd14f3 100644
--- a/source/blender/draw/intern/shaders/common_subdiv_vbo_lnor_comp.glsl
+++ b/source/blender/draw/intern/shaders/common_subdiv_vbo_lnor_comp.glsl
@@ -38,13 +38,18 @@ void main()
     }
   }
   else {
-    /* Face is flat shaded, compute flat face normal from an inscribed triangle. */
-    vec3 verts[3];
-    for (int i = 0; i < 3; i++) {
-      verts[i] = get_vertex_pos(pos_nor[start_loop_index + i]);
-    }
-
-    vec3 face_normal = normalize(cross(verts[1] - verts[0], verts[2] - verts[0]));
+    vec3 v0 = get_vertex_pos(pos_nor[start_loop_index + 0]);
+    vec3 v1 = get_vertex_pos(pos_nor[start_loop_index + 1]);
+    vec3 v2 = get_vertex_pos(pos_nor[start_loop_index + 2]);
+    vec3 v3 = get_vertex_pos(pos_nor[start_loop_index + 3]);
+
+    vec3 face_normal = vec3(0.0);
+    add_newell_cross_v3_v3v3(face_normal, v0, v1);
+    add_newell_cross_v3_v3v3(face_normal, v1, v2);
+    add_newell_cross_v3_v3v3(face_normal, v2, v3);
+    add_newell_cross_v3_v3v3(face_normal, v3, v0);
+
+    face_normal = normalize(face_normal);
     for (int i = 0; i < 4; i++) {
       output_lnor[start_loop_index + i] = face_normal;
     }