GPv3: Move to layer #117244

Merged
Falk David merged 42 commits from mendio/blender:GPv3_OP_Move_to_layer into main 2024-02-05 12:03:11 +01:00
7 changed files with 154 additions and 3 deletions

View File

@ -4642,6 +4642,9 @@ def km_grease_pencil_edit_mode(params):
# Active layer
op_menu("GREASE_PENCIL_MT_layer_active", {"type": 'Y', "value": 'PRESS'}),
# Move to layer
op_menu("GREASE_PENCIL_MT_move_to_layer", {"type": 'M', "value": 'PRESS'}),
# Context menu
*_template_items_context_menu("VIEW3D_MT_greasepencil_edit_context_menu", params.context_menu_event),

View File

@ -55,6 +55,7 @@ class DATA_PT_grease_pencil_layers(DataButtonsPanel, Panel):
col = row.column()
sub = col.column(align=True)
sub.operator_context = 'EXEC_DEFAULT'
sub.operator("grease_pencil.layer_add", icon='ADD', text="")
sub.menu("GREASE_PENCIL_MT_grease_pencil_add_layer_extra", icon='DOWNARROW_HLT', text="")

View File

@ -274,6 +274,30 @@ class GPENCIL_MT_layer_active(Menu):
i -= 1
class GREASE_PENCIL_MT_move_to_layer(Menu):
bl_label = "Move to Layer"
def draw(self, context):
layout = self.layout
layout.operator_context = 'INVOKE_REGION_WIN'
grease_pencil = context.active_object.data
mendio marked this conversation as resolved Outdated

obd -> grease_pencil

`obd` -> `grease_pencil`
mendio marked this conversation as resolved Outdated

trailing whitespace

trailing whitespace
layout.operator("grease_pencil.move_to_layer", text="New Layer", icon='ADD').add_new_layer = True
mendio marked this conversation as resolved Outdated

Use layer_add here. This can also be invoked as a popup if that's what you want to do.

Use `layer_add` here. This can also be invoked as a popup if that's what you want to do.

in GPv2 we follow the Move to Collection behavior, when the user wants to move an object to a new collection, the name of the new collection can be typed in a popup window. Maybe I'm wrong but is this not possible with layer_add? At this point always use the name "Layer" for new layers. This seems like a regression from GPv2 because the user has to change the name later in the layer list, which slows down the workflow. This also applies to set Active Layer operator.

in GPv2 we follow the Move to Collection behavior, when the user wants to move an object to a new collection, the name of the new collection can be typed in a popup window. Maybe I'm wrong but is this not possible with `layer_add`? At this point always use the name "Layer" for new layers. This seems like a regression from GPv2 because the user has to change the name later in the layer list, which slows down the workflow. This also applies to set Active Layer operator.

Right, the layer_add is just missing an invoke :)

Try this:

diff --git a/scripts/startup/bl_ui/properties_data_grease_pencil.py b/scripts/startup/bl_ui/properties_data_grease_pencil.py
index b683847299f..0d2499e911c 100644
--- a/scripts/startup/bl_ui/properties_data_grease_pencil.py
+++ b/scripts/startup/bl_ui/properties_data_grease_pencil.py
@@ -55,6 +55,7 @@ class DATA_PT_grease_pencil_layers(DataButtonsPanel, Panel):
 
         col = row.column()
         sub = col.column(align=True)
+        sub.operator_context = 'EXEC_DEFAULT'
         sub.operator("grease_pencil.layer_add", icon='ADD', text="")
         sub.menu("GREASE_PENCIL_MT_grease_pencil_add_layer_extra", icon='DOWNARROW_HLT', text="")
 
diff --git a/source/blender/editors/grease_pencil/intern/grease_pencil_layers.cc b/source/blender/editors/grease_pencil/intern/grease_pencil_layers.cc
index 3fbd8684a7a..fe168226817 100644
--- a/source/blender/editors/grease_pencil/intern/grease_pencil_layers.cc
+++ b/source/blender/editors/grease_pencil/intern/grease_pencil_layers.cc
@@ -76,13 +76,14 @@ static void GREASE_PENCIL_OT_layer_add(wmOperatorType *ot)
   ot->description = "Add a new Grease Pencil layer in the active object";
 
   /* callbacks */
+  ot->invoke = WM_operator_props_popup_confirm;
   ot->exec = grease_pencil_layer_add_exec;
   ot->poll = active_grease_pencil_poll;
 
   ot->flag = OPTYPE_REGISTER | OPTYPE_UNDO;
 
   PropertyRNA *prop = RNA_def_string(
-      ot->srna, "new_layer_name", nullptr, INT16_MAX, "Name", "Name of the new layer");
+      ot->srna, "new_layer_name", "Layer", INT16_MAX, "Name", "Name of the new layer");
   RNA_def_property_flag(prop, PROP_SKIP_SAVE);
   ot->prop = prop;
 }

And then just use layer_add in the menu :)

Right, the `layer_add` is just missing an `invoke` :) Try this: ``` diff diff --git a/scripts/startup/bl_ui/properties_data_grease_pencil.py b/scripts/startup/bl_ui/properties_data_grease_pencil.py index b683847299f..0d2499e911c 100644 --- a/scripts/startup/bl_ui/properties_data_grease_pencil.py +++ b/scripts/startup/bl_ui/properties_data_grease_pencil.py @@ -55,6 +55,7 @@ class DATA_PT_grease_pencil_layers(DataButtonsPanel, Panel): col = row.column() sub = col.column(align=True) + sub.operator_context = 'EXEC_DEFAULT' sub.operator("grease_pencil.layer_add", icon='ADD', text="") sub.menu("GREASE_PENCIL_MT_grease_pencil_add_layer_extra", icon='DOWNARROW_HLT', text="") diff --git a/source/blender/editors/grease_pencil/intern/grease_pencil_layers.cc b/source/blender/editors/grease_pencil/intern/grease_pencil_layers.cc index 3fbd8684a7a..fe168226817 100644 --- a/source/blender/editors/grease_pencil/intern/grease_pencil_layers.cc +++ b/source/blender/editors/grease_pencil/intern/grease_pencil_layers.cc @@ -76,13 +76,14 @@ static void GREASE_PENCIL_OT_layer_add(wmOperatorType *ot) ot->description = "Add a new Grease Pencil layer in the active object"; /* callbacks */ + ot->invoke = WM_operator_props_popup_confirm; ot->exec = grease_pencil_layer_add_exec; ot->poll = active_grease_pencil_poll; ot->flag = OPTYPE_REGISTER | OPTYPE_UNDO; PropertyRNA *prop = RNA_def_string( - ot->srna, "new_layer_name", nullptr, INT16_MAX, "Name", "Name of the new layer"); + ot->srna, "new_layer_name", "Layer", INT16_MAX, "Name", "Name of the new layer"); RNA_def_property_flag(prop, PROP_SKIP_SAVE); ot->prop = prop; } ``` And then just use `layer_add` in the menu :)

perfect, that works, also the set Active Layer operator is working as intended

perfect, that works, also the set Active Layer operator is working as intended

This is also fine: layout.operator("grease_pencil.layer_add", text="New Layer", icon='ADD').new_layer_name = "Layer"

This is also fine: `layout.operator("grease_pencil.layer_add", text="New Layer", icon='ADD').new_layer_name = "Layer"`

When doing this way the Layer add popup shows empty

When doing this way the Layer add popup shows empty

sorry, the problem was a typo

sorry, the problem was a typo
mendio marked this conversation as resolved Outdated

trailing whitespace

trailing whitespace
if not grease_pencil.layers:
return
layout.separator()
for i in range(len(grease_pencil.layers) - 1, -1, -1):
layer = grease_pencil.layers[i]
if layer == grease_pencil.layers.active:
icon = 'GREASEPENCIL'
else:
icon = 'NONE'
layout.operator("grease_pencil.move_to_layer", text=layer.name, icon=icon).target_layer_name = layer.name
class GREASE_PENCIL_MT_layer_active(Menu):
bl_label = "Change Active Layer"
@ -942,6 +966,7 @@ classes = (
GPENCIL_UL_layer,
GPENCIL_UL_masks,
GREASE_PENCIL_MT_move_to_layer,
GREASE_PENCIL_MT_layer_active,
GreasePencilFlipTintColors,

View File

@ -5855,6 +5855,7 @@ class VIEW3D_MT_edit_greasepencil_stroke(Menu):
layout.separator()
layout.menu("GREASE_PENCIL_MT_move_to_layer")
layout.menu("VIEW3D_MT_grease_pencil_assign_material")
layout.operator("grease_pencil.set_active_material")

View File

@ -13,6 +13,7 @@
#include "BLI_math_vector_types.hh"
#include "BLI_span.hh"
#include "BLI_stack.hh"
#include "BLI_string.h"
mendio marked this conversation as resolved
Review

I think this include is not needed.

I think this include is not needed.
#include "BLT_translation.h"
#include "DNA_material_types.h"
@ -34,13 +35,13 @@
#include "ED_curves.hh"
#include "ED_grease_pencil.hh"
#include "ED_screen.hh"
#include "GEO_join_geometries.hh"
#include "GEO_reorder.hh"
#include "GEO_smooth_curves.hh"
#include "GEO_subdivide_curves.hh"
#include "WM_api.hh"
#include "UI_resources.hh"
mendio marked this conversation as resolved Outdated

This should be removed. If a function needs this, it can be added in that function.

This should be removed. If a function needs this, it can be added in that function.
namespace blender::ed::greasepencil {
@ -1693,6 +1694,122 @@ static void GREASE_PENCIL_OT_stroke_reorder(wmOperatorType *ot)
mendio marked this conversation as resolved Outdated

Double ot->poll =

Double `ot->poll =`
/** \} */
/* -------------------------------------------------------------------- */
/** \name Move To Layer Operator
* \{ */

It might make more sense to make this a RNA_def_string with the layer_name. Because the layer that we move to always has to exist.

It might make more sense to make this a `RNA_def_string` with the `layer_name`. Because the layer that we move to always has to exist.

maybe I don't get what is your idea. Right now layer property stores the layer index and that is what we use in grease_pencil_move_to_layer_exec to determine the target layer. The property that there is no longer needed is new_layer_name because now we use layer_add operator to handle adding layers.

maybe I don't get what is your idea. Right now `layer` property stores the layer index and that is what we use in `grease_pencil_move_to_layer_exec` to determine the target layer. The property that there is no longer needed is `new_layer_name` because now we use `layer_add` operator to handle adding layers.

Yes I'm not talking about the new_layer_name. I mean that the layer (or maybe a better name would be target_layer) that we move the selection into could be a string property with the name rather than an index. Maybe that makes the setup for the macro a bit easier (since we'd need to find the index of the newly added layer, but we know the name already).

Yes I'm not talking about the `new_layer_name`. I mean that the `layer` (or maybe a better name would be `target_layer`) that we move the selection into could be a `string` property with the name rather than an index. Maybe that makes the setup for the macro a bit easier (since we'd need to find the index of the newly added layer, but we know the name already).

I see, just updated the grease_pencil_move_to_layer_exec to use an string property.
I guess we only have to tackle the macro. Do you have a sample for me to see?

I see, just updated the `grease_pencil_move_to_layer_exec` to use an string property. I guess we only have to tackle the macro. Do you have a sample for me to see?
static int grease_pencil_move_to_layer_exec(bContext *C, wmOperator *op)
{
mendio marked this conversation as resolved Outdated

This prop should be removed now.

This prop should be removed now.
using namespace blender::bke;
using namespace bke::greasepencil;
const Scene *scene = CTX_data_scene(C);
bool changed = false;
Object *object = CTX_data_active_object(C);
GreasePencil &grease_pencil = *static_cast<GreasePencil *>(object->data);
int target_layer_name_length;
char *target_layer_name = nullptr;
BLI_SCOPED_DEFER([&] { MEM_SAFE_FREE(target_layer_name); });
const bool add_new_layer = RNA_boolean_get(op->ptr, "add_new_layer");
if (add_new_layer) {
Layer &new_layer = grease_pencil.add_layer("Layer");
target_layer_name = BLI_strdup_null(new_layer.name().c_str());
}
else {
target_layer_name = RNA_string_get_alloc(
op->ptr, "target_layer_name", nullptr, 0, &target_layer_name_length);
}
TreeNode *target_node = grease_pencil.find_node_by_name(target_layer_name);
if (target_node == nullptr) {
BKE_reportf(op->reports, RPT_ERROR, "There is no layer '%s'", target_layer_name);
return OPERATOR_CANCELLED;
}
Layer *layer_dst = &target_node->as_layer();
if (layer_dst->is_locked()) {
BKE_reportf(op->reports, RPT_ERROR, "'%s' Layer is locked", target_layer_name);
return OPERATOR_CANCELLED;
}
/* Iterate through all the drawings at current scene frame. */
const Array<MutableDrawingInfo> drawings_src = retrieve_editable_drawings(*scene, grease_pencil);
for (const MutableDrawingInfo &info : drawings_src) {
bke::CurvesGeometry &curves_src = info.drawing.strokes_for_write();
IndexMaskMemory memory;
const IndexMask selected_strokes = ed::curves::retrieve_selected_curves(curves_src, memory);
if (selected_strokes.is_empty()) {
continue;
}
if (!layer_dst->has_drawing_at(info.frame_number)) {
/* Move geometry to a new drawing in target layer. */
grease_pencil.insert_blank_frame(*layer_dst, info.frame_number, 0, BEZT_KEYTYPE_KEYFRAME);
Drawing &drawing_dst = *grease_pencil.get_editable_drawing_at(*layer_dst, info.frame_number);
drawing_dst.strokes_for_write() = bke::curves_copy_curve_selection(
curves_src, selected_strokes, {});
curves_src.remove_curves(selected_strokes, {});
drawing_dst.tag_topology_changed();
}
else {
/* Append geometry to drawing in target layer. */
Drawing &drawing_dst = *grease_pencil.get_editable_drawing_at(*layer_dst, info.frame_number);
bke::CurvesGeometry selected_elems = curves_copy_curve_selection(
curves_src, selected_strokes, {});
Curves *selected_curves = bke::curves_new_nomain(std::move(selected_elems));
Curves *layer_curves = bke::curves_new_nomain(std::move(drawing_dst.strokes_for_write()));
std::array<GeometrySet, 2> geometry_sets{GeometrySet::from_curves(selected_curves),
GeometrySet::from_curves(layer_curves)};
GeometrySet joined = geometry::join_geometries(geometry_sets, {});
drawing_dst.strokes_for_write() = std::move(joined.get_curves_for_write()->geometry.wrap());
curves_src.remove_curves(selected_strokes, {});
drawing_dst.tag_topology_changed();
}
info.drawing.tag_topology_changed();
changed = true;
}
if (changed) {
/* updates */
DEG_id_tag_update(&grease_pencil.id, ID_RECALC_TRANSFORM | ID_RECALC_GEOMETRY);
WM_event_add_notifier(C, NC_GPENCIL | ND_DATA | NA_EDITED, nullptr);
}
return OPERATOR_FINISHED;
}
static void GREASE_PENCIL_OT_move_to_layer(wmOperatorType *ot)
{
PropertyRNA *prop;
/* identifiers. */
ot->name = "Move to Layer";
ot->idname = "GREASE_PENCIL_OT_move_to_layer";
ot->description = "Move selected strokes to another layer";
/* callbacks. */
ot->exec = grease_pencil_move_to_layer_exec;
ot->poll = editable_grease_pencil_poll;
/* flags. */
ot->flag = OPTYPE_REGISTER | OPTYPE_UNDO;
prop = RNA_def_string(
ot->srna, "target_layer_name", "Layer", INT16_MAX, "Name", "Target Grease Pencil Layer");
RNA_def_property_flag(prop, PROP_HIDDEN | PROP_SKIP_SAVE);
prop = RNA_def_boolean(
ot->srna, "add_new_layer", false, "New Layer", "Move selection to a new layer");
RNA_def_property_flag(prop, PROP_HIDDEN | PROP_SKIP_SAVE);
}
/** \} */
} // namespace blender::ed::greasepencil
void ED_operatortypes_grease_pencil_edit()
@ -1715,4 +1832,5 @@ void ED_operatortypes_grease_pencil_edit()
WM_operatortype_append(GREASE_PENCIL_OT_clean_loose);
WM_operatortype_append(GREASE_PENCIL_OT_stroke_subdivide);
WM_operatortype_append(GREASE_PENCIL_OT_stroke_reorder);
WM_operatortype_append(GREASE_PENCIL_OT_move_to_layer);
}

View File

@ -76,13 +76,14 @@ static void GREASE_PENCIL_OT_layer_add(wmOperatorType *ot)
ot->description = "Add a new Grease Pencil layer in the active object";
/* callbacks */
ot->invoke = WM_operator_props_popup_confirm;
ot->exec = grease_pencil_layer_add_exec;
ot->poll = active_grease_pencil_poll;
ot->flag = OPTYPE_REGISTER | OPTYPE_UNDO;
PropertyRNA *prop = RNA_def_string(
ot->srna, "new_layer_name", nullptr, INT16_MAX, "Name", "Name of the new layer");
ot->srna, "new_layer_name", "Layer", INT16_MAX, "Name", "Name of the new layer");
RNA_def_property_flag(prop, PROP_SKIP_SAVE);
ot->prop = prop;
}

View File

@ -16,6 +16,8 @@
#include "ED_keyframes_edit.hh"
#include "WM_api.hh"
mendio marked this conversation as resolved
Review

I think this include is no longer needed

I think this include is no longer needed
struct bContext;
struct Main;
struct Object;