[Bf-codereview] Camera sensor size (issue 5274047)

[brechtvanlommel at gmail.com]

Sorry to bring this up again, but I'm still a bit confused by the sensor
stuff regarding in the case of a "portrait" instead of "landscape"
aspect ratio, see comments.


http://codereview.appspot.com/5274047/diff/4001/source/blender/blenlib/BLI_math_rotation.h
File source/blender/blenlib/BLI_math_rotation.h (right):

http://codereview.appspot.com/5274047/diff/4001/source/blender/blenlib/BLI_math_rotation.h#newcode185
source/blender/blenlib/BLI_math_rotation.h:185: float
hfov_to_focallength(float hfov, float sensor_x);
I wouldn't name these function specifically for horizontal values, they
work fine for vertical too.

http://codereview.appspot.com/5274047/diff/4001/source/blender/editors/space_view3d/view3d_view.c
File source/blender/editors/space_view3d/view3d_view.c (right):

http://codereview.appspot.com/5274047/diff/4001/source/blender/editors/space_view3d/view3d_view.c#newcode971
source/blender/editors/space_view3d/view3d_view.c:971: float lens,
sensor=32.f, fac, x1, y1, x2, y2;
It might be good to replace these hardcoded 32 values in a few places
with a #define in DNA_camera_types.h. Also minor nitpick is that in some
places it's numbers are types as x.f while in others it's x.0f, the
latter is the de facto standard in the code.

http://codereview.appspot.com/5274047/diff/4001/source/blender/makesrna/intern/rna_camera.c
File source/blender/makesrna/intern/rna_camera.c (right):

http://codereview.appspot.com/5274047/diff/4001/source/blender/makesrna/intern/rna_camera.c#newcode119
source/blender/makesrna/intern/rna_camera.c:119:
RNA_def_property_ui_text(prop, "Field of View", "Camera lens horizontal
field of view in degrees");
This description and the one for the sensor size doesn't seem to be
actually right. It's the horizontal or vertical fov depending on the
aspect ratio.

http://codereview.appspot.com/5274047/diff/4001/source/blender/makesrna/intern/rna_camera.c#newcode141
source/blender/makesrna/intern/rna_camera.c:141: prop=
RNA_def_property(srna, "sensor_width", PROP_FLOAT, PROP_NONE);
Similarly here, this is called sensor_width, but I guess that's not
really accurate because it is the sensor height for certain aspect
ratios? Or maybe it's a matter interpretation, where you assume that
you'd physically rotate the camera to get such images?

http://codereview.appspot.com/5274047/diff/4001/source/gameengine/Rasterizer/RAS_FramingManager.cpp
File source/gameengine/Rasterizer/RAS_FramingManager.cpp (right):

http://codereview.appspot.com/5274047/diff/4001/source/gameengine/Rasterizer/RAS_FramingManager.cpp#newcode52
source/gameengine/Rasterizer/RAS_FramingManager.cpp:52: * ^Deprecated
Comment
Just remove or replace this comment?

http://codereview.appspot.com/5274047/