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

GPv3: Set Uniform operator for Thickness and Opacity #114006

Merged
Antonio Vazquez merged 20 commits from antoniov/blender:GPv3_normalize into main 2023-11-08 16:19:10 +01:00
Conversation 3 Commits 20 Files Changed 2 +124
2 changed files with 124 additions and 0 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
scripts/startup/bl_ui/space_view3d.py
View File

@ -5848,6 +5848,10 @@ class VIEW3D_MT_edit_greasepencil_point(Menu):
layout = self.layout
layout.operator("grease_pencil.stroke_smooth", text="Smooth Points")
layout.separator()
layout.operator("grease_pencil.set_uniform_thickness")
layout.operator("grease_pencil.set_uniform_opacity")
class VIEW3D_MT_edit_curves(Menu):
bl_label = "Curves"

120
source/blender/editors/grease_pencil/intern/grease_pencil_edit.cc
View File

@ -15,6 +15,7 @@
#include "BLI_stack.hh"
#include "BKE_context.h"
#include "BKE_curves_utils.hh"
#include "BKE_grease_pencil.hh"
#include "RNA_access.hh"
@ -1005,6 +1006,123 @@ static void GREASE_PENCIL_OT_set_active_material(wmOperatorType *ot)
}
/** \} */
/* -------------------------------------------------------------------- */
/** \name Set stroke uniform Thickness
* \{ */
static int grease_pencil_set_uniform_thickness_exec(bContext *C, wmOperator *op)
{
const Scene *scene = CTX_data_scene(C);
Object *object = CTX_data_active_object(C);
GreasePencil &grease_pencil = *static_cast<GreasePencil *>(object->data);
antoniov marked this conversation as resolved
Hans Goudey commented 2023-11-07 09:15:50 +01:00
Review
Copy Link

Seems that this should be exposed as "Thickness" and then divided by 2 before it's assigned to the points.

Seems that this should be exposed as "Thickness" and then divided by 2 before it's assigned to the points.
Antonio Vazquez commented 2023-11-07 09:32:47 +01:00
Review
Copy Link

I'm not sure about that. The name "Radius" is the same name used in Blender for the brush size. If we use "Thickness" as a name, we are breaking the thickness logic of the Drawing. Actually, the thickness is the result of applying the radius.

@mendio What do you think here?

I'm not sure about that. The name "Radius" is the same name used in Blender for the brush size. If we use "Thickness" as a name, we are breaking the thickness logic of the Drawing. Actually, the thickness is the result of applying the radius. @mendio What do you think here?
Hans Goudey commented 2023-11-07 09:36:49 +01:00
Review
Copy Link

The thickness is twice the radius. And the operator is called "Set Thickness"

The thickness is twice the radius. And the operator is called "Set Thickness"
Antonio Vazquez commented 2023-11-07 09:42:49 +01:00
Review
Copy Link

Yes, agree. What I mean is, if we use here Thickness then maybe we need change also Radius in the Draw operator, because the "thickness" of the stroke in DRaw mode is set using "radius" parameter.

My concern is to be sure we keep consistency, so I will wait, before change the name, for @mendio because he has been working all these years to keep consistency, so his opinion is important here. If all agree I can change it easily.

Yes, agree. What I mean is, if we use here Thickness then maybe we need change also Radius in the Draw operator, because the "thickness" of the stroke in DRaw mode is set using "radius" parameter. My concern is to be sure we keep consistency, so I will wait, before change the name, for @mendio because he has been working all these years to keep consistency, so his opinion is important here. If all agree I can change it easily.
Matias Mendiola commented 2023-11-07 15:58:16 +01:00
Review
Copy Link

For the UI we use Radius and Strength names only in brush settings, once applied, it becomes Stroke Thickness and Stroke Opacity. So yes, Thickness fot the parameter is correct in this case because we are affecting an already existing stroke.

For the UI we use Radius and Strength names only in brush settings, once applied, it becomes Stroke Thickness and Stroke Opacity. So yes, Thickness fot the parameter is correct in this case because we are affecting an already existing stroke.
Antonio Vazquez commented 2023-11-07 15:58:44 +01:00
Review
Copy Link

I have talked with @mendio and we agree to use Thickness instead of Radius.

I have talked with @mendio and we agree to use Thickness instead of Radius.
/* Radius is half of the thickness. */
const float radius = RNA_float_get(op->ptr, "thickness") * 0.5f;
bool changed = false;
antoniov marked this conversation as resolved Outdated
Hans Goudey commented 2023-10-31 17:31:16 +01:00
Outdated
Review
Copy Link

Retrieve the attribute once outside of the loop, then affect it inside, without checking the mode.
Retrieving the attribute by name is very slow compared to everything else, and it also isn't threadsafe when retrieving with write access.

Retrieve the attribute once outside of the loop, then affect it inside, without checking the mode. Retrieving the attribute by name is very slow compared to everything else, and it also isn't threadsafe when retrieving with write access.
const Array<MutableDrawingInfo> drawings = retrieve_editable_drawings(*scene, grease_pencil);
antoniov marked this conversation as resolved Outdated
Hans Goudey commented 2023-10-31 19:02:34 +01:00
Outdated
Review
Copy Link

Retrieve the necessary attribute in the call to fill_points.

Getting an attribute with write access is potentially expensive. It's important to avoid it when possible.

Retrieve the necessary attribute in the call to `fill_points`. Getting an attribute with write access is potentially expensive. It's important to avoid it when possible.
threading::parallel_for_each(drawings, [&](const MutableDrawingInfo &info) {
bke::CurvesGeometry &curves = info.drawing.strokes_for_write();
IndexMaskMemory memory;
const IndexMask selected_curves = ed::curves::retrieve_selected_curves(curves, memory);
antoniov marked this conversation as resolved Outdated
Hans Goudey commented 2023-10-31 18:04:48 +01:00
Outdated
Review
Copy Link

else if, or even better switch (mode) so that adding a mode without changing this gives a compilation error

`else if`, or even better `switch (mode)` so that adding a mode without changing this gives a compilation error
if (selected_curves.is_empty()) {
return;
}
const OffsetIndices<int> points_by_curve = curves.points_by_curve();
MutableSpan<float> radii = info.drawing.radii_for_write();
bke::curves::fill_points<float>(points_by_curve, selected_curves, radius, radii);
antoniov marked this conversation as resolved Outdated
Falk David commented 2023-10-21 17:24:31 +02:00
Outdated
Review
Copy Link

Use math::clamp(...)

Use `math::clamp(...)`
Antonio Vazquez commented 2023-10-22 13:13:49 +02:00
Outdated
Review
Copy Link

Removed the line because the paramter is filtered already.

Removed the line because the paramter is filtered already.
changed = true;
});
if (changed) {
DEG_id_tag_update(&grease_pencil.id, ID_RECALC_GEOMETRY);
WM_event_add_notifier(C, NC_GEOM | ND_DATA, &grease_pencil);
}
return OPERATOR_FINISHED;
}
static void GREASE_PENCIL_OT_set_uniform_thickness(wmOperatorType *ot)
{
/* Identifiers. */
ot->name = "Set Uniform Thickness";
ot->idname = "GREASE_PENCIL_OT_set_uniform_thickness";
ot->description = "Set all stroke points to same thickness";
/* Callbacks. */
ot->exec = grease_pencil_set_uniform_thickness_exec;
ot->poll = editable_grease_pencil_poll;
/* flags */
ot->flag = OPTYPE_REGISTER | OPTYPE_UNDO;
/* props */
ot->prop = RNA_def_float(
ot->srna, "thickness", 0.1f, 0.0f, 1000.0f, "Thickness", "Thickness", 0.0f, 1000.0f);
}
/** \} */
/* -------------------------------------------------------------------- */
/** \name Set stroke uniform Opacity
* \{ */
static int grease_pencil_set_uniform_opacity_exec(bContext *C, wmOperator *op)
{
const Scene *scene = CTX_data_scene(C);
Object *object = CTX_data_active_object(C);
GreasePencil &grease_pencil = *static_cast<GreasePencil *>(object->data);
const float opacity = RNA_float_get(op->ptr, "opacity");
bool changed = false;
const Array<MutableDrawingInfo> drawings = retrieve_editable_drawings(*scene, grease_pencil);
threading::parallel_for_each(drawings, [&](const MutableDrawingInfo &info) {
bke::CurvesGeometry &curves = info.drawing.strokes_for_write();
IndexMaskMemory memory;
const IndexMask selected_curves = ed::curves::retrieve_selected_curves(curves, memory);
if (selected_curves.is_empty()) {
return;
}
const OffsetIndices<int> points_by_curve = curves.points_by_curve();
MutableSpan<float> opacities = info.drawing.opacities_for_write();
bke::curves::fill_points<float>(points_by_curve, selected_curves, opacity, opacities);
changed = true;
});
if (changed) {
DEG_id_tag_update(&grease_pencil.id, ID_RECALC_GEOMETRY);
WM_event_add_notifier(C, NC_GEOM | ND_DATA, &grease_pencil);
}
return OPERATOR_FINISHED;
}
static void GREASE_PENCIL_OT_set_uniform_opacity(wmOperatorType *ot)
{
/* Identifiers. */
ot->name = "Set Uniform Opacity";
ot->idname = "GREASE_PENCIL_OT_set_uniform_opacity";
ot->description = "Set all stroke points to same opacity";
/* Callbacks. */
ot->exec = grease_pencil_set_uniform_opacity_exec;
ot->poll = editable_grease_pencil_poll;
/* flags */
ot->flag = OPTYPE_REGISTER | OPTYPE_UNDO;
/* props */
ot->prop = RNA_def_float(ot->srna, "opacity", 1.0f, 0.0f, 1.0f, "Opacity", "", 0.0f, 1.0f);
}
/** \} */
/* -------------------------------------------------------------------- */
/** \name Switch Direction Operator
* \{ */
@ -1070,6 +1188,8 @@ void ED_operatortypes_grease_pencil_edit()
WM_operatortype_append(GREASE_PENCIL_OT_cyclical_set);
WM_operatortype_append(GREASE_PENCIL_OT_set_active_material);
WM_operatortype_append(GREASE_PENCIL_OT_stroke_switch_direction);
WM_operatortype_append(GREASE_PENCIL_OT_set_uniform_thickness);
WM_operatortype_append(GREASE_PENCIL_OT_set_uniform_opacity);
}
void ED_keymap_grease_pencil(wmKeyConfig *keyconf)