[Bf-codereview] Camera tracking integration (issue 5285047)

brechtvanlommel at gmail.com brechtvanlommel at gmail.com
Wed Oct 19 19:47:53 CEST 2011


Regarding the naming, it seems to me there are various terms used: Movie
Clip, Reconstruction, Track, Solver. It would be nice if it the naming
made it clearer that the scene panel, 3d view display options,
constraints and movie clip editor fit together. Maybe all should have
Track/Tracker/Tracking in the name, I'm not sure.


http://codereview.appspot.com/5285047/diff/3001/release/scripts/startup/bl_ui/properties_scene.py
File release/scripts/startup/bl_ui/properties_scene.py (right):

http://codereview.appspot.com/5285047/diff/3001/release/scripts/startup/bl_ui/properties_scene.py#newcode223
release/scripts/startup/bl_ui/properties_scene.py:223: bl_label = "Movie
Clip"
I'd suggest to call this panel "Tracker". Also I'm not sure if the
concept of an active movie clip makes much sense outside of the tracker,
perhaps it should renamed to indicate it's only used for that?

http://codereview.appspot.com/5285047/diff/3001/release/scripts/startup/bl_ui/space_clip.py
File release/scripts/startup/bl_ui/space_clip.py (right):

http://codereview.appspot.com/5285047/diff/3001/release/scripts/startup/bl_ui/space_clip.py#newcode42
release/scripts/startup/bl_ui/space_clip.py:42:
sub.menu("CLIP_MT_select")
For consistency with other editors, the menus should be ordered view -
select - clip.

http://codereview.appspot.com/5285047/diff/3001/release/scripts/startup/bl_ui/space_clip.py#newcode62
release/scripts/startup/bl_ui/space_clip.py:62: class
CLIP_PT_tools(Panel):
I'd leave out this panel, this is already possible from the header.

http://codereview.appspot.com/5285047/diff/3001/release/scripts/startup/bl_ui/space_clip.py#newcode380
release/scripts/startup/bl_ui/space_clip.py:380: class
CLIP_PT_display(Panel):
There's not enough space here for two columns, in the default
configuration various words here are cut off, maybe split it up? Or make
the region wider.

http://codereview.appspot.com/5285047/diff/3001/release/scripts/startup/bl_ui/space_clip.py#newcode664
release/scripts/startup/bl_ui/space_clip.py:664: class
CLIP_MT_clip(Menu):
The order in this menu should be reversed I think, usually open is at
the bottom and nested menus at the top.

http://codereview.appspot.com/5285047/diff/3001/release/scripts/startup/bl_ui/space_clip.py#newcode799
release/scripts/startup/bl_ui/space_clip.py:799:
layout.operator("clip.select_all", text="Inverse").action = 'INVERT'
The order of this menu feels a bit strange compared to others. Not all
select menus are consistent, but perhaps you can make this one
consistent with the 3d view select menu?

http://codereview.appspot.com/5285047/diff/3001/source/blender/nodes/composite/nodes/node_composite_transform.c
File source/blender/nodes/composite/nodes/node_composite_transform.c
(right):

http://codereview.appspot.com/5285047/diff/3001/source/blender/nodes/composite/nodes/node_composite_transform.c#newcode39
source/blender/nodes/composite/nodes/node_composite_transform.c:39:
static bNodeSocketTemplate cmp_node_stabilize2d_in[]= {
This variable should be renamed to cmp_node_transform_in, same for the
output sockets.

http://codereview.appspot.com/5285047/diff/3001/source/blender/nodes/composite/nodes/node_composite_transform.c#newcode43
source/blender/nodes/composite/nodes/node_composite_transform.c:43:
{	SOCK_FLOAT,		1,	"Degr",			0.0f, 0.0f, 0.0f, 0.0f, -10000.0f,
10000.0f},
Usually such a property would be called "Angle", but it seems the Rotate
node is inconsistent at this as well. This socket should be set as
PROP_ANGLE.

http://codereview.appspot.com/5285047/diff/3001/source/blender/nodes/composite/nodes/node_composite_transform.c#newcode75
source/blender/nodes/composite/nodes/node_composite_transform.c:75:
mul_serie_m4(mat, lmat, smat, cmat, rmat, icmat, NULL, NULL, NULL);
In other places in Blender the order of transformations is
scale-rotate-translate, this is rotate-scale-translate. Any particular
reason to do it this way?

http://codereview.appspot.com/5285047/


More information about the Bf-codereview mailing list