GPv3: Opacity modifier #116946

Merged
Lukas Tönne merged 52 commits from LukasTonne/blender:gp3-opacity-modifier into main 2024-01-16 16:56:22 +01:00
3 changed files with 152 additions and 13 deletions
Showing only changes of commit b3f163ee63 - Show all commits

View File

@ -2547,10 +2547,26 @@ typedef struct GreasePencilOpacityModifierData {
GreasePencilModifierInfluenceData influence;
/** GreasePencilOpacityModifierFlag */
int flag;
char _pad1[4];
/** GreasePencilModifierColorMode */
char color_mode;
char _pad1[3];
float color_factor;
float hardness_factor;
void *_pad2;
} GreasePencilOpacityModifierData;
/** Which attributes are affected by color modifiers. */
typedef enum GreasePencilModifierColorMode {
MOD_GREASE_PENCIL_COLOR_STROKE = 0,
MOD_GREASE_PENCIL_COLOR_FILL = 1,
MOD_GREASE_PENCIL_COLOR_BOTH = 2,
MOD_GREASE_PENCIL_COLOR_HARDNESS = 3,
} GreasePencilModifierColorMode;
typedef enum GreasePencilOpacityModifierFlag {
MOD_GREASE_PENCIL_OPACITY_OPEN_INFLUENCE_PANEL = (1 << 0),
/* Use vertex group as opacity factors instead of influence. */
MOD_GREASE_PENCIL_OPACITY_USE_VERTEX_GROUP = (1 << 1),
/* Set the opacity for every point in a stroke, otherwise multiply existing opacity. */
MOD_GREASE_PENCIL_OPACITY_USE_UNIFORM_OPACITY = (1 << 2),
} GreasePencilOpacityModifierFlag;

View File

@ -7635,6 +7635,7 @@ static void rna_def_modifier_grease_pencil_opacity(BlenderRNA *brna)
// TODO
RNA_define_lib_overridable(false);
}
void RNA_def_modifier(BlenderRNA *brna)
{

View File

@ -12,6 +12,7 @@
#include "DNA_modifier_types.h"
#include "DNA_scene_types.h"
#include "BKE_colortools.hh"
#include "BKE_curves.hh"
#include "BKE_geometry_set.hh"
#include "BKE_grease_pencil.hh"
@ -94,28 +95,149 @@ static void foreach_ID_link(ModifierData *md, Object *ob, IDWalkFunc walk, void
greasepencil::foreach_influence_ID_link(&omd->influence, ob, walk, user_data);
}
LukasTonne marked this conversation as resolved Outdated

Use the * operator overload to just store the VArray directly here

Use the * operator overload to just store the VArray directly here
/* XXX Placeholder for vertex groupn weights. */
static VArray<float> get_grease_pencil_modifier_vertex_weights(
const bke::CurvesGeometry &curves, const GreasePencilModifierVertexGroupData &vertex_group)
{
const bool use_vertex_group = (vertex_group.vertex_group_name[0] != '\0');
return VArray<float>::ForSingle(use_vertex_group ? 1.0f : 0.0f, curves.point_num);
}
static void modify_stroke_color(const GreasePencilOpacityModifierData &omd,
bke::CurvesGeometry &curves,
const IndexMask &curves_mask)
{
const bool use_uniform_opacity = (omd.flag & MOD_GREASE_PENCIL_OPACITY_USE_UNIFORM_OPACITY);
const bool use_vgroup_opacity = (omd.flag & MOD_GREASE_PENCIL_OPACITY_USE_VERTEX_GROUP);
const bool invert_vertex_group = (omd.influence.vertex_group.flag &
GREASE_PENCIL_INFLUENCE_INVERT_VERTEX_GROUP);
const bool use_curve = (omd.influence.custom_curve.flag &
GREASE_PENCIL_INFLUENCE_USE_CUSTOM_CURVE);
const OffsetIndices<int> points_by_curve = curves.points_by_curve();
const VArray<float> vgroup_weights = get_grease_pencil_modifier_vertex_weights(
curves, omd.influence.vertex_group);
bke::MutableAttributeAccessor attributes = curves.attributes_for_write();
bke::SpanAttributeWriter<float> opacities = attributes.lookup_or_add_for_write_span<float>(
"opacity", bke::AttrDomain::Point);
for (const int64_t i : curves_mask.index_range()) {
const int64_t curve_i = curves_mask[i];
LukasTonne marked this conversation as resolved Outdated

remove this and pass vgroup_weight to clamp fn? 😅

remove this and pass `vgroup_weight` to clamp fn? 😅

Haha yeah, this is the result of me trying to make sense of the logic of the old modifier.

Haha yeah, this is the result of me trying to make sense of the logic of the old modifier.
const IndexRange points_range = points_by_curve[curve_i];
for (const int64_t point_i : points_range) {
const float curve_input = points_range.size() >= 2 ? (float(point_i - points_range.first()) /
float(points_range.size() - 1)) :
0.0f;
const float curve_factor = use_curve ?
BKE_curvemapping_evaluateF(
omd.influence.custom_curve.curve, 0, curve_input) :

Wondering if this could be made much clean by using DefaultMixer?

Wondering if this could be made much clean by using `DefaultMixer`?

That seems a bit unnecessary here.

  • The attributes are all floats, no need to handle different types.
  • The math is weird and undocumented. In the "uniform" mode it uses 1 - current as some kind of cutoff point, and then allows ramping up the influence to 2.0, presumably so one can reach full opacity/hardness again. It all seems very ad-hoc.

I'd prefer to not try and "fix" this too much, since the primary purpose is compatibility with GP2.

That seems a bit unnecessary here. - The attributes are all floats, no need to handle different types. - The math is weird and undocumented. In the "uniform" mode it uses `1 - current` as some kind of cutoff point, and then allows ramping up the influence to 2.0, presumably so one can reach full opacity/hardness again. It all seems very ad-hoc. I'd prefer to not try and "fix" this too much, since the primary purpose is compatibility with GP2.
1.0f;
if (use_uniform_opacity) {
opacities.span[point_i] = std::clamp(curve_factor, 0.0f, 1.0f);
}
else if (use_vgroup_opacity) {
/* Use vertex group weights as opacity factors. */
const float vgroup_weight = vgroup_weights[point_i];
const float point_factor = vgroup_weight;
opacities.span[point_i] = std::clamp(
opacities.span[point_i] + omd.color_factor * curve_factor * point_factor - 1.0f,
0.0f,
1.0f);
}
else {
/* Use vertex group weights as influence factors. */
const float vgroup_weight = vgroup_weights[point_i];
const float vgroup_influence = invert_vertex_group ? 1.0f - vgroup_weight : vgroup_weight;
opacities.span[point_i] = std::clamp(
omd.color_factor * curve_factor * vgroup_influence, 0.0f, 1.0f);
}
}
}
opacities.finish();
}
static void modify_fill_color(const GreasePencilOpacityModifierData &omd,
bke::CurvesGeometry &curves,
const IndexMask &curves_mask)
LukasTonne marked this conversation as resolved
Review

points_range -> points is the cononical variable name here. "range" is already described by the type

`points_range` -> `points` is the cononical variable name here. "range" is already described by the type
{
LukasTonne marked this conversation as resolved
Review

You can count on the fact that curves always have at least one point

You can count on the fact that curves always have at least one point
Review

At least one or at least two? I need two for this length factor calculation.

At least one or at least two? I need two for this length factor calculation.
Review

Falk says curves can have 1 point only, so i still need to check.

Falk says curves can have 1 point only, so i still need to check.
const bool use_vgroup_opacity = (omd.flag & MOD_GREASE_PENCIL_OPACITY_USE_VERTEX_GROUP);
const bool invert_vertex_group = (omd.influence.vertex_group.flag &
GREASE_PENCIL_INFLUENCE_INVERT_VERTEX_GROUP);
const OffsetIndices<int> points_by_curve = curves.points_by_curve();
const VArray<float> vgroup_weights = get_grease_pencil_modifier_vertex_weights(
curves, omd.influence.vertex_group);
bke::MutableAttributeAccessor attributes = curves.attributes_for_write();
/* Fill color opacity per stroke. */
bke::SpanAttributeWriter<float> fill_opacities = attributes.lookup_or_add_for_write_span<float>(
"fill_opacity", bke::AttrDomain::Curve);
for (const int64_t i : curves_mask.index_range()) {
const int64_t curve_i = curves_mask[i];
if (use_vgroup_opacity) {
/* Use the first stroke point as vertex weight. */
const IndexRange points_range = points_by_curve[curve_i];
const float stroke_weight = points_range.is_empty() ? 1.0f :
LukasTonne marked this conversation as resolved
Review

Could this be skipped if there's no hardness attribute already?

Could this be skipped if there's no hardness attribute already?
Review

I suppose it could. The old modifier is quite inconsistent in how the "uniform" setting is applied (confusingly called "normalized" internally). In the color modes (stroke, fill) it then applies the settings as an offset rather than a factor, with some unexplained bias and clamping. But this isn't used for the hardness mode, so 🤷

I suppose it could. The old modifier is quite inconsistent in how the "uniform" setting is applied (confusingly called "normalized" internally). In the color modes (stroke, fill) it then applies the settings as an _offset_ rather than a factor, with some unexplained bias and clamping. But this isn't used for the hardness mode, so 🤷
vgroup_weights[points_range.first()];
const float stroke_influence = invert_vertex_group ? 1.0f - stroke_weight : stroke_weight;
fill_opacities.span[curve_i] = std::clamp(stroke_influence, 0.0f, 1.0f);
}
else {
fill_opacities.span[curve_i] = std::clamp(omd.color_factor, 0.0f, 1.0f);
}
}
fill_opacities.finish();
}
static void modify_hardness(const GreasePencilOpacityModifierData &omd,
bke::CurvesGeometry &curves,
const IndexMask &curves_mask)
LukasTonne marked this conversation as resolved
Review

C++ style casts (elsewhere in this file too)

C++ style casts (elsewhere in this file too)
{
bke::MutableAttributeAccessor attributes = curves.attributes_for_write();
bke::SpanAttributeWriter<float> hardnesses = attributes.lookup_or_add_for_write_span<float>(
"hardness", bke::AttrDomain::Curve);
for (const int64_t i : curves_mask.index_range()) {
const int64_t curve_i = curves_mask[i];
hardnesses.span[curve_i] = std::clamp(
hardnesses.span[curve_i] * omd.hardness_factor, 0.0f, 1.0f);
}
hardnesses.finish();
}
static void modify_curves(ModifierData *md,
const ModifierEvalContext *ctx,
bke::CurvesGeometry &curves)
{
GreasePencilOpacityModifierData *omd = (GreasePencilOpacityModifierData *)md;
bke::MutableAttributeAccessor attributes = curves.attributes_for_write();
bke::SpanAttributeWriter<float> opacities = attributes.lookup_or_add_for_write_span<float>(
"opacity", bke::AttrDomain::Point);
OffsetIndices<int> points_by_curve = curves.points_by_curve();
IndexMaskMemory mask_memory;
IndexMask curves_mask = greasepencil::get_filtered_stroke_mask(
ctx->object, curves, omd->influence.material_filter, mask_memory);
for (const int64_t i : curves_mask.index_range()) {
const int64_t curve_i = curves_mask[i];
for (const int64_t point_i : points_by_curve[curve_i]) {
opacities.span[point_i] *= 0.5f;
}
}
opacities.finish();
switch (omd->color_mode) {
case MOD_GREASE_PENCIL_COLOR_STROKE:
modify_stroke_color(*omd, curves, curves_mask);
break;
case MOD_GREASE_PENCIL_COLOR_FILL:
modify_fill_color(*omd, curves, curves_mask);
break;
case MOD_GREASE_PENCIL_COLOR_BOTH:
modify_stroke_color(*omd, curves, curves_mask);
modify_fill_color(*omd, curves, curves_mask);
break;
case MOD_GREASE_PENCIL_COLOR_HARDNESS:
modify_hardness(*omd, curves, curves_mask);
break;
}
}
LukasTonne marked this conversation as resolved Outdated

threading::parallel_for ?

`threading::parallel_for` ?
static void modify_geometry_set(ModifierData *md,