GPv3: Transform operators (translate, rotate, scale, opacity, radius) #111836
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
4 Participants
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: blender/blender#111836
Loading…
Reference in New Issue
Block a user
No description provided.
Delete Branch "casey-bianco-davis/blender:GPv3-transform"
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 adds the keybindings, menu, and functionally for the edit mode transform operators to grease pencil v3
Have you sure it's already not a wip?
GPv3: Add edit mode transform operator (i.e translate, rotation, scale, opacity, radius)to WIP: GPv3: Add edit mode transform operator (i.e translate, rotation, scale, opacity, radius)WIP: GPv3: Add edit mode transform operator (i.e translate, rotation, scale, opacity, radius)to GPv3: Add edit mode transform operator (i.e translate, rotation, scale, opacity, radius)Thank you for working on this! Very much appreciated.
Overall the code looks very good already. I'd just like to see some more shared code between curves and grease pencil drawings.
@ -32,6 +32,7 @@ set(SRC
transform_convert_curves.cc
transform_convert_gpencil_legacy.cc
transform_convert_graph.cc
transform_convert_greasepencil.cc
greasepencil
->grease_pencil
@ -0,0 +120,4 @@
scene->r.cfra, [&](int /*drawing_index*/, blender::bke::greasepencil::Drawing &drawing) {
bke::CurvesGeometry &curves = drawing.strokes_for_write();
MutableSpan<float3> positions = curves.positions_for_write();
This code shouldn't be duplicated for
Curves
andGreasePencil
. The implementation could be intransform_convert_curves.cc
but it could be declared intransform_convert.hh
so that we can just call it here.Generally, this looks very close already. There are some changes in
main
(like the change toforeach_editable_drawing
) that will have to be merged. Just some minor comments.@ -109,0 +115,4 @@
/**
* Used for both curves and grease pencil objects.
*/
void curvePopulateTransDataStructs(TransDataContainer &tc,
curves_populate_trans_data_structs
@ -109,0 +119,4 @@
blender::bke::CurvesGeometry &curves,
blender::MutableSpan<float> *value_attribute,
const blender::IndexMask &selected_indices,
const bool use_proportional_edit,
Remove
const
in declaration.@ -109,0 +121,4 @@
const blender::IndexMask &selected_indices,
const bool use_proportional_edit,
const bool use_connected_only,
const int data_offset);
data_offset
->trans_data_offset
@ -100,1 +98,3 @@
pseudoinverse_m3_m3(smtx, mtx, PSEUDOINVERSE_EPSILON);
curvePopulateTransDataStructs(tc,
curves,
nullptr /* Currently no attributes. */,
/* Currently no transform for attributes other than position. */
@ -810,0 +819,4 @@
}
grease_pencil.foreach_editable_drawing(
scene->r.cfra, [&](int drawing_index, blender::bke::greasepencil::Drawing &drawing) {
layer_index
(same for other places as well)@ -810,0 +824,4 @@
const bke::crazyspace::GeometryDeformation deformation =
bke::crazyspace::get_evaluated_grease_pencil_drawing_deformation(
*depsgraph, *ob, drawing_index);
layer_index
data_offset
totrans_data_offset
407cab7b85drawing_index
tolayer_index
9de66cdbf9Did a quick test of this code. Apart from the warning I mentioned, works great :)
@ -369,0 +373,4 @@
label="Bend",
icon="ops.gpencil.edit_bend",
widget=None,
keymap=(),
This triggers a warning
@ -56,1 +56,4 @@
int drawing_index);
GeometryDeformation get_evaluated_grease_pencil_drawing_deformation(const Depsgraph &depsgraph,
const Object &ob_orig,
int drawing_index);
This changed to the
layer_index
inmain
so I guess a merge is needed.Overall this looks reasonable. I left some comments, mostly about style things.
@ -109,0 +117,4 @@
*/
void curve_populate_trans_data_structs(TransDataContainer &tc,
blender::bke::CurvesGeometry &curves,
blender::MutableSpan<float> *value_attribute,
Typically it's clearer to use
std::optional
than a pointer for such optional arguments@ -124,3 +124,1 @@
const int point_i = points[i];
TransData &td = tc.data[point_i];
float3 *elem = &positions[point_i];
void curve_populate_trans_data_structs(TransDataContainer &tc,
Could this be in the
blender::ed::transform::curves
namespace as well?I tried moving the code but kept getting error and I don't know c++ good enough to fix them. :|
Okay, that's fine! It can be done later as a more general cleanup
@ -148,3 +174,1 @@
TransData &td = tc.data[points[i]];
td.dist = closest_distances[i];
}
if (value_attribute != nullptr) {
I'd have to dig into this code more to see how possible this is, but it would be nice to keep this
value_attribute
in a separate loop from the location. Even moving it to a separate function would be nice if possible.For some background, I'm dreaming of a time where we can just process these arrays separately directly, rather than stuffing them in a
TransData
struct.@ -0,0 +110,4 @@
layer_points_offset);
if (use_proportional_edit) {
layer_points_offset += curves.point_num;
curves.points_num()
. Just nicer to user the accessor consistently.@ -0,0 +134,4 @@
bke::CurvesGeometry &curves = drawing.strokes_for_write();
curves.calculate_bezier_auto_handles();
curves.tag_positions_changed();
I think the drawing needs the "positions changed" tag too
Yep, otherwise the triangle caches are not tagged as dirty.
drawing_index
tolayer_index
fb01fe7616std::optional
instead of a pointer ab7090aea3This is looking good to me. More cleanup could be done afterwards in main
@ -0,0 +7,4 @@
*/
#include "BLI_array.hh"
#include "BLI_inplace_priority_queue.hh"
Unused include, maybe?
Would you be interested in adding the
SHRINKFATTEN
mode to curves edit mode too (the new "hair" curves type). It would be really nice to be able to take advantage of the shared code and also be able to adjust the radius there too. That change could be a separate PR if you preferred too.@ -0,0 +81,4 @@
const IndexMask selected_indices = selection_per_layer_per_object[i + layer_offset];
std::optional<blender::MutableSpan<float>> value_attribute = {};
= {}
is unnecessary,optional
has a default constructorblender::
is unnecessary here@ -0,0 +121,4 @@
GreasePencil &grease_pencil = *static_cast<GreasePencil *>(tc.obedit->data);
grease_pencil.foreach_editable_drawing(
scene->r.cfra, [&](int /*layer_index*/, blender::bke::greasepencil::Drawing &drawing) {
blender::
is unnecessary hereblender::
2b5d21cd60I may be interested in trying
SHRINKFATTEN
but I think it be better as a separate pull request.GPv3: Add edit mode transform operator (i.e translate, rotation, scale, opacity, radius)to GPv3: Transform operators (translate, rotate, scale, opacity, radius)