Skip to content
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

Feature Addition: Allowing building without the visualizer #250

Open
sarika93 opened this issue Jan 13, 2025 · 1 comment
Open

Feature Addition: Allowing building without the visualizer #250

sarika93 opened this issue Jan 13, 2025 · 1 comment

Comments

@sarika93
Copy link

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:

  1. It’s easy to confirm that something “works” on a development (rather than production) device
  2. Easy to monitor feature detections when doing development with a custom dataset
  3. 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:

  1. 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.
  2. 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)

@sarika93
Copy link
Author

sarika93 commented Jan 30, 2025

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?

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

No branches or pull requests

1 participant