Allow SpaceViews to customize the behavior for how default visualizers are chosen #5050
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jleibs
added
🟦 blueprint
jleibs
force-pushed
the
jleibs/space_view_visualizer_determination
branch
from
a44e34e to
bff6e98
Compare
jleibs
changed the title
|
nice. if we'd now enable visualizer overrides we could solve 
|
Wumpf
approved these changes
Feb 6, 2024
Comment on lines
+168
to
+175
| // Because SeriesLine is our fallback visualizer, also include any entities for which | ||
| // SeriesLine is applicable, even if not indicated. | ||
| if let Some(applicable) = ctx | ||
| .applicable_entities_per_visualizer | ||
| .get(&SeriesLineSystem::identifier()) | ||
| { | ||
| indicated_entities.0.extend(applicable.iter().cloned()); | ||
| } |
There was a problem hiding this comment.
ah great catch! At first I thought a different mechanism needs to do this, but I think that's spot on!
Comment on lines
+255
to
+259
| // If there were no other visualizers, but the SeriesLineSystem is available, use it. | ||
| if visualizers.is_empty() && available_visualizers.contains(&SeriesLineSystem::identifier()) | ||
| { | ||
| visualizers.insert(0, SeriesLineSystem::identifier()); | ||
| } |
There was a problem hiding this comment.
unfortunate that we can't just call into the base for the rest (don't see how either). But otherwise perfect, just as planned!
Wumpf
force-pushed
the
jleibs/space_view_visualizer_determination
branch
from
bff6e98 to
49e3cd9
Compare
7 tasks
teh-cmc
added a commit
that referenced
this pull request
Feb 6, 2024
…ad (#5042) ### What Also fixes missing serialization of `StrokeWidth` & `MarkerSize`  blocked by a few things: * [x] text labels should go on SeriesLine/SeriesPlot #5043 * [x] SeriesPoint doesn't have a `MarkerSize` component yet #5057 * [x] Need to fix heuristic for only having Scalar -> Line #5050 --------- Co-authored-by: Clement Rey <[email protected]>
Closed
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What
We don't want to use Scalar as the indicator for LineSeries or PointSeries or else we would get both any time a user logs a Scalar. Instead we now delegate this choice to the space-view.
Additionally, entities are now always included in the query-results if they are visualizable, even if they aren't indicated. Without this there is no way to access the query-result in order to override its visualizers if necessary.
Checklist
mainbuild: app.rerun.ionightlybuild: app.rerun.io