source/blender/blenkernel/intern/grease_pencil.cc

@@ -1482,95 +1482,126 @@ bool GreasePencil::insert_duplicate_frame(blender::bke::greasepencil::Layer &lay
   return true;
 }
 
-bool GreasePencil::remove_frame_at(blender::bke::greasepencil::Layer &layer,
-                                   const int frame_number)
+bool GreasePencil::remove_frames(blender::bke::greasepencil::Layer &layer,
+                                 blender::Span<int> frame_numbers)
 {
   using namespace blender::bke::greasepencil;
-  if (!layer.frames().contains(frame_number)) {
-    return false;
+  bool removed_any_drawing_user = false;
+  for (const int frame_number : frame_numbers) {
+    if (!layer.frames().contains(frame_number)) {
+      continue;
+    }
+    const GreasePencilFrame frame_to_remove = layer.frames().lookup(frame_number);
+    const int drawing_index_to_remove = frame_to_remove.drawing_index;
+    if (!layer.remove_frame(frame_number)) {
+      /* If removing the frame was not successful, continue. */
+      continue;
+    }
+    if (frame_to_remove.is_null()) {
+      /* Null frames don't reference a drawing, continue. */
+      continue;
+    }
+    GreasePencilDrawingBase *drawing_base = this->drawings(drawing_index_to_remove);
+    if (drawing_base->type != GP_DRAWING) {
+      /* If the drawing is referenced from another object, we don't track it's users because we
+       * cannot delete drawings from another object. */
+      continue;
+    }
+    Drawing &drawing = reinterpret_cast<GreasePencilDrawing *>(drawing_base)->wrap();
+    drawing.remove_user();
+    removed_any_drawing_user = true;
   }
-  const GreasePencilFrame &frame_to_remove = layer.frames().lookup(frame_number);
-  const int drawing_index_to_remove = frame_to_remove.drawing_index;
-  if (!layer.remove_frame(frame_number)) {
-    /* If removing the frame was not successful, return early. */
-    return false;
+  if (removed_any_drawing_user) {
+    this->remove_drawings_with_no_users();
+    return true;
   }
-  GreasePencilDrawingBase *drawing_base = this->drawings(drawing_index_to_remove);
-  if (drawing_base->type != GP_DRAWING) {
-    /* If the drawing is referenced from another object, we don't track it's users because we
-     * cannot delete drawings from another object. Return early. */
-    return false;
-  }
-  Drawing &drawing = reinterpret_cast<GreasePencilDrawing *>(drawing_base)->wrap();
-  drawing.remove_user();
-  if (!drawing.has_users()) {
-    this->remove_drawing(drawing_index_to_remove);
-  }
-  return true;
+  return false;
 }
 
-void GreasePencil::remove_drawing(const int index_to_remove)
+static void remove_drawings_unchecked(GreasePencil &grease_pencil,
+                                      Span<int> sorted_indices_to_remove)
 {
   using namespace blender::bke::greasepencil;
-  /* In order to not change the indices of the drawings, we do the following to the drawing to be
-   * removed:
-   *  - If the drawing (A) is not the last one:
-   *     1.1) Find any frames in the layers that reference the last drawing (B) and point them to
-   *          A's index.
-   *     1.2) Swap drawing A with drawing B.
-   *  2) Destroy A and shrink the array by one.
-   *  3) Remove any frames in the layers that reference the A's index.
-   */
-  BLI_assert(this->drawing_array_num > 0);
-  BLI_assert(index_to_remove >= 0 && index_to_remove < this->drawing_array_num);
+  if (grease_pencil.drawing_array_num == 0 || sorted_indices_to_remove.size() == 0) {
+    return;
+  }
+  const int drawings_to_remove = sorted_indices_to_remove.size();
+  const blender::IndexRange last_drawings_range(
+      grease_pencil.drawings().size() - drawings_to_remove, drawings_to_remove);
 
-  /* Move the drawing that should be removed to the last index. */
-  const int last_drawing_index = this->drawing_array_num - 1;
-  if (index_to_remove != last_drawing_index) {
-    for (Layer *layer : this->layers_for_write()) {
+  /* We keep track of the next available index (for swapping) by iterating from the end and
+   * skipping over drawings that are already in the range to be removed. */
+  auto next_available_index = last_drawings_range.last();
+  auto greatest_index_to_remove_it = std::rbegin(sorted_indices_to_remove);
+  auto get_next_available_index = [&]() {
+    while (next_available_index == *greatest_index_to_remove_it) {
+      greatest_index_to_remove_it = std::prev(greatest_index_to_remove_it);
+      next_available_index--;
+    }
+    return next_available_index;
+  };
+
+  /* Move the drawings to be removed to the end of the array by swapping the pointers. Make sure to
+   * remap any frames pointing to the drawings being swapped. */
+  for (const int index_to_remove : sorted_indices_to_remove) {
+    if (index_to_remove >= last_drawings_range.first()) {
+      /* This drawing and all the next drawings are already in the range to be removed. */
+      break;
+    }
+    const int swap_index = get_next_available_index();
+    /* Remap the drawing_index for frames that point to the drawing to be swapped with. */
+    for (Layer *layer : grease_pencil.layers_for_write()) {
       blender::Map<int, GreasePencilFrame> &frames = layer->frames_for_write();
       for (auto [key, value] : frames.items()) {
-        if (value.drawing_index == last_drawing_index) {
+        if (value.drawing_index == swap_index) {
           value.drawing_index = index_to_remove;
         }
-        else if (value.drawing_index == index_to_remove) {
-          value.drawing_index = last_drawing_index;
-        }
       }
     }
-    std::swap(this->drawings()[index_to_remove], this->drawings()[last_drawing_index]);
+    /* Swap the pointers to the drawings in the drawing array. */
+    std::swap(grease_pencil.drawings()[index_to_remove], grease_pencil.drawings()[swap_index]);
+    next_available_index--;
   }
 
-  /* Delete the last drawing. */
-  GreasePencilDrawingBase *drawing_base_to_remove = this->drawings(last_drawing_index);
-  switch (drawing_base_to_remove->type) {
-    case GP_DRAWING: {
-      GreasePencilDrawing *drawing_to_remove = reinterpret_cast<GreasePencilDrawing *>(
-          drawing_base_to_remove);
-      MEM_delete(&drawing_to_remove->wrap());
-      break;
-    }
-    case GP_DRAWING_REFERENCE: {
-      GreasePencilDrawingReference *drawing_reference_to_remove =
-          reinterpret_cast<GreasePencilDrawingReference *>(drawing_base_to_remove);
-      MEM_freeN(drawing_reference_to_remove);
-      break;
-    }
-  }
-
-  /* Remove any frame that points to the last drawing. */
-  for (Layer *layer : this->layers_for_write()) {
-    blender::Map<int, GreasePencilFrame> &frames = layer->frames_for_write();
-    int64_t frames_removed = frames.remove_if([last_drawing_index](auto item) {
-      return item.value.drawing_index == last_drawing_index;
-    });
-    if (frames_removed > 0) {
-      layer->tag_frames_map_keys_changed();
+  /* Free the last drawings. */
+  for (const int drawing_index : last_drawings_range) {
+    GreasePencilDrawingBase *drawing_base_to_remove = grease_pencil.drawings(drawing_index);
+    switch (drawing_base_to_remove->type) {
+      case GP_DRAWING: {
+        GreasePencilDrawing *drawing_to_remove = reinterpret_cast<GreasePencilDrawing *>(
+            drawing_base_to_remove);
+        MEM_delete(&drawing_to_remove->wrap());
+        break;
+      }
+      case GP_DRAWING_REFERENCE: {
+        GreasePencilDrawingReference *drawing_reference_to_remove =
+            reinterpret_cast<GreasePencilDrawingReference *>(drawing_base_to_remove);
+        MEM_freeN(drawing_reference_to_remove);
+        break;
+      }
     }
   }
 
   /* Shrink drawing array. */
-  shrink_array<GreasePencilDrawingBase *>(&this->drawing_array, &this->drawing_array_num, 1);
+  shrink_array<GreasePencilDrawingBase *>(
+      &grease_pencil.drawing_array, &grease_pencil.drawing_array_num, drawings_to_remove);
+}
+
+void GreasePencil::remove_drawings_with_no_users()
+{
+  using namespace blender;
+  Vector<int> drawings_to_be_removed;
+  for (const int64_t drawing_i : this->drawings().index_range()) {
+    GreasePencilDrawingBase *drawing_base = this->drawings(drawing_i);
+    if (drawing_base->type != GP_DRAWING) {
+      continue;
+    }
+    GreasePencilDrawing *drawing = reinterpret_cast<GreasePencilDrawing *>(drawing_base);
+    if (!drawing->wrap().has_users()) {
+      drawings_to_be_removed.append(drawing_i);
+    }
+  }
+  remove_drawings_unchecked(*this, drawings_to_be_removed.as_span());
 }
 
 blender::bke::greasepencil::Drawing *GreasePencil::get_editable_drawing_at(
@@ -1895,10 +1926,15 @@ void GreasePencil::remove_layer(blender::bke::greasepencil::Layer &layer)
   layer.parent_group().unlink_node(&layer.as_node());
 
   /* Remove drawings. */
-  /* TODO: In the future this should only remove drawings when the user count hits zero. */
   for (GreasePencilFrame frame : layer.frames_for_write().values()) {
-    this->remove_drawing(frame.drawing_index);
+    GreasePencilDrawingBase *drawing_base = this->drawings(frame.drawing_index);
+    if (drawing_base->type != GP_DRAWING) {
+      continue;
+    }
+    GreasePencilDrawing *drawing = reinterpret_cast<GreasePencilDrawing *>(drawing_base);
+    drawing->wrap().remove_user();
   }
+  this->remove_drawings_with_no_users();
 
   /* Delete the layer. */
   MEM_delete(&layer);

source/blender/blenkernel/intern/grease_pencil_test.cc

@@ -53,7 +53,7 @@ TEST(greasepencil, add_empty_drawings)
   EXPECT_EQ(grease_pencil.drawings().size(), 3);
 }
 
-TEST(greasepencil, remove_drawing)
+TEST(greasepencil, remove_drawings)
 {
   GreasePencilIDTestContext ctx;
   GreasePencil &grease_pencil = *static_cast<GreasePencil *>(BKE_id_new(ctx.bmain, ID_GP, "GP"));
@@ -61,7 +61,7 @@ TEST(greasepencil, remove_drawing)
 
   GreasePencilDrawing *drawing = reinterpret_cast<GreasePencilDrawing *>(
       grease_pencil.drawings(1));
-  drawing->geometry.wrap().resize(0, 10);
+  drawing->wrap().strokes_for_write().resize(0, 10);
 
   Layer &layer1 = grease_pencil.root_group().add_layer("Layer1");
   Layer &layer2 = grease_pencil.root_group().add_layer("Layer2");
@@ -71,8 +71,10 @@ TEST(greasepencil, remove_drawing)
   layer1.add_frame(20, 2);
 
   layer2.add_frame(0, 1);
+  drawing->wrap().add_user();
 
-  grease_pencil.remove_drawing(1);
+  grease_pencil.remove_frames(layer1, {10});
+  grease_pencil.remove_frames(layer2, {0});
   EXPECT_EQ(grease_pencil.drawings().size(), 2);
 
   static int expected_frames_size[] = {2, 0};

source/blender/editors/grease_pencil/intern/grease_pencil_frames.cc

@@ -28,15 +28,14 @@ namespace blender::ed::greasepencil {
 
 bool remove_all_selected_frames(GreasePencil &grease_pencil, bke::greasepencil::Layer &layer)
 {
-  bool changed = false;
+  Vector<int> frames_to_remove;
   for (auto [frame_number, frame] : layer.frames().items()) {
     if (!frame.is_selected()) {
       continue;
     }
-    changed |= grease_pencil.remove_frame_at(layer, frame_number);
+    frames_to_remove.append(frame_number);
   }
-
-  return changed;
+  return grease_pencil.remove_frames(layer, frames_to_remove.as_span());
 }
 
 static void select_frame(GreasePencilFrame &frame, const short select_mode)

source/blender/makesdna/DNA_grease_pencil_types.h

@@ -493,9 +493,16 @@ typedef struct GreasePencil {
                               const int dst_frame_number,
                               const bool do_instance);
 
-  bool remove_frame_at(blender::bke::greasepencil::Layer &layer, int frame_number);
-
-  void remove_drawing(int index);
+  /**
+   * Removes all the frames with \a frame_numbers in the \a layer.
+   * \returns true if any frame was removed.
+   */
+  bool remove_frames(blender::bke::greasepencil::Layer &layer, blender::Span<int> frame_numbers);
+  /**
+   * Removes all the drawings that have no users. Will free the drawing data and shrink the
+   * drawings array.
+   */
+  void remove_drawings_with_no_users();
 
   /**
    * Returns an editable drawing on \a layer at frame \a frame_number or `nullptr` if no such