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.

Sign up for GitHub

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

Extract parts of gui into smaller components #7

Merged
merged 10 commits into from
Nov 21, 2023
Merged

Extract parts of gui into smaller components #7

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
1d0a34f
Add top menu bar component
IhsenBouallegue Nov 17, 2023
155a804
Add archive window
IhsenBouallegue Nov 17, 2023
c176bc7
Add bottom status bar component
IhsenBouallegue Nov 17, 2023
4a814c6
Refactor GUI components and create separate header
IhsenBouallegue Nov 17, 2023
b6d3aef
Refactor simulation panel component
IhsenBouallegue Nov 17, 2023
4002192
Move body and bottom_bar back into gui.rs
KoffeinFlummi Nov 20, 2023
64fd4ce
Move panels into structs
KoffeinFlummi Nov 20, 2023
f49103b
Allow downcasting data source trait objects
KoffeinFlummi Nov 20, 2023
bd3c5b2
Move all archive loading functionality into separate struct
KoffeinFlummi Nov 20, 2023
5d40843
Properly request repaints from background threads.
KoffeinFlummi Nov 21, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
226 changes: 19 additions & 207 deletions src/gui.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
//! Main GUI code

use std::io::Write;
use std::path::PathBuf;
use std::sync::mpsc::{Receiver, Sender};
use std::io::Write;

#[cfg(not(target_arch = "wasm32"))]
use std::time::Instant;
Expand All @@ -11,14 +11,15 @@ use web_time::Instant;

use eframe::egui;
use egui::FontFamily::Proportional;
use egui::{TextStyle::*, Align2, ProgressBar, Image};
use egui::{Align, Button, CollapsingHeader, Color32, FontFamily, FontId, Key, Layout, Modifiers, Vec2};
use egui::TextStyle::*;
use egui::{Align, Color32, FontFamily, FontId, Key, Layout, Modifiers, Vec2};

use futures::StreamExt;
use log::*;

use mithril::telemetry::*;

mod components;
mod fc_settings;
mod map;
mod maxi_grid;
Expand All @@ -28,41 +29,21 @@ mod simulation_settings;
mod tabs;
mod theme;
mod top_bar;
pub mod windows; // TODO: make this private (it is public because it has ARCHIVE)

use crate::data_source::*;
use crate::file::*;
use crate::gui::components::body::create_body;
use crate::gui::components::header::create_header;
use crate::gui::components::simulation_panel::create_simulation_panel;
use crate::settings::AppSettings;
use crate::simulation::*;
use crate::state::*;

use crate::gui::fc_settings::*;
use crate::gui::simulation_settings::*;
use crate::gui::components::bottom_status_bar::create_bottom_status_bar;
use crate::gui::components::top_menu_bar::create_top_menu_bar;
use crate::gui::tabs::*;
use crate::gui::theme::*;
use crate::gui::top_bar::*;

// Log files included with the application. These should probably be fetched
// if necessary to reduce application size.
// TODO: migrate old launches
pub const ARCHIVE: [(&str, Option<&'static str>, Option<&'static str>); 5] = [
("Zülpich #1", None, None),
("Zülpich #2", None, None),
(
"DARE (FC A)",
Some("https://raw.githubusercontent.com/tudsat-rocket/sam/main/archive/dare_launch_a_telem_filtered.json"),
Some("https://raw.githubusercontent.com/tudsat-rocket/sam/main/archive/dare_launch_a_flash_filtered.json"),
),
(
"DARE (FC B)",
Some("https://raw.githubusercontent.com/tudsat-rocket/sam/main/archive/dare_launch_b_telem_filtered.json"),
Some("https://raw.githubusercontent.com/tudsat-rocket/sam/main/archive/dare_launch_b_flash_filtered.json"),
),
(
"EuRoC 2023 (ÆSIR Signý)",
Some("https://raw.githubusercontent.com/tudsat-rocket/sam/main/archive/euroc_2023_telem_filtered.json"),
Some("https://raw.githubusercontent.com/tudsat-rocket/sam/main/archive/euroc_2023_flash_filtered.json"),
),
];
use crate::gui::windows::archive::open_archive_window;
Copy link
Contributor

Choose a reason for hiding this comment

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

Definitely a fan of moving this somewhere else. In the future, I'd like to have some sort of Archive struct that is a bit smarter about things like caching, but that would probably be independent of the Window. Also like the structure with a separate windows folder. We might have more of these in the future.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I totally agree, it's more predictable and more similar to how big UI are built


#[derive(Debug)]
enum ArchiveLoadProgress {
Expand Down Expand Up @@ -153,7 +134,7 @@ impl Sam {
Ok(chunk) => {
cursor.write_all(&chunk).unwrap();
progress = u64::min(progress + chunk.len() as u64, total_size);
if progress == total_size || progress > last_progress + 256*1024 {
if progress == total_size || progress > last_progress + 256 * 1024 {
let _ = progress_sender.send(ArchiveLoadProgress::Progress((progress, total_size)));
last_progress = progress;
}
Expand Down Expand Up @@ -394,195 +375,26 @@ impl Sam {
}

// Top menu bar
egui::TopBottomPanel::top("menubar").min_height(30.0).max_height(30.0).show(ctx, |ui| {
ui.set_enabled(!self.archive_window_open);
ui.horizontal_centered(|ui| {
let image = if ui.style().visuals.dark_mode {
Image::new(egui::include_image!("../assets/logo_dark_mode.png"))
} else {
Image::new(egui::include_image!("../assets/logo_light_mode.png"))
};
ui.add(image.max_size(Vec2::new(ui.available_width(), 20.0)));

ui.separator();
egui::widgets::global_dark_light_mode_switch(ui);
ui.separator();

ui.selectable_value(&mut self.tab, GuiTab::Launch, "🚀 Launch (F1)");
ui.selectable_value(&mut self.tab, GuiTab::Plot, "📈 Plot (F2)");
ui.selectable_value(&mut self.tab, GuiTab::Configure, "⚙ Configure (F3)");

ui.separator();

// Opening files manually is not available on web assembly
#[cfg(target_arch = "x86_64")]
if ui.selectable_label(false, "🗁 Open Log File").clicked() {
if let Some(data_source) = open_log_file() {
self.open_log_file(data_source);
}
}

// Toggle archive panel
ui.toggle_value(&mut self.archive_window_open, "🗄 Flight Archive");

// Toggle archive panel
if ui.selectable_label(self.data_source.simulation_settings().is_some(), "💻 Simulate").clicked() {
self.data_source = Box::new(SimulationDataSource::default());
}

// Show a button to the right to close the current log/simulation and go back to
// live view
ui.allocate_ui_with_layout(ui.available_size(), Layout::right_to_left(Align::Center), |ui| {
if self.data_source.is_log_file() || self.data_source.simulation_settings().is_some() {
if ui.button("❌").clicked() {
self.close_data_source();
}
}
});
});
});
create_top_menu_bar(self, ctx);
Copy link
Contributor

Choose a reason for hiding this comment

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

I have to say I'm not the biggest fan of passing self around to other files because that might make it more difficult in the future to tell where the contents of Sam are being modified. On the other hand I can see why it would be easier this way.

I think we could pass in something for set_enabled (and this would probably be done similarly for the other panels), and a reference to the data source, but for everything where we change state from within the panel, we might have to implement a response system similar to other egui widgets. So for instance having the function/method return some sort of enum like:

enum TopBarResponse {
    ArchiveWindowOpened,
    TabSelected(GuiTab),
    // etc
}

That's a bit more involved, so I think it may not be necessary to do that right now, but I think long-term this would be the cleanest solution, even though It would mean a bit more code in src/gui.rs.


// A window to open archived logs directly in the application
let mut archive_open = self.archive_window_open; // necessary to avoid mutably borrowing self
egui::Window::new("Flight Archive").open(&mut archive_open).min_width(300.0).anchor(Align2::CENTER_CENTER, [0.0, 0.0]).resizable(false).collapsible(false).show(ctx, |ui| {
ui.add_space(10.0);

for (i, (title, telem, flash)) in ARCHIVE.iter().enumerate() {
if i != 0 {
ui.separator();
}

ui.horizontal(|ui| {
ui.label(*title);
ui.with_layout(Layout::right_to_left(Align::Center), |ui| {
if ui.add_enabled(flash.is_some(), Button::new("🖴 Flash")).clicked() {
self.open_archive_log(flash.unwrap());
}

if ui.add_enabled(telem.is_some(), Button::new("📡 Telemetry")).clicked() {
self.open_archive_log(telem.unwrap());
}
});
});
}

ui.add_space(10.0);
ui.horizontal(|ui| {
ui.add_visible_ui(self.archive_progress.is_some(), |ui| {
let (done, total) = self.archive_progress.unwrap_or((0, 0));
let f = (total > 0).then(|| done as f32 / total as f32).unwrap_or(0.0);
let text = format!("{:.2}MiB / {:.2}MiB", done as f32 / (1024.0*1024.0), total as f32 / (1024.0*1024.0));
ui.add_sized([ui.available_width(), 20.0], ProgressBar::new(f).text(text));
});
});
ui.add_space(10.0);
open_archive_window(ctx, &mut archive_open, self);
Copy link
Contributor

Choose a reason for hiding this comment

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

Similar to above, great fan of moving some of the more specific UI stuff out of here though. For windows it might be interesting to have a system where windows can return values, in this case a flight log, to src/gui.rs. Something along the lines of:

if let Some(log) = ui.add(archive_window).result() {
    self.open_archive_log(log);
}

Same as above though, this isn't necessarily urgent.


ui.checkbox(&mut self.replay_logs, "Replay logs");
});
self.archive_window_open = archive_open;

if self.data_source.simulation_settings().is_some() {
let old_settings = self.data_source.simulation_settings().unwrap().clone();

egui::SidePanel::left("sim").min_width(300.0).max_width(500.0).resizable(true).show(ctx, |ui| {
ui.set_enabled(!self.archive_window_open);
ui.heading("Simulation");
ui.add_space(20.0);

CollapsingHeader::new("Simulation Parameters").default_open(true).show(ui, |ui| {
let settings = self.data_source.simulation_settings().unwrap();
settings.ui(ui)
});

CollapsingHeader::new("(Simulated) FC Settings").default_open(false).show(ui, |ui| {
let settings = self.data_source.simulation_settings().unwrap();
settings.fc_settings.ui(ui, None)
});

ui.add_space(20.0);

let changed = *self.data_source.simulation_settings().unwrap() != old_settings;
let released = false;
ui.horizontal(|ui| {
if ui.button("Reset").clicked() {
*(self.data_source.simulation_settings().unwrap()) = SimulationSettings::default();
}

if ui.button("↻ Rerun").clicked() || (changed && released) {
self.data_source.reset();
}
});
});
create_simulation_panel(self, ctx);
Copy link
Contributor

Choose a reason for hiding this comment

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

This might be a place where it would be possible to get rid of passing self, by just passing a boolean for enabling and the data source. I also was never a huge fan of how we check for a simulation data source here. Ideally we should try downcasting the boxed trait inside self.data_source to the actual SimulationDataSource type, and then passing that. But again, that's not necessarily relevant to the cleanup.

}

// Top panel containing text indicators and flight mode buttons
if ctx.screen_rect().width() > 1000.0 {
egui::TopBottomPanel::top("topbar").min_height(60.0).max_height(60.0).show(ctx, |ui| {
ui.set_enabled(!self.archive_window_open);
ui.horizontal_centered(|ui| {
self.top_bar(ui, false);
});
});
} else {
egui::TopBottomPanel::top("topbar").min_height(20.0).max_height(300.0).show(ctx, |ui| {
ui.set_enabled(!self.archive_window_open);
CollapsingHeader::new("Status & Controls").default_open(false).show(ui, |ui| {
self.top_bar(ui, true);
ui.add_space(10.0);
});
});
}
create_header(self, ctx);
Copy link
Contributor

Choose a reason for hiding this comment

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

This looks good except for the same concerns about self. I would probably also use the opportunity to move self.top_bar out of here as well.


// Bottom status bar
egui::TopBottomPanel::bottom("bottombar").min_height(30.0).show(ctx, |ui| {
ui.set_enabled(!self.archive_window_open);
ui.horizontal_centered(|ui| {
// Give the data source some space on the left ...
ui.horizontal_centered(|ui| {
ui.set_width(ui.available_width() / 2.0);
self.data_source.status_bar_ui(ui);
});

// ... and the current tab some space on the right.
ui.allocate_ui_with_layout(ui.available_size(), Layout::right_to_left(Align::Center), |ui| {
#[cfg(not(target_arch = "wasm32"))]
ui.add_enabled_ui(!self.data_source.is_log_file(), |ui| {
if ui.button("⏮ Reset").clicked() {
self.data_source.reset();
}
});

#[cfg(not(target_arch = "wasm32"))]
ui.separator();

match self.tab {
GuiTab::Launch => {}
GuiTab::Plot => self.plot_tab.bottom_bar_ui(ui, self.data_source.as_mut()),
GuiTab::Configure => {}
}
});
});
});
create_bottom_status_bar(self, ctx);
Copy link
Contributor

Choose a reason for hiding this comment

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

I was planning to either get rid of this reset button altogether or move it to the data source specific UI, for resetting during long serial sessions. This way, there would be no actual GUI code here, just some generic GUI for the data source and current tab. In that case, I think this could be compact enough to stay here. This way all the tab-dependent stuff would happen here in src/gui.rs (except maybe for the buttons for selecting one, but that could be changed) and if you added a new tab you would only have to add your new tab to src/gui.rs.


// Everything else. This has to be called after all the other panels
// are created.
egui::CentralPanel::default().show(ctx, |ui| {
ui.set_width(ui.available_width());
ui.set_height(ui.available_height());
ui.set_enabled(!self.archive_window_open);

match self.tab {
GuiTab::Launch => {}
GuiTab::Plot => self.plot_tab.main_ui(ui, self.data_source.as_mut()),
GuiTab::Configure => {
let changed = self.configure_tab.main_ui(ui, self.data_source.as_mut(), &mut self.settings);
if changed {
self.data_source.apply_settings(&self.settings);
self.plot_tab.apply_settings(&self.settings);
}
}
}
});
// Everything else. This has to be called after all the other panels are created.
create_body(self, ctx);
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this might be general enough to stay in here. See reasoning above regarding keeping all the match self.tab { ... } stuff in one place.


// If we have live data coming in, we need to tell egui to repaint.
// If we don't, we shouldn't.
Expand Down
24 changes: 24 additions & 0 deletions src/gui/components/body.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
use crate::gui::tabs::GuiTab;
use crate::gui::Sam;

use egui::Context;

pub fn create_body(sam: &mut Sam, ctx: &Context) {
egui::CentralPanel::default().show(ctx, |ui| {
ui.set_width(ui.available_width());
ui.set_height(ui.available_height());
ui.set_enabled(!sam.archive_window_open);

match sam.tab {
GuiTab::Launch => {}
GuiTab::Plot => sam.plot_tab.main_ui(ui, sam.data_source.as_mut()),
GuiTab::Configure => {
let changed = sam.configure_tab.main_ui(ui, sam.data_source.as_mut(), &mut sam.settings);
if changed {
sam.data_source.apply_settings(&sam.settings);
sam.plot_tab.apply_settings(&sam.settings);
}
}
}
});
}
35 changes: 35 additions & 0 deletions src/gui/components/bottom_status_bar.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
use crate::gui::tabs::GuiTab;
use crate::gui::Sam;
use egui::{Align, Context, Layout};

pub fn create_bottom_status_bar(sam: &mut Sam, ctx: &Context) {
egui::TopBottomPanel::bottom("bottombar").min_height(30.0).show(ctx, |ui| {
ui.set_enabled(!sam.archive_window_open);
ui.horizontal_centered(|ui| {
// Give the data source some space on the left ...
ui.horizontal_centered(|ui| {
ui.set_width(ui.available_width() / 2.0);
sam.data_source.status_bar_ui(ui);
});

// ... and the current tab some space on the right.
ui.allocate_ui_with_layout(ui.available_size(), Layout::right_to_left(Align::Center), |ui| {
#[cfg(not(target_arch = "wasm32"))]
ui.add_enabled_ui(!sam.data_source.is_log_file(), |ui| {
if ui.button("⏮ Reset").clicked() {
sam.data_source.reset();
}
});

#[cfg(not(target_arch = "wasm32"))]
ui.separator();

match sam.tab {
GuiTab::Launch => {}
GuiTab::Plot => sam.plot_tab.bottom_bar_ui(ui, sam.data_source.as_mut()),
GuiTab::Configure => {}
}
});
});
});
}
22 changes: 22 additions & 0 deletions src/gui/components/header.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
use crate::gui::Sam;

use egui::{CollapsingHeader, Context};

pub fn create_header(sam: &mut Sam, ctx: &Context) {
if ctx.screen_rect().width() > 1000.0 {
egui::TopBottomPanel::top("topbar").min_height(60.0).max_height(60.0).show(ctx, |ui| {
ui.set_enabled(!sam.archive_window_open);
ui.horizontal_centered(|ui| {
sam.top_bar(ui, false);
});
});
} else {
egui::TopBottomPanel::top("topbar").min_height(20.0).max_height(300.0).show(ctx, |ui| {
ui.set_enabled(!sam.archive_window_open);
CollapsingHeader::new("Status & Controls").default_open(false).show(ui, |ui| {
sam.top_bar(ui, true);
ui.add_space(10.0);
});
});
}
}
5 changes: 5 additions & 0 deletions src/gui/components/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
pub mod body;
pub mod bottom_status_bar;
pub mod header;
pub mod simulation_panel;
pub mod top_menu_bar;
Loading