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

Geraldine Chua chua.gsk at gmail.com
Thu Aug 2 18:54:27 CEST 2018 

Hi Brecht,

Sorry for the late reply, as I've been working on fixing as much of the
stuff you've mentioned as possible.

* 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?
>

Oh, I had already fixed this in Cycles but forgot to add the fix to
intern/openvdb. If you tried to render bunny_cloud.vdb, it would be
actually positioned normally. I have patched it up now. (trying to view
bunny_cloud in the viewport crashes Blender on my computer so I didn't
notice it's wrong transform XD)

* 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.
>

My current way of hiding all other smoke options is really hacky (too many
nots...), so maybe there's a better way of doing it. Incidentally, would it
be acceptable to implement mass import of VDB files for animation purposes?
It basically reuses a lot of the VDB pointcache code, and would make the
importer more immediately viable for users.

* 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.
>

So the way I interpreted this is, if "Deformation" is unchecked, Volume
motion blur is disabled? That's how I implemented it but it seems a little
unintuitive. Doesn't that option refer to dynamic mesh motion blur?

* Please remove device_memory_openvdb.h and related code to use OpenVDB
> accessors in the kernel, we won't merge this into master.
>

:( At least it was an interesting exercise to see if the VDB accessors
worked well in the kernel.

* Rename TILE_SIZE, LAST_TILE_WIDTH_SHIFT to all have a TEX_ prefix, to
> avoid potential confusion about what these are used for.
>

For this, I created a new struct SparseTextureInfo, so now we're not
passing 7 different variables in the kernel -.-

I'm still not entirely satisfied with the variable names in
SparseTextureInfo though. If anyone has suggestions, I am very open to
changing them D:

* 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.
>

I'm guessing that that the stuff in editors/io is fine since it
specifically handles OpenVDB files so I didn't change those but I have
updated everything else.

* 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.
> * ImageManager::name_from_grid_type() can be a static function rather than
> a class member function. Also rename "default" to "dense" in there.
> * 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.
> * 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.
>

Done with all of these :)

* 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();
> * 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.
>

The reason grid_info's memory use doesn't directly get added to
memory_size() is because I believe the function gets used for memory
allocation, freeing, etc. so I didn't want to mess with it. In order to
avoid having allocation errors, grid_info memory check is now logged
separately.

I've been holding back on fixing OpenCL until the CPU implementation was
mostly finalized and I've had to completely rewrite the CUDA sparse grid
generator since the old version worked on some incorrect assumptions and
was generally very bad. I've uploaded my rewrite and in theory it should
now sample properly (when I run the algorithm on CPU it samples fine), but
unfortunately, I can't really test it out since my desktop's graphics card
is broken... I would really be thankful for your help :)

My main concern is how CUDA and OpenCL load textures in the kernel. For CPU
rendering, it seems to be fine to not directly allocate the device_memory
grid_info data and instead store a pointer to the data in the volume
texture. This approach may not work out for CUDA/OpenCL. If it doesn't, I
think the offsets device_memory will need to be allocated as an actual
image.

So right now, the main hurdles left are merging with 2.8(?) and making sure
that sparse grids can be rendered on OpenCL/CUDA, unless there are any more
issues with the current state of the branch.

Best regards,
Geraldine


On Mon, Jul 30, 2018 at 12:20 AM, Brecht Van Lommel <
brechtvanlommel at gmail.com> wrote:

> 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
>>
>
> --
> 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/20180803/264f0252/attachment-0001.html>

More information about the Soc-2018-dev mailing list