You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
I would like to submit a change to Kimera-VIO to allow a user to optionally build with or without the visualizer. This will be done in several PRs to make reviewing the changes easier.
Why?
My team is currently trying to use Kimera-VIO for localization on a palm-sized nano drone. The iMX93 (aarch64) processor that we are using is not very powerful and there isn’t a scenario where we want to run visualization on-board. Furthermore, visualization has a number of heavy dependencies i.e. cv::viz and VTK, which we don’t want to cross-compile and install (large dependency chain that we never plan on using).
However, we still think visualization is useful generally for the following reasons:
It’s easy to confirm that something “works” on a development (rather than production) device
Easy to monitor feature detections when doing development with a custom dataset
Provides quick insight when tuning parameters (trajectory tracking, features)
I definitely think the existing visualizer could still be more useful (e.g. remote visualization) but this might require a complete overhaul. I’ll be creating a separate issue for this some time soon after I look into this a bit more deeply.
Proposed changes
See below for summary of changes and here for full design document:
Introducing a new KIMERA_BUILD_VISUALIZER flag for controlling if visualization is included in a build. This will be on by default and accessed in code using #ifdef preprocessor directives.
Creating new visualizer abstract classes that:
Existing visualizer classes are derived from
That new dummy classes are derived from. These dummy classes will have stub functions of those in the main visualizer
classes.
This will allow minimal changes to be made to existing classes while maintaining existing implementations and minimizing #ifdef use. For classes that are already derived from abstract classes (e.g. DisplayParams) only new dummy classes will be created.
Excluding visualizer header files from other classes via the #ifdef preprocessor directive with KIMERA_BUILD_VISUALIZER.
Redefining the WidgetsMap typedef to a string when building without visualization. Visualizer-definitions.h will be included in this build configuration.
Moving visualization code (e.g. draw3DMesh, spinDisplay, _window) from MeshOptimization into OpenCvVisualizer3D.
If you have a better place in mind for where to include this, definitely let me know.
Adding a toolchain file for cross-compiling for aarch64 with gcc/g++. Tests will be built, but not run.
Excluding visualizer tests when building without visualization.
What we have found is that most test files have nothing visualizer or cv::viz related, but some of them do and I have a few questions about how to handle this when building without the visualizer:
testTracker and testOpticalFlowPredictor both have visualization functions that use cv::viz objects.
These functions are all used when the display flag is true.
Is it reasonable to simply place #ifdef blocks around these? It definitely feels a bit clunky, but moving test specific objects into separate classes adds additional complexity.
testRgbdCamera and testEurocPlayground both use a number of visualizer objects with the display flag.
When building without the visualizer, are you OK with us using the dummy visualizer classes here and setting the display flag to false to prevent test failures where actual visualizer data is required?
Looking forward to a response! Open to discussion if you have any pushback or questions.
-Sarika (Virtana TT Ltd)
The text was updated successfully, but these errors were encountered:
Following up on the above, since the Kimera-VIO-ROS wrapper with Rviz already does most of what the visualizer does, it would be a lot cleaner to delete since we don’t end up with a lot of branching logic.
I would like to go ahead and make these changes in a series of PRs in the following order:
Removing all references to the visualizer in non-visualizer classes and updating related tests accordingly.
Proposal: Submit this in separate PRs by module (e.g. pipeline class changes in one PR, frontend class changes in another PR etc) to make it easier to review.
Updating the Readme accordingly to call out that visualization will exclusively be available using Rviz. Edit: This should be in whatever the first PR is with the visualizer turned off by default.
Removing at CMakeList.txt references to the visualizer.
Deleting the visualizer module.
Does that sound reasonable?
Additionally, should I be pull requesting these changes against the master branch or is there a develop branch somewhere that I should be using instead?
What?
I would like to submit a change to Kimera-VIO to allow a user to optionally build with or without the visualizer. This will be done in several PRs to make reviewing the changes easier.
Why?
My team is currently trying to use Kimera-VIO for localization on a palm-sized nano drone. The iMX93 (aarch64) processor that we are using is not very powerful and there isn’t a scenario where we want to run visualization on-board. Furthermore, visualization has a number of heavy dependencies i.e.
cv::viz
and VTK, which we don’t want to cross-compile and install (large dependency chain that we never plan on using).However, we still think visualization is useful generally for the following reasons:
I definitely think the existing visualizer could still be more useful (e.g. remote visualization) but this might require a complete overhaul. I’ll be creating a separate issue for this some time soon after I look into this a bit more deeply.
Proposed changes
See below for summary of changes and here for full design document:
KIMERA_BUILD_VISUALIZER
flag for controlling if visualization is included in a build. This will be on by default and accessed in code using#ifdef
preprocessor directives.classes.
This will allow minimal changes to be made to existing classes while maintaining existing implementations and minimizing
#ifdef
use. For classes that are already derived from abstract classes (e.g.DisplayParams
) only new dummy classes will be created.Redefining the
WidgetsMap
typedef to a string when building without visualization.Visualizer-definitions.h
will be included in this build configuration.draw3DMesh
,spinDisplay
,_window)
fromMeshOptimization
intoOpenCvVisualizer3D
.If you have a better place in mind for where to include this, definitely let me know.
What we have found is that most test files have nothing visualizer or cv::viz related, but some of them do and I have a few questions about how to handle this when building without the visualizer:
testTracker
andtestOpticalFlowPredictor
both have visualization functions that usecv::viz
objects.#ifdef
blocks around these? It definitely feels a bit clunky, but moving test specific objects into separate classes adds additional complexity.testRgbdCamera
andtestEurocPlayground
both use a number of visualizer objects with the display flag.Looking forward to a response! Open to discussion if you have any pushback or questions.
-Sarika (Virtana TT Ltd)
The text was updated successfully, but these errors were encountered: