Page MenuHome
Differential D17063

Curves: Add initial transform support
ClosedPublic
Actions

Authored by Falk David (filedescriptor) on Fri, Jan 20, 12:44 PM.

Details

Reviewers
Hans Goudey (HooglyBoogly)
Commits
rB50387964b6f1: Curves: Add initial transform support
Summary

This adds basic support for the transform operators for curves.

Diff Detail

Repository
rB Blender

Event Timeline

Falk David (filedescriptor) requested review of this revision.Fri, Jan 20, 12:44 PM
Falk David (filedescriptor) created this revision.
Harbormaster completed remote builds in B25479: Diff 59712.Fri, Jan 20, 12:44 PM
Falk David (filedescriptor) retitled this revision from Add initial transform support for curves to Curves: Add initial transform support.Fri, Jan 20, 12:45 PM
Falk David (filedescriptor) edited the summary of this revision. (Show Details)
Falk David (filedescriptor) added a project: Nodes & Physics.
Philipp Oeser (lichtwerk) added a subscriber: Philipp Oeser (lichtwerk).Fri, Jan 20, 1:24 PM
Hans Goudey (HooglyBoogly) requested changes to this revision.Fri, Jan 20, 4:02 PM
Hans Goudey (HooglyBoogly) added a subscriber: Hans Goudey (HooglyBoogly).
Hans Goudey (HooglyBoogly) added inline comments.
source/blender/editors/transform/transform_convert_curves.cc
25
30

The indices might be empty if everything is selected, so two separate arrays will be necessary-- one of index masks and one of the vectors

39–47

This is already a utility-- curves::retrieve_selected_points

71–83

Might as well parallelize this

This revision now requires changes to proceed.Fri, Jan 20, 4:02 PM
Hans Goudey (HooglyBoogly) awarded a token.Fri, Jan 20, 4:03 PM
Falk David (filedescriptor) added inline comments.Fri, Jan 20, 4:07 PM
source/blender/editors/transform/transform_convert_curves.cc
39–47

I saw that, but I couldn't include things from editor in this file. Maybe there is a way?

Hans Goudey (HooglyBoogly) added inline comments.Fri, Jan 20, 4:13 PM
source/blender/editors/transform/transform_convert_curves.cc
39–47

Hmm, including ED_curves.h worked for me, and should be covered by the ../include item in cmake. Maybe there's something else going on though.

Falk David (filedescriptor) added inline comments.Fri, Jan 20, 4:46 PM
source/blender/editors/transform/transform_convert_curves.cc
39–47

Changing the namespace like you did made it work!

Falk David (filedescriptor) updated this revision to Diff 59732.Fri, Jan 20, 5:30 PM
Falk David (filedescriptor) edited the summary of this revision. (Show Details)
  • Merge branch 'master' into curves-transform
  • Cleanup + support for no selection attribute case
Harbormaster completed remote builds in B25490: Diff 59732.Fri, Jan 20, 5:30 PM
Falk David (filedescriptor) added inline comments.Fri, Jan 20, 5:32 PM
source/blender/editors/transform/transform_convert_curves.cc
67–68

This is still very ugly, but the transform code needs a raw pointer to work, so not sure what else to do here.

Hans Goudey (HooglyBoogly) accepted this revision.Fri, Jan 20, 6:54 PM

This works well and it's relatively fast considering how early it is!

Maybe adding a basic keymap and active tools is the next step? Or some more selection operators I guess.

release/scripts/startup/bl_ui/space_view3d.py
5338–5342
source/blender/editors/curves/intern/curves_selection.cc
58–59

I strongly suggest setting up format on save for your editor, it'll save a lot of time!

source/blender/editors/transform/transform_convert_curves.cc
14

This one is redundant with BKE_curves.hh

33

What about selection_per_object? The index_mask part is included in the type I think

90–91

Might as well put this here now. It won't make Bezier's work fully, but it will help a bit.

103

Missing newline

This revision is now accepted and ready to land.Fri, Jan 20, 6:54 PM
Falk David (filedescriptor) updated this revision to Diff 59757.Sat, Jan 21, 2:19 PM
  • Address feedback
Harbormaster completed remote builds in B25496: Diff 59757.Sat, Jan 21, 2:19 PM
Hans Goudey (HooglyBoogly) accepted this revision.Sat, Jan 21, 6:36 PM

A great first step!

source/blender/editors/transform/transform_convert_curves.cc
35
50
84

Careful about making copies by mistake, I noticed this earlier with Vector<int64_t> too (not in the patch anymore)

Iliya Katueshenock (Moder) awarded a token.Sat, Jan 21, 6:58 PM
Falk David (filedescriptor) edited the summary of this revision. (Show Details)Sat, Jan 21, 7:56 PM
Falk David (filedescriptor) updated this revision to Diff 59765.Sat, Jan 21, 7:57 PM
  • Use references instead of pointers
Harbormaster completed remote builds in B25502: Diff 59765.Sat, Jan 21, 7:57 PM
Closed by commit rB50387964b6f1: Curves: Add initial transform support (authored by Falk David (filedescriptor)). · Explain WhySat, Jan 21, 8:39 PM
This revision was automatically updated to reflect the committed changes.
Falk David (filedescriptor) added a commit: rB50387964b6f1: Curves: Add initial transform support.

Revision Contents
Changeset List

PathSize
release/
scripts/
startup/
bl_ui/
5 lines
source/
blender/
editors/
curves/
intern/
3 lines
include/
1 line
transform/
1 line
4 lines
5 lines
102 lines

Diff 59766

View Options

release/scripts/startup/bl_ui/space_view3d.py

Loading...
View Options

source/blender/editors/curves/intern/curves_selection.cc

Loading...
View Options

source/blender/editors/include/ED_curves.h

Loading...
View Options

source/blender/editors/transform/CMakeLists.txt

Loading...
View Options

source/blender/editors/transform/transform_convert.h

Loading...
View Options

source/blender/editors/transform/transform_convert.c

Loading...
View Options

source/blender/editors/transform/transform_convert_curves.cc

Loading...