Sculpt: Visibility bugfixes

* Unhide all is no longer part of face_set_change_visibility.
* Implemented a few visibility API methods for PBVH_BMESH
* Fixed bug with unhide all not freeing all multires grid_hidden
  bitmaps.

5 files changed, 106 insertions, 67 deletions

diff --git a/release/scripts/presets/keyconfig/keymap_data/blender_default.py b/release/scripts/presets/keyconfig/keymap_data/blender_default.py
index 75d0717184e..5ab9cdb542a 100644
--- a/release/scripts/presets/keyconfig/keymap_data/blender_default.py
+++ b/release/scripts/presets/keyconfig/keymap_data/blender_default.py
@@ -5069,8 +5069,8 @@ def km_sculpt(params):
          {"properties": [("mode", 'TOGGLE')]}),
         ("sculpt.face_set_change_visibility", {"type": 'H', "value": 'PRESS', "shift": True},
          {"properties": [("mode", 'HIDE_ACTIVE')]}),
-        ("sculpt.face_set_change_visibility", {"type": 'H', "value": 'PRESS', "alt": True},
-         {"properties": [("mode", 'SHOW_ALL')]}),
+        ("sculpt.reveal_all", {"type": 'H', "value": 'PRESS', "alt": True},
+         {"properties": []}),
 
         ("sculpt.face_set_edit", {"type": 'W', "value": 'PRESS', "ctrl": True},
          {"properties": [("mode", 'GROW')]}),
diff --git a/source/blender/blenkernel/intern/paint.cc b/source/blender/blenkernel/intern/paint.cc
index ea4360b1626..9e3a044ff1c 100644
--- a/source/blender/blenkernel/intern/paint.cc
+++ b/source/blender/blenkernel/intern/paint.cc
@@ -2134,7 +2134,7 @@ void BKE_sculpt_sync_face_visibility_to_grids(Mesh *mesh, SubdivCCG *subdiv_ccg)
       ".hide_poly", ATTR_DOMAIN_FACE, false);
   if (hide_poly.is_single() && !hide_poly.get_internal_single()) {
     /* Nothing is hidden, so we can just remove all visibility bitmaps. */
-    for (const int i : hide_poly.index_range()) {
+    for (const int i : IndexRange(subdiv_ccg->num_grids)) {
       BKE_subdiv_ccg_grid_hidden_free(subdiv_ccg, i);
     }
     return;
diff --git a/source/blender/editors/sculpt_paint/sculpt.c b/source/blender/editors/sculpt_paint/sculpt.c
index 89998f19476..85a300484c3 100644
--- a/source/blender/editors/sculpt_paint/sculpt.c
+++ b/source/blender/editors/sculpt_paint/sculpt.c
@@ -641,7 +641,6 @@ void SCULPT_visibility_sync_all_from_faces(Object *ob)
     }
     case PBVH_BMESH: {
       BMIter iter;
-      BMVert *v;
       BMFace *f;
 
       /* Hide all verts and edges attached to faces.*/
@@ -655,7 +654,7 @@ void SCULPT_visibility_sync_all_from_faces(Object *ob)
 
       /* Unhide verts and edges attached to visible faces. */
       BM_ITER_MESH (f, &iter, ss->bm, BM_FACES_OF_MESH) {
-        if (!(BM_elem_flag_test(f, BM_ELEM_HIDDEN))) {
+        if (BM_elem_flag_test(f, BM_ELEM_HIDDEN)) {
           continue;
         }
 
diff --git a/source/blender/editors/sculpt_paint/sculpt_face_set.cc b/source/blender/editors/sculpt_paint/sculpt_face_set.cc
index 34717888966..894fa0a5d64 100644
--- a/source/blender/editors/sculpt_paint/sculpt_face_set.cc
+++ b/source/blender/editors/sculpt_paint/sculpt_face_set.cc
@@ -785,7 +785,6 @@ typedef enum eSculptFaceGroupVisibilityModes {
   SCULPT_FACE_SET_VISIBILITY_SHOW_ACTIVE = 1,
   SCULPT_FACE_SET_VISIBILITY_HIDE_ACTIVE = 2,
   SCULPT_FACE_SET_VISIBILITY_INVERT = 3,
-  SCULPT_FACE_SET_VISIBILITY_SHOW_ALL = 4,
 } eSculptFaceGroupVisibilityModes;
 
 static EnumPropertyItem prop_sculpt_face_sets_change_visibility_types[] = {
@@ -817,13 +816,6 @@ static EnumPropertyItem prop_sculpt_face_sets_change_visibility_types[] = {
         "Invert Face Set Visibility",
         "Invert Face Set Visibility",
     },
-    {
-        SCULPT_FACE_SET_VISIBILITY_SHOW_ALL,
-        "SHOW_ALL",
-        0,
-        "Show All Face Sets",
-        "Show All Face Sets",
-    },
     {0, nullptr, 0, nullptr, nullptr},
 };
 
@@ -837,56 +829,13 @@ static int sculpt_face_sets_change_visibility_exec(bContext *C, wmOperator *op)
 
   BKE_sculpt_update_object_for_edit(depsgraph, ob, true, true, false);
 
-  /* Dyntopo not supported except for SCULPT_FACE_SET_VISIBILITY_SHOW_ALL. */
+  /* Not supported for dyntopo. */
   if (BKE_pbvh_type(ss->pbvh) == PBVH_BMESH) {
-    if (ss->pbvh && ss->bm &&
-        RNA_enum_get(op->ptr, "mode") == SCULPT_FACE_SET_VISIBILITY_SHOW_ALL) {
-      PBVHNode **nodes;
-      int totnode;
-
-      BKE_pbvh_search_gather(ss->pbvh, nullptr, nullptr, &nodes, &totnode);
-
-      if (!totnode) {
-        return OPERATOR_CANCELLED;
-      }
-
-      SCULPT_undo_push_begin(ob, op);
-      SCULPT_undo_push_node(ob, nodes[0], SCULPT_UNDO_COORDS);
-
-      for (int i = 0; i < totnode; i++) {
-        BKE_pbvh_node_mark_update_visibility(nodes[i]);
-      }
-
-      BMIter iter;
-      BMFace *f;
-      BMVert *v;
-      const int cd_mask = CustomData_get_offset(&ss->bm->vdata, CD_PAINT_MASK);
-
-      BM_ITER_MESH (v, &iter, ss->bm, BM_VERTS_OF_MESH) {
-        BM_log_vert_before_modified(ss->bm_log, v, cd_mask);
-      }
-      BM_ITER_MESH (f, &iter, ss->bm, BM_FACES_OF_MESH) {
-        BM_log_face_modified(ss->bm_log, f);
-      }
-
-      SCULPT_face_visibility_all_set(ss, true);
-      SCULPT_visibility_sync_all_from_faces(ob);
-
-      SCULPT_undo_push_end(ob);
-      MEM_SAFE_FREE(nodes);
-
-      return OPERATOR_FINISHED;
-    }
-
     return OPERATOR_CANCELLED;
   }
 
-  if (!ss->face_sets) {
-    return OPERATOR_CANCELLED;
-  }
-
-  const int tot_vert = SCULPT_vertex_count_get(ss);
   const int mode = RNA_enum_get(op->ptr, "mode");
+  const int tot_vert = SCULPT_vertex_count_get(ss);
   const int active_face_set = SCULPT_active_face_set_get(ss);
 
   SCULPT_undo_push_begin(ob, op);
@@ -940,15 +889,6 @@ static int sculpt_face_sets_change_visibility_exec(bContext *C, wmOperator *op)
     }
   }
 
-  if (mode == SCULPT_FACE_SET_VISIBILITY_SHOW_ALL) {
-    /* As an optimization, free the hide attribute when making all geometry visible. This allows
-     * reduced memory usage without manually clearing it later, and allows sculpt operations to
-     * avoid checking element's hide status. */
-    CustomData_free_layer_named(&mesh->pdata, ".hide_poly", mesh->totpoly);
-    ss->hide_poly = nullptr;
-    BKE_pbvh_update_hide_attributes_from_mesh(pbvh);
-  }
-
   if (mode == SCULPT_FACE_SET_VISIBILITY_SHOW_ACTIVE) {
     ss->hide_poly = BKE_sculpt_hide_poly_ensure(mesh);
     SCULPT_face_visibility_all_set(ss, false);
diff --git a/source/blender/editors/sculpt_paint/sculpt_ops.c b/source/blender/editors/sculpt_paint/sculpt_ops.c
index ea2261559ef..ae119b9e8a5 100644
--- a/source/blender/editors/sculpt_paint/sculpt_ops.c
+++ b/source/blender/editors/sculpt_paint/sculpt_ops.c
@@ -1270,6 +1270,105 @@ static void SCULPT_OT_mask_from_cavity(wmOperatorType *ot)
   RNA_def_boolean(ot->srna, "invert", false, "Cavity (Inverted)", "");
 }
 
+static int sculpt_reveal_all_exec(bContext *C, wmOperator *op)
+{
+  Object *ob = CTX_data_active_object(C);
+  SculptSession *ss = ob->sculpt;
+  Depsgraph *depsgraph = CTX_data_depsgraph_pointer(C);
+
+  Mesh *mesh = BKE_object_get_original_mesh(ob);
+
+  BKE_sculpt_update_object_for_edit(depsgraph, ob, true, true, false);
+
+  if (!ss->pbvh) {
+    return OPERATOR_CANCELLED;
+  }
+
+  PBVHNode **nodes;
+  int totnode;
+  bool with_bmesh = BKE_pbvh_type(ss->pbvh) == PBVH_BMESH;
+
+  BKE_pbvh_search_gather(ss->pbvh, NULL, NULL, &nodes, &totnode);
+
+  if (!totnode) {
+    return OPERATOR_CANCELLED;
+  }
+
+  /* Propagate face hide state to verts for undo. */
+  SCULPT_visibility_sync_all_from_faces(ob);
+
+  SCULPT_undo_push_begin(ob, op);
+
+  for (int i = 0; i < totnode; i++) {
+    BKE_pbvh_node_mark_update_visibility(nodes[i]);
+
+    if (!with_bmesh) {
+      SCULPT_undo_push_node(ob, nodes[i], SCULPT_UNDO_HIDDEN);
+    }
+  }
+
+  if (!with_bmesh) {
+    /* As an optimization, free the hide attribute when making all geometry visible. This allows
+     * reduced memory usage without manually clearing it later, and allows sculpt operations to
+     * avoid checking element's hide status. */
+    CustomData_free_layer_named(&mesh->pdata, ".hide_poly", mesh->totpoly);
+    ss->hide_poly = NULL;
+  }
+  else {
+    SCULPT_undo_push_node(ob, nodes[0], SCULPT_UNDO_HIDDEN);
+
+    BMIter iter;
+    BMFace *f;
+    BMVert *v;
+    const int cd_mask = CustomData_get_offset(&ss->bm->vdata, CD_PAINT_MASK);
+
+    BM_ITER_MESH (v, &iter, ss->bm, BM_VERTS_OF_MESH) {
+      BM_log_vert_before_modified(ss->bm_log, v, cd_mask);
+    }
+    BM_ITER_MESH (f, &iter, ss->bm, BM_FACES_OF_MESH) {
+      BM_log_face_modified(ss->bm_log, f);
+    }
+
+    SCULPT_face_visibility_all_set(ss, true);
+  }
+
+  SCULPT_visibility_sync_all_from_faces(ob);
+
+  /* Note: SCULPT_visibility_sync_all_from_faces may have deleted
+   * pbvh->hide_vert if hide_poly did not exist, which is why
+   * we call BKE_pbvh_update_hide_attributes_from_mesh here instead of
+   * after CustomData_free_layer_named above.
+   */
+  if (!with_bmesh) {
+    BKE_pbvh_update_hide_attributes_from_mesh(ss->pbvh);
+  }
+
+  BKE_pbvh_update_visibility(ss->pbvh);
+
+  SCULPT_undo_push_end(ob);
+  MEM_SAFE_FREE(nodes);
+
+  SCULPT_tag_update_overlays(C);
+  DEG_id_tag_update(&ob->id, ID_RECALC_SHADING);
+  ED_region_tag_redraw(CTX_wm_region(C));
+
+  return OPERATOR_FINISHED;
+}
+
+void SCULPT_OT_reveal_all(wmOperatorType *ot)
+{
+  /* Identifiers. */
+  ot->name = "Reveal All";
+  ot->idname = "SCULPT_OT_reveal_all";
+  ot->description = "Unhide all geometry";
+
+  /* Api callbacks. */
+  ot->exec = sculpt_reveal_all_exec;
+  ot->poll = SCULPT_mode_poll;
+
+  ot->flag = OPTYPE_REGISTER | OPTYPE_UNDO;
+}
+
 void ED_operatortypes_sculpt(void)
 {
   WM_operatortype_append(SCULPT_OT_brush_stroke);
@@ -1305,4 +1404,5 @@ void ED_operatortypes_sculpt(void)
 
   WM_operatortype_append(SCULPT_OT_expand);
   WM_operatortype_append(SCULPT_OT_mask_from_cavity);
+  WM_operatortype_append(SCULPT_OT_reveal_all);
 }