Cycles: enable NanoVDB by default #81454
Labels
No Label
Interest
Alembic
Interest
Animation & Rigging
Interest
Asset Browser
Interest
Asset Browser Project Overview
Interest
Audio
Interest
Automated Testing
Interest
Blender Asset Bundle
Interest
BlendFile
Interest
Collada
Interest
Compatibility
Interest
Compositing
Interest
Core
Interest
Cycles
Interest
Dependency Graph
Interest
Development Management
Interest
EEVEE
Interest
EEVEE & Viewport
Interest
Freestyle
Interest
Geometry Nodes
Interest
Grease Pencil
Interest
ID Management
Interest
Images & Movies
Interest
Import Export
Interest
Line Art
Interest
Masking
Interest
Metal
Interest
Modeling
Interest
Modifiers
Interest
Motion Tracking
Interest
Nodes & Physics
Interest
OpenGL
Interest
Overlay
Interest
Overrides
Interest
Performance
Interest
Physics
Interest
Pipeline, Assets & IO
Interest
Platforms, Builds & Tests
Interest
Python API
Interest
Render & Cycles
Interest
Render Pipeline
Interest
Sculpt, Paint & Texture
Interest
Text Editor
Interest
Translations
Interest
Triaging
Interest
Undo
Interest
USD
Interest
User Interface
Interest
UV Editing
Interest
VFX & Video
Interest
Video Sequencer
Interest
Virtual Reality
Interest
Vulkan
Interest
Wayland
Interest
Workbench
Interest: X11
Legacy
Blender 2.8 Project
Legacy
Milestone 1: Basic, Local Asset Browser
Legacy
OpenGL Error
Meta
Good First Issue
Meta
Papercut
Meta
Retrospective
Meta
Security
Module
Animation & Rigging
Module
Core
Module
Development Management
Module
EEVEE & Viewport
Module
Grease Pencil
Module
Modeling
Module
Nodes & Physics
Module
Pipeline, Assets & IO
Module
Platforms, Builds & Tests
Module
Python API
Module
Render & Cycles
Module
Sculpt, Paint & Texture
Module
Triaging
Module
User Interface
Module
VFX & Video
Platform
FreeBSD
Platform
Linux
Platform
macOS
Platform
Windows
Priority
High
Priority
Low
Priority
Normal
Priority
Unbreak Now!
Status
Archived
Status
Confirmed
Status
Duplicate
Status
Needs Info from Developers
Status
Needs Information from User
Status
Needs Triage
Status
Resolved
Type
Bug
Type
Design
Type
Known Issue
Type
Patch
Type
Report
Type
To Do
No Milestone
No project
No Assignees
7 Participants
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: blender/blender#81454
Loading…
Reference in New Issue
No description provided.
Delete Branch "%!s(<nil>)"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
For 2.92 we want to enable NanoVDB by default in Cycles. The implementation is available
3df90de6c2, but disabled by default.716b7a60df)Potential features to add:
a8c81ffa83)Changed status from 'Needs Triage' to: 'Confirmed'
Added subscribers: @brecht, @pmoursnv, @Stefan_Werner
Added subscriber: @SteffenD
Added subscribers: @sebbas, @LazyDodo, @mont29, @Sergey
Dear platform maintainers, can you add the NanoVDB library?
It's enabled by default in the CMake deps builder. This will install as a header only library in a new
nanovdbfolder, theopenvdblibrary does not need to be updated.@Sergey @sebbas @LazyDodo @mont29
Added the libs, however enabling it leads to a single failed test
{F9142033, width=100%}
Added subscriber: @juang3d
This comment was removed by @juang3d
With Linux I get a redeclaration error as soon as I enable NanoVDB, without it Blender builds fine, with it I get this:
Just with CUDA/Optix BTW, without CUDA/Optix it builds fine with NanoVDB
That's because NanoVDB includes stdint.h, but Cycles already declared those types in kernel_compat_cuda/optix.h (for NVRTC compatibility I guess). NanoVDB does the same when
__CUDACC_RTC__is defined, so I guess we could define that before including NanoVDB even in the NVCC case. Or instead too include stdint.h in the compat headers when__CUDACC_RTC__is not defined instead of typedefing (probably the cleaner solution). @brecht Thoughts?I'm thinking of this in the compat headers:
@pmoursnv, the suggested code looks good to me. The typedefs are indeed there for NVRTC.
Sorry for my lack of knowledge here, but I'm not sure if there is a typo here or if it's correct.
Meaning I'm not sure if its #ifdef or #ifndef, looking at other parts of the code I see several #ifndef in the kernel_compat files, also in the same way I'm not sure if it's #endif or #endinf.
Again this can be my lack of knowledge, I'm tyring to apply this code to kernel_compat_cuda.h and kernel_compat_optix.h, not sure if it's needed somewhere else too.
P.S.: ok, just changing the #endinf wiht an #endif solved it, I assume it was a typo in the code, and there is no need for #ifndef instead of #ifdef :)
This issue was referenced by
cf7343a355That was a typo and was supposed to be
#endif, yes =P.I detected the same as @LazyDodo the volume is shifted to the right of the picture, I compared a simple scene from 2.91 with 2.92
@pmoursnv not a problem, I was unsure if #endinf was something that I didn't knew, since there was #ifndef in the code, and I didn't knew that either :)
Thanks!
I have a question regarding the implementation of this feature, I've noticed that the memory usage is greatly reduce, vastly I would say, so it's awesome, but there is a difference in render time, in some test my time went from 19 seconds to 25 seconds, making it slower, @brecht do you think that could be interesting to locate a check box in performance tab to enable / disable this?
I don't think we should add an option. I don't want to maintain two different code paths for the same thing long term, and also for the GPU kernels isn't not great to increase in size that much.
Better focus would be to improve e.g. volume stepping performance by relying on NanoVDB traversal in addition to sampling.
I also imagine for that for CPU rendering there is still room for optimization in NanoVDB.
Ok, understood, if you think there is room for improvement then there is no reason to keep somethign older for the user :)
This issue was referenced by
fd9124ed6bOK, so after almost a day of work I managed to get
install_depsto build this (why can't they use default standard way of doing things???), but when trying to build Blender I get lots of those errors:Any insight?
This issue was referenced by blender/cycles@69aa84faa8
This issue was referenced by
118e31a0a9The regression test suite completes to 100% with NanoVDB enabled now for me. There were some changes that probably fix the above errors too.
I wonder what was the urgency to add this lib? Current building/install process for it is completely unconventional to say the least, and it's extremely unstable...
BTW, next time please specify exact revision needed, since their current
HEADis already 100% incompatible with our own code (and the expectede62f7a0bf1e27397223c61ddeaaf57edf111b77fwe use with Blender). Had to dig into cmake files to find that, lost a lot of time on this piece of unfinished software. :(And I really don't think such experimental lib should be enabled by default, this is call for all kind of issues imho. Would recommend waiting for it to be properly integrated in OpenVDB at least, i.e. be in their master, and be installable in a decent, standard way,
I'll admit, the whole, "oh just build this random git hash, but leave the original openvdb we have alone, and just copy these headers, it'll work" is somewhat unorthodox, but given @brecht signed off on it I didn't question it.
NanoVDB is standalone and header-only, so I don't really see the problem. Being able to just copy the headers without invoking a build system is the whole point of that. And it's only part of the official OpenVDB repository for simplicity in finding it, but really wouldn't have to be. Handling it as a separate library in Blender makes it easier to update when NanoVDB changes, without having to update OpenVDB too, which may have wider implications. Building random Git hashes in Blender isn't new either, the same is done for some other dependencies (CLEW, CUEW, HIDAPI, ...). Could we have waited for it to land in some versioned OpenVDB release? Probably. But that is a) likely months away (the OpenVDB release cycles isn't all that quick) and b) would still throw up the question of whether we really want to update the entire OpenVDB library (based on the VFX reference platform requirements etc.). Keeping it separate avoids all that.
I'm sorry there have been API changes since the last revision was chosen, which I had missed, that prolonged your time investment. That was unfortunate and my mistake for not keeping track of upstream changes. But there is no need to call NanoVDB unfinished for it. We are mostly using only the data structure in Cycles, which is stable (see bottom of https://github.com/AcademySoftwareFoundation/openvdb/blob/feature/nanovdb/nanovdb/docs/FAQ.md). There are API improvements still happening around that, but that doesn't change how it works at its core.
This issue was referenced by
bd6bfba64dIt's an important feature for volume rendering, that goes along with the volume object. It's unfortunate that this complicates building with
install_deps.sh, but to me it's all the more reason to simplify that system to become something to use only system packages, and not bother trying to match Blender release builds by building libraries from source.This issue was referenced by
a63208823cChanged status from 'Confirmed' to: 'Resolved'
This issue was referenced by
a8c81ffa83