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

Use egui_kittest for the graph view #8578

Merged
merged 17 commits into from
Jan 7, 2025
3 changes: 3 additions & 0 deletions Cargo.lock
Original file line number Diff line number Diff line change
Expand Up @@ -6648,10 +6648,12 @@ version = "0.22.0-alpha.1+dev"
dependencies = [
"ahash",
"egui",
"egui_kittest",
"fjadra",
"itertools 0.13.0",
"nohash-hasher",
"re_chunk",
"re_chunk_store",
"re_data_ui",
"re_entity_db",
"re_format",
Expand All @@ -6664,6 +6666,7 @@ dependencies = [
"re_ui",
"re_view",
"re_viewer_context",
"re_viewport",
"re_viewport_blueprint",
]

Expand Down
11 changes: 8 additions & 3 deletions crates/store/re_dataframe/src/query.rs
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,10 @@ impl<E: StorageEngineLike> QueryHandle<E> {
re_tracing::profile_scope!("init");

// The timeline doesn't matter if we're running in static-only mode.
let filtered_index = self.query.filtered_index.unwrap_or_default();
let filtered_index = self
.query
.filtered_index
.unwrap_or_else(|| Timeline::new_sequence(""));

// 1. Compute the schema for the query.
let view_contents = store.schema_for_query(&self.query);
Expand Down Expand Up @@ -331,8 +334,10 @@ impl<E: StorageEngineLike> QueryHandle<E> {
.map(|(_view_idx, descr)| match descr {
ColumnDescriptor::Time(_) => None,
ColumnDescriptor::Component(descr) => {
let query =
re_chunk::LatestAtQuery::new(Timeline::default(), TimeInt::STATIC);
let query = re_chunk::LatestAtQuery::new(
Timeline::new_sequence(""),
TimeInt::STATIC,
);

let results = cache.latest_at(
&query,
Expand Down
15 changes: 0 additions & 15 deletions crates/store/re_log_types/src/time_point/timeline.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,6 @@ re_string_interner::declare_new_type!(
pub struct TimelineName;
);

impl Default for TimelineName {
fn default() -> Self {
Self::from(String::default())
}
}

// ----------------------------------------------------------------------------

/// A time frame/space, e.g. `log_time` or `frame_nr`, coupled with the type of time
Expand All @@ -27,15 +21,6 @@ pub struct Timeline {
typ: TimeType,
}

impl Default for Timeline {
fn default() -> Self {
Self {
name: TimelineName::default(),
typ: TimeType::Sequence,
}
}
}

Comment on lines -30 to -38
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what's the rational there?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we tripped over default timelines being empty string and figured that this is practically never a valid timeline name (for all we know it is practically still valid, but even then we don't want to encourage it as the "default")

impl Timeline {
/// For absolute or relative time.
#[inline]
Expand Down
21 changes: 16 additions & 5 deletions crates/viewer/re_view_dataframe/src/dataframe_ui.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use re_chunk_store::external::re_chunk::Arrow2Array;
use re_chunk_store::{ColumnDescriptor, LatestAtQuery};
use re_dataframe::external::re_query::StorageEngineArcReadGuard;
use re_dataframe::QueryHandle;
use re_log_types::{EntityPath, TimeInt, Timeline, TimelineName};
use re_log_types::{EntityPath, TimeInt, TimeType, Timeline, TimelineName};
use re_types_core::ComponentName;
use re_ui::UiExt as _;
use re_viewer_context::{SystemCommandSender, ViewId, ViewerContext};
Expand Down Expand Up @@ -203,7 +203,11 @@ impl egui_table::TableDelegate for DataframeTableDelegate<'_> {
re_tracing::profile_function!();

// TODO(ab): actual static-only support
let filtered_index = self.query_handle.query().filtered_index.unwrap_or_default();
let filtered_index = self
.query_handle
.query()
.filtered_index
.unwrap_or_else(|| Timeline::new("", TimeType::Sequence));

self.query_handle
.seek_to_row(info.visible_rows.start as usize);
Expand Down Expand Up @@ -270,8 +274,11 @@ impl egui_table::TableDelegate for DataframeTableDelegate<'_> {
let column = &self.selected_columns[cell.col_range.start];

// TODO(ab): actual static-only support
let filtered_index =
self.query_handle.query().filtered_index.unwrap_or_default();
let filtered_index = self
.query_handle
.query()
.filtered_index
.unwrap_or_else(|| Timeline::new("", TimeType::Sequence));

// if this column can actually be hidden, then that's the corresponding action
let hide_action = match column {
Expand Down Expand Up @@ -393,7 +400,11 @@ impl egui_table::TableDelegate for DataframeTableDelegate<'_> {
.unwrap_or(TimeInt::MAX);

// TODO(ab): actual static-only support
let filtered_index = self.query_handle.query().filtered_index.unwrap_or_default();
let filtered_index = self
.query_handle
.query()
.filtered_index
.unwrap_or_else(|| Timeline::new("", TimeType::Sequence));
let latest_at_query = LatestAtQuery::new(filtered_index, timestamp);

if ui.is_sizing_pass() {
Expand Down
7 changes: 7 additions & 0 deletions crates/viewer/re_view_graph/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -39,3 +39,10 @@ egui.workspace = true
fjadra.workspace = true
itertools.workspace = true
nohash-hasher.workspace = true


# TODO:
#[dev-dependencies]
egui_kittest.workspace = true
re_chunk_store.workspace = true
re_viewport.workspace = true
2 changes: 0 additions & 2 deletions crates/viewer/re_view_graph/src/layout/geometry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@ impl EdgeGeometry {
}

/// The starting position of an edge.
#[expect(unused)]
pub fn source_pos(&self) -> Pos2 {
match self.path {
PathGeometry::Line { source, .. } | PathGeometry::CubicBezier { source, .. } => source,
Expand All @@ -54,7 +53,6 @@ impl EdgeGeometry {
}

/// The direction of the edge at the source node (normalized).
#[expect(unused)]
pub fn source_arrow_direction(&self) -> Vec2 {
use PathGeometry::{CubicBezier, Line};
match self.path {
Expand Down
1 change: 0 additions & 1 deletion crates/viewer/re_view_graph/src/layout/result.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@ impl Layout {
}

/// Gets the shape of an edge in the final layout.
#[expect(unused)]
pub fn get_edge(&self, edge: &EdgeId) -> Option<&[EdgeGeometry]> {
self.edges.get(edge).map(|es| es.as_slice())
}
Expand Down
1 change: 1 addition & 0 deletions crates/viewer/re_view_graph/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,4 +9,5 @@ mod ui;
mod view;
mod visualizers;

pub use ui::GraphViewState;
pub use view::GraphView;
4 changes: 2 additions & 2 deletions crates/viewer/re_view_graph/src/visualizers/edges.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ pub struct EdgesVisualizer {

pub struct EdgeInstance {
// We will need this in the future, when we want to select individual edges.
pub _instance: Instance,
pub instance: Instance,
pub source: GraphNode,
pub target: GraphNode,
pub source_index: NodeId,
Expand Down Expand Up @@ -76,7 +76,7 @@ impl VisualizerSystem for EdgesVisualizer {
let target_index = NodeId::from_entity_node(entity_path, &target);

EdgeInstance {
_instance: Instance::from(i as u64),
instance: Instance::from(i as u64),
source,
target,
source_index,
Expand Down
Loading
Loading