Fisheye Panorama for Cycles

- created a new camera type (panorama)
- implemented the fisheye panorama
... and opened the room for the cylindrical panorama.

I didn't implement  cylindrical yet because I'm still thinking whether or not we need it. It's easy to rebuild a cylindrical panorama from a equirectangular (although you will be rendering more than you need in the final image).

Apart from the open hooks for the cylinder panorama (and the lack of description for the rna props), the patch should be good to go (2.64).
And in the near future we may want to have properties to set the angles for the equirectangular too. So I'm not so sure panorama_fov is a good name for the fisheye fov.

Anyhoo, I look forward for the review. There are probably a few things done in a non efficient-cycles way. (fisheye_to_direction could likely be optimized as well, I first wanted to make sure my math was correct though).

For the UI I set 10 to 360 as soft values and 10 to 3600 as hard values.

edit: commited on rev. 46287

[brechtvl, 2012-04-25 08:38:39]

Looks good, but there's still a bunch of code missing for adding panorama as a
proper camera type. It needs a version patch, changes in blender internal, maybe
3d view drawing and any place that checks camera type, and removal of the old
use_panorama.

http://codereview.appspot.com/6111052/diff/1/intern/cycles/kernel/kernel_came...
File intern/cycles/kernel/kernel_camera.h (right):

http://codereview.appspot.com/6111052/diff/1/intern/cycles/kernel/kernel_came...
intern/cycles/kernel/kernel_camera.h:96: float3 Pcamera =
transform_perspective(&rastertocamera, make_float3(raster_x, raster_y,
kernel_data.cam.panorama_fov));
Orthographic camera code should not be modified.

http://codereview.appspot.com/6111052/diff/1/intern/cycles/kernel/kernel_came...
intern/cycles/kernel/kernel_camera.h:153: ray->dD.dx =
normalize(transform_direction(&cameratoworld,
equirectangular_to_direction(Pcamera.x, Pcamera.y))) - ray->D;
This also needs the if/else for the camera type.

[dfelinto, 2012-04-25 18:26:16]

"It needs a version patch, changes in blender internal, maybe 3d view drawing
and any place that checks camera type, and removal of the old use_panorama."

That's what I was wondering when I asked on the plans for the panorama camera.
What I wasn't sure (and still am not) is:
(1) get rid of 'use_panorama' leaving the panorama as an option only for cycles
(2) leave 'use_panorama' working for the internal renderer as it is and panorama
camera for cycles.
(3) implement a real panorama camera for internal renderer.

I know the (3) would be ideal, but I'm not sure I can accomplish it without some
guidance (and if the workload is too big it may not be ready so soon). I don't
think we can get it to work without doing multiple renders and stitching it
together. I also always assumed 'you' (ton actually) wouldn't want a bunch of
'if (dome)' polluting the render core code ;)

Thoughts?
(attaching next a small fix for the 2 raised issues + removing cylinder code)

[dfelinto, 2012-04-25 18:26:57]

removing cylinder, fix ortographic and fix differential check

[brechtvl, 2012-04-25 18:31:51]

Hi,

On 2012/04/25 18:26:16, dfelinto wrote:
> (1) get rid of 'use_panorama' leaving the panorama as an option only for
cycles
> (2) leave 'use_panorama' working for the internal renderer as it is and
panorama
> camera for cycles.
> (3) implement a real panorama camera for internal renderer.

I was thinking (4), to just make the panorama type working for Blender Internal,
doing the same as use_panorama does now. It would just be a different UI to
expose the same functionality, no need to actually implement any new type of
camera in Blender Internal.

[dfelinto, 2012-04-27 04:25:09]

all the issues addressed now. good to go?

[brechtvl, 2012-04-27 10:29:53]

Looks good to commit, with these two small tweaks:

In collada/CameraExporter.cpp, it makes a bit more sense to export panorama as
perspective rather than ortho, since that's how it shows in the viewport.

http://codereview.appspot.com/6111052/diff/12001/intern/cycles/kernel/kernel_...
File intern/cycles/kernel/kernel_montecarlo.h (right):

http://codereview.appspot.com/6111052/diff/12001/intern/cycles/kernel/kernel_...
intern/cycles/kernel/kernel_montecarlo.h:235: float phi = acosf(u/r);
(0.5, 0.5) will give division by zero and so a NaN here, change to:
acosf((r != 0.0f)? u/r: 0.0f);

[dfelinto, 2012-04-27 14:38:30]

last issues addressed (+ fix in readfile from last patch). planning to commit
tmw after trunk is ope

[brechtvl, 2012-04-27 14:42:56]

LGTM. :)

[dfelinto, 2012-05-03 15:17:35]

photographic fisheye (equisolid) implemented too. probably need to rename some
defines/tooltip/ui

[dfelinto, 2012-05-03 15:34:30]

additional notes.
the main issues I have are:

1) how to name the modes
2) whether it's ok to take sensor into consideration but to ignore the render
dimensions (width,height of the image always map to UV 0,0:1,1) - just like
equirectangular mode was doing before (the 'use panorama').

http://codereview.appspot.com/6111052/diff/25001/intern/cycles/app/cycles_xml...
File intern/cycles/app/cycles_xml.cpp (right):

http://codereview.appspot.com/6111052/diff/25001/intern/cycles/app/cycles_xml...
intern/cycles/app/cycles_xml.cpp:298: else if(xml_equal_string(node,
"panorama_type", "fisheye_equidistant"))
there is something (not specific to this file) I'm not sure on what is better.

Technically this fisheye_equidistant is an hemispherical fulldome render. To be
a real Equidistant I should take sensors into account (which I'm not since that
would complicate a lot the life of dome people) (which would need to find the
perfect arrange of lens [which is also depending of the desired FOV] and sensor
dimensions [i.e. square] to get a circular fisheye out of it.

Thoughts?

http://codereview.appspot.com/6111052/diff/25001/intern/cycles/kernel/kernel_...
File intern/cycles/kernel/kernel_montecarlo.h (right):

http://codereview.appspot.com/6111052/diff/25001/intern/cycles/kernel/kernel_...
intern/cycles/kernel/kernel_montecarlo.h:256: return make_float3(0.f);
Without this the render goes beyond 180deg. If someone wants a 'circular
fisheye' lens this need to be set to 180. this renders white though, the ideal
would be to render black. dont know how

http://codereview.appspot.com/6111052/diff/25001/release/scripts/startup/bl_u...
File release/scripts/startup/bl_ui/properties_data_camera.py (right):

http://codereview.appspot.com/6111052/diff/25001/release/scripts/startup/bl_u...
release/scripts/startup/bl_ui/properties_data_camera.py:99: row.prop(ccam,
"fisheye_fov")
The fov here is not the field of view of the image. The fov of the image will
depend on multiple factors.

This fov is often referred as the diagonal field of view of the lens when using
a 35mm sensor. wonder if needs to be clearer for the user.

[brechtvl, 2012-05-03 15:46:12]

Some quick comments on the code, don't have an opinion on the FOV/sensor yet.

http://codereview.appspot.com/6111052/diff/25001/intern/cycles/kernel/kernel_...
File intern/cycles/kernel/kernel_camera.h (right):

http://codereview.appspot.com/6111052/diff/25001/intern/cycles/kernel/kernel_...
intern/cycles/kernel/kernel_camera.h:147: ray->D =
equirectangular_to_direction(Pcamera.x, Pcamera.y);
At this point it might be good to create a panorama_to_direction function, that
takes the panorama type as argument and does the switch() there, instead of
duplicating it 3x.

http://codereview.appspot.com/6111052/diff/25001/intern/cycles/kernel/kernel_...
File intern/cycles/kernel/kernel_montecarlo.h (right):

http://codereview.appspot.com/6111052/diff/25001/intern/cycles/kernel/kernel_...
intern/cycles/kernel/kernel_montecarlo.h:256: return make_float3(0.f);
Is it white or NaN? In any case, the proper solution would be to change this in
kernel_path.h, at the line:
float4 L = kernel_path_integrate(kg, &rng, sample, ray, buffer);

Set it and then check ray.t == 0, and just set L to make_float4(0.0f, 0.0f,
0.0f, 0.0f);

http://codereview.appspot.com/6111052/diff/25001/intern/cycles/kernel/kernel_...
intern/cycles/kernel/kernel_montecarlo.h:256: return make_float3(0.f);
Also, don't use return make_float3(0.f);, it works only on the CPU actually,
probably need to make that more clear. It needs to be make_float3(0.0f, 0.0f,
0.0f);

[dfelinto, 2012-05-04 07:04:13]

addressing issues pointed by brecht (code cleanup will come later, no meld atm)

[dfelinto, 2012-05-04 07:05:20]

http://codereview.appspot.com/6111052/diff/25001/intern/cycles/kernel/kernel_...
File intern/cycles/kernel/kernel_montecarlo.h (right):

http://codereview.appspot.com/6111052/diff/25001/intern/cycles/kernel/kernel_...
intern/cycles/kernel/kernel_montecarlo.h:256: return make_float3(0.f);
It's actually not white, but the background colour (which happened to be white
in my test file). I can't test for ray.t after kernel_path_integrate because
ray->t is set there.
See if it's good in the latest patch.


On 2012/05/03 15:46:12, brechtvl wrote:
> Is it white or NaN? In any case, the proper solution would be to change this
in
> kernel_path.h, at the line:
> float4 L = kernel_path_integrate(kg, &rng, sample, ray, buffer);
> 
> Set it and then check ray.t == 0, and just set L to make_float4(0.0f, 0.0f,
> 0.0f, 0.0f);

[dfelinto, 2012-05-04 16:24:15]

committed on rev. 46287. I introduced a small regression issue though.

I don't know to access cycles data in readfile.c and I decided to change the
default value for the panorama type enum to Fisheye Equisolid. So a file that
had 'use_panorama' will now have a panorama camera BUT not the equirectangular
one. Should that be a concern?

Brecht thanks for all the help with the review.