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
Branch
curves-transform (branched from master)
Build Status
Buildable 25479
Build 25479: arc lint + arc unit

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
24
29

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

38–46

This is already a utility-- curves::retrieve_selected_points

70–82

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
38–46

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
38–46

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
38–46

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
5332–5336
source/blender/editors/curves/intern/curves_selection.cc
59–60 ↗(On Diff #59732)

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.

108

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/
6 lines
source/
blender/
editors/
transform/
1 line
4 lines
5 lines
107 lines

Diff 59712

View Options

release/scripts/startup/bl_ui/space_view3d.py

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...