Skip to content

Commit

Permalink
Remove the feature flag for the dataframe view (#7663)
Browse files Browse the repository at this point in the history
### What

- Fixed #7494 

### Checklist
* [x] I have read and agree to [Contributor
Guide](https://github.com/rerun-io/rerun/blob/main/CONTRIBUTING.md) and
the [Code of
Conduct](https://github.com/rerun-io/rerun/blob/main/CODE_OF_CONDUCT.md)
* [x] I've included a screenshot or gif (if applicable)
* [x] I have tested the web demo (if applicable):
* Using examples from latest `main` build:
[rerun.io/viewer](https://rerun.io/viewer/pr/7663?manifest_url=https://app.rerun.io/version/main/examples_manifest.json)
* Using full set of examples from `nightly` build:
[rerun.io/viewer](https://rerun.io/viewer/pr/7663?manifest_url=https://app.rerun.io/version/nightly/examples_manifest.json)
* [x] The PR title and labels are set such as to maximize their
usefulness for the next release's CHANGELOG
* [x] If applicable, add a new check to the [release
checklist](https://github.com/rerun-io/rerun/blob/main/tests/python/release_checklist)!
* [x] If have noted any breaking changes to the log API in
`CHANGELOG.md` and the migration guide

- [PR Build Summary](https://build.rerun.io/pr/7663)
- [Recent benchmark results](https://build.rerun.io/graphs/crates.html)
- [Wasm size tracking](https://build.rerun.io/graphs/sizes.html)

To run all checks from `main`, comment on the PR with `@rerun-bot
full-check`.
  • Loading branch information
abey79 authored Oct 10, 2024
1 parent c6d842b commit e4aaee1
Show file tree
Hide file tree
Showing 5 changed files with 18 additions and 68 deletions.
46 changes: 11 additions & 35 deletions crates/viewer/re_viewer/src/app.rs
Original file line number Diff line number Diff line change
Expand Up @@ -257,12 +257,11 @@ impl App {
}

let mut space_view_class_registry = SpaceViewClassRegistry::default();
if let Err(err) = populate_space_view_class_registry_with_builtin(
&mut space_view_class_registry,
state.app_options(),
) {
if let Err(err) =
populate_space_view_class_registry_with_builtin(&mut space_view_class_registry)
{
re_log::error!(
"Failed to populate space view type registry with built-in space views: {}",
"Failed to populate the view type registry with built-in space views: {}",
err
);
}
Expand All @@ -289,15 +288,12 @@ impl App {

let panel_state_overrides = startup_options.panel_state_overrides;

let reflection = match crate::reflection::generate_reflection() {
Ok(reflection) => reflection,
Err(err) => {
re_log::error!(
"Failed to create list of serialized default values for components: {err}"
);
Default::default()
}
};
let reflection = crate::reflection::generate_reflection().unwrap_or_else(|err| {
re_log::error!(
"Failed to create list of serialized default values for components: {err}"
);
Default::default()
});

Self {
build_info,
Expand Down Expand Up @@ -537,22 +533,6 @@ impl App {
SystemCommand::EnableInspectBlueprintTimeline(show) => {
self.app_options_mut().inspect_blueprint_timeline = show;
}
SystemCommand::EnableExperimentalDataframeSpaceView(enabled) => {
let result = if enabled {
self.space_view_class_registry
.add_class::<re_space_view_dataframe::DataframeSpaceView>()
} else {
self.space_view_class_registry
.remove_class::<re_space_view_dataframe::DataframeSpaceView>()
};

if let Err(err) = result {
re_log::warn_once!(
"Failed to {} experimental dataframe space view: {err}",
if enabled { "enable" } else { "disable" }
);
}
}

SystemCommand::SetSelection(item) => {
self.state.selection_state.set_selection(item);
Expand Down Expand Up @@ -1724,7 +1704,6 @@ impl eframe::App for App {
/// Add built-in space views to the registry.
fn populate_space_view_class_registry_with_builtin(
space_view_class_registry: &mut SpaceViewClassRegistry,
app_options: &AppOptions,
) -> Result<(), SpaceViewClassRegistryError> {
re_tracing::profile_function!();
space_view_class_registry.add_class::<re_space_view_bar_chart::BarChartSpaceView>()?;
Expand All @@ -1734,10 +1713,7 @@ fn populate_space_view_class_registry_with_builtin(
space_view_class_registry.add_class::<re_space_view_text_document::TextDocumentSpaceView>()?;
space_view_class_registry.add_class::<re_space_view_text_log::TextSpaceView>()?;
space_view_class_registry.add_class::<re_space_view_time_series::TimeSeriesSpaceView>()?;

if app_options.experimental_dataframe_space_view {
space_view_class_registry.add_class::<re_space_view_dataframe::DataframeSpaceView>()?;
}
space_view_class_registry.add_class::<re_space_view_dataframe::DataframeSpaceView>()?;

Ok(())
}
Expand Down
29 changes: 6 additions & 23 deletions crates/viewer/re_viewer/src/ui/rerun_menu.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use egui::NumExt as _;

use re_log_types::TimeZone;
use re_ui::{UICommand, UiExt as _};
use re_viewer_context::{StoreContext, SystemCommand, SystemCommandSender};
use re_viewer_context::StoreContext;

use crate::App;

Expand Down Expand Up @@ -295,7 +295,7 @@ fn render_state_ui(ui: &mut egui::Ui, render_state: &egui_wgpu::RenderState) {
}

fn options_menu_ui(
command_sender: &re_viewer_context::CommandSender,
_command_sender: &re_viewer_context::CommandSender,
ui: &mut egui::Ui,
frame: &eframe::Frame,
app_options: &mut re_viewer_context::AppOptions,
Expand Down Expand Up @@ -355,7 +355,7 @@ fn options_menu_ui(
{
ui.add_space(SPACING);
ui.label("Experimental features:");
experimental_feature_ui(command_sender, ui, app_options);
experimental_feature_ui(ui, app_options);
}

if let Some(_backend) = frame
Expand All @@ -368,37 +368,20 @@ fn options_menu_ui(
{
ui.add_space(SPACING);
if _backend == wgpu::Backend::BrowserWebGpu {
UICommand::RestartWithWebGl.menu_button_ui(ui, command_sender);
UICommand::RestartWithWebGl.menu_button_ui(ui, _command_sender);
} else {
UICommand::RestartWithWebGpu.menu_button_ui(ui, command_sender);
UICommand::RestartWithWebGpu.menu_button_ui(ui, _command_sender);
}
}
}
}

fn experimental_feature_ui(
command_sender: &re_viewer_context::CommandSender,
ui: &mut egui::Ui,
app_options: &mut re_viewer_context::AppOptions,
) {
fn experimental_feature_ui(ui: &mut egui::Ui, app_options: &mut re_viewer_context::AppOptions) {
#[cfg(not(target_arch = "wasm32"))]
ui
.re_checkbox(&mut app_options.experimental_space_view_screenshots, "Space view screenshots")
.on_hover_text("Allow taking screenshots of 2D and 3D space views via their context menu. Does not contain labels.");

if ui
.re_checkbox(
&mut app_options.experimental_dataframe_space_view,
"Dataframe space view",
)
.on_hover_text("Enable the experimental dataframe space view.")
.clicked()
{
command_sender.send_system(SystemCommand::EnableExperimentalDataframeSpaceView(
app_options.experimental_dataframe_space_view,
));
}

ui.re_checkbox(
&mut app_options.plot_query_clamping,
"Plots: query clamping",
Expand Down
5 changes: 0 additions & 5 deletions crates/viewer/re_viewer_context/src/app_options.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,6 @@ pub struct AppOptions {
#[cfg(not(target_arch = "wasm32"))]
pub experimental_space_view_screenshots: bool,

/// Enable experimental dataframe space views.
pub experimental_dataframe_space_view: bool,

/// Toggle query clamping for the plot visualizers.
pub plot_query_clamping: bool,

Expand Down Expand Up @@ -53,8 +50,6 @@ impl Default for AppOptions {
#[cfg(not(target_arch = "wasm32"))]
experimental_space_view_screenshots: false,

experimental_dataframe_space_view: false,

plot_query_clamping: true,

show_picking_debug_overlay: false,
Expand Down
3 changes: 0 additions & 3 deletions crates/viewer/re_viewer_context/src/command_sender.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,9 +62,6 @@ pub enum SystemCommand {
#[cfg(debug_assertions)]
EnableInspectBlueprintTimeline(bool),

/// Enable or disable the experimental dataframe space views.
EnableExperimentalDataframeSpaceView(bool),

/// Set the item selection.
SetSelection(crate::Item),

Expand Down
3 changes: 1 addition & 2 deletions crates/viewer/re_viewer_context/src/test_context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -161,8 +161,7 @@ impl TestContext {
| SystemCommand::ClearAndGenerateBlueprint
| SystemCommand::ActivateRecording(_)
| SystemCommand::CloseStore(_)
| SystemCommand::CloseAllRecordings
| SystemCommand::EnableExperimentalDataframeSpaceView(_) => handled = false,
| SystemCommand::CloseAllRecordings => handled = false,

#[cfg(debug_assertions)]
SystemCommand::EnableInspectBlueprintTimeline(_) => handled = false,
Expand Down

0 comments on commit e4aaee1

Please sign in to comment.