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

Issue 4280080: Radial control RNA update (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
11 years, 1 month ago by nicholasbishop
Modified:
10 years, 2 months ago
Reviewers:
brechtvl, bf-codereview
Base URL:
https://svn.blender.org/svnroot/bf-blender/trunk/blender/
Visibility:
Public.

Description

Patch to make the radial control more generic with RNA. In the current radial control code in trunk, generic parts of the radial control are implemented as an incomplete operator within WM. Then each different user of the radial control has to implement a separate operator to actually pass in specific brush data -- e.g. sculpt's brush size, vpaint's brush size, etc. This patch removes all the extra operators and makes the WM operator do everything. It now takes several RNA path strings as its properties -- the only required property is data_path, which specifies the data to be modified by the radial control. The other paths affect display in various ways, e.g. rotation, color, etc. In addition to decreasing some duplicate paint brush code, these updates make it pretty easy to enable radial control for other purposes (and it can be set up entirely though python or keymaps, no extra C code needed.)

Patch Set 1 #

Total comments: 10

Patch Set 2 : Updated patch; should address everything except for the brush GL texture issue #

Patch Set 3 : Removed brush image RNA function #

Unified diffs Side-by-side diffs Delta from patch set Stats (+487 lines, -606 lines) Patch
source/blender/blenkernel/BKE_brush.h View 1 2 1 chunk +1 line, -2 lines 0 comments Download
source/blender/blenkernel/intern/brush.c View 1 2 2 chunks +1 line, -45 lines 0 comments Download
source/blender/editors/physics/particle_edit.c View 1 2 1 chunk +0 lines, -74 lines 0 comments Download
source/blender/editors/physics/physics_intern.h View 1 2 1 chunk +0 lines, -1 line 0 comments Download
source/blender/editors/physics/physics_ops.c View 1 2 3 chunks +9 lines, -3 lines 0 comments Download
source/blender/editors/sculpt_paint/paint_image.c View 1 2 3 chunks +0 lines, -89 lines 0 comments Download
source/blender/editors/sculpt_paint/paint_intern.h View 1 2 2 chunks +0 lines, -4 lines 0 comments Download
source/blender/editors/sculpt_paint/paint_ops.c View 1 2 12 chunks +60 lines, -19 lines 0 comments Download
source/blender/editors/sculpt_paint/paint_vertex.c View 1 2 1 chunk +0 lines, -104 lines 0 comments Download
source/blender/editors/sculpt_paint/sculpt.c View 1 2 2 chunks +0 lines, -60 lines 0 comments Download
source/blender/makesdna/DNA_windowmanager_types.h View 1 2 1 chunk +0 lines, -6 lines 0 comments Download
source/blender/makesrna/intern/rna_sculpt_paint.c View 1 2 1 chunk +2 lines, -2 lines 0 comments Download
source/blender/makesrna/intern/rna_space.c View 1 2 2 chunks +27 lines, -0 lines 0 comments Download
source/blender/windowmanager/WM_api.h View 1 2 1 chunk +0 lines, -6 lines 0 comments Download
source/blender/windowmanager/intern/wm_operators.c View 1 2 3 chunks +386 lines, -191 lines 0 comments Download
source/blenderplayer/bad_level_call_stubs/stubs.c View 1 2 1 chunk +1 line, -0 lines 0 comments Download

Messages

Total messages: 8
nicholasbishop
11 years, 1 month ago (2011-03-30 03:31:46 UTC) #1
brechtvl
I like this a lot, works much more like it should now. Comments inline. http://codereview.appspot.com/4280080/diff/1/source/blender/editors/sculpt_paint/paint_ops.c ...
11 years, 1 month ago (2011-04-01 09:02:50 UTC) #2
nicholasbishop
Updated patch; should address everything except for the brush GL texture issue
11 years, 1 month ago (2011-04-03 05:23:03 UTC) #3
nicholasbishop
Thanks for the detailed review :) I uploaded a new patch to address most of ...
11 years, 1 month ago (2011-04-03 05:31:44 UTC) #4
brechtvl
On 2011/04/03 05:31:44, nicholasbishop wrote: > A question about improving the brush texture-getter: > > ...
11 years, 1 month ago (2011-04-03 16:00:01 UTC) #5
nicholasbishop
Removed brush image RNA function
11 years, 1 month ago (2011-04-03 17:24:08 UTC) #6
nicholasbishop
That makes good sense, updated the patch to remove the extra RNA stuff.
11 years, 1 month ago (2011-04-03 17:26:02 UTC) #7
brechtvl
11 years, 1 month ago (2011-04-15 11:57:28 UTC) #8
Also here, LGTM after latest changes.
Sign in to reply to this message.

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