From b24788918e3955660b162fe6b556784c29bd11eb Mon Sep 17 00:00:00 2001 From: Luca Della Vedova Date: Wed, 5 Jul 2023 13:38:59 +0800 Subject: [PATCH 01/17] First draft of fuel asset gallery Signed-off-by: Luca Della Vedova --- rmf_site_editor/Cargo.toml | 1 + .../src/interaction/camera_controls.rs | 3 + rmf_site_editor/src/interaction/mod.rs | 4 + .../src/interaction/model_preview.rs | 100 ++++++++++ rmf_site_editor/src/site/mod.rs | 1 + rmf_site_editor/src/site/model.rs | 57 +++++- rmf_site_editor/src/site/sdf.rs | 4 +- rmf_site_editor/src/widgets/create.rs | 3 + rmf_site_editor/src/widgets/mod.rs | 15 ++ rmf_site_editor/src/widgets/new_model.rs | 188 ++++++++++++++++++ 10 files changed, 364 insertions(+), 12 deletions(-) create mode 100644 rmf_site_editor/src/interaction/model_preview.rs create mode 100644 rmf_site_editor/src/widgets/new_model.rs diff --git a/rmf_site_editor/Cargo.toml b/rmf_site_editor/Cargo.toml index 5737d7da..f35a9ffa 100644 --- a/rmf_site_editor/Cargo.toml +++ b/rmf_site_editor/Cargo.toml @@ -42,6 +42,7 @@ rfd = "0.11" urdf-rs = "0.7" # sdformat_rs = { path = "../../sdf_rust_experimental/sdformat_rs"} sdformat_rs = { git = "https://github.com/open-rmf/sdf_rust_experimental", rev = "f86344f"} +gz-fuel = { git = "https://github.com/luca-della-vedova/gz-fuel/", branch = "first_implementation" } # only enable the 'dynamic' feature if we're not building for web or windows [target.'cfg(all(not(target_arch = "wasm32"), not(target_os = "windows")))'.dependencies] diff --git a/rmf_site_editor/src/interaction/camera_controls.rs b/rmf_site_editor/src/interaction/camera_controls.rs index 0350c610..44c32c11 100644 --- a/rmf_site_editor/src/interaction/camera_controls.rs +++ b/rmf_site_editor/src/interaction/camera_controls.rs @@ -49,6 +49,9 @@ pub const HOVERED_OUTLINE_LAYER: u8 = 4; /// The X-Ray layer is used to show visual cues that need to be rendered /// above anything that would be obstructing them. pub const XRAY_RENDER_LAYER: u8 = 5; +/// The Model Preview layer is used by model previews to spawn and render +/// models in the engine without having them being visible to general cameras +pub const MODEL_PREVIEW_LAYER: u8 = 6; #[derive(Resource)] struct MouseLocation { diff --git a/rmf_site_editor/src/interaction/mod.rs b/rmf_site_editor/src/interaction/mod.rs index ff1484dd..24532dbd 100644 --- a/rmf_site_editor/src/interaction/mod.rs +++ b/rmf_site_editor/src/interaction/mod.rs @@ -47,6 +47,9 @@ pub use light::*; pub mod mode; pub use mode::*; +pub mod model_preview; +pub use model_preview::*; + pub mod outline; pub use outline::*; @@ -137,6 +140,7 @@ impl Plugin for InteractionPlugin { .add_plugin(PickingPlugin) .add_plugin(OutlinePlugin) .add_plugin(CameraControlsPlugin) + .add_plugin(ModelPreviewPlugin) .add_system_set( SystemSet::on_update(InteractionState::Enable) .with_system(make_lift_doormat_gizmo) diff --git a/rmf_site_editor/src/interaction/model_preview.rs b/rmf_site_editor/src/interaction/model_preview.rs new file mode 100644 index 00000000..56f45f0e --- /dev/null +++ b/rmf_site_editor/src/interaction/model_preview.rs @@ -0,0 +1,100 @@ +/* + * Copyright (C) 2023 Open Source Robotics Foundation + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * +*/ + +use crate::interaction::MODEL_PREVIEW_LAYER; +use bevy::prelude::*; +use bevy::render::render_resource::{ + Extent3d, TextureDescriptor, TextureDimension, TextureFormat, TextureUsages, +}; +use bevy::render::{camera::RenderTarget, primitives::Aabb, view::RenderLayers}; +use bevy_egui::{egui::TextureId, EguiContext}; + +#[derive(Resource)] +pub struct ModelPreviewCamera { + pub camera_entity: Entity, + pub egui_handle: TextureId, + pub model_entity: Entity, +} + +pub struct ModelPreviewPlugin; + +// TODO(luca) implement this system to scale the view based on the model's Aabb +fn scale_preview_for_model_bounding_box( + aabbs: Query<&Aabb, Changed>, + parents: Query<&Parent>, + model_preview: Res, + camera_transforms: Query<&mut Transform, With>, +) { +} + +impl FromWorld for ModelPreviewCamera { + fn from_world(world: &mut World) -> Self { + // camera + let image_size = Extent3d { + width: 320, + height: 240, + depth_or_array_layers: 1, + }; + let mut preview_image = Image { + texture_descriptor: TextureDescriptor { + label: None, + size: image_size, + dimension: TextureDimension::D2, + format: TextureFormat::Bgra8UnormSrgb, + mip_level_count: 1, + sample_count: 1, + usage: TextureUsages::TEXTURE_BINDING + | TextureUsages::COPY_DST + | TextureUsages::RENDER_ATTACHMENT, + }, + ..default() + }; + preview_image.resize(image_size); + let mut images = world.get_resource_mut::>().unwrap(); + let preview_image = images.add(preview_image); + let mut egui_context = world.get_resource_mut::().unwrap(); + // Attach the bevy image to the egui image + let egui_handle = egui_context.add_image(preview_image.clone()); + let camera_entity = world + .spawn(Camera3dBundle { + transform: Transform::from_xyz(-2.0, 2.5, 5.0).looking_at(Vec3::ZERO, Vec3::Z), + camera: Camera { + target: RenderTarget::Image(preview_image), + ..default() + }, + ..default() + }) + .insert(RenderLayers::from_layers(&[MODEL_PREVIEW_LAYER])) + .id(); + let model_entity = world + .spawn(RenderLayers::from_layers(&[MODEL_PREVIEW_LAYER])) + .id(); + + Self { + camera_entity, + egui_handle, + model_entity, + } + } +} + +impl Plugin for ModelPreviewPlugin { + fn build(&self, app: &mut App) { + app.init_resource::() + .add_system(scale_preview_for_model_bounding_box); + } +} diff --git a/rmf_site_editor/src/site/mod.rs b/rmf_site_editor/src/site/mod.rs index fa526f75..f2156b69 100644 --- a/rmf_site_editor/src/site/mod.rs +++ b/rmf_site_editor/src/site/mod.rs @@ -260,6 +260,7 @@ impl Plugin for SitePlugin { .with_system(handle_new_sdf_roots) .with_system(update_model_scales) .with_system(make_models_selectable) + .with_system(propagate_model_render_layers) .with_system(handle_new_mesh_primitives) .with_system(add_drawing_visuals) .with_system(handle_loaded_drawing) diff --git a/rmf_site_editor/src/site/model.rs b/rmf_site_editor/src/site/model.rs index e7d6aa25..6c93f415 100644 --- a/rmf_site_editor/src/site/model.rs +++ b/rmf_site_editor/src/site/model.rs @@ -16,11 +16,11 @@ */ use crate::{ - interaction::{DragPlaneBundle, Selectable}, + interaction::{DragPlaneBundle, PickableBundle, Selectable, MODEL_PREVIEW_LAYER}, site::{Category, PreventDeletion, SiteAssets}, SdfRoot, }; -use bevy::{asset::LoadState, gltf::Gltf, prelude::*}; +use bevy::{asset::LoadState, gltf::Gltf, prelude::*, render::view::RenderLayers}; use bevy_mod_outline::OutlineMeshExt; use rmf_site_format::{AssetSource, ModelMarker, Pending, Pose, Scale, UrdfRoot}; use smallvec::SmallVec; @@ -85,7 +85,13 @@ pub fn update_model_scenes( >, asset_server: Res, loading_models: Query< - (Entity, &TentativeModelFormat, &PendingSpawning, &Scale), + ( + Entity, + &TentativeModelFormat, + &PendingSpawning, + &Scale, + Option<&RenderLayers>, + ), With, >, spawned_models: Query< @@ -154,7 +160,7 @@ pub fn update_model_scenes( // For each model that is loading, check if its scene has finished loading // yet. If the scene has finished loading, then insert it as a child of the // model entity and make it selectable. - for (e, tentative_format, h, scale) in loading_models.iter() { + for (e, tentative_format, h, scale, render_layer) in loading_models.iter() { if asset_server.get_load_state(&h.0) == LoadState::Loaded { let model_id = if let Some(gltf) = gltfs.get(&h.typed_weak::()) { Some(commands.entity(e).add_children(|parent| { @@ -214,10 +220,10 @@ pub fn update_model_scenes( None }; if let Some(id) = model_id { - commands - .entity(e) - .insert(ModelSceneRoot) - .insert(Selectable::new(e)); + commands.entity(e).insert(ModelSceneRoot); + if !render_layer.is_some_and(|l| l.iter().all(|l| l == MODEL_PREVIEW_LAYER)) { + commands.entity(e).insert(Selectable::new(e)); + } commands.entity(e).remove::(); current_scenes.get_mut(e).unwrap().entity = Some(id); } @@ -313,7 +319,7 @@ pub fn make_models_selectable( mut commands: Commands, new_scene_roots: Query, Without)>, parents: Query<&Parent>, - scene_roots: Query<&Selectable, With>, + scene_roots: Query<(&Selectable, Option<&RenderLayers>), With>, all_children: Query<&Children>, mesh_handles: Query<&Handle>, mut mesh_assets: ResMut>, @@ -328,10 +334,16 @@ pub fn make_models_selectable( // A root might be a child of another root, for example for SDF models that have multiple // submeshes. We need to traverse up to find the highest level scene to use for selecting // behavior - let selectable = AncestorIter::new(&parents, model_scene_root) + let Some((selectable, render_layers)) = AncestorIter::new(&parents, model_scene_root) .filter_map(|p| scene_roots.get(p).ok()) .last() - .unwrap_or(scene_roots.get(model_scene_root).unwrap()); + else { + continue; + }; + // If layer should not be visible, don't make it selectable + if render_layers.is_some_and(|r| r.iter().all(|l| l == MODEL_PREVIEW_LAYER)) { + continue; + } queue.push(model_scene_root); while let Some(e) = queue.pop() { @@ -359,3 +371,26 @@ pub fn make_models_selectable( } } } + +/// Assigns the render layer of the root, if present, to all the children +pub fn propagate_model_render_layers( + mut commands: Commands, + new_scene_roots: Query>, + scene_roots: Query<&RenderLayers, With>, + parents: Query<&Parent>, + mesh_entities: Query>>, + children: Query<&Children>, +) { + for e in &new_scene_roots { + let Some(render_layers) = AncestorIter::new(&parents, e) + .filter_map(|p| scene_roots.get(p).ok()) + .last() else { + continue; + }; + for c in DescendantIter::new(&children, e) { + if mesh_entities.get(c).is_ok() { + commands.entity(c).insert(render_layers.clone()); + } + } + } +} diff --git a/rmf_site_editor/src/site/sdf.rs b/rmf_site_editor/src/site/sdf.rs index 7b585bfc..d3d00f0d 100644 --- a/rmf_site_editor/src/site/sdf.rs +++ b/rmf_site_editor/src/site/sdf.rs @@ -31,7 +31,9 @@ use rmf_site_format::{ // TODO(luca) reduce chances for panic and do proper error handling here fn compute_model_source(path: &str, uri: &str) -> AssetSource { - let binding = path.strip_prefix("search://").unwrap(); + let binding = path + .strip_prefix("search://") + .unwrap_or_else(|| path.strip_prefix("rmf-server://").unwrap()); if let Some(stripped) = uri.strip_prefix("model://") { // Get the org name from context, model name from this and combine let org_name = binding.split("/").next().unwrap(); diff --git a/rmf_site_editor/src/widgets/create.rs b/rmf_site_editor/src/widgets/create.rs index 2c4b3668..576a165b 100644 --- a/rmf_site_editor/src/widgets/create.rs +++ b/rmf_site_editor/src/widgets/create.rs @@ -124,6 +124,9 @@ impl<'a, 'w, 's> CreateWidget<'a, 'w, 's> { if let Ok((_e, source, _scale)) = self.events.pending_asset_sources.get_single() { + if ui.button("Browse fuel").clicked() { + self.events.new_model.asset_gallery_status.show = true; + } if ui.button("Spawn model").clicked() { let model = Model { source: source.clone(), diff --git a/rmf_site_editor/src/widgets/mod.rs b/rmf_site_editor/src/widgets/mod.rs index 25a1133f..5c3931d9 100644 --- a/rmf_site_editor/src/widgets/mod.rs +++ b/rmf_site_editor/src/widgets/mod.rs @@ -62,6 +62,9 @@ use inspector::{InspectorParams, InspectorWidget}; pub mod move_layer; pub use move_layer::*; +pub mod new_model; +pub use new_model::*; + #[derive(Debug, Hash, PartialEq, Eq, Clone, SystemLabel)] pub enum UiUpdateLabel { DrawUi, @@ -76,6 +79,8 @@ impl Plugin for StandardUiLayout { .init_resource::() .init_resource::() .init_resource::() + .init_resource::() + .init_resource::() .init_resource::() .add_system_set(SystemSet::on_enter(AppState::MainMenu).with_system(init_ui_style)) .add_system_set( @@ -184,6 +189,7 @@ pub struct AppEvents<'w, 's> { pub app_state: Res<'w, State>, pub pending_asset_sources: Query<'w, 's, (Entity, &'static AssetSource, &'static Scale), With>, + pub new_model: NewModelParams<'w, 's>, } fn site_ui_layout( @@ -290,6 +296,15 @@ fn site_ui_layout( }); }); + if events.new_model.asset_gallery_status.show { + egui::SidePanel::left("left_panel") + .resizable(true) + .exact_width(320.0) + .show(egui_context.ctx_mut(), |ui| { + NewModel::new(&mut events).show(ui); + }); + } + let egui_context = egui_context.ctx_mut(); let ui_has_focus = egui_context.wants_pointer_input() || egui_context.wants_keyboard_input() diff --git a/rmf_site_editor/src/widgets/new_model.rs b/rmf_site_editor/src/widgets/new_model.rs new file mode 100644 index 00000000..7ee6a371 --- /dev/null +++ b/rmf_site_editor/src/widgets/new_model.rs @@ -0,0 +1,188 @@ +/* + * Copyright (C) 2023 Open Source Robotics Foundation + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * +*/ + +use crate::interaction::{ChangeMode, ModelPreviewCamera, SelectAnchor3D}; +use crate::site::{AssetSource, Model}; +use crate::AppEvents; +use bevy::tasks::AsyncComputeTaskPool; +use bevy::{ecs::system::SystemParam, prelude::*}; +use bevy_egui::egui::{ + widgets::Image as EguiImage, Button, Color32, ComboBox, Context, Frame, Pos2, Rect, ScrollArea, + Ui, Window, +}; +use bevy_egui::EguiContext; +use gz_fuel::{FuelClient as GzFuelClient, FuelModel}; + +#[derive(Resource, Default, Deref, DerefMut)] +pub struct FuelClient(GzFuelClient); + +/// Filters applied to models in the fuel list +pub struct ShowAssetFilters { + pub owner: Option, +} + +/// Used to signals whether to show or hide the left side panel with the asset gallery +#[derive(Resource)] +pub struct AssetGalleryStatus { + pub show: bool, + pub selected: Option, + pub filters: ShowAssetFilters, +} + +impl Default for ShowAssetFilters { + fn default() -> Self { + Self { + owner: Some("OpenRobotics".into()), + } + } +} + +impl Default for AssetGalleryStatus { + fn default() -> Self { + Self { + show: true, + selected: None, + filters: Default::default(), + } + } +} + +#[derive(SystemParam)] +pub struct NewModelParams<'w, 's> { + pub fuel_client: ResMut<'w, FuelClient>, + // TODO(luca) refactor to see whether we need + pub asset_gallery_status: ResMut<'w, AssetGalleryStatus>, + pub model_preview_camera: Res<'w, ModelPreviewCamera>, + pub image_assets: Res<'w, Assets>, + _ignore: Query<'w, 's, ()>, +} + +pub struct NewModel<'a, 'w, 's> { + events: &'a mut AppEvents<'w, 's>, +} + +impl<'a, 'w, 's> NewModel<'a, 'w, 's> { + pub fn new(events: &'a mut AppEvents<'w, 's>) -> Self { + Self { events } + } + + pub fn show(mut self, ui: &mut Ui) { + let fuel_client = &mut self.events.new_model.fuel_client; + let owners = fuel_client.get_owners_cached().unwrap(); + match &fuel_client.models { + Some(models) => { + // TODO(luca) remove unwrap + let mut owner_filter = self + .events + .new_model + .asset_gallery_status + .filters + .owner + .clone(); + let mut owner_filter_enabled = owner_filter.is_some(); + ui.checkbox(&mut owner_filter_enabled, "Owners"); + match owner_filter_enabled { + true => { + let mut selected = match owner_filter { + Some(s) => s, + None => owners[0].clone(), + }; + ComboBox::from_id_source("Asset Owner Filter") + .selected_text(selected.clone()) + .show_ui(ui, |ui| { + for owner in owners.into_iter() { + ui.selectable_value(&mut selected, owner.clone(), owner); + } + ui.end_row(); + }); + owner_filter = Some(selected); + } + false => { + owner_filter = None; + } + } + let models = match &owner_filter { + Some(owner) => fuel_client.as_ref().models_by_owner(&owner).unwrap(), + None => models.clone(), + }; + // Show models + ScrollArea::vertical().max_height(500.0).show(ui, |ui| { + for model in models { + let sel = self + .events + .new_model + .asset_gallery_status + .selected + .as_ref() + .is_some_and(|s| s.name == model.name); + if ui.selectable_label(sel, &model.name).clicked() { + self.events.new_model.asset_gallery_status.selected = Some(model); + } + } + }); + // Set the model source to what is selected + if let Some(selected) = &self.events.new_model.asset_gallery_status.selected { + let model_entity = self.events.new_model.model_preview_camera.model_entity; + let model = Model { + source: AssetSource::Remote( + selected.owner.clone() + "/" + &selected.name + "/model.sdf", + ), + ..default() + }; + self.events.commands.entity(model_entity).insert(model); + } + + ui.image( + self.events.new_model.model_preview_camera.egui_handle, + bevy_egui::egui::Vec2::new(320.0, 240.0), + ); + + if owner_filter != self.events.new_model.asset_gallery_status.filters.owner { + self.events.new_model.asset_gallery_status.filters.owner = owner_filter; + } + if let Some(selected) = &self.events.new_model.asset_gallery_status.selected { + if ui.button("Spawn model").clicked() { + let source = + AssetSource::Search(selected.owner.clone() + "/" + &selected.name); + let model = Model { + source: source.clone(), + ..default() + }; + self.events.request.change_mode.send(ChangeMode::To( + SelectAnchor3D::create_new_point().for_model(model).into(), + )); + } + } + } + None => { + ui.label("No models found"); + if ui.add(Button::new("Fetch models")).clicked() { + // TODO(luca) make this async to avoid blocking the whole application + fuel_client.update_cache_blocking(); + /* + AsyncComputeTaskPool::get().spawn( + async move {fuel_client.update_cache()}) + .detach(); + */ + } + } + } + if ui.add(Button::new("Close")).clicked() { + self.events.new_model.asset_gallery_status.show = false; + } + } +} From b3b838c80f47bf1937c7a069694befed98de0f60 Mon Sep 17 00:00:00 2001 From: Luca Della Vedova Date: Thu, 6 Jul 2023 09:00:33 +0800 Subject: [PATCH 02/17] Remove spurious panics, fully support local remote and search fuel assets Signed-off-by: Luca Della Vedova --- rmf_site_editor/src/site/model.rs | 3 ++ rmf_site_editor/src/site/sdf.rs | 65 ++++++++++++++++++------ rmf_site_editor/src/site_asset_io.rs | 9 ---- rmf_site_editor/src/widgets/new_model.rs | 7 +-- rmf_site_format/src/asset_source.rs | 8 +-- rmf_site_format/src/workcell.rs | 2 +- 6 files changed, 62 insertions(+), 32 deletions(-) diff --git a/rmf_site_editor/src/site/model.rs b/rmf_site_editor/src/site/model.rs index 6c93f415..ba6b1944 100644 --- a/rmf_site_editor/src/site/model.rs +++ b/rmf_site_editor/src/site/model.rs @@ -37,6 +37,7 @@ pub struct ModelScene { #[derive(Component, Debug, Default, Clone, PartialEq)] pub enum TentativeModelFormat { #[default] + Plain, GlbFlat, Obj, Stl, @@ -48,6 +49,7 @@ impl TentativeModelFormat { pub fn next(&self) -> Option { use TentativeModelFormat::*; match self { + Plain => Some(GlbFlat), GlbFlat => Some(Obj), Obj => Some(Stl), Stl => Some(GlbFolder), @@ -61,6 +63,7 @@ impl TentativeModelFormat { pub fn to_string(&self, model_name: &str) -> String { use TentativeModelFormat::*; match self { + Plain => "".to_owned(), Obj => ("/".to_owned() + model_name + ".obj").into(), GlbFlat => ".glb".into(), Stl => ".stl".into(), diff --git a/rmf_site_editor/src/site/sdf.rs b/rmf_site_editor/src/site/sdf.rs index d3d00f0d..d9a9ad70 100644 --- a/rmf_site_editor/src/site/sdf.rs +++ b/rmf_site_editor/src/site/sdf.rs @@ -29,23 +29,58 @@ use rmf_site_format::{ ModelMarker, NameInSite, Pose, Rotation, Scale, WorkcellCollisionMarker, WorkcellVisualMarker, }; -// TODO(luca) reduce chances for panic and do proper error handling here +// TODO(luca) cleanup this, there are many ways models are referenced and have to be resolved in +// SDF between local, fuel and cached paths so the logic becomes quite complicated. fn compute_model_source(path: &str, uri: &str) -> AssetSource { - let binding = path - .strip_prefix("search://") - .unwrap_or_else(|| path.strip_prefix("rmf-server://").unwrap()); - if let Some(stripped) = uri.strip_prefix("model://") { - // Get the org name from context, model name from this and combine - let org_name = binding.split("/").next().unwrap(); - let path = org_name.to_owned() + "/" + stripped; - AssetSource::Remote(path) - } else if let Some(path_idx) = binding.rfind("/") { - // It's a path relative to this model, remove file and append uri - let (model_path, _model_name) = binding.split_at(path_idx); - AssetSource::Remote(model_path.to_owned() + "/" + uri) - } else { - AssetSource::Remote("".into()) + let mut asset_source = AssetSource::from(path); + match asset_source { + AssetSource::Remote(ref mut p) | AssetSource::Search(ref mut p) => { + let binding = p.clone(); + *p = if let Some(stripped) = uri.strip_prefix("model://") { + // Get the org name from context, model name from this and combine + let org_name = binding.split("/").next().unwrap(); + org_name.to_owned() + "/" + stripped + } else if let Some(path_idx) = binding.rfind("/") { + // It's a path relative to this model, remove file and append uri + let (model_path, _model_name) = binding.split_at(path_idx); + model_path.to_owned() + "/" + uri + } else { + println!( + "WARNING: Invalid model found path is {} and model uri is {}", + path, uri + ); + "".into() + }; + } + AssetSource::Local(ref mut p) + | AssetSource::Bundled(ref mut p) + | AssetSource::Package(ref mut p) => { + let binding = p.clone(); + *p = if let Some(stripped) = uri.strip_prefix("model://") { + // Search for a model with the requested name in the same folder as the sdf file + // Note that this will not play well if the requested model shares files with other + // models that are placed in different folders or are in fuel, but should work for + // most local, self contained, models. + // Get the org name from context, model name from this and combine + let model_folder = binding.rsplitn(3, "/").skip(2).next().unwrap(); + model_folder.to_owned() + "/" + stripped + } else if let Some(path_idx) = binding.rfind("/") { + // It's a path relative to this model, remove file and append uri + let (model_path, _model_name) = binding.split_at(path_idx); + model_path.to_owned() + "/" + uri + } else { + println!( + "WARNING: Invalid model found path is {} and model uri is {}", + path, uri + ); + "".into() + }; + } + AssetSource::Bundled(_) | AssetSource::Package(_) => { + println!("WARNING: Requested asset source {:?} type not supported for SDFs, might behave unexpectedly", asset_source); + } } + asset_source } fn parse_scale(scale: &Option) -> Scale { diff --git a/rmf_site_editor/src/site_asset_io.rs b/rmf_site_editor/src/site_asset_io.rs index f50dc4f4..ecf6eca7 100644 --- a/rmf_site_editor/src/site_asset_io.rs +++ b/rmf_site_editor/src/site_asset_io.rs @@ -254,7 +254,6 @@ impl AssetIo for SiteAssetIo { // Order should be: // Relative to the building.yaml location, TODO, relative paths are tricky // Relative to some paths read from an environment variable (.. need to check what gz uses for models) - // For SDF Only: // Relative to a cache directory // Attempt to fetch from the server and save it to the cache directory @@ -267,14 +266,6 @@ impl AssetIo for SiteAssetIo { } } - if !asset_name.ends_with(".sdf") { - return Box::pin(async move { - Err(AssetIoError::Io(io::Error::new( - io::ErrorKind::Other, - format!("Asset {} not found", asset_name), - ))) - }); - } // Try local cache #[cfg(not(target_arch = "wasm32"))] { diff --git a/rmf_site_editor/src/widgets/new_model.rs b/rmf_site_editor/src/widgets/new_model.rs index 7ee6a371..5d21304e 100644 --- a/rmf_site_editor/src/widgets/new_model.rs +++ b/rmf_site_editor/src/widgets/new_model.rs @@ -156,10 +156,11 @@ impl<'a, 'w, 's> NewModel<'a, 'w, 's> { } if let Some(selected) = &self.events.new_model.asset_gallery_status.selected { if ui.button("Spawn model").clicked() { - let source = - AssetSource::Search(selected.owner.clone() + "/" + &selected.name); + let source = AssetSource::Remote( + selected.owner.clone() + "/" + &selected.name + "/model.sdf", + ); let model = Model { - source: source.clone(), + source, ..default() }; self.events.request.change_mode.send(ChangeMode::To( diff --git a/rmf_site_format/src/asset_source.rs b/rmf_site_format/src/asset_source.rs index 0ba87cc0..e6825380 100644 --- a/rmf_site_format/src/asset_source.rs +++ b/rmf_site_format/src/asset_source.rs @@ -52,8 +52,8 @@ impl Default for AssetSource { // Utility functions to add / strip prefixes for using AssetSource in AssetIo objects impl From<&Path> for AssetSource { fn from(path: &Path) -> Self { - if let Some(path) = path.to_str().and_then(|p| Some(String::from(p))) { - AssetSource::from(&path) + if let Some(path) = path.to_str() { + AssetSource::from(path) } else { AssetSource::default() } @@ -61,8 +61,8 @@ impl From<&Path> for AssetSource { } // Utility functions to add / strip prefixes for using AssetSource in AssetIo objects -impl From<&String> for AssetSource { - fn from(path: &String) -> Self { +impl From<&str> for AssetSource { + fn from(path: &str) -> Self { // TODO(luca) pattern matching here would make sure unimplemented variants are a compile error if let Some(path) = path.strip_prefix("rmf-server://").map(|p| p.to_string()) { return AssetSource::Remote(path); diff --git a/rmf_site_format/src/workcell.rs b/rmf_site_format/src/workcell.rs index 0d331acb..3506d488 100644 --- a/rmf_site_format/src/workcell.rs +++ b/rmf_site_format/src/workcell.rs @@ -250,7 +250,7 @@ impl WorkcellModel { // TODO(luca) Make a bundle for workcell models to avoid manual insertion here commands.insert(( NameInWorkcell(self.name.clone()), - AssetSource::from(filename), + AssetSource::from(filename.as_str()), self.pose.clone(), ConstraintDependents::default(), scale, From 769557f9cd064d63235acd13587d8e4da3a7a9dc Mon Sep 17 00:00:00 2001 From: Luca Della Vedova Date: Thu, 6 Jul 2023 13:30:44 +0800 Subject: [PATCH 03/17] Improve UX, add tags filter Signed-off-by: Luca Della Vedova --- rmf_site_editor/src/widgets/new_model.rs | 185 +++++++++++++++-------- 1 file changed, 126 insertions(+), 59 deletions(-) diff --git a/rmf_site_editor/src/widgets/new_model.rs b/rmf_site_editor/src/widgets/new_model.rs index 5d21304e..f09aadd4 100644 --- a/rmf_site_editor/src/widgets/new_model.rs +++ b/rmf_site_editor/src/widgets/new_model.rs @@ -21,8 +21,8 @@ use crate::AppEvents; use bevy::tasks::AsyncComputeTaskPool; use bevy::{ecs::system::SystemParam, prelude::*}; use bevy_egui::egui::{ - widgets::Image as EguiImage, Button, Color32, ComboBox, Context, Frame, Pos2, Rect, ScrollArea, - Ui, Window, + widgets::Image as EguiImage, Button, Color32, ComboBox, Context, Frame, Pos2, Rect, RichText, + ScrollArea, Ui, Window, }; use bevy_egui::EguiContext; use gz_fuel::{FuelClient as GzFuelClient, FuelModel}; @@ -33,6 +33,9 @@ pub struct FuelClient(GzFuelClient); /// Filters applied to models in the fuel list pub struct ShowAssetFilters { pub owner: Option, + pub recall_owner: Option, + pub tag: Option, + pub recall_tag: Option, } /// Used to signals whether to show or hide the left side panel with the asset gallery @@ -40,6 +43,8 @@ pub struct ShowAssetFilters { pub struct AssetGalleryStatus { pub show: bool, pub selected: Option, + pub cached_owners: Option>, + pub cached_tags: Option>, pub filters: ShowAssetFilters, } @@ -47,6 +52,9 @@ impl Default for ShowAssetFilters { fn default() -> Self { Self { owner: Some("OpenRobotics".into()), + recall_owner: None, + tag: None, + recall_tag: None, } } } @@ -56,6 +64,8 @@ impl Default for AssetGalleryStatus { Self { show: true, selected: None, + cached_owners: None, + cached_tags: None, filters: Default::default(), } } @@ -82,85 +92,142 @@ impl<'a, 'w, 's> NewModel<'a, 'w, 's> { pub fn show(mut self, ui: &mut Ui) { let fuel_client = &mut self.events.new_model.fuel_client; - let owners = fuel_client.get_owners_cached().unwrap(); + let gallery_status = &mut self.events.new_model.asset_gallery_status; + ui.label(RichText::new("Asset Gallery").size(18.0)); + ui.add_space(10.0); match &fuel_client.models { Some(models) => { - // TODO(luca) remove unwrap - let mut owner_filter = self - .events - .new_model - .asset_gallery_status - .filters - .owner - .clone(); + // Note, unwraps here are safe because the client will return None only if models + // are not populated which will not happen in this match branch + let mut owner_filter = gallery_status.filters.owner.clone(); let mut owner_filter_enabled = owner_filter.is_some(); - ui.checkbox(&mut owner_filter_enabled, "Owners"); - match owner_filter_enabled { - true => { - let mut selected = match owner_filter { - Some(s) => s, - None => owners[0].clone(), - }; - ComboBox::from_id_source("Asset Owner Filter") - .selected_text(selected.clone()) - .show_ui(ui, |ui| { - for owner in owners.into_iter() { - ui.selectable_value(&mut selected, owner.clone(), owner); - } - ui.end_row(); - }); - owner_filter = Some(selected); - } - false => { - owner_filter = None; - } - } - let models = match &owner_filter { - Some(owner) => fuel_client.as_ref().models_by_owner(&owner).unwrap(), + ui.label(RichText::new("Filters").size(14.0)); + ui.add_space(5.0); + ui.horizontal(|ui| { + ui.checkbox(&mut owner_filter_enabled, "Owners"); + gallery_status.filters.owner = match owner_filter_enabled { + true => { + let owners = gallery_status + .cached_owners + .clone() + .or_else(|| fuel_client.get_owners()) + .unwrap(); + let mut selected = match &owner_filter { + Some(s) => s.clone(), + None => gallery_status + .filters + .recall_owner + .clone() + .unwrap_or(owners[0].clone()), + }; + ComboBox::from_id_source("Asset Owner Filter") + .selected_text(selected.clone()) + .show_ui(ui, |ui| { + for owner in owners.into_iter() { + ui.selectable_value(&mut selected, owner.clone(), owner); + } + ui.end_row(); + }); + gallery_status.filters.recall_owner = Some(selected.clone()); + Some(selected) + } + false => None, + }; + }); + ui.add_space(5.0); + // TODO(luca) should we cache the models by owner result to avoid calling at every + // frame? + let mut models = match &owner_filter { + Some(owner) => fuel_client.as_ref().models_by_owner(None, &owner).unwrap(), None => models.clone(), }; + + let mut tag_filter = gallery_status.filters.tag.clone(); + let mut tag_filter_enabled = tag_filter.is_some(); + ui.horizontal(|ui| { + ui.checkbox(&mut tag_filter_enabled, "Tags"); + gallery_status.filters.tag = match tag_filter_enabled { + true => { + let tags = gallery_status + .cached_tags + .clone() + .or_else(|| fuel_client.get_tags()) + .unwrap(); + let mut selected = match &tag_filter { + Some(s) => s.clone(), + None => gallery_status + .filters + .recall_tag + .clone() + .unwrap_or(tags[0].clone()), + }; + ComboBox::from_id_source("Asset Tag Filter") + .selected_text(selected.clone()) + .show_ui(ui, |ui| { + for tag in tags.into_iter() { + ui.selectable_value(&mut selected, tag.clone(), tag); + } + ui.end_row(); + }); + gallery_status.filters.recall_tag = Some(selected.clone()); + Some(selected) + } + false => None, + }; + }); + + if let Some(tag) = &tag_filter { + models = fuel_client + .as_ref() + .models_by_tag(Some(&models), &tag) + .unwrap(); + } + ui.add_space(10.0); + + ui.label(RichText::new("Models").size(14.0)); + ui.add_space(5.0); // Show models - ScrollArea::vertical().max_height(500.0).show(ui, |ui| { + let mut new_selected = None; + ScrollArea::vertical().max_height(400.0).show(ui, |ui| { for model in models { - let sel = self - .events - .new_model - .asset_gallery_status + let sel = gallery_status .selected .as_ref() - .is_some_and(|s| s.name == model.name); + .is_some_and(|s| *s == model); if ui.selectable_label(sel, &model.name).clicked() { - self.events.new_model.asset_gallery_status.selected = Some(model); + new_selected = Some(model); } } }); - // Set the model source to what is selected - if let Some(selected) = &self.events.new_model.asset_gallery_status.selected { - let model_entity = self.events.new_model.model_preview_camera.model_entity; - let model = Model { - source: AssetSource::Remote( - selected.owner.clone() + "/" + &selected.name + "/model.sdf", - ), - ..default() - }; - self.events.commands.entity(model_entity).insert(model); - } + ui.add_space(10.0); ui.image( self.events.new_model.model_preview_camera.egui_handle, bevy_egui::egui::Vec2::new(320.0, 240.0), ); + ui.add_space(10.0); - if owner_filter != self.events.new_model.asset_gallery_status.filters.owner { - self.events.new_model.asset_gallery_status.filters.owner = owner_filter; + if gallery_status.selected != new_selected { + if let Some(selected) = new_selected { + // Set the model preview source to what is selected + let model_entity = self.events.new_model.model_preview_camera.model_entity; + let model = Model { + source: AssetSource::Remote( + selected.owner.clone() + "/" + &selected.name + "/model.sdf", + ), + ..default() + }; + self.events.commands.entity(model_entity).insert(model); + gallery_status.selected = Some(selected); + } } - if let Some(selected) = &self.events.new_model.asset_gallery_status.selected { + + if let Some(selected) = &gallery_status.selected { if ui.button("Spawn model").clicked() { - let source = AssetSource::Remote( - selected.owner.clone() + "/" + &selected.name + "/model.sdf", - ); let model = Model { - source, + source: AssetSource::Remote( + selected.owner.clone() + "/" + &selected.name + "/model.sdf", + ), ..default() }; self.events.request.change_mode.send(ChangeMode::To( From bfece66853c76f42bd98bb1dbbbef29d95079a79 Mon Sep 17 00:00:00 2001 From: Luca Della Vedova Date: Thu, 6 Jul 2023 15:20:56 +0800 Subject: [PATCH 04/17] Add async update to fuel cache Signed-off-by: Luca Della Vedova --- rmf_site_editor/src/site/fuel_cache.rs | 81 ++++++++++++++++++++++++ rmf_site_editor/src/site/mod.rs | 7 ++ rmf_site_editor/src/widgets/new_model.rs | 25 ++++---- 3 files changed, 101 insertions(+), 12 deletions(-) create mode 100644 rmf_site_editor/src/site/fuel_cache.rs diff --git a/rmf_site_editor/src/site/fuel_cache.rs b/rmf_site_editor/src/site/fuel_cache.rs new file mode 100644 index 00000000..56b5bb6b --- /dev/null +++ b/rmf_site_editor/src/site/fuel_cache.rs @@ -0,0 +1,81 @@ +/* + * Copyright (C) 2023 Open Source Robotics Foundation + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * +*/ + +use bevy::prelude::*; +use bevy::tasks::AsyncComputeTaskPool; +// TODO(luca) move FuelClient here +use crate::widgets::{AssetGalleryStatus, FuelClient}; +use crossbeam_channel::{Receiver, Sender}; + +/// Event used to request an update to the fuel cache +pub struct UpdateFuelCache; + +#[derive(Deref, DerefMut)] +pub struct FuelCacheUpdated(Option); + +/// Using channels instead of events to allow usage in wasm since, unlike event writers, they can +/// be cloned and moved into async functions therefore don't have lifetime issues +#[derive(Debug, Resource)] +pub struct UpdateFuelCacheChannels { + pub sender: Sender, + pub receiver: Receiver, +} + +impl Default for UpdateFuelCacheChannels { + fn default() -> Self { + let (sender, receiver) = crossbeam_channel::unbounded(); + Self { sender, receiver } + } +} + +pub fn handle_update_fuel_cache_requests( + mut events: EventReader, + mut gallery_status: ResMut, + mut fuel_client: ResMut, + mut channels: ResMut, +) { + if let Some(e) = events.iter().last() { + println!("Updating fuel cache"); + gallery_status.fetching_cache = true; + let mut fuel_client = fuel_client.clone(); + let sender = channels.sender.clone(); + AsyncComputeTaskPool::get() + .spawn(async move { + // Send client if update was successful + let res = fuel_client + .update_cache() + .await + .and_then(|()| Some(fuel_client)); + sender.send(FuelCacheUpdated(res)); + }) + .detach(); + } +} + +pub fn read_update_fuel_cache_results( + mut channels: ResMut, + mut fuel_client: ResMut, + mut gallery_status: ResMut, +) { + if let Ok(result) = channels.receiver.try_recv() { + println!("Fuel cache updated"); + if let Some(client) = result.0 { + *fuel_client = client; + } + gallery_status.fetching_cache = false; + } +} diff --git a/rmf_site_editor/src/site/mod.rs b/rmf_site_editor/src/site/mod.rs index f2156b69..6da7bf55 100644 --- a/rmf_site_editor/src/site/mod.rs +++ b/rmf_site_editor/src/site/mod.rs @@ -39,6 +39,9 @@ pub use drawing::*; pub mod floor; pub use floor::*; +pub mod fuel_cache; +pub use fuel_cache::*; + pub mod lane; pub use lane::*; @@ -141,6 +144,7 @@ impl Plugin for SitePlugin { .init_resource::() .init_resource::() .init_resource::() + .init_resource::() .add_event::() .add_event::() .add_event::() @@ -150,6 +154,7 @@ impl Plugin for SitePlugin { .add_event::() .add_event::() .add_event::() + .add_event::() .add_plugin(ChangePlugin::>::default()) .add_plugin(RecallPlugin::>::default()) .add_plugin(ChangePlugin::::default()) @@ -271,6 +276,8 @@ impl Plugin for SitePlugin { .with_system(add_wall_visual) .with_system(update_wall_edge) .with_system(update_wall_for_moved_anchors) + .with_system(handle_update_fuel_cache_requests) + .with_system(read_update_fuel_cache_results) .with_system(update_transforms_for_changed_poses) .with_system(export_lights), ); diff --git a/rmf_site_editor/src/widgets/new_model.rs b/rmf_site_editor/src/widgets/new_model.rs index f09aadd4..c913ee97 100644 --- a/rmf_site_editor/src/widgets/new_model.rs +++ b/rmf_site_editor/src/widgets/new_model.rs @@ -16,7 +16,7 @@ */ use crate::interaction::{ChangeMode, ModelPreviewCamera, SelectAnchor3D}; -use crate::site::{AssetSource, Model}; +use crate::site::{AssetSource, Model, UpdateFuelCache}; use crate::AppEvents; use bevy::tasks::AsyncComputeTaskPool; use bevy::{ecs::system::SystemParam, prelude::*}; @@ -27,7 +27,7 @@ use bevy_egui::egui::{ use bevy_egui::EguiContext; use gz_fuel::{FuelClient as GzFuelClient, FuelModel}; -#[derive(Resource, Default, Deref, DerefMut)] +#[derive(Resource, Clone, Default, Deref, DerefMut)] pub struct FuelClient(GzFuelClient); /// Filters applied to models in the fuel list @@ -46,6 +46,7 @@ pub struct AssetGalleryStatus { pub cached_owners: Option>, pub cached_tags: Option>, pub filters: ShowAssetFilters, + pub fetching_cache: bool, } impl Default for ShowAssetFilters { @@ -67,6 +68,7 @@ impl Default for AssetGalleryStatus { cached_owners: None, cached_tags: None, filters: Default::default(), + fetching_cache: false, } } } @@ -78,7 +80,7 @@ pub struct NewModelParams<'w, 's> { pub asset_gallery_status: ResMut<'w, AssetGalleryStatus>, pub model_preview_camera: Res<'w, ModelPreviewCamera>, pub image_assets: Res<'w, Assets>, - _ignore: Query<'w, 's, ()>, + pub update_cache: EventWriter<'w, 's, UpdateFuelCache>, } pub struct NewModel<'a, 'w, 's> { @@ -238,15 +240,14 @@ impl<'a, 'w, 's> NewModel<'a, 'w, 's> { } None => { ui.label("No models found"); - if ui.add(Button::new("Fetch models")).clicked() { - // TODO(luca) make this async to avoid blocking the whole application - fuel_client.update_cache_blocking(); - /* - AsyncComputeTaskPool::get().spawn( - async move {fuel_client.update_cache()}) - .detach(); - */ - } + } + } + ui.add_space(10.0); + if gallery_status.fetching_cache == true { + ui.label("Updating model cache..."); + } else { + if ui.add(Button::new("Update model cache")).clicked() { + self.events.new_model.update_cache.send(UpdateFuelCache); } } if ui.add(Button::new("Close")).clicked() { From b603027c1ec1c7258eec9f6355bbeaf99ebc9514 Mon Sep 17 00:00:00 2001 From: Luca Della Vedova Date: Thu, 6 Jul 2023 16:05:12 +0800 Subject: [PATCH 05/17] Clear compile warnings Signed-off-by: Luca Della Vedova --- rmf_site_editor/src/site/fuel_cache.rs | 16 +++++++++------- rmf_site_editor/src/site/model.rs | 2 +- rmf_site_editor/src/site/sdf.rs | 4 +--- rmf_site_editor/src/widgets/new_model.rs | 13 ++++--------- 4 files changed, 15 insertions(+), 20 deletions(-) diff --git a/rmf_site_editor/src/site/fuel_cache.rs b/rmf_site_editor/src/site/fuel_cache.rs index 56b5bb6b..d48b609f 100644 --- a/rmf_site_editor/src/site/fuel_cache.rs +++ b/rmf_site_editor/src/site/fuel_cache.rs @@ -16,7 +16,7 @@ */ use bevy::prelude::*; -use bevy::tasks::AsyncComputeTaskPool; +use bevy::tasks::IoTaskPool; // TODO(luca) move FuelClient here use crate::widgets::{AssetGalleryStatus, FuelClient}; use crossbeam_channel::{Receiver, Sender}; @@ -45,29 +45,31 @@ impl Default for UpdateFuelCacheChannels { pub fn handle_update_fuel_cache_requests( mut events: EventReader, mut gallery_status: ResMut, - mut fuel_client: ResMut, - mut channels: ResMut, + fuel_client: Res, + channels: Res, ) { - if let Some(e) = events.iter().last() { + if events.iter().last().is_some() { println!("Updating fuel cache"); gallery_status.fetching_cache = true; let mut fuel_client = fuel_client.clone(); let sender = channels.sender.clone(); - AsyncComputeTaskPool::get() + IoTaskPool::get() .spawn(async move { // Send client if update was successful let res = fuel_client .update_cache() .await .and_then(|()| Some(fuel_client)); - sender.send(FuelCacheUpdated(res)); + sender + .send(FuelCacheUpdated(res)) + .expect("Failed sending fuel cache update event"); }) .detach(); } } pub fn read_update_fuel_cache_results( - mut channels: ResMut, + channels: Res, mut fuel_client: ResMut, mut gallery_status: ResMut, ) { diff --git a/rmf_site_editor/src/site/model.rs b/rmf_site_editor/src/site/model.rs index ba6b1944..57a06d40 100644 --- a/rmf_site_editor/src/site/model.rs +++ b/rmf_site_editor/src/site/model.rs @@ -16,7 +16,7 @@ */ use crate::{ - interaction::{DragPlaneBundle, PickableBundle, Selectable, MODEL_PREVIEW_LAYER}, + interaction::{DragPlaneBundle, Selectable, MODEL_PREVIEW_LAYER}, site::{Category, PreventDeletion, SiteAssets}, SdfRoot, }; diff --git a/rmf_site_editor/src/site/sdf.rs b/rmf_site_editor/src/site/sdf.rs index d9a9ad70..4d814415 100644 --- a/rmf_site_editor/src/site/sdf.rs +++ b/rmf_site_editor/src/site/sdf.rs @@ -52,9 +52,7 @@ fn compute_model_source(path: &str, uri: &str) -> AssetSource { "".into() }; } - AssetSource::Local(ref mut p) - | AssetSource::Bundled(ref mut p) - | AssetSource::Package(ref mut p) => { + AssetSource::Local(ref mut p) => { let binding = p.clone(); *p = if let Some(stripped) = uri.strip_prefix("model://") { // Search for a model with the requested name in the same folder as the sdf file diff --git a/rmf_site_editor/src/widgets/new_model.rs b/rmf_site_editor/src/widgets/new_model.rs index c913ee97..6b9f1729 100644 --- a/rmf_site_editor/src/widgets/new_model.rs +++ b/rmf_site_editor/src/widgets/new_model.rs @@ -18,13 +18,8 @@ use crate::interaction::{ChangeMode, ModelPreviewCamera, SelectAnchor3D}; use crate::site::{AssetSource, Model, UpdateFuelCache}; use crate::AppEvents; -use bevy::tasks::AsyncComputeTaskPool; use bevy::{ecs::system::SystemParam, prelude::*}; -use bevy_egui::egui::{ - widgets::Image as EguiImage, Button, Color32, ComboBox, Context, Frame, Pos2, Rect, RichText, - ScrollArea, Ui, Window, -}; -use bevy_egui::EguiContext; +use bevy_egui::egui::{Button, ComboBox, RichText, ScrollArea, Ui}; use gz_fuel::{FuelClient as GzFuelClient, FuelModel}; #[derive(Resource, Clone, Default, Deref, DerefMut)] @@ -92,7 +87,7 @@ impl<'a, 'w, 's> NewModel<'a, 'w, 's> { Self { events } } - pub fn show(mut self, ui: &mut Ui) { + pub fn show(self, ui: &mut Ui) { let fuel_client = &mut self.events.new_model.fuel_client; let gallery_status = &mut self.events.new_model.asset_gallery_status; ui.label(RichText::new("Asset Gallery").size(18.0)); @@ -101,7 +96,7 @@ impl<'a, 'w, 's> NewModel<'a, 'w, 's> { Some(models) => { // Note, unwraps here are safe because the client will return None only if models // are not populated which will not happen in this match branch - let mut owner_filter = gallery_status.filters.owner.clone(); + let owner_filter = gallery_status.filters.owner.clone(); let mut owner_filter_enabled = owner_filter.is_some(); ui.label(RichText::new("Filters").size(14.0)); ui.add_space(5.0); @@ -144,7 +139,7 @@ impl<'a, 'w, 's> NewModel<'a, 'w, 's> { None => models.clone(), }; - let mut tag_filter = gallery_status.filters.tag.clone(); + let tag_filter = gallery_status.filters.tag.clone(); let mut tag_filter_enabled = tag_filter.is_some(); ui.horizontal(|ui| { ui.checkbox(&mut tag_filter_enabled, "Tags"); From f29bc125077583343bfc9b36f3bb6e8fa730aa24 Mon Sep 17 00:00:00 2001 From: Luca Della Vedova Date: Fri, 7 Jul 2023 12:33:30 +0800 Subject: [PATCH 06/17] Cleanup and refactors to model logic Signed-off-by: Luca Della Vedova --- rmf_site_editor/src/site/fuel_cache.rs | 6 +- rmf_site_editor/src/site/mod.rs | 2 + rmf_site_editor/src/site/model.rs | 127 +++++++++++------------ rmf_site_editor/src/widgets/mod.rs | 1 - rmf_site_editor/src/widgets/new_model.rs | 6 +- 5 files changed, 68 insertions(+), 74 deletions(-) diff --git a/rmf_site_editor/src/site/fuel_cache.rs b/rmf_site_editor/src/site/fuel_cache.rs index d48b609f..19647f95 100644 --- a/rmf_site_editor/src/site/fuel_cache.rs +++ b/rmf_site_editor/src/site/fuel_cache.rs @@ -18,8 +18,12 @@ use bevy::prelude::*; use bevy::tasks::IoTaskPool; // TODO(luca) move FuelClient here -use crate::widgets::{AssetGalleryStatus, FuelClient}; +use crate::widgets::AssetGalleryStatus; use crossbeam_channel::{Receiver, Sender}; +use gz_fuel::FuelClient as GzFuelClient; + +#[derive(Resource, Clone, Default, Deref, DerefMut)] +pub struct FuelClient(GzFuelClient); /// Event used to request an update to the fuel cache pub struct UpdateFuelCache; diff --git a/rmf_site_editor/src/site/mod.rs b/rmf_site_editor/src/site/mod.rs index 6da7bf55..fa2bd449 100644 --- a/rmf_site_editor/src/site/mod.rs +++ b/rmf_site_editor/src/site/mod.rs @@ -140,6 +140,7 @@ impl Plugin for SitePlugin { .add_state_to_stage(CoreStage::PostUpdate, SiteState::Off) .insert_resource(ClearColor(Color::rgb(0., 0., 0.))) .insert_resource(FloorVisibility::default()) + .init_resource::() .init_resource::() .init_resource::() .init_resource::() @@ -261,6 +262,7 @@ impl Plugin for SitePlugin { .with_system(add_measurement_visuals) .with_system(update_changed_measurement) .with_system(update_measurement_for_moved_anchors) + .with_system(handle_model_loaded_events) .with_system(update_model_scenes) .with_system(handle_new_sdf_roots) .with_system(update_model_scales) diff --git a/rmf_site_editor/src/site/model.rs b/rmf_site_editor/src/site/model.rs index 57a06d40..44601b8c 100644 --- a/rmf_site_editor/src/site/model.rs +++ b/rmf_site_editor/src/site/model.rs @@ -80,13 +80,8 @@ pub struct PendingSpawning(HandleUntyped); #[derive(Component, Debug, Clone, Copy)] pub struct ModelSceneRoot; -pub fn update_model_scenes( +pub fn handle_model_loaded_events( mut commands: Commands, - changed_models: Query< - (Entity, &AssetSource, &Pose, &TentativeModelFormat), - (Changed, With), - >, - asset_server: Res, loading_models: Query< ( Entity, @@ -97,15 +92,8 @@ pub fn update_model_scenes( ), With, >, - spawned_models: Query< - Entity, - ( - Without, - With, - With, - ), - >, mut current_scenes: Query<&mut ModelScene>, + asset_server: Res, site_assets: Res, meshes: Res>, scenes: Res>, @@ -113,53 +101,6 @@ pub fn update_model_scenes( urdfs: Res>, sdfs: Res>, ) { - fn spawn_model( - e: Entity, - source: &AssetSource, - pose: &Pose, - asset_server: &AssetServer, - tentative_format: &TentativeModelFormat, - commands: &mut Commands, - ) { - let mut commands = commands.entity(e); - commands - .insert(ModelScene { - source: source.clone(), - format: tentative_format.clone(), - entity: None, - }) - .insert(SpatialBundle { - transform: pose.transform(), - ..default() - }) - .insert(Category::Model); - - // For search assets, look at subfolders and iterate through file formats - // TODO(luca) This will also iterate for non search assets, fix - let asset_source = match source { - AssetSource::Search(name) => { - let model_name = name.split('/').last().unwrap(); - AssetSource::Search(name.to_owned() + &tentative_format.to_string(model_name)) - } - _ => source.clone(), - }; - let handle = asset_server.load_untyped(&String::from(&asset_source)); - commands - .insert(PreventDeletion::because( - "Waiting for model to spawn".to_string(), - )) - .insert(PendingSpawning(handle)); - } - - // There is a bug(?) in bevy scenes, which causes panic when a scene is despawned - // immediately after it is spawned. - // Work around it by checking the `spawned` container BEFORE updating it so that - // entities are only despawned at the next frame. This also ensures that entities are - // "fully spawned" before despawning. - for e in spawned_models.iter() { - commands.entity(e).remove::(); - } - // For each model that is loading, check if its scene has finished loading // yet. If the scene has finished loading, then insert it as a child of the // model entity and make it selectable. @@ -223,15 +164,64 @@ pub fn update_model_scenes( None }; if let Some(id) = model_id { - commands.entity(e).insert(ModelSceneRoot); + let mut cmd = commands.entity(e); + cmd.insert(ModelSceneRoot) + .remove::<(PreventDeletion, PendingSpawning)>(); if !render_layer.is_some_and(|l| l.iter().all(|l| l == MODEL_PREVIEW_LAYER)) { - commands.entity(e).insert(Selectable::new(e)); + cmd.insert(Selectable::new(e)); } - commands.entity(e).remove::(); current_scenes.get_mut(e).unwrap().entity = Some(id); } } } +} + +pub fn update_model_scenes( + mut commands: Commands, + changed_models: Query< + (Entity, &AssetSource, &Pose, &TentativeModelFormat), + (Changed, With), + >, + asset_server: Res, + mut current_scenes: Query<&mut ModelScene>, +) { + fn spawn_model( + e: Entity, + source: &AssetSource, + pose: &Pose, + asset_server: &AssetServer, + tentative_format: &TentativeModelFormat, + commands: &mut Commands, + ) { + let mut commands = commands.entity(e); + commands + .insert(ModelScene { + source: source.clone(), + format: tentative_format.clone(), + entity: None, + }) + .insert(SpatialBundle { + transform: pose.transform(), + ..default() + }) + .insert(Category::Model); + + // For search assets, look at subfolders and iterate through file formats + // TODO(luca) This will also iterate for non search assets, fix + let asset_source = match source { + AssetSource::Search(name) => { + let model_name = name.split('/').last().unwrap(); + AssetSource::Search(name.to_owned() + &tentative_format.to_string(model_name)) + } + _ => source.clone(), + }; + let handle = asset_server.load_untyped(&String::from(&asset_source)); + commands + .insert(PreventDeletion::because( + "Waiting for model to spawn".to_string(), + )) + .insert(PendingSpawning(handle)); + } // update changed models for (e, source, pose, tentative_format) in changed_models.iter() { @@ -240,7 +230,6 @@ pub fn update_model_scenes( if current_scene.source != *source || current_scene.format != *tentative_format { if let Some(scene_entity) = current_scene.entity { commands.entity(scene_entity).despawn_recursive(); - commands.entity(e).remove_children(&[scene_entity]); commands.entity(e).remove::(); } // Updated model @@ -291,13 +280,17 @@ pub fn update_model_tentative_formats( LoadState::Failed => { if let Some(fmt) = tentative_format.next() { *tentative_format = fmt; - commands.entity(e).remove::(); + commands + .entity(e) + .remove::<(PreventDeletion, PendingSpawning)>(); } else { println!( "WARNING: Model with source {} not found", String::from(source) ); - commands.entity(e).remove::(); + commands + .entity(e) + .remove::<(PreventDeletion, TentativeModelFormat)>(); } } _ => {} diff --git a/rmf_site_editor/src/widgets/mod.rs b/rmf_site_editor/src/widgets/mod.rs index 5c3931d9..aca9efa5 100644 --- a/rmf_site_editor/src/widgets/mod.rs +++ b/rmf_site_editor/src/widgets/mod.rs @@ -79,7 +79,6 @@ impl Plugin for StandardUiLayout { .init_resource::() .init_resource::() .init_resource::() - .init_resource::() .init_resource::() .init_resource::() .add_system_set(SystemSet::on_enter(AppState::MainMenu).with_system(init_ui_style)) diff --git a/rmf_site_editor/src/widgets/new_model.rs b/rmf_site_editor/src/widgets/new_model.rs index 6b9f1729..00122610 100644 --- a/rmf_site_editor/src/widgets/new_model.rs +++ b/rmf_site_editor/src/widgets/new_model.rs @@ -16,15 +16,12 @@ */ use crate::interaction::{ChangeMode, ModelPreviewCamera, SelectAnchor3D}; -use crate::site::{AssetSource, Model, UpdateFuelCache}; +use crate::site::{AssetSource, FuelClient, Model, UpdateFuelCache}; use crate::AppEvents; use bevy::{ecs::system::SystemParam, prelude::*}; use bevy_egui::egui::{Button, ComboBox, RichText, ScrollArea, Ui}; use gz_fuel::{FuelClient as GzFuelClient, FuelModel}; -#[derive(Resource, Clone, Default, Deref, DerefMut)] -pub struct FuelClient(GzFuelClient); - /// Filters applied to models in the fuel list pub struct ShowAssetFilters { pub owner: Option, @@ -74,7 +71,6 @@ pub struct NewModelParams<'w, 's> { // TODO(luca) refactor to see whether we need pub asset_gallery_status: ResMut<'w, AssetGalleryStatus>, pub model_preview_camera: Res<'w, ModelPreviewCamera>, - pub image_assets: Res<'w, Assets>, pub update_cache: EventWriter<'w, 's, UpdateFuelCache>, } From 314c2bdb25ec741631b6c323807c1bf675606fa1 Mon Sep 17 00:00:00 2001 From: Luca Della Vedova Date: Fri, 7 Jul 2023 13:43:04 +0800 Subject: [PATCH 07/17] Minor cleanups Signed-off-by: Luca Della Vedova --- rmf_site_editor/src/site/model.rs | 10 ++++------ rmf_site_editor/src/widgets/new_model.rs | 23 +++++++++++++---------- 2 files changed, 17 insertions(+), 16 deletions(-) diff --git a/rmf_site_editor/src/site/model.rs b/rmf_site_editor/src/site/model.rs index 44601b8c..db85a26b 100644 --- a/rmf_site_editor/src/site/model.rs +++ b/rmf_site_editor/src/site/model.rs @@ -278,19 +278,17 @@ pub fn update_model_tentative_formats( for (e, mut tentative_format, h, source) in loading_models.iter_mut() { match asset_server.get_load_state(&h.0) { LoadState::Failed => { + let mut cmd = commands.entity(e); + cmd.remove::(); if let Some(fmt) = tentative_format.next() { *tentative_format = fmt; - commands - .entity(e) - .remove::<(PreventDeletion, PendingSpawning)>(); + cmd.remove::(); } else { println!( "WARNING: Model with source {} not found", String::from(source) ); - commands - .entity(e) - .remove::<(PreventDeletion, TentativeModelFormat)>(); + cmd.remove::(); } } _ => {} diff --git a/rmf_site_editor/src/widgets/new_model.rs b/rmf_site_editor/src/widgets/new_model.rs index 00122610..8345a918 100644 --- a/rmf_site_editor/src/widgets/new_model.rs +++ b/rmf_site_editor/src/widgets/new_model.rs @@ -181,17 +181,20 @@ impl<'a, 'w, 's> NewModel<'a, 'w, 's> { ui.add_space(5.0); // Show models let mut new_selected = None; - ScrollArea::vertical().max_height(400.0).show(ui, |ui| { - for model in models { - let sel = gallery_status - .selected - .as_ref() - .is_some_and(|s| *s == model); - if ui.selectable_label(sel, &model.name).clicked() { - new_selected = Some(model); + ScrollArea::vertical() + .max_height(350.0) + .auto_shrink([false, false]) + .show(ui, |ui| { + for model in models { + let sel = gallery_status + .selected + .as_ref() + .is_some_and(|s| *s == model); + if ui.selectable_label(sel, &model.name).clicked() { + new_selected = Some(model); + } } - } - }); + }); ui.add_space(10.0); ui.image( From 410ac0758a149f9127c3eb128fb2cbea915443a8 Mon Sep 17 00:00:00 2001 From: Luca Della Vedova Date: Wed, 12 Jul 2023 15:28:37 +0800 Subject: [PATCH 08/17] Add private models and API key support Signed-off-by: Luca Della Vedova --- rmf_site_editor/src/site/fuel_cache.rs | 30 +++++++-- rmf_site_editor/src/site/mod.rs | 2 + rmf_site_editor/src/site/model.rs | 44 ++++++++------ rmf_site_editor/src/site_asset_io.rs | 18 +++--- rmf_site_editor/src/widgets/new_model.rs | 77 ++++++++++++++++++++++-- 5 files changed, 133 insertions(+), 38 deletions(-) diff --git a/rmf_site_editor/src/site/fuel_cache.rs b/rmf_site_editor/src/site/fuel_cache.rs index 19647f95..e25fe622 100644 --- a/rmf_site_editor/src/site/fuel_cache.rs +++ b/rmf_site_editor/src/site/fuel_cache.rs @@ -15,10 +15,11 @@ * */ +use crate::site::{AssetSource, ModelMarker, ModelSceneRoot, TentativeModelFormat}; +use crate::site_asset_io::FUEL_API_KEY; +use crate::widgets::AssetGalleryStatus; use bevy::prelude::*; use bevy::tasks::IoTaskPool; -// TODO(luca) move FuelClient here -use crate::widgets::AssetGalleryStatus; use crossbeam_channel::{Receiver, Sender}; use gz_fuel::FuelClient as GzFuelClient; @@ -31,6 +32,10 @@ pub struct UpdateFuelCache; #[derive(Deref, DerefMut)] pub struct FuelCacheUpdated(Option); +/// Event used to set the fuel API key from the UI. Will also trigger a reload for failed assets +#[derive(Deref, DerefMut)] +pub struct SetFuelApiKey(pub String); + /// Using channels instead of events to allow usage in wasm since, unlike event writers, they can /// be cloned and moved into async functions therefore don't have lifetime issues #[derive(Debug, Resource)] @@ -53,7 +58,7 @@ pub fn handle_update_fuel_cache_requests( channels: Res, ) { if events.iter().last().is_some() { - println!("Updating fuel cache"); + info!("Updating fuel cache"); gallery_status.fetching_cache = true; let mut fuel_client = fuel_client.clone(); let sender = channels.sender.clone(); @@ -78,10 +83,23 @@ pub fn read_update_fuel_cache_results( mut gallery_status: ResMut, ) { if let Ok(result) = channels.receiver.try_recv() { - println!("Fuel cache updated"); - if let Some(client) = result.0 { - *fuel_client = client; + match result.0 { + Some(client) => *fuel_client = client, + None => error!("Failed updating fuel cache"), } gallery_status.fetching_cache = false; } } + +pub fn reload_failed_models_with_new_api_key( + mut commands: Commands, + mut api_key_events: EventReader, + failed_models: Query, Without)>, +) { + if let Some(key) = api_key_events.iter().last() { + *FUEL_API_KEY.lock().unwrap() = Some((**key).clone()); + for e in &failed_models { + commands.entity(e).insert(TentativeModelFormat::default()); + } + } +} diff --git a/rmf_site_editor/src/site/mod.rs b/rmf_site_editor/src/site/mod.rs index fa2bd449..ca1b4987 100644 --- a/rmf_site_editor/src/site/mod.rs +++ b/rmf_site_editor/src/site/mod.rs @@ -156,6 +156,7 @@ impl Plugin for SitePlugin { .add_event::() .add_event::() .add_event::() + .add_event::() .add_plugin(ChangePlugin::>::default()) .add_plugin(RecallPlugin::>::default()) .add_plugin(ChangePlugin::::default()) @@ -280,6 +281,7 @@ impl Plugin for SitePlugin { .with_system(update_wall_for_moved_anchors) .with_system(handle_update_fuel_cache_requests) .with_system(read_update_fuel_cache_results) + .with_system(reload_failed_models_with_new_api_key) .with_system(update_transforms_for_changed_poses) .with_system(export_lights), ); diff --git a/rmf_site_editor/src/site/model.rs b/rmf_site_editor/src/site/model.rs index 16fd1781..9ea07841 100644 --- a/rmf_site_editor/src/site/model.rs +++ b/rmf_site_editor/src/site/model.rs @@ -18,6 +18,7 @@ use crate::{ interaction::{DragPlaneBundle, Selectable, MODEL_PREVIEW_LAYER}, site::{Category, PreventDeletion, SiteAssets}, + site_asset_io::MODEL_ENVIRONMENT_VARIABLE, SdfRoot, }; use bevy::{asset::LoadState, gltf::Gltf, prelude::*, render::view::RenderLayers}; @@ -83,13 +84,7 @@ pub struct ModelSceneRoot; pub fn handle_model_loaded_events( mut commands: Commands, loading_models: Query< - ( - Entity, - &TentativeModelFormat, - &PendingSpawning, - &Scale, - Option<&RenderLayers>, - ), + (Entity, &PendingSpawning, &Scale, Option<&RenderLayers>), With, >, mut current_scenes: Query<&mut ModelScene>, @@ -104,7 +99,7 @@ pub fn handle_model_loaded_events( // For each model that is loading, check if its scene has finished loading // yet. If the scene has finished loading, then insert it as a child of the // model entity and make it selectable. - for (e, tentative_format, h, scale, render_layer) in loading_models.iter() { + for (e, h, scale, render_layer) in loading_models.iter() { if asset_server.get_load_state(&h.0) == LoadState::Loaded { let model_id = if let Some(gltf) = gltfs.get(&h.typed_weak::()) { Some(commands.entity(e).add_children(|parent| { @@ -270,28 +265,39 @@ pub fn update_model_tentative_formats( >, asset_server: Res, ) { + static SUPPORTED_EXTENSIONS: &[&str] = &["obj", "stl", "sdf", "glb", "gltf"]; for e in changed_models.iter() { // Reset to the first format commands.entity(e).insert(TentativeModelFormat::default()); } // Check from the asset server if any format failed, if it did try the next for (e, mut tentative_format, h, source) in loading_models.iter_mut() { - match asset_server.get_load_state(&h.0) { - LoadState::Failed => { - let mut cmd = commands.entity(e); - cmd.remove::(); + if matches!(asset_server.get_load_state(&h.0), LoadState::Failed) { + let mut cmd = commands.entity(e); + cmd.remove::(); + // We want to iterate only for search asset types, for others just print an error + if matches!(source, AssetSource::Search(_)) { if let Some(fmt) = tentative_format.next() { *tentative_format = fmt; cmd.remove::(); - } else { - warn!( - "WARNING: Model with source {} not found", - String::from(source) - ); - cmd.remove::(); + continue; } } - _ => {} + let path = String::from(source); + let model_ext = path + .rsplit_once('.') + .map(|s| s.1.to_owned()) + .unwrap_or_else(|| tentative_format.to_string("")); + let reason = if !SUPPORTED_EXTENSIONS.iter().any(|e| model_ext.ends_with(e)) { + "Format not supported".to_owned() + } else { + match source { + AssetSource::Search(_) | AssetSource::Remote(_) => format!("Model not found, try using an API key if it belongs to a private organization, or add its path to the {} environment variable", MODEL_ENVIRONMENT_VARIABLE), + _ => "Failed parsing file".to_owned(), + } + }; + warn!("Failed loading Model with source {}: {}", path, reason); + cmd.remove::(); } } } diff --git a/rmf_site_editor/src/site_asset_io.rs b/rmf_site_editor/src/site_asset_io.rs index ecf6eca7..56e42f4a 100644 --- a/rmf_site_editor/src/site_asset_io.rs +++ b/rmf_site_editor/src/site_asset_io.rs @@ -10,6 +10,7 @@ use std::fs; use std::io; use std::io::prelude::*; use std::path::{Path, PathBuf}; +use std::sync::Mutex; use crate::urdf_loader::UrdfPlugin; use urdf_rs::utils::expand_package_path; @@ -29,7 +30,9 @@ struct SiteAssetIo { } const FUEL_BASE_URI: &str = "https://fuel.gazebosim.org/1.0"; -const MODEL_ENVIRONMENT_VARIABLE: &str = "GZ_SIM_RESOURCE_PATH"; +pub const MODEL_ENVIRONMENT_VARIABLE: &str = "GZ_SIM_RESOURCE_PATH"; + +pub static FUEL_API_KEY: Mutex> = Mutex::new(None); #[derive(Deserialize)] struct FuelErrorMsg { @@ -61,12 +64,13 @@ impl SiteAssetIo { asset_name: String, ) -> BoxedFuture<'a, Result, AssetIoError>> { Box::pin(async move { - let bytes = surf::get(remote_url.clone()) - .recv_bytes() - .await - .map_err(|e| { - AssetIoError::Io(io::Error::new(io::ErrorKind::Other, e.to_string())) - })?; + let mut req = surf::get(remote_url.clone()); + if let Some(key) = &(*FUEL_API_KEY.lock().unwrap()) { + req = req.header("Private-token", key.clone()); + } + let bytes = req.recv_bytes().await.map_err(|e| { + AssetIoError::Io(io::Error::new(io::ErrorKind::Other, e.to_string())) + })?; match serde_json::from_slice::(&bytes) { Ok(error) => { diff --git a/rmf_site_editor/src/widgets/new_model.rs b/rmf_site_editor/src/widgets/new_model.rs index 8345a918..39332690 100644 --- a/rmf_site_editor/src/widgets/new_model.rs +++ b/rmf_site_editor/src/widgets/new_model.rs @@ -16,11 +16,11 @@ */ use crate::interaction::{ChangeMode, ModelPreviewCamera, SelectAnchor3D}; -use crate::site::{AssetSource, FuelClient, Model, UpdateFuelCache}; +use crate::site::{AssetSource, FuelClient, Model, SetFuelApiKey, UpdateFuelCache}; use crate::AppEvents; use bevy::{ecs::system::SystemParam, prelude::*}; -use bevy_egui::egui::{Button, ComboBox, RichText, ScrollArea, Ui}; -use gz_fuel::{FuelClient as GzFuelClient, FuelModel}; +use bevy_egui::egui::{Button, ComboBox, RichText, ScrollArea, Ui, Window}; +use gz_fuel::FuelModel; /// Filters applied to models in the fuel list pub struct ShowAssetFilters { @@ -28,6 +28,8 @@ pub struct ShowAssetFilters { pub recall_owner: Option, pub tag: Option, pub recall_tag: Option, + pub private: Option, + pub recall_private: Option, } /// Used to signals whether to show or hide the left side panel with the asset gallery @@ -38,7 +40,9 @@ pub struct AssetGalleryStatus { pub cached_owners: Option>, pub cached_tags: Option>, pub filters: ShowAssetFilters, + pub proposed_api_key: String, pub fetching_cache: bool, + pub show_api_window: bool, } impl Default for ShowAssetFilters { @@ -48,6 +52,8 @@ impl Default for ShowAssetFilters { recall_owner: None, tag: None, recall_tag: None, + private: None, + recall_private: None, } } } @@ -60,7 +66,9 @@ impl Default for AssetGalleryStatus { cached_owners: None, cached_tags: None, filters: Default::default(), + proposed_api_key: Default::default(), fetching_cache: false, + show_api_window: false, } } } @@ -72,6 +80,7 @@ pub struct NewModelParams<'w, 's> { pub asset_gallery_status: ResMut<'w, AssetGalleryStatus>, pub model_preview_camera: Res<'w, ModelPreviewCamera>, pub update_cache: EventWriter<'w, 's, UpdateFuelCache>, + pub set_api_key: EventWriter<'w, 's, SetFuelApiKey>, } pub struct NewModel<'a, 'w, 's> { @@ -127,7 +136,6 @@ impl<'a, 'w, 's> NewModel<'a, 'w, 's> { false => None, }; }); - ui.add_space(5.0); // TODO(luca) should we cache the models by owner result to avoid calling at every // frame? let mut models = match &owner_filter { @@ -175,6 +183,42 @@ impl<'a, 'w, 's> NewModel<'a, 'w, 's> { .models_by_tag(Some(&models), &tag) .unwrap(); } + + let private_filter = gallery_status.filters.private.clone(); + let mut private_filter_enabled = private_filter.is_some(); + ui.horizontal(|ui| { + ui.checkbox(&mut private_filter_enabled, "Private"); + gallery_status.filters.private = match private_filter_enabled { + true => { + let mut selected = match &private_filter { + Some(s) => s.clone(), + None => gallery_status.filters.recall_private.unwrap_or(false), + }; + ComboBox::from_id_source("Asset Private Filter") + .selected_text(selected.to_string()) + .show_ui(ui, |ui| { + for private in [true, false].into_iter() { + ui.selectable_value( + &mut selected, + private, + private.to_string(), + ); + } + ui.end_row(); + }); + gallery_status.filters.recall_private = Some(selected); + Some(selected) + } + false => None, + }; + }); + + if let Some(private) = &private_filter { + models = fuel_client + .as_ref() + .models_by_private(Some(&models), *private) + .unwrap(); + } ui.add_space(10.0); ui.label(RichText::new("Models").size(14.0)); @@ -182,7 +226,7 @@ impl<'a, 'w, 's> NewModel<'a, 'w, 's> { // Show models let mut new_selected = None; ScrollArea::vertical() - .max_height(350.0) + .max_height(300.0) .auto_shrink([false, false]) .show(ui, |ui| { for model in models { @@ -237,6 +281,27 @@ impl<'a, 'w, 's> NewModel<'a, 'w, 's> { } } ui.add_space(10.0); + if gallery_status.show_api_window { + Window::new("API Key").show(ui.ctx(), |ui| { + ui.label("Key"); + ui.text_edit_singleline(&mut gallery_status.proposed_api_key); + if ui.add(Button::new("Save")).clicked() { + // Take it to avoid leaking the information in the dialog + self.events + .new_model + .set_api_key + .send(SetFuelApiKey(gallery_status.proposed_api_key.clone())); + fuel_client.token = Some(std::mem::take(&mut gallery_status.proposed_api_key)); + gallery_status.show_api_window = false; + } else if ui.add(Button::new("Close")).clicked() { + gallery_status.proposed_api_key = Default::default(); + gallery_status.show_api_window = false; + } + }); + } + if ui.add(Button::new("Set API key")).clicked() { + gallery_status.show_api_window = true; + } if gallery_status.fetching_cache == true { ui.label("Updating model cache..."); } else { @@ -245,7 +310,7 @@ impl<'a, 'w, 's> NewModel<'a, 'w, 's> { } } if ui.add(Button::new("Close")).clicked() { - self.events.new_model.asset_gallery_status.show = false; + gallery_status.show = false; } } } From 7bb2c3c1157a4e1000b8950fa85d55d70969289e Mon Sep 17 00:00:00 2001 From: Luca Della Vedova Date: Wed, 12 Jul 2023 17:38:37 +0800 Subject: [PATCH 09/17] Optimize model filtering Signed-off-by: Luca Della Vedova --- rmf_site_editor/src/widgets/new_model.rs | 47 +++++++++++------------- 1 file changed, 22 insertions(+), 25 deletions(-) diff --git a/rmf_site_editor/src/widgets/new_model.rs b/rmf_site_editor/src/widgets/new_model.rs index 39332690..12d8392d 100644 --- a/rmf_site_editor/src/widgets/new_model.rs +++ b/rmf_site_editor/src/widgets/new_model.rs @@ -136,12 +136,6 @@ impl<'a, 'w, 's> NewModel<'a, 'w, 's> { false => None, }; }); - // TODO(luca) should we cache the models by owner result to avoid calling at every - // frame? - let mut models = match &owner_filter { - Some(owner) => fuel_client.as_ref().models_by_owner(None, &owner).unwrap(), - None => models.clone(), - }; let tag_filter = gallery_status.filters.tag.clone(); let mut tag_filter_enabled = tag_filter.is_some(); @@ -177,13 +171,6 @@ impl<'a, 'w, 's> NewModel<'a, 'w, 's> { }; }); - if let Some(tag) = &tag_filter { - models = fuel_client - .as_ref() - .models_by_tag(Some(&models), &tag) - .unwrap(); - } - let private_filter = gallery_status.filters.private.clone(); let mut private_filter_enabled = private_filter.is_some(); ui.horizontal(|ui| { @@ -213,14 +200,27 @@ impl<'a, 'w, 's> NewModel<'a, 'w, 's> { }; }); - if let Some(private) = &private_filter { - models = fuel_client - .as_ref() - .models_by_private(Some(&models), *private) - .unwrap(); - } ui.add_space(10.0); + // TODO(luca) should we cache the models by owner result to avoid calling at every + // frame? + let models = models + .iter() + .filter(|m| { + owner_filter.is_none() + | owner_filter.as_ref().is_some_and(|owner| m.owner == *owner) + }) + .filter(|m| { + private_filter.is_none() + | private_filter + .as_ref() + .is_some_and(|private| m.private == *private) + }) + .filter(|m| { + tag_filter.is_none() + | tag_filter.as_ref().is_some_and(|tag| m.tags.contains(&tag)) + }); + ui.label(RichText::new("Models").size(14.0)); ui.add_space(5.0); // Show models @@ -230,10 +230,7 @@ impl<'a, 'w, 's> NewModel<'a, 'w, 's> { .auto_shrink([false, false]) .show(ui, |ui| { for model in models { - let sel = gallery_status - .selected - .as_ref() - .is_some_and(|s| *s == model); + let sel = gallery_status.selected.as_ref().is_some_and(|s| s == model); if ui.selectable_label(sel, &model.name).clicked() { new_selected = Some(model); } @@ -247,7 +244,7 @@ impl<'a, 'w, 's> NewModel<'a, 'w, 's> { ); ui.add_space(10.0); - if gallery_status.selected != new_selected { + if gallery_status.selected.as_ref() != new_selected { if let Some(selected) = new_selected { // Set the model preview source to what is selected let model_entity = self.events.new_model.model_preview_camera.model_entity; @@ -258,7 +255,7 @@ impl<'a, 'w, 's> NewModel<'a, 'w, 's> { ..default() }; self.events.commands.entity(model_entity).insert(model); - gallery_status.selected = Some(selected); + gallery_status.selected = Some(selected.clone()); } } From 19a8c3b95f482024b923edf84fa25b4b555c0127 Mon Sep 17 00:00:00 2001 From: Luca Della Vedova Date: Fri, 14 Jul 2023 14:06:22 +0800 Subject: [PATCH 10/17] Remove writing to disk in wasm builds Signed-off-by: Luca Della Vedova --- rmf_site_editor/src/site/fuel_cache.rs | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/rmf_site_editor/src/site/fuel_cache.rs b/rmf_site_editor/src/site/fuel_cache.rs index e25fe622..b4994958 100644 --- a/rmf_site_editor/src/site/fuel_cache.rs +++ b/rmf_site_editor/src/site/fuel_cache.rs @@ -21,7 +21,7 @@ use crate::widgets::AssetGalleryStatus; use bevy::prelude::*; use bevy::tasks::IoTaskPool; use crossbeam_channel::{Receiver, Sender}; -use gz_fuel::FuelClient as GzFuelClient; +use gz_fuel::{FuelClient as GzFuelClient, FuelModel}; #[derive(Resource, Clone, Default, Deref, DerefMut)] pub struct FuelClient(GzFuelClient); @@ -30,7 +30,7 @@ pub struct FuelClient(GzFuelClient); pub struct UpdateFuelCache; #[derive(Deref, DerefMut)] -pub struct FuelCacheUpdated(Option); +pub struct FuelCacheUpdated(Option>); /// Event used to set the fuel API key from the UI. Will also trigger a reload for failed assets #[derive(Deref, DerefMut)] @@ -64,11 +64,13 @@ pub fn handle_update_fuel_cache_requests( let sender = channels.sender.clone(); IoTaskPool::get() .spawn(async move { + // Only write to cache in non wasm, no file system in web + #[cfg(target_arch = "wasm32")] + let write_to_disk = false; + #[cfg(not(target_arch = "wasm32"))] + let write_to_disk = true; // Send client if update was successful - let res = fuel_client - .update_cache() - .await - .and_then(|()| Some(fuel_client)); + let res = fuel_client.update_cache(write_to_disk).await; sender .send(FuelCacheUpdated(res)) .expect("Failed sending fuel cache update event"); @@ -84,7 +86,7 @@ pub fn read_update_fuel_cache_results( ) { if let Ok(result) = channels.receiver.try_recv() { match result.0 { - Some(client) => *fuel_client = client, + Some(models) => fuel_client.models = Some(models), None => error!("Failed updating fuel cache"), } gallery_status.fetching_cache = false; From 12672a919dc757b4601e41d9ed9f077ba35aeed7 Mon Sep 17 00:00:00 2001 From: Luca Della Vedova Date: Fri, 14 Jul 2023 14:08:54 +0800 Subject: [PATCH 11/17] Minor format, info message on API key setting Signed-off-by: Luca Della Vedova --- rmf_site_editor/src/site/fuel_cache.rs | 1 + rmf_site_editor/src/site/model.rs | 7 ++++++- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/rmf_site_editor/src/site/fuel_cache.rs b/rmf_site_editor/src/site/fuel_cache.rs index b4994958..b0ac753c 100644 --- a/rmf_site_editor/src/site/fuel_cache.rs +++ b/rmf_site_editor/src/site/fuel_cache.rs @@ -99,6 +99,7 @@ pub fn reload_failed_models_with_new_api_key( failed_models: Query, Without)>, ) { if let Some(key) = api_key_events.iter().last() { + info!("New API Key set, attempting to re-download failed models"); *FUEL_API_KEY.lock().unwrap() = Some((**key).clone()); for e in &failed_models { commands.entity(e).insert(TentativeModelFormat::default()); diff --git a/rmf_site_editor/src/site/model.rs b/rmf_site_editor/src/site/model.rs index 9ea07841..44f84aed 100644 --- a/rmf_site_editor/src/site/model.rs +++ b/rmf_site_editor/src/site/model.rs @@ -292,7 +292,12 @@ pub fn update_model_tentative_formats( "Format not supported".to_owned() } else { match source { - AssetSource::Search(_) | AssetSource::Remote(_) => format!("Model not found, try using an API key if it belongs to a private organization, or add its path to the {} environment variable", MODEL_ENVIRONMENT_VARIABLE), + AssetSource::Search(_) | AssetSource::Remote(_) => format!( + "Model not found, try using an API key if it belongs to \ + a private organization, or add its path to the {} \ + environment variable", + MODEL_ENVIRONMENT_VARIABLE + ), _ => "Failed parsing file".to_owned(), } }; From 13ca1807cbb862c6b203e50bf83acbf0b443d3bd Mon Sep 17 00:00:00 2001 From: Luca Della Vedova Date: Fri, 14 Jul 2023 14:24:29 +0800 Subject: [PATCH 12/17] Minor cleanups Signed-off-by: Luca Della Vedova --- .../src/interaction/model_preview.rs | 12 +------ rmf_site_editor/src/site/fuel_cache.rs | 2 +- rmf_site_editor/src/widgets/new_model.rs | 35 +++---------------- 3 files changed, 6 insertions(+), 43 deletions(-) diff --git a/rmf_site_editor/src/interaction/model_preview.rs b/rmf_site_editor/src/interaction/model_preview.rs index 56f45f0e..9b9d6eb7 100644 --- a/rmf_site_editor/src/interaction/model_preview.rs +++ b/rmf_site_editor/src/interaction/model_preview.rs @@ -32,15 +32,6 @@ pub struct ModelPreviewCamera { pub struct ModelPreviewPlugin; -// TODO(luca) implement this system to scale the view based on the model's Aabb -fn scale_preview_for_model_bounding_box( - aabbs: Query<&Aabb, Changed>, - parents: Query<&Parent>, - model_preview: Res, - camera_transforms: Query<&mut Transform, With>, -) { -} - impl FromWorld for ModelPreviewCamera { fn from_world(world: &mut World) -> Self { // camera @@ -94,7 +85,6 @@ impl FromWorld for ModelPreviewCamera { impl Plugin for ModelPreviewPlugin { fn build(&self, app: &mut App) { - app.init_resource::() - .add_system(scale_preview_for_model_bounding_box); + app.init_resource::(); } } diff --git a/rmf_site_editor/src/site/fuel_cache.rs b/rmf_site_editor/src/site/fuel_cache.rs index b0ac753c..e284ca0b 100644 --- a/rmf_site_editor/src/site/fuel_cache.rs +++ b/rmf_site_editor/src/site/fuel_cache.rs @@ -58,7 +58,7 @@ pub fn handle_update_fuel_cache_requests( channels: Res, ) { if events.iter().last().is_some() { - info!("Updating fuel cache"); + info!("Updating fuel cache, this might take a few minutes"); gallery_status.fetching_cache = true; let mut fuel_client = fuel_client.clone(); let sender = channels.sender.clone(); diff --git a/rmf_site_editor/src/widgets/new_model.rs b/rmf_site_editor/src/widgets/new_model.rs index 12d8392d..fbb50132 100644 --- a/rmf_site_editor/src/widgets/new_model.rs +++ b/rmf_site_editor/src/widgets/new_model.rs @@ -23,6 +23,7 @@ use bevy_egui::egui::{Button, ComboBox, RichText, ScrollArea, Ui, Window}; use gz_fuel::FuelModel; /// Filters applied to models in the fuel list +#[derive(Default)] pub struct ShowAssetFilters { pub owner: Option, pub recall_owner: Option, @@ -33,7 +34,7 @@ pub struct ShowAssetFilters { } /// Used to signals whether to show or hide the left side panel with the asset gallery -#[derive(Resource)] +#[derive(Resource, Default)] pub struct AssetGalleryStatus { pub show: bool, pub selected: Option, @@ -45,34 +46,6 @@ pub struct AssetGalleryStatus { pub show_api_window: bool, } -impl Default for ShowAssetFilters { - fn default() -> Self { - Self { - owner: Some("OpenRobotics".into()), - recall_owner: None, - tag: None, - recall_tag: None, - private: None, - recall_private: None, - } - } -} - -impl Default for AssetGalleryStatus { - fn default() -> Self { - Self { - show: true, - selected: None, - cached_owners: None, - cached_tags: None, - filters: Default::default(), - proposed_api_key: Default::default(), - fetching_cache: false, - show_api_window: false, - } - } -} - #[derive(SystemParam)] pub struct NewModelParams<'w, 's> { pub fuel_client: ResMut<'w, FuelClient>, @@ -202,7 +175,7 @@ impl<'a, 'w, 's> NewModel<'a, 'w, 's> { ui.add_space(10.0); - // TODO(luca) should we cache the models by owner result to avoid calling at every + // TODO(luca) should we cache the models by filters result to avoid calling at every // frame? let models = models .iter() @@ -299,7 +272,7 @@ impl<'a, 'w, 's> NewModel<'a, 'w, 's> { if ui.add(Button::new("Set API key")).clicked() { gallery_status.show_api_window = true; } - if gallery_status.fetching_cache == true { + if gallery_status.fetching_cache { ui.label("Updating model cache..."); } else { if ui.add(Button::new("Update model cache")).clicked() { From 62c7b5174bce628e628bc3647be8e4a2e4188b78 Mon Sep 17 00:00:00 2001 From: Luca Della Vedova Date: Fri, 18 Aug 2023 17:14:28 +0800 Subject: [PATCH 13/17] Add missing systems to make gallery work in workcell editor mode Signed-off-by: Luca Della Vedova --- rmf_site_editor/src/widgets/create.rs | 3 +++ rmf_site_editor/src/widgets/mod.rs | 9 +++++++++ rmf_site_editor/src/workcell/mod.rs | 14 ++++++++++++-- 3 files changed, 24 insertions(+), 2 deletions(-) diff --git a/rmf_site_editor/src/widgets/create.rs b/rmf_site_editor/src/widgets/create.rs index a0621f8b..d346fb01 100644 --- a/rmf_site_editor/src/widgets/create.rs +++ b/rmf_site_editor/src/widgets/create.rs @@ -212,6 +212,9 @@ impl<'a, 'w1, 'w2, 's1, 's2> CreateWidget<'a, 'w1, 'w2, 's1, 's2> { } } AppState::WorkcellEditor => { + if ui.button("Browse fuel").clicked() { + self.events.new_model.asset_gallery_status.show = true; + } if ui.button("Spawn visual").clicked() { let workcell_model = WorkcellModel { geometry: Geometry::Mesh { diff --git a/rmf_site_editor/src/widgets/mod.rs b/rmf_site_editor/src/widgets/mod.rs index 583227dd..9d6cab16 100644 --- a/rmf_site_editor/src/widgets/mod.rs +++ b/rmf_site_editor/src/widgets/mod.rs @@ -591,6 +591,15 @@ fn workcell_ui_layout( &mut events.visibility_parameters, ); + if events.new_model.asset_gallery_status.show { + egui::SidePanel::left("left_panel") + .resizable(true) + .exact_width(320.0) + .show(egui_context.ctx_mut(), |ui| { + NewModel::new(&mut events).show(ui); + }); + } + let egui_context = egui_context.ctx_mut(); let ui_has_focus = egui_context.wants_pointer_input() || egui_context.wants_keyboard_input() diff --git a/rmf_site_editor/src/workcell/mod.rs b/rmf_site_editor/src/workcell/mod.rs index 39a1428d..6a8d0a09 100644 --- a/rmf_site_editor/src/workcell/mod.rs +++ b/rmf_site_editor/src/workcell/mod.rs @@ -43,8 +43,11 @@ use crate::AppState; use crate::{ shapes::make_infinite_grid, site::{ - handle_new_mesh_primitives, make_models_selectable, update_anchor_transforms, - update_model_scenes, update_model_tentative_formats, update_transforms_for_changed_poses, + handle_model_loaded_events, handle_new_mesh_primitives, handle_new_sdf_roots, + handle_update_fuel_cache_requests, make_models_selectable, propagate_model_render_layers, + read_update_fuel_cache_results, reload_failed_models_with_new_api_key, + update_anchor_transforms, update_model_scales, update_model_scenes, + update_model_tentative_formats, update_transforms_for_changed_poses, }, }; @@ -99,12 +102,19 @@ impl Plugin for WorkcellEditorPlugin { SystemSet::on_update(AppState::WorkcellEditor) .with_system(add_wireframe_to_meshes) .with_system(update_constraint_dependents) + .with_system(handle_model_loaded_events) .with_system(update_model_scenes) + .with_system(update_model_scales) .with_system(update_model_tentative_formats) + .with_system(propagate_model_render_layers) .with_system(make_models_selectable) + .with_system(handle_update_fuel_cache_requests) + .with_system(read_update_fuel_cache_results) + .with_system(reload_failed_models_with_new_api_key) .with_system(handle_workcell_keyboard_input) .with_system(handle_new_mesh_primitives) .with_system(change_workcell.before(load_workcell)) + .with_system(handle_new_sdf_roots) .with_system(handle_new_urdf_roots), ) .add_system(load_workcell) From e52612cbfd221db16e1953d100dd4b7eef77f01d Mon Sep 17 00:00:00 2001 From: "Michael X. Grey" Date: Wed, 23 Aug 2023 12:01:07 +0800 Subject: [PATCH 14/17] Leave a note about cargo-check precommit hook Signed-off-by: Michael X. Grey --- .pre-commit-config.yaml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 082d42ac..929c58a4 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -19,3 +19,7 @@ repos: rev: master hooks: - id: fmt + # cargo-check is failing to recognize From<&str> for AssetSource, causing a + # false positive in its error detection. + # TODO(@mxgrey): Attempt to re-enable this at a later time. + # - id: cargo-check From bd22711863ef657de47537287a05cd4e066a8c72 Mon Sep 17 00:00:00 2001 From: "Michael X. Grey" Date: Wed, 23 Aug 2023 16:25:32 +0800 Subject: [PATCH 15/17] Bring back cargo-check precommit hook Signed-off-by: Michael X. Grey --- .pre-commit-config.yaml | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 929c58a4..d4e15bf4 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -19,7 +19,4 @@ repos: rev: master hooks: - id: fmt - # cargo-check is failing to recognize From<&str> for AssetSource, causing a - # false positive in its error detection. - # TODO(@mxgrey): Attempt to re-enable this at a later time. - # - id: cargo-check + - id: cargo-check From 91a8256eee8a8ff6ffffade21b518a20c7df31f1 Mon Sep 17 00:00:00 2001 From: Luca Della Vedova Date: Thu, 24 Aug 2023 09:09:52 +0800 Subject: [PATCH 16/17] Address feedback Signed-off-by: Luca Della Vedova --- rmf_site_editor/Cargo.toml | 2 +- rmf_site_editor/src/site/sdf.rs | 29 +++++++++++++++++++--------- rmf_site_editor/src/site_asset_io.rs | 14 ++++++++++++-- 3 files changed, 33 insertions(+), 12 deletions(-) diff --git a/rmf_site_editor/Cargo.toml b/rmf_site_editor/Cargo.toml index ecb7730c..c8ce8bc2 100644 --- a/rmf_site_editor/Cargo.toml +++ b/rmf_site_editor/Cargo.toml @@ -48,7 +48,7 @@ tracing-subscriber = "0.3.1" rfd = "0.11" urdf-rs = "0.7" sdformat_rs = { git = "https://github.com/open-rmf/sdf_rust_experimental", rev = "f86344f"} -gz-fuel = { git = "https://github.com/luca-della-vedova/gz-fuel/", branch = "first_implementation" } +gz-fuel = { git = "https://github.com/open-rmf/gz-fuel-rs", branch = "first_implementation" } pathdiff = "*" # only enable the 'dynamic' feature if we're not building for web or windows diff --git a/rmf_site_editor/src/site/sdf.rs b/rmf_site_editor/src/site/sdf.rs index 94a7ae7a..7293ee56 100644 --- a/rmf_site_editor/src/site/sdf.rs +++ b/rmf_site_editor/src/site/sdf.rs @@ -38,15 +38,22 @@ fn compute_model_source(path: &str, uri: &str) -> AssetSource { let binding = p.clone(); *p = if let Some(stripped) = uri.strip_prefix("model://") { // Get the org name from context, model name from this and combine - let org_name = binding.split("/").next().unwrap(); - org_name.to_owned() + "/" + stripped + if let Some(org_name) = binding.split("/").next() { + org_name.to_owned() + "/" + stripped + } else { + error!( + "Unable to extract organization name from asset source [{}]", + uri + ); + "".into() + } } else if let Some(path_idx) = binding.rfind("/") { // It's a path relative to this model, remove file and append uri let (model_path, _model_name) = binding.split_at(path_idx); model_path.to_owned() + "/" + uri } else { - println!( - "WARNING: Invalid model found path is {} and model uri is {}", + error!( + "Invalid SDF model path, Path is [{}] and model uri is [{}]", path, uri ); "".into() @@ -60,22 +67,26 @@ fn compute_model_source(path: &str, uri: &str) -> AssetSource { // models that are placed in different folders or are in fuel, but should work for // most local, self contained, models. // Get the org name from context, model name from this and combine - let model_folder = binding.rsplitn(3, "/").skip(2).next().unwrap(); - model_folder.to_owned() + "/" + stripped + if let Some(model_folder) = binding.rsplitn(3, "/").skip(2).next() { + model_folder.to_owned() + "/" + stripped + } else { + error!("Unable to extract model folder from asset source [{}]", uri); + "".into() + } } else if let Some(path_idx) = binding.rfind("/") { // It's a path relative to this model, remove file and append uri let (model_path, _model_name) = binding.split_at(path_idx); model_path.to_owned() + "/" + uri } else { - println!( - "WARNING: Invalid model found path is {} and model uri is {}", + error!( + "Invalid SDF model path, Path is [{}] and model uri is [{}]", path, uri ); "".into() }; } AssetSource::Bundled(_) | AssetSource::Package(_) => { - println!("WARNING: Requested asset source {:?} type not supported for SDFs, might behave unexpectedly", asset_source); + warn!("Requested asset source {:?} type not supported for SDFs, might behave unexpectedly", asset_source); } } asset_source diff --git a/rmf_site_editor/src/site_asset_io.rs b/rmf_site_editor/src/site_asset_io.rs index 17e07dca..3a8a9f64 100644 --- a/rmf_site_editor/src/site_asset_io.rs +++ b/rmf_site_editor/src/site_asset_io.rs @@ -65,8 +65,18 @@ impl SiteAssetIo { ) -> BoxedFuture<'a, Result, AssetIoError>> { Box::pin(async move { let mut req = surf::get(remote_url.clone()); - if let Some(key) = &(*FUEL_API_KEY.lock().unwrap()) { - req = req.header("Private-token", key.clone()); + match FUEL_API_KEY.lock() { + Ok(key) => { + if let Some(key) = key.clone() { + req = req.header("Private-token", key); + } + } + Err(_) => { + return Err(AssetIoError::Io(io::Error::new( + io::ErrorKind::Other, + format!("Lock poisoning detected when reading fuel API key, please set it again."), + ))); + } } let bytes = req.recv_bytes().await.map_err(|e| { AssetIoError::Io(io::Error::new(io::ErrorKind::Other, e.to_string())) From b13f1f076425932de0abec6b933e3bf29dfd614c Mon Sep 17 00:00:00 2001 From: "Michael X. Grey" Date: Thu, 24 Aug 2023 12:09:52 +0800 Subject: [PATCH 17/17] Automatically reset poisoned API key Signed-off-by: Michael X. Grey --- rmf_site_editor/src/site/fuel_cache.rs | 6 +++++- rmf_site_editor/src/site_asset_io.rs | 11 +++++++---- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/rmf_site_editor/src/site/fuel_cache.rs b/rmf_site_editor/src/site/fuel_cache.rs index e284ca0b..aa6eae6f 100644 --- a/rmf_site_editor/src/site/fuel_cache.rs +++ b/rmf_site_editor/src/site/fuel_cache.rs @@ -100,7 +100,11 @@ pub fn reload_failed_models_with_new_api_key( ) { if let Some(key) = api_key_events.iter().last() { info!("New API Key set, attempting to re-download failed models"); - *FUEL_API_KEY.lock().unwrap() = Some((**key).clone()); + let mut key_guard = match FUEL_API_KEY.lock() { + Ok(key) => key, + Err(poisoned) => poisoned.into_inner(), + }; + *key_guard = Some((**key).clone()); for e in &failed_models { commands.entity(e).insert(TentativeModelFormat::default()); } diff --git a/rmf_site_editor/src/site_asset_io.rs b/rmf_site_editor/src/site_asset_io.rs index 3a8a9f64..6a0212f6 100644 --- a/rmf_site_editor/src/site_asset_io.rs +++ b/rmf_site_editor/src/site_asset_io.rs @@ -71,7 +71,9 @@ impl SiteAssetIo { req = req.header("Private-token", key); } } - Err(_) => { + Err(poisoned_key) => { + // Reset the key to None + *poisoned_key.into_inner() = None; return Err(AssetIoError::Io(io::Error::new( io::ErrorKind::Other, format!("Lock poisoning detected when reading fuel API key, please set it again."), @@ -93,9 +95,10 @@ impl SiteAssetIo { ))); } Err(_) => { - // This is okay. When a GET from fuel was successful, it - // will not return a JSON that can be interpreted as a - // FuelErrorMsg + // This is actually the happy path. When a GET from fuel was + // successful, it will not return a JSON that can be + // interpreted as a FuelErrorMsg, so our attempt to parse an + // error message will fail. } }