Feature/edge colors #52
Conversation
Schmluk
commented
Jan 21, 2026
Schmluk
commented
- Implements edge coloring with color adapters.
- Updated visualizer config to have the same defaults.
- Tested on uHumans2.
nathanhhughes
left a comment
nathanhhughes
left a comment
There was a problem hiding this comment.
LGTM, thanks! Pretty much exactly how I would have implemented it. Some minor cleanup things (and I think the virtual config for the adapter isn't parsed anywhere yet)
| * @param edge Edge to get color for | ||
| * @returns Visualizer color for source and target ends of the edge. | ||
| */ | ||
| virtual std::pair<spark_dsg::Color, spark_dsg::Color> getColor( |
There was a problem hiding this comment.
(minor) a typedef or a quick struct (e.g., EdgeColor) would be nice here to make the line break go away potentially
| renderer: | ||
| layer_z_step: 5.5 # unit separation between layers | ||
| collapse_layers: false # whether or not to apply offsets to each of the layers | ||
| layer_z_step: 5.5 # unit separation between layers |
There was a problem hiding this comment.
I'd probably just checkout the config from develop for now and/or delete use_color from the config (given that the uniform color functor is the default). I think vs code has a different yaml formatter configured than I do for pre-commit and/or something is weird
| field(config.alpha, "alpha"); | ||
| enum_field(config.color, "color"); | ||
| field(config.use_color, "use_color"); | ||
| field(config.color, "color"); |
There was a problem hiding this comment.
| field(config.color, "color"); | |
| field(config.color, "color"); | |
| field(config.edge_color, "edge_color"); |
I think this is missing?
There was a problem hiding this comment.
I think edge_color as a field does not exist, but it's edge.color, so this should be covered. I got rid of 'use_color` since this is now also embedded in the virtual conf, so 1 less field.
Edit: Tested this and it gets parsed correctly.
| // TODO(lschmid): This is not the prettiest, but e.g. handing the layerinfo to the | ||
| // adapters or so also didn't seem right. | ||
| edge_color = [this, &graph](const SceneGraphEdge& edge) { | ||
| return std::make_pair(node_color(graph.getNode(edge.source)), | ||
| node_color(graph.getNode(edge.target))); |
There was a problem hiding this comment.
I think this is the best way to do this for now (the only other way would be to pass the node colors as optional inputs to the edge adapter, but that's kinda akward)
| //! @brief show interlayer edge using node colors | ||
| bool interlayer_use_color = true; | ||
| //! @brief If true color mesh edges | ||
| bool interlayer_use_color; |
There was a problem hiding this comment.
This should probably have a default (true or false should be fine) for now (I've also been meaning to handle the configuration of interlayer edges and partitions better, which would make it easier to use adapter for interlayer edges)
There was a problem hiding this comment.
Yup! I first implemented also color adapters for this but then realized it's only used for the 2D places so I reverted it. Turns out the round trip was not lossless 😆
| max_value_(1.0), | ||
| functor_(config::create<EdgeValueFunctor>(config.value_functor)), | ||
| colormap_(config.colormap) { | ||
| CHECK(functor_) << "invalid functor type: " << config.value_functor; |
There was a problem hiding this comment.
I don't think a check makes sense here, I'd just have a warning (and add single quotes for the functor name)
There was a problem hiding this comment.
Copied this from the node colors, will update both!
Edit: Was thinking about adding a warning but I think cu will warn if the specified functor does not exist.
Co-authored-by: Nathan Hughes <nathan.h.hughes@gmail.com>
* first draft of edge color adapters * revert interlayer use color in config * Update hydra_visualizer/src/drawing.cpp Co-authored-by: Nathan Hughes <nathan.h.hughes@gmail.com> * implement edge color type * PR feedback * fix compilation and test --------- Co-authored-by: Nathan Hughes <nathan.h.hughes@gmail.com>
* first draft of edge color adapters * revert interlayer use color in config * Update hydra_visualizer/src/drawing.cpp Co-authored-by: Nathan Hughes <nathan.h.hughes@gmail.com> * implement edge color type * PR feedback * fix compilation and test --------- Co-authored-by: Nathan Hughes <nathan.h.hughes@gmail.com>
