[Soc-2018-dev] Weekly Report #11 - Further Development for Cycles' Volume Rendering

[Brecht Van Lommel]

Great work! I committed Stefan's patch now.

So overall loading VDB files and rendering them as sparse grids seems to be
working well, which is what I hoped for from this project. Here's my notes
from reviewing the code:

* Build error, CUDA and OpenCL device code needs to be updated to use the
same code as CPU device. Add grid_info size directly in
device_memory::memory_size() so the code is not duplicated.
    intern/cycles/device/device_cuda.cpp:1024:25: error: ‘void*’ is not a
pointer-to-object type
        size += mem.grid_info->memory_size();
* Building with OSL seems to be broken due to some conflict with our
foreach() macro and OpenVDB headers. I'll fix this in master as part of
making C++11 a minimum requirement, then we don't need a custom foreach
macro anymore and the conflict will be solved.
* Please remove device_memory_openvdb.h and related code to use OpenVDB
accessors in the kernel, we won't merge this into master.
* File > Import of bunny_cloud.vdb still shows wrong results in the
viewport, I think you figure out that this was due to the grids starting at
a negative index or something like that. Is there a fix for this?
* For the UI, when using OpenVDB import for smoke I suggest to just remove
all the buttons except for the file path and clipping. The "OpenVDB Import"
can be a hidden setting that is enabled when using File > Import, and there
is no need to be able to switch the smoke type, divisions, .. when it's all
coming from OpenVDB. It's all a bit of a hack but we might as well hide it
a bit more.
* In shader_eval_volume(), move setting sd->P_v = volume_get_position(kg,
sd); out of #ifdef __OBJECT_MOTION__. We still need this even when
compiling without motion blur support.
* In ShaderData, don't put P_v at the very end of the struct, maybe put it
after the ray differentials. The closures need to be the last member since
that array has a dynamic size.
* Rename TILE_SIZE, LAST_TILE_WIDTH_SHIFT to all have a TEX_ prefix, to
avoid potential confusion about what these are used for.
* The CUDA code in compute_sparse_coordinates() I guess is unfinished,
since *ix is being used without initialized and the result of modff() is
not used, etc. I can help finishing this code if needed.
* ImageManager::name_from_grid_type() can be a static function rather than
a class member function. Also rename "default" to "dense" in there.
* In MeshManager::device_update_attributes(), add (mesh->motion_steps > 1)
to the test to only request velocity if motion blur is enabled.
* Once this is done, the "Deformation" setting in the object "Motion Blur"
panel should allow disabling of volume motion blur. The tooltip of that
setting can then be adjusted to explain that.
* For OpenVDB, I prefer to get rid of the dependency on openvdb_capi.h and
intern/openvdb_reader.h. These mostly exists because other Blender code is
C only, but for Cycles it's already using the OpenVDB headers itself, so we
might as well use them everywhere. At least as far as I can tell the code
would not become much more complicated, and it's easier to make it work for
Cycles integration into other software then.
* On the Blender side, rename filter_openvdb to filter_volume,
FILE_TYPE_OPENVDB to FILE_TYPE_VOLUME, is_openvdb to use_volume_file, and
openvdb_filepath to volume_filepath. Not so important now, but a bit more
future proof in case we support more file types in the future.
* If you have the time, it would be nice to add support for loading boolean
and double OpenVDB grids as well. These would be converted to floats, and I
guess most handled by templating.


On Sat, Jul 28, 2018 at 6:03 PM Geraldine Chua <chua.gsk at gmail.com> wrote:

> This week was spent mostly reviewing and testing everything done in the
> past 10 weeks, and making revisions for better optimizations.
>
> Some changes:
>
>    - For motion blur, advected position is now cached in ShaderData and
>    only one sample is done (originally 3 samples were taken due to some
>    confusion over what was causing lack of blur). As an aside, normalized
>    position is also now cached, so there may be some modest speed-ups now in
>    rendering volumes even without motion blur.
>    - Sparse grids are now directly loaded into device memory rather than
>    loaded into a vector and then memcpy to device memory (used to be based on
>    how scaling images works).
>    - OpenVDBs are directly loaded as sparse grids (originally they were
>    loaded first as dense grids to allow some post-processing and then
>    converted to sparse grids).
>    - External VDBs may have background values that are not 0.0. In
>    theory, that means voxel values should be shifted to account for a
>    background value of 0.0 in Cycles, which is now done to all imported grids,
>    though whether this effect is required or not depends on the VDB imported.
>    - Changed how VDB values are iterated through from using accessors to
>    iterating through leaf nodes and acessing their buffers, greatly decreasing
>    iteration time (thanks to suggestion from Brecht!)
>
>
> Unrelated to my tasks, but the volume mesh builder has significant
> slowdown with very large volumes (e.g. a single 577x572x438 density grid
> takes ~25 seconds to build a mesh for), which is probably only somewhat
> obvious now when testing volumes from external VDBs. Stefan posted a patch
> that looks to fix this issue (https://developer.blender.org/P756).
>
> *To-dos next week*
>
> Next week should be mostly wrapping up, minor revisions, and merging. I
> will await code review to see if any new changes need to be made.
> --
> Soc-2018-dev mailing list
> Soc-2018-dev at blender.org
> https://lists.blender.org/mailman/listinfo/soc-2018-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.blender.org/pipermail/soc-2018-dev/attachments/20180729/eeb95ca2/attachment.html>