GPv3: Return std::optional from frame_key_at

Previously, `frame_key_at` would return `-1` for when it couldn't find
a key for the given frame.
This could lead to issues though, because `-1` is a valid key (it is a valid scene frame number).

This changes the API to return an optional instead. This makes it very clear
when no key was found, and should make it less error prone.
This commit is contained in:
Falk David
2024-01-19 14:30:53 +01:00
parent 0602683ec2
commit b6ed5eea8d
5 changed files with 24 additions and 23 deletions

View File

@@ -409,9 +409,9 @@ class Layer : public ::GreasePencilLayer {
bool has_drawing_at(const int frame_number) const;
/**
* \returns the key of the active frame at \a frame_number or -1 if there is no frame.
* \returns the key of the active frame at \a frame_number or std::nullopt if there is no frame.
*/
FramesMapKey frame_key_at(int frame_number) const;
std::optional<FramesMapKey> frame_key_at(int frame_number) const;
/**
* \returns a pointer to the active frame at \a frame_number or nullptr if there is no frame.

View File

@@ -814,16 +814,16 @@ Span<FramesMapKey> Layer::sorted_keys() const
return this->runtime->sorted_keys_cache_.data();
}
FramesMapKey Layer::frame_key_at(const int frame_number) const
std::optional<FramesMapKey> Layer::frame_key_at(const int frame_number) const
{
Span<int> sorted_keys = this->sorted_keys();
/* No keyframes, return no drawing. */
if (sorted_keys.is_empty()) {
return -1;
return {};
}
/* Before the first drawing, return no drawing. */
if (frame_number < sorted_keys.first()) {
return -1;
return {};
}
/* After or at the last drawing, return the last drawing. */
if (frame_number >= sorted_keys.last()) {
@@ -832,21 +832,21 @@ FramesMapKey Layer::frame_key_at(const int frame_number) const
/* Search for the drawing. upper_bound will get the drawing just after. */
SortedKeysIterator it = std::upper_bound(sorted_keys.begin(), sorted_keys.end(), frame_number);
if (it == sorted_keys.end() || it == sorted_keys.begin()) {
return -1;
return {};
}
return *std::prev(it);
}
const GreasePencilFrame *Layer::frame_at(const int frame_number) const
{
const FramesMapKey frame_key = this->frame_key_at(frame_number);
return (frame_key == -1) ? nullptr : this->frames().lookup_ptr(frame_key);
const std::optional<FramesMapKey> frame_key = this->frame_key_at(frame_number);
return frame_key ? this->frames().lookup_ptr(*frame_key) : nullptr;
}
GreasePencilFrame *Layer::frame_at(const int frame_number)
{
const FramesMapKey frame_key = this->frame_key_at(frame_number);
return (frame_key == -1) ? nullptr : this->frames_for_write().lookup_ptr(frame_key);
const std::optional<FramesMapKey> frame_key = this->frame_key_at(frame_number);
return frame_key ? this->frames_for_write().lookup_ptr(*frame_key) : nullptr;
}
int Layer::drawing_index_at(const int frame_number) const
@@ -862,11 +862,11 @@ bool Layer::has_drawing_at(const int frame_number) const
int Layer::get_frame_duration_at(const int frame_number) const
{
const FramesMapKey frame_key = this->frame_key_at(frame_number);
if (frame_key == -1) {
const std::optional<FramesMapKey> frame_key = this->frame_key_at(frame_number);
if (!frame_key) {
return -1;
}
SortedKeysIterator frame_number_it = std::next(this->sorted_keys().begin(), frame_key);
SortedKeysIterator frame_number_it = std::next(this->sorted_keys().begin(), *frame_key);
if (*frame_number_it == this->sorted_keys().last()) {
return -1;
}

View File

@@ -746,14 +746,14 @@ static int grease_pencil_delete_frame_exec(bContext *C, wmOperator *op)
bool changed = false;
if (mode == DeleteFrameMode::ACTIVE_FRAME && grease_pencil.has_active_layer()) {
bke::greasepencil::Layer &layer = *grease_pencil.get_active_layer();
if (layer.is_editable()) {
changed |= grease_pencil.remove_frames(layer, {layer.frame_key_at(current_frame)});
if (layer.is_editable() && layer.frame_key_at(current_frame)) {
changed |= grease_pencil.remove_frames(layer, {*layer.frame_key_at(current_frame)});
}
}
else if (mode == DeleteFrameMode::ALL_FRAMES) {
for (bke::greasepencil::Layer *layer : grease_pencil.layers_for_write()) {
if (layer->is_editable()) {
changed |= grease_pencil.remove_frames(*layer, {layer->frame_key_at(current_frame)});
if (layer->is_editable() && layer->frame_key_at(current_frame)) {
changed |= grease_pencil.remove_frames(*layer, {*layer->frame_key_at(current_frame)});
}
}
}

View File

@@ -156,15 +156,15 @@ static int grease_pencil_stroke_invoke(bContext *C, wmOperator *op, const wmEven
/* If auto-key is on and the drawing at the current frame starts before the current frame a new
* keyframe needs to be inserted. */
if (blender::animrig::is_autokey_on(scene) &&
active_layer.frame_key_at(current_frame) < current_frame)
if (blender::animrig::is_autokey_on(scene) && active_layer.frame_key_at(current_frame) &&
*active_layer.frame_key_at(current_frame) < current_frame)
{
const ToolSettings *ts = CTX_data_tool_settings(C);
if ((ts->gpencil_flags & GP_TOOL_FLAG_RETAIN_LAST) != 0) {
/* For additive drawing, we duplicate the frame that's currently visible and insert it at the
* current frame. */
grease_pencil.insert_duplicate_frame(
active_layer, active_layer.frame_key_at(current_frame), current_frame, false);
active_layer, *active_layer.frame_key_at(current_frame), current_frame, false);
}
else {
/* Otherwise we just insert a blank keyframe at the current frame. */

View File

@@ -795,8 +795,9 @@ static void insert_grease_pencil_key(bAnimContext *ac,
bool changed = false;
if (hold_previous) {
const FramesMapKey active_frame_number = layer->frame_key_at(current_frame_number);
if ((active_frame_number == -1) || layer->frames().lookup(active_frame_number).is_null()) {
const std::optional<FramesMapKey> active_frame_number = layer->frame_key_at(
current_frame_number);
if (!active_frame_number || layer->frames().lookup(*active_frame_number).is_null()) {
/* There is no active frame to hold to, or it's a null frame. Therefore just insert a blank
* frame. */
changed = grease_pencil->insert_blank_frame(
@@ -805,7 +806,7 @@ static void insert_grease_pencil_key(bAnimContext *ac,
else {
/* Duplicate the active frame. */
changed = grease_pencil->insert_duplicate_frame(
*layer, active_frame_number, current_frame_number, false);
*layer, *active_frame_number, current_frame_number, false);
}
}
else {