Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(314)

Issue 5091046: RNA write access for CurveMap

Can't Edit
Can't Publish+Mail
Start Review
Created:
10 years, 4 months ago by lukas.toenne
Modified:
10 years, 4 months ago
Reviewers:
ideasman42, bf-codereview
Base URL:
https://svn.blender.org/svnroot/bf-blender/trunk/blender/
Visibility:
Public.

Patch Set 1 #

Total comments: 3
Unified diffs Side-by-side diffs Delta from patch set Stats (+100 lines, -9 lines) Patch
M source/blender/makesrna/intern/rna_color.c View 7 chunks +100 lines, -9 lines 3 comments Download

Messages

Total messages: 3
lukas.toenne
10 years, 4 months ago (2011-09-21 11:50:35 UTC) #1
lukas.toenne
Further explanation: Updating points in a CurveMap requires update calls on the CurveMapping struct. Since ...
10 years, 4 months ago (2011-09-21 11:57:32 UTC) #2
ideasman42
10 years, 4 months ago (2011-09-22 07:07:13 UTC) #3
Generally functionality is ok, but would like to have this match Curve and
FCurve style access, comments inline.

http://codereview.appspot.com/5091046/diff/1/source/blender/makesrna/intern/r...
File source/blender/makesrna/intern/rna_color.c (right):

http://codereview.appspot.com/5091046/diff/1/source/blender/makesrna/intern/r...
source/blender/makesrna/intern/rna_color.c:161: static void
rna_CurveMapping_set_point_handles(CurveMapping *cumap, int cuma_index, int
handle_type)
Why add this when we have CurveMapPoint.handle_type ???

http://codereview.appspot.com/5091046/diff/1/source/blender/makesrna/intern/r...
source/blender/makesrna/intern/rna_color.c:425: prop= RNA_def_property(srna,
"points", PROP_COLLECTION, PROP_NONE);
add/remove points function should be added here.

I'd like this to follow Bezier curve / FCurve api style where possible.

http://codereview.appspot.com/5091046/diff/1/source/blender/makesrna/intern/r...
source/blender/makesrna/intern/rna_color.c:486: func= RNA_def_function(srna,
"reset", "rna_CurveMapping_reset");
Rather then passing an index this should be a member of CurveMap.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b