blender
/
blender
136
407
Fork 589
Code Issues 6.3k Pull Requests 767 Projects 19 Wiki Activity

GPv3: Hide Others operator #115038

Merged
Antonio Vazquez merged 8 commits from antoniov/blender:GPV3_hide_others into main 2023-11-23 16:03:39 +01:00
Conversation 0 Commits 8 Files Changed 2 +61
2 changed files with 61 additions and 0 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
scripts/startup/bl_ui/properties_material_gpencil.py
View File

@ -19,6 +19,7 @@ class GPENCIL_MT_material_context_menu(Menu):
layout = self.layout
if context.preferences.experimental.use_grease_pencil_version3:
layout.operator("grease_pencil.material_reveal", icon='RESTRICT_VIEW_OFF', text="Show All")
layout.operator("grease_pencil.material_hide", icon='RESTRICT_VIEW_ON', text="Hide Others").invert = True
layout.separator()

60
source/blender/editors/grease_pencil/intern/grease_pencil_material.cc
View File

@ -16,6 +16,9 @@
#include "ED_grease_pencil.hh"
#include "RNA_access.hh"
#include "RNA_define.hh"
#include "WM_api.hh"
namespace blender::ed::greasepencil {
@ -63,6 +66,62 @@ static void GREASE_PENCIL_OT_material_reveal(wmOperatorType *ot)
/** \} */
/* -------------------------------------------------------------------- */
/** \name Hide Others Materials Operator
* \{ */
static int grease_pencil_material_hide_exec(bContext *C, wmOperator *op)
{
Object *object = CTX_data_active_object(C);
GreasePencil &grease_pencil = *static_cast<GreasePencil *>(object->data);
const bool invert = RNA_boolean_get(op->ptr, "invert");
bool changed = false;
const int material_index = object->actcol - 1;
for (const int i : IndexRange(object->totcol)) {
if (invert && i == material_index) {
continue;
}
antoniov marked this conversation as resolved Outdated
Hans Goudey commented 2023-11-23 15:42:40 +01:00
Outdated
Review
Copy Link

Putting object->actcol - 1 in a variable would make this look a little simpler.

Putting `object->actcol - 1` in a variable would make this look a little simpler.
if (!invert && i != material_index) {
continue;
}
antoniov marked this conversation as resolved Outdated
Falk David commented 2023-11-17 11:19:42 +01:00
Outdated
Review
Copy Link

I think this should use BKE_object_material_get. It doesn't seem correct to change the default material in case BKE_object_material_get returns nullptr.

I think this should use `BKE_object_material_get`. It doesn't seem correct to change the default material in case `BKE_object_material_get` returns `nullptr`.
if (Material *ma = BKE_object_material_get(object, i + 1)) {
antoniov marked this conversation as resolved Outdated
Falk David commented 2023-11-17 11:21:13 +01:00
Outdated
Review
Copy Link

Since this should not be nullptr there should be an BLI_assert(gp_style != nullptr) here.

Since this should not be `nullptr` there should be an `BLI_assert(gp_style != nullptr)` here.
MaterialGPencilStyle &gp_style = *ma->gp_style;
gp_style.flag |= GP_MATERIAL_HIDE;
DEG_id_tag_update(&ma->id, ID_RECALC_COPY_ON_WRITE);
changed = true;
}
}
if (changed) {
DEG_id_tag_update(&grease_pencil.id, ID_RECALC_GEOMETRY);
WM_event_add_notifier(C, NC_GEOM | ND_DATA | NA_EDITED, &grease_pencil);
}
return OPERATOR_FINISHED;
}
static void GREASE_PENCIL_OT_material_hide(wmOperatorType *ot)
{
/* Identifiers. */
antoniov marked this conversation as resolved Outdated
Falk David commented 2023-11-17 11:15:36 +01:00
Outdated
Review
Copy Link

This seems like a copy-paste error.

This seems like a copy-paste error.
ot->name = "Hide Materials";
ot->idname = "GREASE_PENCIL_OT_material_hide";
antoniov marked this conversation as resolved Outdated
Falk David commented 2023-11-17 11:26:51 +01:00
Outdated
Review
Copy Link

The operator description seems to suggest that only selected/unselected materials get hidden, but the code hides the active material. That's counter intuitive to me. Maybe this should just say "Hide active/inactive Grease Pencil material(s)" ?

The operator description seems to suggest that only selected/unselected materials get hidden, but the code hides the active material. That's counter intuitive to me. Maybe this should just say "Hide active/inactive Grease Pencil material(s)" ?
ot->description = "Hide active/inactive Grease Pencil material(s)";
/* Callbacks. */
ot->exec = grease_pencil_material_hide_exec;
ot->poll = active_grease_pencil_poll;
ot->flag = OPTYPE_REGISTER | OPTYPE_UNDO;
/* props */
RNA_def_boolean(
antoniov marked this conversation as resolved Outdated
Falk David commented 2023-11-17 11:32:34 +01:00
Outdated
Review
Copy Link

Maybe "invert" would work better ? And "Hide inactive materials instead of the active one" ?

Maybe "invert" would work better ? And "Hide inactive materials instead of the active one" ?
ot->srna, "invert", false, "Invert", "Hide inactive materials instead of the active one");
}
/** \} */
/* -------------------------------------------------------------------- */
/** \name Lock All Materials Operator
* \{ */
@ -200,6 +259,7 @@ void ED_operatortypes_grease_pencil_material()
{
using namespace blender::ed::greasepencil;
WM_operatortype_append(GREASE_PENCIL_OT_material_reveal);
WM_operatortype_append(GREASE_PENCIL_OT_material_hide);
WM_operatortype_append(GREASE_PENCIL_OT_material_lock_all);
WM_operatortype_append(GREASE_PENCIL_OT_material_unlock_all);
WM_operatortype_append(GREASE_PENCIL_OT_material_lock_unused);