Add multilens cameras #2914
Add multilens cameras #2914
Conversation
There was a problem hiding this comment.
Partial review.
I read all the code, but didn't test it yet.
Generally, it looks good. I think most things are minors.
Maybe it's worth it to have a LensCamera class (or so), which has all the extract_... functions in it, which are used by this camera as well as the thin lens camera (and possible future camera models with lenses).
| <parameter name="film_dimensions" value="0.025 0.01875" /> | ||
| <parameter name="focal_distance" value="1.0" /> | ||
| <parameter name="focal_length" value="0.035" /> | ||
| <parameter name="lens_file" value="C:\Users\willi\Documents\appleseed-dev\appleseed\sandbox\lenses\double-gauss-tronnier.txt" /> |
There was a problem hiding this comment.
Repetition of your TODO: Change to relative paths.
| // This software is released under the MIT license. | ||
| // | ||
| // Copyright (c) 2010-2013 Francois Beaune, Jupiter Jazz Limited | ||
| // Copyright (c) 2014-2018 Francois Beaune, The appleseedhq Organization |
|
|
||
| typedef ImageImportanceSampler<Vector2d, float> ImageImportanceSamplerType; | ||
|
|
||
| class ImageSampler |
There was a problem hiding this comment.
This class is part of the thinlenscamera as well.
| // Extract autofocus status. | ||
| m_autofocus_enabled = m_params.get_optional<bool>("autofocus_enabled", true); | ||
|
|
||
| m_derivatives_enabled = m_params.get_optional<bool>("derivatives_enabled", true); |
There was a problem hiding this comment.
I think this was only for testing purposes and can be removed again?
| // Extract diaphragm configuration. | ||
| m_diaphragm_map_bound = build_diaphragm_importance_sampler(*project.get_scene()); | ||
| extract_diaphragm_blade_count(); | ||
| m_diaphragm_tilt_angle = |
There was a problem hiding this comment.
Why did you move this out of a function?
|
|
||
| for (auto lens_iter = m_lens_container.rbegin() + start_index; lens_iter != m_lens_container.rend(); ++lens_iter) | ||
| { | ||
| LensElement current_element = *lens_iter; |
| current_z -= current_element.thickness; | ||
|
|
||
| double t = 0; // Parameter, at which ray intersects element | ||
| Vector3d normal(0, 0, 0); // Normal vector |
| return true; | ||
| } | ||
|
|
||
| bool trace_ray_from_film(Ray3d& ray, const int start_index = 0, const bool ignore_aperture_shape = false) const |
There was a problem hiding this comment.
These two functions are almost alike. Is there a way to merge them, while still having readable code?
There was a problem hiding this comment.
They were originally merged, but I split them at some point as it kept confusing me. The problem is that the iteration direction and the index of refraction computation is different.
| ray.m_org = transform.point_to_parent(ray.m_org); | ||
| ray.m_dir = normalize(transform.vector_to_parent(ray.m_dir)); | ||
|
|
||
| if (m_derivatives_enabled && ndc.has_derivatives()) |
There was a problem hiding this comment.
I'm unsure if derivatives should have an enable/disable option.
|
|
||
| void extract_lens_file() | ||
| { | ||
| if (has_param("lens_file")) |
There was a problem hiding this comment.
What happens in the overall workflow, if the parameter doesn't exist? If you're sure it's always there, remove this check, if it may not exist, adapt the rest of the workflow.
|
Thanks for the first look! |
There was a problem hiding this comment.
Hey,
Great work!
Since the CI is currently not working, I manually compiled this PR using Ubuntu with GCC-6, GCC-7 and Clang-8. All compilers didn't throw any warning or error under Debug or Release build.
On top of that, I ran the full testsuite, which resulted in 141 failures, which is exactly the number we had before (see here for the old log)
Thus, from my side, this PR is ready to go.
Summary
This PR contains the implementation of a physically-based multi-lens camera created as a bachelor thesis
supervised by @LZaw. It partially covers the topics of the issue "Physically-based camera" #1740 and consists of the complete code as well as test scenes.
src/appleseed/renderer/modeling/camera/multilenscamera.cppandsrc/appleseed/renderer/modeling/camera/multilenscamera.h.sandbox/tests/test scenes/multilens camera/.sandbox/lenses/.TODOs
It is currently a draft pull request, as there are some open tasks that will be completed once I submit the thesis:
connect_vertex()Problem Description
All cameras currently available in appleseed are approximations of real models and sacrifice realistic camera phenomena for performance. There are however scenarios, where images as if shot on a real camera are desired, for example when combining real images with computer-generated footage.
Solution Description
To achieve photorealistic images, a simulation of a real lens with many lens elements is required. In literature, there are mainly two approaches for that, which I like to call "ray simulation" and "polynomial optics". Below, I list some characteristics of the two.
Ray simulation
Polynomial Optics
In the end, my choice fell on ray simulation, more precisely the model described in "Realistic rendering of bokeh effect based on optical aberrations" with elements from "An Accurate and Practical Camera Lens Model for Rendering Realistic Lens Effects" and Physically Based Rendering: From Theory to Implementation. For more details on the the comparison and the choice, please see Chapter 3 of my thesis.
Implementation
As discussed, this PR extends the cameras by another one and is located in
src/appleseed/renderer/modeling/camera. This PR contains the following changes:MultiLensCamerathat inherits fromPerspectiveCamera. The most important parts of it are:sandbox/lenses/.MultiLensCameraFactorytoCameraFactoryRegistrarvoid spawn_ray(..)tobool spawn_ray(..). This is required, as rays can be blocked by the aperture and should then not contribute to the illumination calculation. This change affectscamera.hand all<name>camera.cppfiles, as well assrc/appleseed/renderer/kernel/rendering/generic/genericsamplerenderer.cppandsrc/appleseed/renderer/kernel/rendering/scenepicker.cpp.extract_focal_length(..)fromPerspectiveCamera::on_render_begin(..)to all childen (pinhole, thinlens, fisheye). This is becauseMultiLensCamerahas its own focal length extraction that allows for not specifying a focal length.Results
The implemented model can achieve the following:
It cannot produce:
Performance
I spent quite some time analyzing the performance of the implementation in relation to pinhole and thin lens camera. For details, see Chapter 6.2 of my thesis. The main takeaways were:
Conclusion
The implemented model is not a thin lens replacement, but offers advantages in some scenarios, especially when bokeh or vignetting is important. Although performance is a bit worse than pinhole and thin lens, the overhead is constant and therefore too significant in complex scenes. With simple scenes though, the multi-lens camera is probably not worthwhile to use.