Fix #142056: Grease Pencil: Assert on duplicating layer

The asserts were added in 45ab790e80
for better error detection.

The assert was happening because the duplicated layer
was referencing frames of the source layer by default,
causing the actual user counts of the drawings to be
in an invalid state (they they were getting fixed
after the call to `duplicate_layer`, but they could remain
invalid! ).

To fix this, we ensure that the user counts are correct
after calling `duplicate_layer`. New parameters are added
to allow duplicating the frames (with empty drawings) or
duplicating the frames and the drawings.

Pull Request: https://projects.blender.org/blender/blender/pulls/142201
This commit is contained in:
Falk David
2025-07-18 10:37:46 +02:00
committed by Falk David
parent 67ffbcd0ee
commit d2f4d80ac9
3 changed files with 73 additions and 83 deletions

View File

@@ -3772,7 +3772,9 @@ void GreasePencil::add_layers_for_eval(const int num_new_layers)
}
blender::bke::greasepencil::Layer &GreasePencil::duplicate_layer(
const blender::bke::greasepencil::Layer &duplicate_layer)
const blender::bke::greasepencil::Layer &duplicate_layer,
const bool duplicate_frames,
const bool duplicate_drawings)
{
using namespace blender;
std::string unique_name = unique_layer_name(duplicate_layer.name());
@@ -3792,6 +3794,25 @@ blender::bke::greasepencil::Layer &GreasePencil::duplicate_layer(
attr.finish();
});
/* When a layer is duplicated, the frames are shared by default. Clear the frames, to ensure a
* valid state. */
new_layer->frames_for_write().clear();
if (duplicate_frames) {
for (auto [frame_number, frame] : duplicate_layer.frames().items()) {
const int duration = duplicate_layer.get_frame_duration_at(frame_number);
bke::greasepencil::Drawing *dst_drawing = this->insert_frame(
*new_layer, frame_number, duration, eBezTriple_KeyframeType(frame.type));
if (duplicate_drawings) {
BLI_assert(dst_drawing != nullptr);
/* TODO: This can fail (return `nullptr`) if the drawing is a drawing reference! */
const bke::greasepencil::Drawing &src_drawing = *this->get_drawing_at(duplicate_layer,
frame_number);
/* Duplicate the drawing. */
*dst_drawing = src_drawing;
}
}
}
this->update_drawing_users_for_layer(*new_layer);
new_layer->set_name(unique_name);
return *new_layer;
@@ -3799,10 +3820,13 @@ blender::bke::greasepencil::Layer &GreasePencil::duplicate_layer(
blender::bke::greasepencil::Layer &GreasePencil::duplicate_layer(
blender::bke::greasepencil::LayerGroup &parent_group,
const blender::bke::greasepencil::Layer &duplicate_layer)
const blender::bke::greasepencil::Layer &duplicate_layer,
const bool duplicate_frames,
const bool duplicate_drawings)
{
using namespace blender;
bke::greasepencil::Layer &new_layer = this->duplicate_layer(duplicate_layer);
bke::greasepencil::Layer &new_layer = this->duplicate_layer(
duplicate_layer, duplicate_frames, duplicate_drawings);
move_node_into(new_layer.as_node(), parent_group);
return new_layer;
}

View File

@@ -649,27 +649,14 @@ static wmOperatorStatus grease_pencil_layer_duplicate_exec(bContext *C, wmOperat
/* Duplicate layer. */
Layer &active_layer = *grease_pencil.get_active_layer();
Layer &new_layer = grease_pencil.duplicate_layer(active_layer);
const bool duplicate_frames = true;
const bool duplicate_drawings = !empty_keyframes;
Layer &new_layer = grease_pencil.duplicate_layer(
active_layer, duplicate_frames, duplicate_drawings);
WM_msg_publish_rna_prop(
CTX_wm_message_bus(C), &grease_pencil.id, &grease_pencil, GreasePencilv3, layers);
/* Clear source keyframes and recreate them with duplicated drawings. */
new_layer.frames_for_write().clear();
for (auto [frame_number, frame] : active_layer.frames().items()) {
const int duration = active_layer.get_frame_duration_at(frame_number);
Drawing *dst_drawing = grease_pencil.insert_frame(
new_layer, frame_number, duration, eBezTriple_KeyframeType(frame.type));
if (!empty_keyframes) {
BLI_assert(dst_drawing != nullptr);
/* TODO: This can fail (return `nullptr`) if the drawing is a drawing reference! */
const Drawing &src_drawing = *grease_pencil.get_drawing_at(active_layer, frame_number);
/* Duplicate the drawing. */
*dst_drawing = src_drawing;
}
}
grease_pencil.move_node_after(new_layer.as_node(), active_layer.as_node());
grease_pencil.set_active_layer(&new_layer);
@@ -1099,73 +1086,48 @@ enum class DuplicateCopyMode {
Active,
};
static void duplicate_layer_and_frames(GreasePencil &dst_grease_pencil,
const GreasePencil &src_grease_pencil,
const blender::bke::greasepencil::Layer &src_layer,
const DuplicateCopyMode copy_frame_mode,
const int current_frame)
static void copy_layer_and_frames_to_target_object(
GreasePencil &dst_grease_pencil,
const GreasePencil &src_grease_pencil,
const blender::bke::greasepencil::Layer &src_layer,
const DuplicateCopyMode copy_frame_mode,
const int current_frame)
{
using namespace blender::bke::greasepencil;
BLI_assert(&src_grease_pencil != &dst_grease_pencil);
if (&dst_grease_pencil == &src_grease_pencil) {
/* Duplicating frames is valid if copying from the same object.
* The resulting frames will reference existing drawings, which is more efficient than making
* full copies. */
Layer &dst_layer = dst_grease_pencil.duplicate_layer(src_layer);
/* When copying from another object a new layer is created and all drawings are copied. */
const int src_layer_index = *src_grease_pencil.get_layer_index(src_layer);
dst_layer.frames_for_write().clear();
for (const auto [frame_number, frame] : src_layer.frames().items()) {
if ((copy_frame_mode == DuplicateCopyMode::Active) &&
(&frame != src_layer.frame_at(current_frame)))
{
continue;
}
const int duration = src_layer.get_frame_duration_at(frame_number);
Layer &dst_layer = dst_grease_pencil.add_layer(src_layer.name());
const int dst_layer_index = dst_grease_pencil.layers().size() - 1;
Drawing *dst_drawing = dst_grease_pencil.insert_frame(
dst_layer, frame_number, duration, eBezTriple_KeyframeType(frame.type));
if (dst_drawing != nullptr) {
/* Duplicate drawing. */
const Drawing &src_drawing = *src_grease_pencil.get_drawing_at(src_layer, frame_number);
*dst_drawing = src_drawing;
}
BKE_grease_pencil_copy_layer_parameters(src_layer, dst_layer);
const bke::AttributeAccessor src_attributes = src_grease_pencil.attributes();
bke::MutableAttributeAccessor dst_attributes = dst_grease_pencil.attributes_for_write();
src_attributes.foreach_attribute([&](const bke::AttributeIter &iter) {
if (iter.domain != bke::AttrDomain::Layer) {
return;
}
}
else {
/* When copying from another object a new layer is created and all drawings are copied. */
const int src_layer_index = *src_grease_pencil.get_layer_index(src_layer);
Layer &dst_layer = dst_grease_pencil.add_layer(src_layer.name());
const int dst_layer_index = dst_grease_pencil.layers().size() - 1;
BKE_grease_pencil_copy_layer_parameters(src_layer, dst_layer);
const bke::AttributeAccessor src_attributes = src_grease_pencil.attributes();
bke::MutableAttributeAccessor dst_attributes = dst_grease_pencil.attributes_for_write();
src_attributes.foreach_attribute([&](const bke::AttributeIter &iter) {
if (iter.domain != bke::AttrDomain::Layer) {
return;
}
bke::GAttributeReader reader = src_attributes.lookup(iter.name, iter.domain, iter.data_type);
BLI_assert(reader);
bke::GAttributeWriter writer = dst_attributes.lookup_or_add_for_write(
iter.name, iter.domain, iter.data_type);
if (writer) {
const CPPType &cpptype = bke::attribute_type_to_cpp_type(iter.data_type);
BUFFER_FOR_CPP_TYPE_VALUE(cpptype, buffer);
reader.varray.get(src_layer_index, buffer);
writer.varray.set_by_copy(dst_layer_index, buffer);
}
writer.finish();
});
std::optional<int> frame_select = std::nullopt;
if (copy_frame_mode == DuplicateCopyMode::Active) {
frame_select = current_frame;
bke::GAttributeReader reader = src_attributes.lookup(iter.name, iter.domain, iter.data_type);
BLI_assert(reader);
bke::GAttributeWriter writer = dst_attributes.lookup_or_add_for_write(
iter.name, iter.domain, iter.data_type);
if (writer) {
const CPPType &cpptype = bke::attribute_type_to_cpp_type(iter.data_type);
BUFFER_FOR_CPP_TYPE_VALUE(cpptype, buffer);
reader.varray.get(src_layer_index, buffer);
writer.varray.set_by_copy(dst_layer_index, buffer);
}
dst_grease_pencil.copy_frames_from_layer(
dst_layer, src_grease_pencil, src_layer, frame_select);
writer.finish();
});
std::optional<int> frame_select = std::nullopt;
if (copy_frame_mode == DuplicateCopyMode::Active) {
frame_select = current_frame;
}
dst_grease_pencil.copy_frames_from_layer(dst_layer, src_grease_pencil, src_layer, frame_select);
}
static wmOperatorStatus grease_pencil_layer_duplicate_object_exec(bContext *C, wmOperator *op)
@@ -1186,12 +1148,12 @@ static wmOperatorStatus grease_pencil_layer_duplicate_object_exec(bContext *C, w
if (only_active) {
const Layer &active_layer = *src_grease_pencil.get_active_layer();
duplicate_layer_and_frames(
copy_layer_and_frames_to_target_object(
dst_grease_pencil, src_grease_pencil, active_layer, copy_frame_mode, current_frame);
}
else {
for (const Layer *layer : src_grease_pencil.layers()) {
duplicate_layer_and_frames(
copy_layer_and_frames_to_target_object(
dst_grease_pencil, src_grease_pencil, *layer, copy_frame_mode, current_frame);
}
}

View File

@@ -563,11 +563,15 @@ typedef struct GreasePencil {
bool check_name_is_unique = true);
/** Duplicates a layer from the same object to the top of the root group. */
blender::bke::greasepencil::Layer &duplicate_layer(
const blender::bke::greasepencil::Layer &duplicate_layer);
const blender::bke::greasepencil::Layer &duplicate_layer,
bool duplicate_frames = false,
bool duplicate_drawings = false);
/** Duplicates a layer from the same object to the top of the given group. */
blender::bke::greasepencil::Layer &duplicate_layer(
blender::bke::greasepencil::LayerGroup &parent_group,
const blender::bke::greasepencil::Layer &duplicate_layer);
const blender::bke::greasepencil::Layer &duplicate_layer,
bool duplicate_frames = false,
bool duplicate_drawings = false);
/** Add new layer group into the root group. */
blender::bke::greasepencil::LayerGroup &add_layer_group(blender::StringRef name,
bool check_name_is_unique = true);