-
Notifications
You must be signed in to change notification settings - Fork 40
Add Custom Spatial Fields Support #212
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add Custom Spatial Fields Support #212
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR introduces support for analytical (procedural) spatial fields in VisRTX, enabling GPU-computed volumetric fields that generate values on-the-fly rather than storing pre-computed data. The implementation adds a plugin-based architecture allowing external applications to register custom field types at runtime.
Changes:
- Add analytical field infrastructure with registry pattern for extensibility
- Implement GPU-side OptiX callable programs for analytical field sampling
- Extend CMake configuration to handle target conflicts when building alongside other projects
Reviewed changes
Copilot reviewed 19 out of 19 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| devices/rtx/external/stb_image/CMakeLists.txt | Add guards to handle stb_image target conflicts with external dependencies (e.g., OWL) |
| devices/rtx/external/CMakeLists.txt | Add target existence checks for fmt, nonstd, and stb_image to prevent duplicate definitions |
| devices/rtx/device/spatial_field/SpatialFieldRegistry.h | Define registry system for registering and creating custom spatial field types |
| devices/rtx/device/spatial_field/SpatialField.cpp | Integrate registry lookup into factory method for custom field types |
| devices/rtx/device/spatial_field/RegisterAnalyticalFields.cpp | Provide public API for external applications to register analytical fields |
| devices/rtx/device/spatial_field/AnalyticalFieldSampler_ptx.cu | Implement OptiX callable programs for analytical field sampling on GPU |
| devices/rtx/device/spatial_field/AnalyticalFieldSampler.h | Define PTX wrapper interface for analytical field samplers |
| devices/rtx/device/spatial_field/AnalyticalFieldSampler.cpp | Implement PTX blob retrieval for analytical field sampler |
| devices/rtx/device/spatial_field/AnalyticalField.h | Define abstract base class for analytical spatial fields |
| devices/rtx/device/sampler/Image3D.h | Add public accessors for texture object and image size |
| devices/rtx/device/sampler/Image3D.cpp | Implement imageSize() accessor method |
| devices/rtx/device/sampler/Image2D.h | Add public accessor for texture object |
| devices/rtx/device/renderer/Renderer.cpp | Register analytical field sampler callables in OptiX pipeline |
| devices/rtx/device/optix_visrtx.h | Add analytical field module to device state |
| devices/rtx/device/gpu/shadingState.h | Add analytical data field to volume sampling state union |
| devices/rtx/device/gpu/sbt.h | Add analytical field sampler entry point to callable enum |
| devices/rtx/device/gpu/gpu_objects.h | Define AnalyticalData structure and add to SpatialFieldGPUData union |
| devices/rtx/device/VisRTXDevice.cpp | Initialize analytical field sampler module during OptiX setup |
| devices/rtx/device/CMakeLists.txt | Add analytical field source files and PTX generation targets |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Resolves target name conflicts between VisRTX's stb_image (STATIC library) and Barney/OWL's stb_image (INTERFACE library) when building projects that depend on both. Changes: - Add guard to skip stb_image subdirectory if target already exists - Create visrtx_stb_image as alternative target when stb_image exists - Update tsd_tinygltf to handle both stb_image variants - Add proper include directories for OWL's stb/ subdirectory layout This allows VolumetricPlanets and other projects to build successfully when including both Barney and VisRTX/TSD in the same CMake project.
Extends TSD UpdateDelegate architecture to support animation time change notifications, enabling applications to respond when scene.setAnimationTime() is called. Changes: - Add signalAnimationTimeChanged(float time) to BaseUpdateDelegate interface - Implement signal propagation in MultiUpdateDelegate - Call signal from Scene::setAnimationTime() to notify all delegates - Add no-op implementation in RenderIndex - Update CameraPoses offline rendering to set animation time This enables applications to register custom delegates that respond to animation time changes, useful for updating time-dependent spatial fields during both interactive playback and offline rendering. Architecture benefits: - Clean observer pattern: TSD core decoupled from application code - Extensible: Multiple animation time observers can be registered - Works across all rendering modes: interactive, offline, camera paths
- Add textureObject() and imageSize() accessors to Image2D and Image3D for analytical field texture sampling - Add lat/lon bounds (minLat, maxLat, minLon, maxLon) to CloudFieldData for geographic region filtering
- Add VISRTX_ANALYTICAL_FIELD_DATA_HEADER for external field data definitions - Add VISRTX_ANALYTICAL_SAMPLERS_HEADER for external sampler implementations - Add VISRTX_ANALYTICAL_SAMPLE_DISPATCH macro for dispatch logic - Fix 8-byte alignment for fieldData array to support cudaTextureObject_t - Remove AnalyticalFieldData.h (moved to consuming project) - Add AnalyticalData to VolumeSamplingState union for sampler access This allows external projects (like VolumetricPlanets) to provide custom analytical field implementations without modifying VisRTX core.
The previous fix attempted to handle different stb_image variants but introduced a CI failure. Simplify to just use the stb_image target and its INTERFACE_INCLUDE_DIRECTORIES property directly. This ensures stb_image.h is found regardless of which stb_image implementation is being used (TSD's own or from an external source).
The stb_image conflict fixes should be in devices/rtx/external only, not in TSD's tsd_tinygltf. TSD builds its own stb_image and the link dependency is sufficient to propagate include directories. This fixes the CI failure where stb_image.h couldn't be found.
5aa002d to
0cef498
Compare
tarcila
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@favreau As discussed, I've been rebasing this PR and renamed Analytic to Custom.
Please have a round of testing with your code to confirm this is still working as designed.
One question about the (Analytical/Custom)FieldData. Mostly for the sake of it, as there is no real need to change that.
| // External projects can use this to store | ||
| // their custom field parameters and reinterpret_cast as needed | ||
| // Aligned to 8 bytes to support cudaTextureObject_t and other 64-bit types | ||
| alignas(8) uint8_t fieldData[256]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This adds quite a bunch to the union where this new type is used. Going from 160B to 352B.
Probably not that big of a deal given that those SpatialFieldGPUData instances are always in global memory , and the wasted space is then unused. Tho, I wonder if it would it make sense to replace the fieldData array by a device pointer?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point! There are only a handful of field instances in global memory, so the ~192 bytes of waste per non-analytical field is negligible. A device pointer would save union space but adds a pointer chase on every ray-march sample, not worth the tradeoff for the tiny amount of memory saved I think.
| int numChannels() const override; | ||
|
|
||
| // Public accessor for texture object (used by custom fields) | ||
| cudaTextureObject_t textureObject() const |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't exactly like those accessor. They are brittle, as the actual state of the sampler will depend on whether the ImageXD sampler has been (re)finalized or not.
A probably better solution is to directly go through the image and create resources from there, as done in ImageX::finalize(). Might require exposing some helpers.
Let's merge this PR as-is and revisit that later on.
Also, it would be nice to have the same exposure to other image sampler types.
favreau
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I could upgrade all apps and it's all working great! Only just the CustomData has to be changed to CustomFieldData in devices/rtx/device/spatial_field/CustomFieldSampler_ptx.cu.
Many thanks 👍
| // External projects can use this to store | ||
| // their custom field parameters and reinterpret_cast as needed | ||
| // Aligned to 8 bytes to support cudaTextureObject_t and other 64-bit types | ||
| alignas(8) uint8_t fieldData[256]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point! There are only a handful of field instances in global memory, so the ~192 bytes of waste per non-analytical field is negligible. A device pointer would save union space but adds a pointer chase on every ray-march sample, not worth the tradeoff for the tiny amount of memory saved I think.
`CustomData` should be C`ustomFieldData` Co-authored-by: Cyrille Favreau <cfavreau@nvidia.com>
This PR introduces support for analytical (procedural) spatial fields in VisRTX, enabling GPU-computed volumetric fields that generate values on-the-fly rather than storing pre-computed data.
Architecture:
Device-Side changes (VisRTX):