source/blender/blenkernel/BKE_grease_pencil.hh

@@ -258,6 +258,18 @@ class Layer : public ::GreasePencilLayer {
    * \returns a pointer to the added frame on success, otherwise nullptr.
    */
   GreasePencilFrame *add_frame(int frame_number, int drawing_index, int duration = 0);
+  /**
+   * Removes a frame with \a start_frame_number from the frames map.
+   *
+   * Fails if the map does not contain a frame with \a frame_number or in the specific case where
+   * the previous frame has a fixed duration (is not marked as an implicit hold) and the frame to
+   * remove is a null frame.
+   *
+   * Will remove null frames after the frame to remove.
+   * \param start_frame_number: the first frame number of the frame to be removed.
+   * \return true on success.
+   */
+  bool remove_frame(int start_frame_number);
 
   /**
    * Returns the sorted (start) frame numbers of the frames of this layer.
@@ -284,9 +296,17 @@ class Layer : public ::GreasePencilLayer {
    */
   void tag_frames_map_keys_changed();
 
+ private:
+  using SortedKeysIterator = const int *;
  private:
   GreasePencilFrame *add_frame_internal(int frame_number, int drawing_index);
   int frame_index_at(int frame_number) const;
+  /**
+   * Removes null frames starting from \a begin until \a end (excluded) or until a non-null frame is reached.
+   * \param begin, end: Iterators into the `sorted_keys` span.
+   * \returns an iterator to the element after the last null-frame that was removed.
+   */
+  SortedKeysIterator remove_leading_null_frames_in_range(SortedKeysIterator begin, SortedKeysIterator end);
 };
 
 class LayerGroupRuntime {

source/blender/blenkernel/intern/grease_pencil.cc

@@ -601,6 +601,18 @@ bool Layer::is_selected() const
   return ((this->base.flag & GP_LAYER_TREE_NODE_SELECT) != 0);
 }
 
+Layer::SortedKeysIterator Layer::remove_leading_null_frames_in_range(
+    Layer::SortedKeysIterator begin, Layer::SortedKeysIterator end)
+{
+  Layer::SortedKeysIterator next_it = begin;
+  while (next_it != end && this->frames().lookup(*next_it).is_null()) {
+    this->frames_for_write().remove(*next_it);
+    this->tag_frames_map_keys_changed();
+    next_it = std::next(next_it);
+  }
+  return next_it;
+}
+
 GreasePencilFrame *Layer::add_frame_internal(const int frame_number, const int drawing_index)
 {
   BLI_assert(drawing_index != -1);
@@ -634,20 +646,14 @@ GreasePencilFrame *Layer::add_frame(const int frame_number,
   Span<int> sorted_keys = this->sorted_keys();
   const int end_frame_number = frame_number + duration;
   /* Finds the next greater frame_number that is stored in the map. */
-  auto next_frame_number_it = std::upper_bound(
+  SortedKeysIterator next_frame_number_it = std::upper_bound(
       sorted_keys.begin(), sorted_keys.end(), frame_number);
   /* If the next frame we found is at the end of the frame we're inserting, then we are done. */
   if (next_frame_number_it != sorted_keys.end() && *next_frame_number_it == end_frame_number) {
     return frame;
   }
-  /* While the next frame is a null frame, remove it. */
-  while (next_frame_number_it != sorted_keys.end() &&
-         this->frames().lookup(*next_frame_number_it).is_null())
-  {
-    this->frames_for_write().remove(*next_frame_number_it);
-    this->tag_frames_map_keys_changed();
-    next_frame_number_it = std::next(next_frame_number_it);
-  }
+  next_frame_number_it = this->remove_leading_null_frames_in_range(next_frame_number_it,
+                                                                   sorted_keys.end());
   /* If the duration is set to 0, the frame is marked as an implicit hold.*/
   if (duration == 0) {
     frame->flag |= GP_FRAME_IMPLICIT_HOLD;
@@ -662,6 +668,45 @@ GreasePencilFrame *Layer::add_frame(const int frame_number,
   return frame;
 }
 
+bool Layer::remove_frame(const int start_frame_number)
+{
+  /* If the frame number is not in the frames map, do nothing. */
+  if (!this->frames().contains(start_frame_number)) {
+    return false;
+  }
+  if (this->frames().size() == 1) {
+    this->frames_for_write().remove(start_frame_number);
+    this->tag_frames_map_keys_changed();
+    return true;
+  }
+  Span<int> sorted_keys = this->sorted_keys();
+  /* Find the index of the frame to remove in the `sorted_keys` array. */
+  SortedKeysIterator remove_frame_number_it = std::lower_bound(
+      sorted_keys.begin(), sorted_keys.end(), start_frame_number);
+  /* If there is a next frame: */
+  if (std::next(remove_frame_number_it) != sorted_keys.end()) {
+    SortedKeysIterator next_frame_number_it = std::next(remove_frame_number_it);
+    this->remove_leading_null_frames_in_range(next_frame_number_it, sorted_keys.end());
+  }
+  /* If there is a previous frame: */
+  if (remove_frame_number_it != sorted_keys.begin()) {
+    SortedKeysIterator prev_frame_number_it = std::prev(remove_frame_number_it);
+    const GreasePencilFrame &prev_frame = this->frames().lookup(*prev_frame_number_it);
+    /* If the previous frame is not an implicit hold (e.g. it has a fixed duration) and it's not a
+     * null frame, we cannot just delete the frame. We need to replace it with a null frame. */
+    if (!prev_frame.is_implicit_hold() && !prev_frame.is_null()) {
+      this->frames_for_write().lookup(start_frame_number) = GreasePencilFrame::null();
+      /* Since the original frame was replaced with a null frame, we consider the frame to be
+       * successfully removed here. */
+      return true;
+    }
+  }
+  /* Finally, remove the actual frame. */
+  this->frames_for_write().remove(start_frame_number);
+  this->tag_frames_map_keys_changed();
+  return true;
+}
+
 Span<int> Layer::sorted_keys() const
 {
   this->runtime->sorted_keys_cache_.ensure([&](Vector<int> &r_data) {
@@ -691,7 +736,7 @@ int Layer::frame_index_at(const int frame_number) const
     return sorted_keys.last();
   }
   /* Search for the drawing. upper_bound will get the drawing just after. */
-  auto it = std::upper_bound(sorted_keys.begin(), sorted_keys.end(), frame_number);
+  SortedKeysIterator it = std::upper_bound(sorted_keys.begin(), sorted_keys.end(), frame_number);
   if (it == sorted_keys.end() || it == sorted_keys.begin()) {
     return -1;
   }

source/blender/blenkernel/intern/grease_pencil_test.cc

@@ -254,4 +254,64 @@ TEST(greasepencil, add_frame_duration_override_null_frames)
   EXPECT_EQ(sorted_keys[2], 11);
 }
 
+TEST(greasepencil, remove_frame_single)
+{
+  Layer layer;
+  layer.add_frame(0, 1);
+  layer.remove_frame(0);
+  EXPECT_EQ(layer.frames().size(), 0);
+}
+
+TEST(greasepencil, remove_frame_first)
+{
+  Layer layer;
+  layer.add_frame(0, 1);
+  layer.add_frame(5, 2);
+  layer.remove_frame(0);
+  EXPECT_EQ(layer.frames().size(), 1);
+  EXPECT_EQ(layer.frames().lookup(5).drawing_index, 2);
+}
+
+TEST(greasepencil, remove_frame_last)
+{
+  Layer layer;
+  layer.add_frame(0, 1);
+  layer.add_frame(5, 2);
+  layer.remove_frame(5);
+  EXPECT_EQ(layer.frames().size(), 1);
+  EXPECT_EQ(layer.frames().lookup(0).drawing_index, 1);
+}
+
+TEST(greasepencil, remove_frame_implicit_hold)
+{
+  Layer layer;
+  layer.add_frame(0, 1, 4);
+  layer.add_frame(5, 2);
+  layer.remove_frame(5);
+  EXPECT_EQ(layer.frames().size(), 2);
+  EXPECT_EQ(layer.frames().lookup(0).drawing_index, 1);
+  EXPECT_TRUE(layer.frames().lookup(4).is_null());
+}
+
+TEST(greasepencil, remove_frame_fixed_duration_end)
+{
+  Layer layer;
+  layer.add_frame(0, 1, 5);
+  layer.add_frame(5, 2);
+  layer.remove_frame(0);
+  EXPECT_EQ(layer.frames().size(), 1);
+  EXPECT_EQ(layer.frames().lookup(5).drawing_index, 2);
+}
+
+TEST(greasepencil, remove_frame_fixed_duration_overwrite_end)
+{
+  Layer layer;
+  layer.add_frame(0, 1, 5);
+  layer.add_frame(5, 2);
+  layer.remove_frame(5);
+  EXPECT_EQ(layer.frames().size(), 2);
+  EXPECT_EQ(layer.frames().lookup(0).drawing_index, 1);
+  EXPECT_TRUE(layer.frames().lookup(5).is_null());
+}
+
 }  // namespace blender::bke::greasepencil::tests