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

Handle no model argument being passed #1286

Merged
merged 7 commits into from
Oct 30, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
543 changes: 398 additions & 145 deletions Cargo.lock

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion crates/fj-app/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ fn main() -> anyhow::Result<()> {

fn no_model_error() -> anyhow::Error {
anyhow!(
"You must specify a model to start Fornjot.\n\
"You must specify a model to start Fornjot in export only mode.\n\
- Pass a model as a command-line argument. See `fj-app --help`.\n\
- Specify a default model in the configuration file."
)
Expand Down
25 changes: 11 additions & 14 deletions crates/fj-host/src/host.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,21 +13,18 @@ impl Host {
///
/// This is only useful, if you want to continuously watch the model for
/// changes. If you don't just keep using `Model`.
pub fn from_model(model: Option<Model>) -> Option<Result<Self, Error>> {
if let Some(model) = model {
let watch_path = model.watch_path();
let evaluator = Evaluator::from_model(model);
let _watcher = match Watcher::watch_model(&watch_path, &evaluator) {
Ok(_watcher) => _watcher,
Err(e) => return Some(Err(e)),
};
pub fn from_model(model: Model) -> Result<Self, Error> {
let watch_path = model.watch_path();
let evaluator = Evaluator::from_model(model);
let _watcher = match Watcher::watch_model(&watch_path, &evaluator) {
Ok(_watcher) => _watcher,
Err(e) => return Err(e),
};
hannobraun marked this conversation as resolved.
Show resolved Hide resolved

return Some(Ok(Self {
evaluator,
_watcher,
}));
}
None
return Ok(Self {
evaluator,
_watcher,
});
}

/// Access a channel with evaluation events
Expand Down
6 changes: 5 additions & 1 deletion crates/fj-viewer/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,15 @@ egui-wgpu = "0.19.0"
fj-interop.workspace = true
fj-math.workspace = true
raw-window-handle = "0.4.3"
rfd = "0.10.0"
thiserror = "1.0.35"
tracing = "0.1.37"
wgpu_glyph = "0.17.0"

[dependencies.rfd]
version = "0.10.0"
default_features = false
features = ["xdg-portal"]

[dependencies.wgpu]
version = "0.13.1"
features = ["webgl"]
Expand Down
8 changes: 4 additions & 4 deletions crates/fj-viewer/src/graphics/renderer.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use std::{io, mem::size_of};
use std::{io, mem::size_of, path::PathBuf};

use crossbeam_channel::{Receiver, Sender};
use thiserror::Error;
Expand All @@ -8,7 +8,7 @@ use wgpu_glyph::ab_glyph::InvalidFont;

use crate::{
camera::Camera,
gui::{Gui, GuiEvent},
gui::Gui,
screen::{Screen, ScreenSize},
};

Expand Down Expand Up @@ -159,8 +159,8 @@ impl Renderer {

pub(crate) fn init_gui(
&self,
event_rx: Receiver<GuiEvent>,
event_tx: Sender<GuiEvent>,
event_rx: Receiver<()>,
event_tx: Sender<PathBuf>,
) -> Gui {
Gui::new(&self.device, self.surface_config.format, event_rx, event_tx)
}
Expand Down
30 changes: 10 additions & 20 deletions crates/fj-viewer/src/gui.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,12 +24,6 @@ use fj_math::{Aabb, Scalar};

use crate::graphics::DrawConfig;

/// Event that are passed between the event_loop and gui
pub enum GuiEvent {
AskModel,
LoadModel(PathBuf),
}
Copy link
Owner

Choose a reason for hiding this comment

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

Removing GuiEvent outright is perfectly fine, but just for the record, I think it would have been slightly nicer to keep a struct AskModel; and a struct LoadModel(PathBuf); around, to make those Sender/Receiver types below a bit more descriptive (Sender<AskModel> instead of Sender<()>, for example).

But this is only a minor thing! Just mentioning it for future reference.


struct GuiState {
has_model: bool,
}
Expand All @@ -45,17 +39,17 @@ pub struct Gui {
context: egui::Context,
render_pass: egui_wgpu::renderer::RenderPass,
options: Options,
event_rx: Receiver<GuiEvent>,
event_tx: Sender<GuiEvent>,
event_rx: Receiver<()>,
event_tx: Sender<PathBuf>,
state: GuiState,
}

impl Gui {
pub(crate) fn new(
device: &wgpu::Device,
texture_format: wgpu::TextureFormat,
event_rx: Receiver<GuiEvent>,
event_tx: Sender<GuiEvent>,
event_rx: Receiver<()>,
event_tx: Sender<PathBuf>,
) -> Self {
// The implementation of the integration with `egui` is likely to need
// to change "significantly" depending on what architecture approach is
Expand Down Expand Up @@ -126,15 +120,10 @@ impl Gui {
})
.ok();

let gui_event = match gui_event {
Some(gui_event) => gui_event,
match gui_event {
Some(_) => self.state.has_model = false,
None => break,
};

match gui_event {
GuiEvent::AskModel => self.state.has_model = false,
_ => {}
}
}

self.context.set_pixels_per_point(pixels_per_point);
Expand Down Expand Up @@ -298,17 +287,18 @@ impl Gui {
.anchor(egui::Align2::CENTER_CENTER, [0_f32, -5_f32])
.show(&self.context, |ui| {
ui.vertical_centered(|ui| {
ui.label(
ui.label(egui::RichText::new(
"No model selected please choose a model to view.",
);
).color(egui::Color32::BLACK)
.background_color(egui::Color32::WHITE));
if ui
.button(egui::RichText::new("Pick a model"))
.clicked()
{
let model_dir = show_file_dialog();
if let Some(model_dir) = model_dir {
self.event_tx
.send(GuiEvent::LoadModel(model_dir))
.send(model_dir)
.expect("Channel is disconnected");

self.state.has_model = true;
Expand Down
2 changes: 1 addition & 1 deletion crates/fj-viewer/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ mod viewer;
pub use self::{
camera::Camera,
graphics::{DrawConfig, Renderer, RendererInitError},
gui::{Gui, GuiEvent},
gui::Gui,
input::{InputEvent, InputHandler},
screen::{NormalizedScreenPosition, Screen, ScreenSize},
viewer::Viewer,
Expand Down
12 changes: 6 additions & 6 deletions crates/fj-viewer/src/viewer.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
use std::path::PathBuf;

use fj_interop::{
processed_shape::ProcessedShape, status_report::StatusReport,
};
Expand All @@ -7,10 +9,8 @@ use tracing::warn;
use crossbeam_channel::{Receiver, Sender};

use crate::{
camera::FocusPoint,
gui::{Gui, GuiEvent},
Camera, DrawConfig, InputEvent, InputHandler, NormalizedScreenPosition,
Renderer, RendererInitError, Screen, ScreenSize,
camera::FocusPoint, gui::Gui, Camera, DrawConfig, InputEvent, InputHandler,
NormalizedScreenPosition, Renderer, RendererInitError, Screen, ScreenSize,
};

/// The Fornjot model viewer
Expand Down Expand Up @@ -44,8 +44,8 @@ impl Viewer {
/// Construct a new instance of `Viewer`
pub async fn new(
screen: &impl Screen,
event_rx: Receiver<GuiEvent>,
event_tx: Sender<GuiEvent>,
event_rx: Receiver<()>,
event_tx: Sender<PathBuf>,
) -> Result<Self, RendererInitError> {
let renderer = Renderer::new(screen).await?;
let gui = renderer.init_gui(event_rx, event_tx);
Expand Down
Loading