Skip to content

Implement the rendering capability for FSI-SPH in Chrono::Sensor#738

Merged
rserban merged 19 commits intoprojectchrono:mainfrom
uwsbel:feature/crm-rendering
May 9, 2026
Merged

Implement the rendering capability for FSI-SPH in Chrono::Sensor#738
rserban merged 19 commits intoprojectchrono:mainfrom
uwsbel:feature/crm-rendering

Conversation

@uebian
Copy link
Copy Markdown
Contributor

@uebian uebian commented May 4, 2026

No description provided.

@rserban
Copy link
Copy Markdown
Member

rserban commented May 6, 2026

@uebian - a few comments:

  1. There is no good reason to push to the repository all these 100 obj files with regolith particles. For the demo you added, it is enough to use a couple only to illustrate the use of the new capability. Application or project codes that need to use all of them will have to manage their assets elsewhere.
  2. These geometry files are not Chrono::Sensor-specific. Please move the 2 that you will be keeping under data/models./regolith/ or something along those lines.
  3. As it is, demo_ROBOT_Viper_CRM_Wheelsinkage is extremely slow. There is no need for a demo to use defaults that make it so computationally intensive. Furthermore, this demo is highlighting a specific new feature: rendering of CRM terrain with Chrono::Sensor; as such, it should not try to show all sort of other features that are already illustrated in other demos.
    Please change the demo to:
    ... rename it to something more relevant (again, this demo is about Chrono::Sensor rendering, not wheel sinkage)
    ... use a rectangular terrain patch (no other option).
    ... make the patch dimensions smaller (say, a length of 5).
    ... as discussed above, use only 1 or 2 regolith particle shapes.
  4. Please use clang-format throughout
  5. Remove the function ChFsiFluidSystemSPH::GetParticleData. I don't see where/why that is needed. Please only add new functions when they are actually needed. Also, even if you were to keep this function, a comment such as "Nevi added this" is unnecessary and pointless. Please provide proper and meaningful documentation for new functions that can be processed by doxygen.
  6. Same as above for FsiDataManager::GetParticleData(). Is this needed?! If so, where?
  7. In ChSensorManager.h why are the new functions in lines 89-99 only conditioned by the FSI-SPH module being enabled? Can they ever be useful if OptiX is not available? Please check building the code with OptiX support disabled. I think you'll find you may also need some fixes to CMakeLists.txt.

Thanks!

@uebian
Copy link
Copy Markdown
Contributor Author

uebian commented May 6, 2026

Yes, I agreed that GetParticleData should be removed. I missed it somehow. I pushed a fix. I'm currently working on other issues.

@uebian
Copy link
Copy Markdown
Contributor Author

uebian commented May 9, 2026

I have completed all requested changes, and the PR should be ready for review again:

There is no good reason to push to the repository all these 100 obj files with regolith particles. For the demo you added, it is enough to use a couple only to illustrate the use of the new capability. Application or project codes that need to use all of them will have to manage their assets elsewhere.

I reduced the number of particles to 3, and put the script used to re-generate them.

These geometry files are not Chrono::Sensor-specific. Please move the 2 that you will be keeping under data/models./regolith/ or something along those lines.

I relocated them.

As it is, demo_ROBOT_Viper_CRM_Wheelsinkage is extremely slow. There is no need for a demo to use defaults that make it so computationally intensive. Furthermore, this demo is highlighting a specific new feature: rendering of CRM terrain with Chrono::Sensor; as such, it should not try to show all sort of other features that are already illustrated in other demos.

I have increased the simulation spacing, and it runs on RTF=50 on my end now. I have renamed it to demo_SEN_CRM_Rendering.

Please use clang-format throughout

done.

Remove the function ChFsiFluidSystemSPH::GetParticleData. I don't see where/why that is needed. Please only add new functions when they are actually needed. Also, even if you were to keep this function, a comment such as "Nevi added this" is unnecessary and pointless. Please provide proper and meaningful documentation for new functions that can be processed by doxygen.

Same as above for FsiDataManager::GetParticleData(). Is this needed?! If so, where?

They are all removed.

In ChSensorManager.h why are the new functions in lines 89-99 only conditioned by the FSI-SPH module being enabled? Can they ever be useful if OptiX is not available? Please check building the code with OptiX support disabled. I think you'll find you may also need some fixes to CMakeLists.txt.

Regarding lines 89-88, I don't think attaching an FSI-SPH system to SensorManager needs OptiX specifically. It's just registering it with the SensorManager. Render it through OptiX cameras will require OptiX, but not attaching it. If in the future we have another kind of cameras, for example, cameras based on VulkanRT, they should be able to see the FSI-SPH data without OptiX. However, I do agree that ChSensorManager.cpp needs to be updated to ensure it can work without OptiX. I have made necessary changes.

I have tried compiling the project without OptiX, and it works on my end (Linux).

@rserban rserban merged commit cb72352 into projectchrono:main May 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants