Fix T62282: multires sculpting does not update smooth normals
authorBrecht Van Lommel <brechtvanlommel@gmail.com>
Fri, 31 May 2019 10:51:12 +0000 (12:51 +0200)
committerBrecht Van Lommel <brechtvanlommel@gmail.com>
Fri, 31 May 2019 15:17:03 +0000 (17:17 +0200)
It may be good to move the normals update out of the drawing code. But it was
already there for the non-multires sculpt cases, and does not have an obvious
place since we bypass the depsgraph and want to avoid the cost of updating the
normals multiple times when multiple events are handled before a redraw.

source/blender/blenkernel/BKE_pbvh.h
source/blender/blenkernel/intern/paint.c
source/blender/blenkernel/intern/pbvh.c
source/blender/draw/intern/draw_manager_data.c
source/blender/editors/sculpt_paint/paint_vertex.c
source/blender/editors/sculpt_paint/sculpt.c
source/blender/editors/sculpt_paint/sculpt_undo.c

index 315c0224b11ead1be7586c557b6b1d6e36f40ebd..0a6f3a7d4d9c4626937b68f0b5ca7393f558285a 100644 (file)
@@ -41,6 +41,7 @@ struct MPoly;
 struct MVert;
 struct PBVH;
 struct PBVHNode;
+struct SubdivCCG;
 
 typedef struct PBVH PBVH;
 typedef struct PBVHNode PBVHNode;
@@ -166,8 +167,6 @@ bool BKE_pbvh_node_find_nearest_to_ray(PBVH *bvh,
 
 void BKE_pbvh_draw_cb(PBVH *bvh,
                       float (*planes)[4],
-                      float (*fnors)[3],
-                      bool show_vcol,
                       void (*draw_fn)(void *user_data, struct GPU_PBVH_Buffers *buffers),
                       void *user_data);
 
@@ -256,9 +255,11 @@ struct GSet *BKE_pbvh_bmesh_node_faces(PBVHNode *node);
 void BKE_pbvh_bmesh_node_save_orig(PBVHNode *node);
 void BKE_pbvh_bmesh_after_stroke(PBVH *bvh);
 
-/* Update Normals/Bounding Box/Redraw and clear flags */
+/* Update Bounding Box/Redraw and clear flags */
 
-void BKE_pbvh_update(PBVH *bvh, int flags, float (*face_nors)[3]);
+void BKE_pbvh_update_bounds(PBVH *bvh, int flags);
+void BKE_pbvh_update_normals(PBVH *bvh, struct SubdivCCG *subdiv_ccg);
+void BKE_pbvh_update_draw_buffers(PBVH *bvh, bool show_vcol);
 void BKE_pbvh_redraw_BB(PBVH *bvh, float bb_min[3], float bb_max[3]);
 void BKE_pbvh_get_grid_updates(PBVH *bvh, bool clear, void ***r_gridfaces, int *r_totface);
 void BKE_pbvh_grids_update(PBVH *bvh,
index 441ae311404db667a6bae36f971197502a09eb59..f1f49070f1daee14a491036f12883f98239ece83 100644 (file)
@@ -987,6 +987,21 @@ void BKE_sculptsession_bm_to_me(Object *ob, bool reorder)
   }
 }
 
+static void sculptsession_free_pbvh(Object *object)
+{
+  SculptSession *ss = object->sculpt;
+
+  if (ss && ss->pbvh) {
+    /* Ensure all normals are updated before freeing the PBVH, because
+     * we skip updating them for performance when we don't draw the PBVH. */
+    Mesh *mesh = object->data;
+    BKE_pbvh_update_normals(ss->pbvh, mesh->runtime.subdiv_ccg);
+
+    BKE_pbvh_free(ss->pbvh);
+    ss->pbvh = NULL;
+  }
+}
+
 void BKE_sculptsession_bm_to_me_for_render(Object *object)
 {
   if (object && object->sculpt) {
@@ -999,11 +1014,6 @@ void BKE_sculptsession_bm_to_me_for_render(Object *object)
        */
       BKE_object_free_derived_caches(object);
 
-      if (object->sculpt->pbvh) {
-        BKE_pbvh_free(object->sculpt->pbvh);
-        object->sculpt->pbvh = NULL;
-      }
-
       sculptsession_bm_to_me_update_data_only(object, false);
 
       /* In contrast with sculptsession_bm_to_me no need in
@@ -1024,9 +1034,8 @@ void BKE_sculptsession_free(Object *ob)
       BM_mesh_free(ss->bm);
     }
 
-    if (ss->pbvh) {
-      BKE_pbvh_free(ss->pbvh);
-    }
+    sculptsession_free_pbvh(ob);
+
     MEM_SAFE_FREE(ss->pmap);
     MEM_SAFE_FREE(ss->pmap_mem);
     if (ss->bm_log) {
@@ -1269,10 +1278,7 @@ void BKE_sculpt_update_object_before_eval(Object *ob)
       /* We free pbvh on changes, except in the middle of drawing a stroke
        * since it can't deal with changing PVBH node organization, we hope
        * topology does not change in the meantime .. weak. */
-      if (ss->pbvh) {
-        BKE_pbvh_free(ss->pbvh);
-        ss->pbvh = NULL;
-      }
+      sculptsession_free_pbvh(ob);
 
       BKE_sculptsession_free_deformMats(ob->sculpt);
 
index f1d5347c48b050066fddb014eaeb4f025eade1e8..a49d2da8cfc8f2f8fd6406098f04f31e34d54095 100644 (file)
@@ -31,8 +31,7 @@
 
 #include "BKE_pbvh.h"
 #include "BKE_ccg.h"
-#include "BKE_subsurf.h"
-#include "BKE_DerivedMesh.h"
+#include "BKE_subdiv_ccg.h"
 #include "BKE_mesh.h" /* for BKE_mesh_calc_normals */
 #include "BKE_paint.h"
 
@@ -990,7 +989,6 @@ typedef struct PBVHUpdateData {
   PBVHNode **nodes;
   int totnode;
 
-  float (*fnors)[3];
   float (*vnors)[3];
   int flag;
 } PBVHUpdateData;
@@ -1003,7 +1001,6 @@ static void pbvh_update_normals_accum_task_cb(void *__restrict userdata,
 
   PBVH *bvh = data->bvh;
   PBVHNode *node = data->nodes[n];
-  float(*fnors)[3] = data->fnors;
   float(*vnors)[3] = data->vnors;
 
   if ((node->flag & PBVH_UpdateNormals)) {
@@ -1027,11 +1024,6 @@ static void pbvh_update_normals_accum_task_cb(void *__restrict userdata,
         const MPoly *mp = &bvh->mpoly[lt->poly];
         BKE_mesh_calc_poly_normal(mp, &bvh->mloop[mp->loopstart], bvh->verts, fn);
         mpoly_prev = lt->poly;
-
-        if (fnors) {
-          /* We can assume a face is only present in one node ever. */
-          copy_v3_v3(fnors[lt->poly], fn);
-        }
       }
 
       for (int j = sides; j--;) {
@@ -1080,23 +1072,11 @@ static void pbvh_update_normals_store_task_cb(void *__restrict userdata,
   }
 }
 
-static void pbvh_update_normals(PBVH *bvh, PBVHNode **nodes, int totnode, float (*fnors)[3])
+static void pbvh_faces_update_normals(PBVH *bvh, PBVHNode **nodes, int totnode)
 {
-  float(*vnors)[3];
-
-  if (bvh->type == PBVH_BMESH) {
-    BLI_assert(fnors == NULL);
-    pbvh_bmesh_normals_update(nodes, totnode);
-    return;
-  }
-
-  if (bvh->type != PBVH_FACES) {
-    return;
-  }
-
   /* could be per node to save some memory, but also means
    * we have to store for each vertex which node it is in */
-  vnors = MEM_callocN(sizeof(*vnors) * bvh->totvert, __func__);
+  float(*vnors)[3] = MEM_callocN(sizeof(*vnors) * bvh->totvert, __func__);
 
   /* subtle assumptions:
    * - We know that for all edited vertices, the nodes with faces
@@ -1111,7 +1091,6 @@ static void pbvh_update_normals(PBVH *bvh, PBVHNode **nodes, int totnode, float
   PBVHUpdateData data = {
       .bvh = bvh,
       .nodes = nodes,
-      .fnors = fnors,
       .vnors = vnors,
   };
 
@@ -1273,7 +1252,7 @@ static int pbvh_flush_bb(PBVH *bvh, PBVHNode *node, int flag)
   return update;
 }
 
-void BKE_pbvh_update(PBVH *bvh, int flag, float (*fnors)[3])
+void BKE_pbvh_update_bounds(PBVH *bvh, int flag)
 {
   if (!bvh->nodes) {
     return;
@@ -1284,10 +1263,6 @@ void BKE_pbvh_update(PBVH *bvh, int flag, float (*fnors)[3])
 
   BKE_pbvh_search_gather(bvh, update_search_cb, POINTER_FROM_INT(flag), &nodes, &totnode);
 
-  if (flag & PBVH_UpdateNormals) {
-    pbvh_update_normals(bvh, nodes, totnode, fnors);
-  }
-
   if (flag & (PBVH_UpdateBB | PBVH_UpdateOriginalBB | PBVH_UpdateRedraw)) {
     pbvh_update_BB_redraw(bvh, nodes, totnode, flag);
   }
@@ -2216,32 +2191,60 @@ static void pbvh_node_draw_cb(PBVHNode *node, void *data_v)
   }
 }
 
-/**
- * Version of #BKE_pbvh_draw that runs a callback.
- */
-void BKE_pbvh_draw_cb(PBVH *bvh,
-                      float (*planes)[4],
-                      float (*fnors)[3],
-                      bool show_vcol,
-                      void (*draw_fn)(void *user_data, GPU_PBVH_Buffers *buffers),
-                      void *user_data)
+void BKE_pbvh_update_normals(PBVH *bvh, struct SubdivCCG *subdiv_ccg)
+{
+  /* Update normals */
+  PBVHNode **nodes;
+  int totnode;
+
+  BKE_pbvh_search_gather(
+      bvh, update_search_cb, POINTER_FROM_INT(PBVH_UpdateNormals), &nodes, &totnode);
+
+  if (bvh->type == PBVH_BMESH) {
+    pbvh_bmesh_normals_update(nodes, totnode);
+  }
+  else if (bvh->type == PBVH_FACES) {
+    pbvh_faces_update_normals(bvh, nodes, totnode);
+  }
+  else if (bvh->type == PBVH_GRIDS) {
+    struct CCGFace **faces;
+    int num_faces;
+    BKE_pbvh_get_grid_updates(bvh, true, (void ***)&faces, &num_faces);
+    if (num_faces > 0) {
+      BKE_subdiv_ccg_update_normals(subdiv_ccg, faces, num_faces);
+      MEM_freeN(faces);
+    }
+  }
+
+  if (nodes) {
+    MEM_freeN(nodes);
+  }
+}
+
+void BKE_pbvh_update_draw_buffers(PBVH *bvh, bool show_vcol)
 {
+  /* Update GPU buffers */
   PBVHNode **nodes;
   int totnode;
 
-  BKE_pbvh_search_gather(bvh,
-                         update_search_cb,
-                         POINTER_FROM_INT(PBVH_UpdateNormals | PBVH_UpdateDrawBuffers),
-                         &nodes,
-                         &totnode);
+  BKE_pbvh_search_gather(
+      bvh, update_search_cb, POINTER_FROM_INT(PBVH_UpdateDrawBuffers), &nodes, &totnode);
 
-  pbvh_update_normals(bvh, nodes, totnode, fnors);
   pbvh_update_draw_buffers(bvh, nodes, totnode, show_vcol);
 
   if (nodes) {
     MEM_freeN(nodes);
   }
+}
 
+/**
+ * Version of #BKE_pbvh_draw that runs a callback.
+ */
+void BKE_pbvh_draw_cb(PBVH *bvh,
+                      float (*planes)[4],
+                      void (*draw_fn)(void *user_data, GPU_PBVH_Buffers *buffers),
+                      void *user_data)
+{
   PBVHNodeDrawCallbackData draw_data = {
       .draw_fn = draw_fn,
       .user_data = user_data,
@@ -2362,8 +2365,7 @@ void BKE_pbvh_apply_vertCos(PBVH *pbvh, float (*vertCos)[3], const int totvert)
       BKE_pbvh_node_mark_update(&pbvh->nodes[a]);
     }
 
-    BKE_pbvh_update(pbvh, PBVH_UpdateBB, NULL);
-    BKE_pbvh_update(pbvh, PBVH_UpdateOriginalBB, NULL);
+    BKE_pbvh_update_bounds(pbvh, PBVH_UpdateBB | PBVH_UpdateOriginalBB);
   }
 }
 
index 09855769daed528e7c9c712d514e7dfc711f3d52..9c9b91eaaedcedfbea7810bcc97e60c9427a80a2 100644 (file)
@@ -665,8 +665,11 @@ static void drw_sculpt_generate_calls(DRWSculptCallbackData *scd, bool use_vcol)
     }
   }
 
-  BKE_pbvh_draw_cb(
-      pbvh, planes, NULL, use_vcol, (void (*)(void *, GPU_PBVH_Buffers *))sculpt_draw_cb, scd);
+  Mesh *mesh = scd->ob->data;
+  BKE_pbvh_update_normals(pbvh, mesh->runtime.subdiv_ccg);
+  BKE_pbvh_update_draw_buffers(pbvh, use_vcol);
+
+  BKE_pbvh_draw_cb(pbvh, planes, (void (*)(void *, GPU_PBVH_Buffers *))sculpt_draw_cb, scd);
 
 #ifdef SCULPT_DEBUG_BUFFERS
   int node_nr = 0;
index bdb8ccb09a439abe7486df75da2e9b4fa0a53fad..439c237cd57d9a8e99672ce09d019a56b6fefdb3 100644 (file)
@@ -1497,8 +1497,7 @@ static void vwpaint_update_cache_variants(bContext *C, VPaint *vp, Object *ob, P
   cache->radius_squared = cache->radius * cache->radius;
 
   if (ss->pbvh) {
-    BKE_pbvh_update(ss->pbvh, PBVH_UpdateRedraw, NULL);
-    BKE_pbvh_update(ss->pbvh, PBVH_UpdateBB, NULL);
+    BKE_pbvh_update_bounds(ss->pbvh, PBVH_UpdateRedraw | PBVH_UpdateBB);
   }
 }
 
index 2855e3181c066412c1f8a513dea76d2c14a2b490..4e5c2a74023fc7f0a5ae1556f7c85c23eb4e1baa 100644 (file)
@@ -506,7 +506,7 @@ void ED_sculpt_redraw_planes_get(float planes[4][4], ARegion *ar, Object *ob)
 
   /* clear redraw flag from nodes */
   if (pbvh) {
-    BKE_pbvh_update(pbvh, PBVH_UpdateRedraw, NULL);
+    BKE_pbvh_update_bounds(pbvh, PBVH_UpdateRedraw);
   }
 }
 
@@ -5208,7 +5208,7 @@ static void sculpt_flush_update_step(bContext *C)
      * only the part of the 3D viewport where changes happened. */
     rcti r;
 
-    BKE_pbvh_update(ss->pbvh, PBVH_UpdateBB, NULL);
+    BKE_pbvh_update_bounds(ss->pbvh, PBVH_UpdateBB);
     /* Update the object's bounding box too so that the object
      * doesn't get incorrectly clipped during drawing in
      * draw_mesh_object(). [#33790] */
@@ -5255,7 +5255,7 @@ static void sculpt_flush_update_done(const bContext *C, Object *ob)
     }
   }
 
-  BKE_pbvh_update(ss->pbvh, PBVH_UpdateOriginalBB, NULL);
+  BKE_pbvh_update_bounds(ss->pbvh, PBVH_UpdateOriginalBB);
 
   if (BKE_pbvh_type(ss->pbvh) == PBVH_BMESH) {
     BKE_pbvh_bmesh_after_stroke(ss->pbvh);
index 3c26a0e2541d2ee3a4b8b7825af7519ced6ae6eb..75a2af4526f87aae2449a6eb3f709f1772b739fb 100644 (file)
@@ -549,9 +549,7 @@ static void sculpt_undo_restore_list(bContext *C, ListBase *lb)
     else {
       BKE_pbvh_search_callback(ss->pbvh, NULL, NULL, update_cb, &rebuild);
     }
-    BKE_pbvh_update(ss->pbvh,
-                    PBVH_UpdateBB | PBVH_UpdateOriginalBB | PBVH_UpdateRedraw | PBVH_UpdateNormals,
-                    NULL);
+    BKE_pbvh_update_bounds(ss->pbvh, PBVH_UpdateBB | PBVH_UpdateOriginalBB | PBVH_UpdateRedraw);
 
     if (BKE_sculpt_multires_active(scene, ob)) {
       if (rebuild) {