GPv3: Vertex Weight Angle modifier #117846
No reviewers
Labels
No Label
Interest
Alembic
Interest
Animation & Rigging
Interest
Asset Browser
Interest
Asset Browser Project Overview
Interest
Audio
Interest
Automated Testing
Interest
Blender Asset Bundle
Interest
BlendFile
Interest
Collada
Interest
Compatibility
Interest
Compositing
Interest
Core
Interest
Cycles
Interest
Dependency Graph
Interest
Development Management
Interest
EEVEE
Interest
EEVEE & Viewport
Interest
Freestyle
Interest
Geometry Nodes
Interest
Grease Pencil
Interest
ID Management
Interest
Images & Movies
Interest
Import Export
Interest
Line Art
Interest
Masking
Interest
Metal
Interest
Modeling
Interest
Modifiers
Interest
Motion Tracking
Interest
Nodes & Physics
Interest
OpenGL
Interest
Overlay
Interest
Overrides
Interest
Performance
Interest
Physics
Interest
Pipeline, Assets & IO
Interest
Platforms, Builds & Tests
Interest
Python API
Interest
Render & Cycles
Interest
Render Pipeline
Interest
Sculpt, Paint & Texture
Interest
Text Editor
Interest
Translations
Interest
Triaging
Interest
Undo
Interest
USD
Interest
User Interface
Interest
UV Editing
Interest
VFX & Video
Interest
Video Sequencer
Interest
Virtual Reality
Interest
Vulkan
Interest
Wayland
Interest
Workbench
Interest: X11
Legacy
Blender 2.8 Project
Legacy
Milestone 1: Basic, Local Asset Browser
Legacy
OpenGL Error
Meta
Good First Issue
Meta
Papercut
Meta
Retrospective
Meta
Security
Module
Animation & Rigging
Module
Core
Module
Development Management
Module
EEVEE & Viewport
Module
Grease Pencil
Module
Modeling
Module
Nodes & Physics
Module
Pipeline, Assets & IO
Module
Platforms, Builds & Tests
Module
Python API
Module
Render & Cycles
Module
Sculpt, Paint & Texture
Module
Triaging
Module
User Interface
Module
VFX & Video
Platform
FreeBSD
Platform
Linux
Platform
macOS
Platform
Windows
Priority
High
Priority
Low
Priority
Normal
Priority
Unbreak Now!
Status
Archived
Status
Confirmed
Status
Duplicate
Status
Needs Info from Developers
Status
Needs Information from User
Status
Needs Triage
Status
Resolved
Type
Bug
Type
Design
Type
Known Issue
Type
Patch
Type
Report
Type
To Do
No Milestone
No project
No Assignees
4 Participants
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: blender/blender#117846
Loading…
Reference in New Issue
No description provided.
Delete Branch "ChengduLittleA/blender:gp3-mod-wangle"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Vertex Weight Angle modifier is migrated to Grease Pencil v3.
First pass
@ -0,0 +92,4 @@
modifier::greasepencil::read_influence_data(reader, &mmd->influence);
}
static void deform_drawing(const ModifierData &md,
deform_drawing
->write_weights_for_drawing
?@ -0,0 +109,4 @@
}
bke::MutableAttributeAccessor attributes = curves.attributes_for_write();
bke::SpanAttributeWriter<float> target = attributes.lookup_or_add_for_write_span<float>(mmd.target_vgname,bke::AttrDomain::Point);
Maybe
dst_weights
?@ -0,0 +117,4 @@
mmd.influence.vertex_group_name, bke::AttrDomain::Point, 1.0f);
/* Use default Z up. */
float3 vec_axis(0.0f, 0.0f, 1.0f);
const float3 z_up(0.0f, 0.0f, 1.0f);
@ -0,0 +136,4 @@
const Span<float3> positions = curves.positions();
strokes.foreach_index(GrainSize(512), [&](const int stroke) {
IndexRange points = points_by_curve[stroke];
Use
const
@ -0,0 +142,4 @@
return;
}
for (const int point : points.drop_front(1)) {
float3 p1 = math::transform_point(obmat3x3, positions[point]);
All these vectors can be
const
@ -0,0 +146,4 @@
float3 p2 = math::transform_point(obmat3x3, positions[point - 1]);
float3 vec = p2 - p1;
const float angle = angle_on_axis_v3v3_v3(vec_ref, vec, axis);
float weight = target.span[point] = 1.0f - sin(angle);
Hm this looks strange, is it suppoesd to be just
const float weight =1.0f - math::sin(angle);
?@ -119,0 +120,4 @@
"GREASE_PENCIL_VERTEX_WEIGHT_ANGLE",
ICON_MOD_VERTEX_WEIGHT,
"Vertex Weight Angle",
"Set the vertex group weights based on the angle of the stroke"},
Better to just keep the old description:
Generate vertex weights base on stroke angle
@ -0,0 +96,4 @@
const Object &ob,
bke::greasepencil::Drawing &drawing)
{
auto &mmd = reinterpret_cast<const GPWeightAngleModifierData &>(md);
const auto
@ -0,0 +114,4 @@
if (dst_weights.span.is_empty()) {
return;
}
const VArray<float> input_weights = *attributes.lookup_or_default<float>(
I think this isn't right when the name is set but the vertex group has no data. There's a function added for this case instead:
get_influence_vertex_weights
@ -0,0 +124,4 @@
float3 vec_ref;
/* Apply modifier rotation (sub 90 degrees for Y axis due Z-Up vector). */
const float rot_angle = mmd.angle - ((mmd.axis == 1) ? M_PI_2 : 0.0f);
rotate_normalized_v3_v3v3fl(vec_ref, z_up, axis, rot_angle);
You can use the
AxisAngle
type andto_quaternion
here I thinkLet me try fiddle with it...
I don't think I understood the math here to convert this into new vector api 😅 Can't get it to work correctly
@ -0,0 +159,4 @@
dst_weights.span[point] = math::clamp(dst_weights.span[point], mmd.min_weight, 1.0f);
}
/* First point has the same weight as the second one. */
dst_weights.span[0] = dst_weights.span[1];
Don't you meant the first point of the curve, rather than the first point in all curves?
@ -0,0 +169,4 @@
const ModifierEvalContext *ctx,
bke::GeometrySet *geometry_set)
{
GPWeightAngleModifierData *mmd = reinterpret_cast<GPWeightAngleModifierData *>(md);
const
Looks good.
@ -0,0 +109,4 @@
}
bke::MutableAttributeAccessor attributes = curves.attributes_for_write();
bke::SpanAttributeWriter<float> dst_weights = attributes.lookup_or_add_for_write_span<float>(
Is the output meant to be a vertex group? Seems it probably should be to have compatibility with the old modifier. In that case, vertex groups can't be created with the attribute API
The modifier isn't supposed to create the vertex group, only write to it.
So I guess we just need to check if the vertex group is present, otherwise return.
In that case it's misleading to use
lookup_or_add_for_write_span
(specifically the "or_add" part).Right, should be
lookup_for_write_span
Apparently
lookup_for_write_span
is not effective when no points has been assigned to the target vertex group (And it will naturally happen when you just prepare some empty vertex groups for passing stuff inside modifier). It will crash if I skip thedst_weights.span.is_empty()
check.I think the crash should be fixed then, because semantically that seems like the right way to do it.
I'm not sure how to fix this in this case... I think we need to make sure the result of
lookup_for_write_span
is still writeable whenis_empty()
? But that would meanlookup_or_add_for_write_span
. I think it won't change any original data since it's only doing things in the modifier geometry set.To better make sure that we stick to the "don't create vertex group in the modifier" thing, I guess there can be a check for "whether the vertex group exists", and if the vertex group does exist while no points have been assigned to it, should
dst_weights.span.is_empty()
return true?Right, I think it should be:
grease_penci.vertex_group_names
. If it doesn't, returnlookup_for_write_span
Now it seems to be working.
The RNA error was likely broken by the merge, fixed.
@ -0,0 +93,4 @@
modifier::greasepencil::read_influence_data(reader, &mmd->influence);
}
static int ensure_vertex_group(const StringRef name, ListBase &vertex_group_names)
StringRef.data()
can't be compared with C strings because it isn't necessarily null terminated. In that case you have to useStringRefNull
and.c_str()
.I have some final comments but I don't think this will need another round of review from me.
@ -0,0 +29,4 @@
#include "UI_interface.hh"
#include "UI_resources.hh"
#include "GEO_smooth_curves.hh"
Unused include
@ -0,0 +99,4 @@
&vertex_group_names, name.c_str(), offsetof(bDeformGroup, name));
if (def_nr < 0) {
bDeformGroup *defgroup = MEM_cnew<bDeformGroup>(__func__);
STRNCPY(defgroup->name, name.data());
Call
.c_str()
instead of data to make the intentions ("I need the null character") clear@ -0,0 +134,4 @@
}
/* Make sure that the target vertex group is added to this drawing so we can write to it. */
ensure_vertex_group(mmd.target_vgname, drawing.strokes_for_write().vertex_group_names);
drawing.strokes_for_write().vertex_group_names
->curves.vertex_group_names
@ -0,0 +154,4 @@
const float rot_angle = mmd.angle - ((mmd.axis == 1) ? M_PI_2 : 0.0f);
rotate_normalized_v3_v3v3fl(vec_ref, z_up, axis, rot_angle);
const float3x3 obmat3x3(float4x4(ob.object_to_world()));
object_to_world
already returns the 4x4 matrix type, so that can be removed@blender-bot build
@ -0,0 +93,4 @@
modifier::greasepencil::read_influence_data(reader, &mmd->influence);
}
static int ensure_vertex_group(const StringRefNull name, ListBase &vertex_group_names)
This will be needed in other places too, so IMHO it's better to have this in
blenkernel/internel/grease_pencil_vertex_groups.cc
.Yes I'm working on a refactor that will add this.
Hummm should I do anything about it?
Because there are more places than just this one, for this PR to stay local to the modifier, you don't need to do anything. I'll replace the static function with a kernel one after.
Some final cleanup comments from me
@ -8650,6 +8658,7 @@ static void rna_def_modifier_grease_pencil_thickness(BlenderRNA *brna)
}
static void rna_def_modifier_grease_pencil_lattice(BlenderRNA *brna)
Seems like this empty line slipped in
@ -0,0 +264,4 @@
} // namespace blender
ModifierTypeInfo modifierType_GPWeightAngle = {
The name can be
GreasePencilWeightAngle
everywhere now. Lukas fixed that@blender-bot build