Skip to content

Commit

Permalink
Undo some of the refactoring on ViewProperties to simplify for now
Browse files Browse the repository at this point in the history
  • Loading branch information
jleibs committed Jun 6, 2024
1 parent 329d805 commit ecc2aa6
Show file tree
Hide file tree
Showing 20 changed files with 164 additions and 246 deletions.
24 changes: 9 additions & 15 deletions crates/re_selection_panel/src/override_ui.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@ use re_log_types::{DataCell, DataRow, RowId, StoreKind};
use re_types_core::{components::VisualizerOverrides, ComponentName};
use re_ui::{ContextExt as _, UiExt as _};
use re_viewer_context::{
ComponentUiTypes, DataResult, OverridePath, QueryContext, SpaceViewClassExt as _,
SystemCommand, SystemCommandSender as _, ViewContext, ViewSystemIdentifier, ViewerContext,
ComponentUiTypes, DataResult, OverridePath, SpaceViewClassExt as _, SystemCommand,
SystemCommandSender as _, ViewContext, ViewSystemIdentifier, ViewerContext,
};
use re_viewport_blueprint::SpaceViewBlueprint;

Expand Down Expand Up @@ -79,7 +79,9 @@ pub fn override_ui(
&data_result,
);

let Some(overrides) = data_result.property_overrides else {
// TODO(jleibs): This clone is annoying, but required because QueryContext wants
// a reference to the EntityPath. We could probably refactor this to avoid the clone.
let Some(overrides) = data_result.property_overrides.clone() else {
return;
};

Expand All @@ -88,6 +90,8 @@ pub fn override_ui(
.into_iter()
.sorted_by_key(|(c, _)| *c);

let query_context = ctx.query_context(&data_result, &query);

re_ui::list_item::list_item_scope(ui, "overrides", |ui| {
ui.spacing_mut().item_spacing.y = 0.0;
for (
Expand Down Expand Up @@ -132,12 +136,7 @@ pub fn override_ui(

if let Some(results) = component_data {
ctx.viewer_ctx.component_ui_registry.singleline_edit_ui(
&QueryContext {
view_ctx: ctx,
target_entity_path: &instance_path.entity_path,
archetype_name: None,
query: &query,
},
&query_context,
ui,
origin_db,
entity_path_overridden,
Expand Down Expand Up @@ -195,12 +194,7 @@ pub fn add_new_override(
opened = true;
ui.style_mut().wrap_mode = Some(egui::TextWrapMode::Extend);

let query_context = QueryContext {
view_ctx: ctx,
target_entity_path: &data_result.entity_path,
archetype_name: None,
query,
};
let query_context = ctx.query_context(data_result, query);

// Present the option to add new components for each component that doesn't
// already have an active override.
Expand Down
14 changes: 4 additions & 10 deletions crates/re_selection_panel/src/selection_panel.rs
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ impl SelectionPanel {
override_visualizer_ui(ctx, view, instance_path, ui);
});

let view_ctx = view.bundle_context(ctx, view_states);
let view_ctx = view.bundle_context_with_states(ctx, view_states);

ui.large_collapsing_header("Component Overrides", true, |ui| {
override_ui(&view_ctx, view, instance_path, ui);
Expand Down Expand Up @@ -220,19 +220,13 @@ impl SelectionPanel {
}

Item::DataResult(space_view_id, instance_path) => {
let Some(space_view) = blueprint.space_view(space_view_id) else {
let Some(view) = blueprint.space_view(space_view_id) else {
return;
};

let view_ctx = view.bundle_context(ctx, view_states);
let view_ctx = view.bundle_context_with_states(ctx, view_states);

blueprint_ui_for_data_result(
&view_ctx,
space_view,
ui,
*space_view_id,
instance_path,
);
blueprint_ui_for_data_result(&view_ctx, view, ui, *space_view_id, instance_path);
}
}
}
Expand Down
4 changes: 3 additions & 1 deletion crates/re_space_view/src/results_ext.rs
Original file line number Diff line number Diff line change
Expand Up @@ -90,10 +90,12 @@ impl<'a> HybridLatestAtResults<'a> {
.best_fallback_for(self.ctx, component_name)?;

let query_context = QueryContext {
view_ctx: self.ctx,
viewer_ctx: self.ctx.viewer_ctx,
target_entity_path: &self.data_result.entity_path,
archetype_name: None, // TODO(jleibs): Do we need this?
query: &self.query,
view_state: self.ctx.view_state,
view_ctx: None,
};

fallback_provider
Expand Down
66 changes: 31 additions & 35 deletions crates/re_space_view/src/view_property_ui.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use ahash::HashMap;
use re_types_core::{Archetype, ArchetypeFieldInfo, ArchetypeInfo, ComponentName};
use re_ui::{list_item, UiExt as _};
use re_viewer_context::{
ComponentFallbackProvider, ComponentUiTypes, QueryContext, SpaceViewId, ViewContext,
ComponentFallbackProvider, ComponentUiTypes, QueryContext, SpaceViewId, SpaceViewState,
ViewerContext,
};
use re_viewport_blueprint::entity_path_for_view_property;
Expand All @@ -11,32 +11,36 @@ use re_viewport_blueprint::entity_path_for_view_property;
///
/// Note that this will show default values for components that are null.
pub fn view_property_ui<A: Archetype>(
ctx: &ViewContext<'_>,
ctx: &ViewerContext<'_>,
ui: &mut egui::Ui,
view_id: SpaceViewId,
fallback_provider: &dyn ComponentFallbackProvider,
view_state: &dyn SpaceViewState,
) {
view_property_ui_impl(ctx, ui, view_id, A::info(), fallback_provider);
view_property_ui_impl(ctx, ui, view_id, A::info(), view_state, fallback_provider);
}

fn view_property_ui_impl(
ctx: &ViewContext<'_>,
ctx: &ViewerContext<'_>,
ui: &mut egui::Ui,
view_id: SpaceViewId,
archetype: ArchetypeInfo,
view_state: &dyn SpaceViewState,
fallback_provider: &dyn ComponentFallbackProvider,
) {
let blueprint_path =
entity_path_for_view_property(view_id, ctx.blueprint_db().tree(), archetype.name);
let query_ctx = QueryContext {
view_ctx: ctx,
viewer_ctx: ctx,
target_entity_path: &blueprint_path,
archetype_name: Some(archetype.name),
query: ctx.viewer_ctx.blueprint_query,
query: ctx.blueprint_query,
view_state,
view_ctx: None,
};

let component_results = ctx.blueprint_db().latest_at(
ctx.viewer_ctx.blueprint_query,
ctx.blueprint_query,
&blueprint_path,
archetype.component_names.iter().copied(),
);
Expand Down Expand Up @@ -128,7 +132,6 @@ fn view_property_component_ui(
);

let ui_types = ctx
.view_ctx
.viewer_ctx
.component_ui_registry
.registered_ui_types(component_name);
Expand All @@ -144,18 +147,15 @@ fn view_property_component_ui(
default_open,
singleline_list_item_content,
|ui| {
ctx.view_ctx
.viewer_ctx
.component_ui_registry
.multiline_edit_ui(
ctx,
ui,
ctx.view_ctx.blueprint_db(),
blueprint_path,
component_name,
component_results,
fallback_provider,
);
ctx.viewer_ctx.component_ui_registry.multiline_edit_ui(
ctx,
ui,
ctx.viewer_ctx.blueprint_db(),
blueprint_path,
component_name,
component_results,
fallback_provider,
);
},
)
.item_response
Expand All @@ -171,7 +171,7 @@ fn view_property_component_ui(
}

view_property_context_menu(
ctx.view_ctx.viewer_ctx,
ctx.viewer_ctx,
&list_item_response,
blueprint_path,
component_name,
Expand Down Expand Up @@ -222,22 +222,18 @@ fn singleline_list_item_content<'a>(
) -> list_item::PropertyContent<'a> {
list_item::PropertyContent::new(display_name)
.action_button(&re_ui::icons::RESET, move || {
ctx.view_ctx
.viewer_ctx
ctx.viewer_ctx
.reset_blueprint_component_by_name(blueprint_path, component_name);
})
.value_fn(move |ui, _| {
ctx.view_ctx
.viewer_ctx
.component_ui_registry
.singleline_edit_ui(
ctx,
ui,
ctx.view_ctx.blueprint_db(),
blueprint_path,
component_name,
component_results,
fallback_provider,
);
ctx.viewer_ctx.component_ui_registry.singleline_edit_ui(
ctx,
ui,
ctx.viewer_ctx.blueprint_db(),
blueprint_path,
component_name,
component_results,
fallback_provider,
);
})
}
9 changes: 5 additions & 4 deletions crates/re_space_view_spatial/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ mod view_3d_properties;
mod visualizers;

use re_space_view::DataResultQuery as _;
use re_viewer_context::ViewContext;
use re_viewer_context::{ViewContext, ViewerContext};
pub use view_2d::SpatialSpaceView2D;
pub use view_3d::SpatialSpaceView3D;

Expand Down Expand Up @@ -113,14 +113,15 @@ fn query_pinhole_legacy(
}

pub(crate) fn configure_background(
ctx: &ViewContext<'_>,
ctx: &ViewerContext<'_>,
background: &ViewProperty<'_>,
render_ctx: &RenderContext,
view_system: &dyn re_viewer_context::ComponentFallbackProvider,
state: &dyn re_viewer_context::SpaceViewState,
) -> Result<(Option<re_renderer::QueueableDrawData>, re_renderer::Rgba), ViewPropertyQueryError> {
use re_renderer::renderer;

let kind: BackgroundKind = background.component_or_fallback(ctx, view_system)?;
let kind: BackgroundKind = background.component_or_fallback(ctx, view_system, state)?;

match kind {
BackgroundKind::GradientDark => Ok((
Expand All @@ -146,7 +147,7 @@ pub(crate) fn configure_background(
)),

BackgroundKind::SolidColor => {
let color: Color = background.component_or_fallback(ctx, view_system)?;
let color: Color = background.component_or_fallback(ctx, view_system, state)?;
Ok((None, color.into()))
}
}
Expand Down
39 changes: 10 additions & 29 deletions crates/re_space_view_spatial/src/ui_2d.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,8 @@ use re_types::{
components::ViewCoordinates,
};
use re_viewer_context::{
gpu_bridge, ItemSpaceContext, SpaceViewClass as _, SpaceViewId, SpaceViewSystemExecutionError,
SystemExecutionOutput, ViewContext, ViewQuery, ViewerContext,
gpu_bridge, ItemSpaceContext, SpaceViewId, SpaceViewSystemExecutionError,
SystemExecutionOutput, ViewQuery, ViewerContext,
};
use re_viewport_blueprint::ViewProperty;

Expand All @@ -35,18 +35,19 @@ use crate::{

/// Pan and zoom, and return the current transform.
fn ui_from_scene(
ctx: &ViewContext<'_>,
ctx: &ViewerContext<'_>,
view_id: SpaceViewId,
response: &egui::Response,
view_class: &SpatialSpaceView2D,
view_state: &SpatialSpaceViewState,
) -> RectTransform {
let bounds_property = ViewProperty::from_archetype::<VisualBounds2D>(
ctx.blueprint_db(),
ctx.viewer_ctx.blueprint_query,
ctx.blueprint_query,
view_id,
);
let bounds: blueprint_components::VisualBounds2D = bounds_property
.component_or_fallback(ctx, view_class)
.component_or_fallback(ctx, view_class, view_state)
.ok_or_log_error()
.unwrap_or_default();
let mut bounds_rect: egui::Rect = bounds.into();
Expand Down Expand Up @@ -109,10 +110,9 @@ fn ui_from_scene(
// Update blueprint if changed
let updated_bounds: blueprint_components::VisualBounds2D = bounds_rect.into();
if response.double_clicked() {
bounds_property
.reset_blueprint_component::<blueprint_components::VisualBounds2D>(ctx.viewer_ctx);
bounds_property.reset_blueprint_component::<blueprint_components::VisualBounds2D>(ctx);
} else if bounds != updated_bounds {
bounds_property.save_blueprint_component(ctx.viewer_ctx, &updated_bounds);
bounds_property.save_blueprint_component(ctx, &updated_bounds);
}

RectTransform::from_to(letterboxed_bounds, response.rect)
Expand Down Expand Up @@ -177,19 +177,8 @@ impl SpatialSpaceView2D {
let (mut response, painter) =
ui.allocate_painter(ui.available_size(), egui::Sense::click_and_drag());

let visualizer_collection = ctx
.space_view_class_registry
.new_visualizer_collection(Self::identifier());

let view_context = ViewContext {
viewer_ctx: ctx,
view_id: query.space_view_id,
view_state: state,
visualizer_collection: &visualizer_collection,
};

// Convert ui coordinates to/from scene coordinates.
let ui_from_scene = ui_from_scene(&view_context, query.space_view_id, &response, self);
let ui_from_scene = ui_from_scene(ctx, query.space_view_id, &response, self, state);
let scene_from_ui = ui_from_scene.inverse();

// TODO(andreas): Use the same eye & transformations as in `setup_target_config`.
Expand Down Expand Up @@ -244,14 +233,6 @@ impl SpatialSpaceView2D {
)?;
}

// TODO(jleibs): Creating (and recreating this) is annoying. Sort out when we need state to be mutable.
let view_context = ViewContext {
viewer_ctx: ctx,
view_id: query.space_view_id,
view_state: state,
visualizer_collection: &visualizer_collection,
};

for draw_data in draw_data {
view_builder.queue_draw(draw_data);
}
Expand All @@ -262,7 +243,7 @@ impl SpatialSpaceView2D {
query.space_view_id,
);
let (background_drawable, clear_color) =
crate::configure_background(&view_context, &background, render_ctx, self)?;
crate::configure_background(ctx, &background, render_ctx, self, state)?;
if let Some(background_drawable) = background_drawable {
view_builder.queue_draw(background_drawable);
}
Expand Down
18 changes: 3 additions & 15 deletions crates/re_space_view_spatial/src/ui_3d.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@ use re_types::{
blueprint::archetypes::Background, components::ViewCoordinates, view_coordinates::SignedAxis3,
};
use re_viewer_context::{
gpu_bridge, Item, ItemSpaceContext, SpaceViewClass as _, SpaceViewSystemExecutionError,
SystemExecutionOutput, ViewContext, ViewQuery, ViewerContext,
gpu_bridge, Item, ItemSpaceContext, SpaceViewSystemExecutionError, SystemExecutionOutput,
ViewQuery, ViewerContext,
};

use crate::{
Expand Down Expand Up @@ -681,20 +681,8 @@ impl SpatialSpaceView3D {
ctx.blueprint_query,
query.space_view_id,
);

let visualizer_collection = ctx
.space_view_class_registry
.new_visualizer_collection(Self::identifier());

let view_ctx = ViewContext {
viewer_ctx: ctx,
view_id: query.space_view_id,
view_state: state,
visualizer_collection: &visualizer_collection,
};

let (background_drawable, clear_color) =
crate::configure_background(&view_ctx, &background, render_ctx, self)?;
crate::configure_background(ctx, &background, render_ctx, self, state)?;
if let Some(background_drawable) = background_drawable {
view_builder.queue_draw(background_drawable);
}
Expand Down
Loading

0 comments on commit ecc2aa6

Please sign in to comment.