GPv3: Add remove_frame
function to layers #110679
|
@ -258,6 +258,19 @@ 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 frame_number from the frames map. The \a frame_number is expected to
|
||||
* be the first frame number of the frame.
|
||||
*
|
||||
* 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.
|
||||
*
|
||||
* \return true on success.
|
||||
*/
|
||||
bool remove_frame(int frame_number);
|
||||
|
||||
/**
|
||||
* Returns the sorted (start) frame numbers of the frames of this layer.
|
||||
|
@ -287,6 +300,7 @@ class Layer : public ::GreasePencilLayer {
|
|||
private:
|
||||
GreasePencilFrame *add_frame_internal(int frame_number, int drawing_index);
|
||||
int frame_index_at(int frame_number) const;
|
||||
const int *remove_all_null_frames_in_range(const int *begin, const int *end);
|
||||
filedescriptor marked this conversation as resolved
Outdated
|
||||
};
|
||||
|
||||
class LayerGroupRuntime {
|
||||
|
|
|
@ -601,6 +601,17 @@ bool Layer::is_selected() const
|
|||
return ((this->base.flag & GP_LAYER_TREE_NODE_SELECT) != 0);
|
||||
}
|
||||
|
||||
const int *Layer::remove_all_null_frames_in_range(const int *begin, const int *end)
|
||||
{
|
||||
auto next_it = begin;
|
||||
filedescriptor marked this conversation as resolved
Amélie Fondevilla
commented
Just curious, why use Just curious, why use `auto` instead of `int *next_it = begin;` here ?
Is it gonna compute some more fancy iterator type ?
Falk David
commented
I cleaned this up by defining an alias I cleaned this up by defining an alias `SortedKeysIterator ` for `const int *`. That way the whole function (and other parts of the `Layer` class functions) should read better.
|
||||
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);
|
||||
|
@ -641,13 +652,8 @@ GreasePencilFrame *Layer::add_frame(const int 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_all_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,44 @@ GreasePencilFrame *Layer::add_frame(const int frame_number,
|
|||
return frame;
|
||||
}
|
||||
|
||||
bool Layer::remove_frame(const int frame_number)
|
||||
{
|
||||
/* If the frame number is not in the frames map, do nothing. */
|
||||
if (!this->frames().contains(frame_number)) {
|
||||
return false;
|
||||
}
|
||||
if (this->frames().size() == 1) {
|
||||
this->frames_for_write().remove(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. */
|
||||
auto remove_frame_number_it = std::lower_bound(
|
||||
sorted_keys.begin(), sorted_keys.end(), frame_number);
|
||||
/* If there is a next frame: */
|
||||
if (std::next(remove_frame_number_it) != sorted_keys.end()) {
|
||||
auto next_frame_number_it = std::next(remove_frame_number_it);
|
||||
/* While the next frame is a null frame, remove it. */
|
||||
this->remove_all_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()) {
|
||||
auto 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. */
|
||||
filedescriptor marked this conversation as resolved
Outdated
Amélie Fondevilla
commented
Is there something to be done with the drawing here ? Like remove the drawing or decrease user count ? Or is it done at a higher level ? Is there something to be done with the drawing here ? Like remove the drawing or decrease user count ? Or is it done at a higher level ?
Falk David
commented
Good point. I changed it to return true when the frame is replaced. Good point. I changed it to return true when the frame is replaced.
|
||||
if (!prev_frame.is_implicit_hold() && !prev_frame.is_null()) {
|
||||
this->frames_for_write().lookup(frame_number) = GreasePencilFrame::null();
|
||||
return false;
|
||||
}
|
||||
}
|
||||
/* Finally, remove the actual frame. */
|
||||
this->frames_for_write().remove(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) {
|
||||
|
|
|
@ -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
|
||||
|
|
Loading…
Reference in New Issue
Maybe a bit more clear : "Removes null frames starting from \a begin to \a end (excluded), or until a non-null frame is reached."