CurvesGeometry: Add initial vertex group support #106944
No reviewers
Labels
No Label
Interest
Alembic
Interest
Animation & Rigging
Interest
Asset System
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
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
Viewport & EEVEE
Interest
Virtual Reality
Interest
Vulkan
Interest
Wayland
Interest
Workbench
Interest: X11
Legacy
Asset Browser Project
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
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
Module
Viewport & EEVEE
Platform
FreeBSD
Platform
Linux
Platform
macOS
Platform
Windows
Severity
High
Severity
Low
Severity
Normal
Severity
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
3 Participants
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: blender/blender#106944
Loading…
Reference in New Issue
Block a user
No description provided.
Delete Branch "filedescriptor/blender:curves-deform-verts"
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?
This PR adds vertex groups to
CurvesGeometry
as well as an attribute read/write accessor.This is also in preparation for GPv3. Since the goal is to have full compatibility with the current grease pencil features, vertex groups need to be supported.
Grease Pencil allows filtering by vertex group for modifiers.To support this, it also makes sense to have read/write access for vertex groups in the attributes API.
In the future, vertex groups should be just another custom attribute on meshes/curves/grease pencil. There are some more issues to be solved before that can happen. This step gets us a bit closer since the vertex weight data is stored in
CustomData
.@ -440,1 +440,4 @@
Span<MDeformVert> CurvesGeometry::deform_verts() const
{
const MDeformVert *dverts = (const MDeformVert *)CustomData_get_layer(&this->point_data,
Use
static_cast
here and belowb1328470d1
toa0aee11edd
WIP: Add vertex groups to CurvesGeometryto Curves: Add initial vertex group supporteab28c4e09
to9dbc164680
Like I mentioned in chat, I'd rather not expose this in the UI or RNA API for
Curves
, since longer term we'd like to replace the storage of vertex groups, and it would be nice to not have to deprecate this version for the new curves type.@ -0,0 +21,4 @@
namespace blender::bke {
class VArrayImpl_For_VertexWeights final : public VMutableArrayImpl<float> {
This entire implementation shouldn't be in a header. Since virtual arrays use virtual inheritance, none of it needs to be in a header actually.
I'm not sure I understand. Should I move this entire code to a
.cc
file? Why are all other classes inheriting fromVMutableArrayImpl
defined in a header then?They are templated to work on any type, this is not. You can move this to a cc file and just expose a function that returns a virtual array (
VArray
) for a vertex group.@ -60,6 +61,10 @@ CurvesGeometry::CurvesGeometry(const int point_num, const int curve_num)
CustomData_reset(&this->point_data);
CustomData_reset(&this->curve_data);
/* Make sure to clear this before using the attributes API. Otherwise the vertex group accessor
Not sure this comment is really necessary, it's just the same thing as the
CustomData_reset
calls above.@ -144,2 +152,4 @@
std::swap(dst.curve_offsets, src.curve_offsets);
dst.vertex_group_active_index = src.vertex_group_active_index;
std::swap(dst.vertex_group_names.first, src.vertex_group_names.first);
Can just keep it simpler by swapping everything.
Hm I'm unsure how I can remove the Python RNA API for this 🤔
I think undoing the change in
BKE_id_supports_vertex_groups
should do it.9dbc164680
to84d6b5448b
The PR description should be updated to give some context, describe the goal a bit more, and remove the screenshot.
Curves: Add initial vertex group supportto Curves: Add vertex group supportSeems more of the changes can be reverted still, since we don't add support for the
Curves
object type in this commit.Also, sorry for the nitpicking, but some comments about the PR description:
Those two things don't really feel connected to me, I guess it assumes that the modifiers would access vertex groups with the attribute API. "needs to" is just a bit strong wording then.
Not really sure how this gets us closer, vertex group data is stored in
CustomData
with or without this change.@ -293,2 +298,4 @@
float *r_weights);
namespace blender::bke {
VArray<float> varray_for_deform_verts(Span<MDeformVert> dverts, const int dvert_index);
const int
->int
dvert_index
->defgroup_index
/vgroup_index
@ -461,0 +486,4 @@
if (dvert != nullptr) {
return {dvert, this->point_num};
}
return {(MDeformVert *)CustomData_add_layer(
Casting style
@ -489,2 +494,4 @@
return &gpd->vertex_group_active_index;
}
case OB_CURVES: {
const Curves *curves_id = static_cast<const Curves *>(ob->data);
Remove
ID_CV
andOB_CURVES
from these, since we don't want to expose curves object vertex groups right now.@ -334,0 +408,4 @@
for (MDeformVert &dvert : dverts.slice(range)) {
MDeformWeight *weight = BKE_defvert_find_index(&dvert, index);
BKE_defvert_remove_group(&dvert, weight);
for (MDeformWeight &weight : MutableSpan(dvert.dw, dvert.totweight)) {
It would be nice to not duplicate this code between meshes and curves too, could that be shared in
BKE_deform.h
too?@ -122,0 +123,4 @@
return lt->dvert;
}
case ID_CV: {
Curves *curves_id = reinterpret_cast<Curves *>(id);
Same here,
ID_CV
andOB_CURVES
don't need to be supported right now (at least in this commit)f95bcdd709
toc58e8d58a2
How so for Grease Pencil?
BKE_defgroup_listbase_name_find
d028ed7a5fJust one last comment I think. The commit message should mention there are no functional changes too.
@ -146,0 +151,4 @@
std::swap(dst.vertex_group_names.first, src.vertex_group_names.first);
std::swap(dst.vertex_group_names.last, src.vertex_group_names.last);
std::swap(dst.vertex_group_active_index, src.vertex_group_active_index);
src.vertex_group_active_index = 0;
src.vertex_group_active_index = 0;
That part is unnecessary I think@ -25,6 +25,8 @@
#include "BKE_attribute_math.hh"
#include "BKE_curves.h"
#include "DNA_meshdata_types.h"
Does it work to forward declare
MDeformVert
? It would be nice not to includeDNA_meshdata_types.h
in the curves header.Curves: Add vertex group supportto CurvesGeometry: Add initial vertex group supportMDeformVert
c1047f5902