From df380ca5ea02b64e343fd468f9dd17dc21f6cd9b Mon Sep 17 00:00:00 2001 From: Armin Sander Date: Mon, 2 Feb 2026 15:45:11 +0100 Subject: [PATCH 1/3] Remove comment --- desktop/src/desktop.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/desktop/src/desktop.rs b/desktop/src/desktop.rs index 4298672f..56a2e2a9 100644 --- a/desktop/src/desktop.rs +++ b/desktop/src/desktop.rs @@ -217,7 +217,6 @@ impl Desktop { let focused = self.interaction.focused(); let originating_instance = focused.instance(); - // Simplify: Use the currently focused instance for determining the originating one. let band_location = focused .band_location() .expect("Failed to start an instance without a focused instance target"); From 3d6e43486b5d7d9dabc38a24a93d704aa61ba6b2 Mon Sep 17 00:00:00 2001 From: Armin Sander Date: Tue, 3 Feb 2026 06:47:48 +0100 Subject: [PATCH 2/3] Make the DesktopTargets more granular, and derive the originating instance and also the focus from what's currently focused --- desktop/src/desktop.rs | 39 +++-- desktop/src/desktop_interaction.rs | 32 +--- desktop/src/desktop_presenter.rs | 225 ++++++++++++++++------------- 3 files changed, 142 insertions(+), 154 deletions(-) diff --git a/desktop/src/desktop.rs b/desktop/src/desktop.rs index 56a2e2a9..4e8f1fb3 100644 --- a/desktop/src/desktop.rs +++ b/desktop/src/desktop.rs @@ -14,7 +14,7 @@ use massive_renderer::RenderPacing; use massive_shell::{ApplicationContext, FontManager, Scene, ShellEvent}; use massive_shell::{AsyncWindowRenderer, ShellWindow}; -use crate::desktop_presenter::{BandLocation, DesktopFocusPath}; +use crate::desktop_presenter::{BandLocation, DesktopFocusPath, DesktopTarget}; use crate::projects::Project; use crate::{ DesktopEnvironment, DesktopInteraction, DesktopPresenter, UserIntent, @@ -106,11 +106,13 @@ impl Desktop { instance_manager.add_view(primary_instance, &creation_info); let ui = DesktopInteraction::new( - DesktopFocusPath::from_instance_and_view( - BandLocation::TopBand, - primary_instance, - primary_view, - ), + vec![ + DesktopTarget::Desktop, + DesktopTarget::TopBand, + DesktopTarget::Instance(primary_instance), + DesktopTarget::View(primary_view), + ] + .into(), &instance_manager, &mut presenter, &scene, @@ -217,21 +219,16 @@ impl Desktop { let focused = self.interaction.focused(); let originating_instance = focused.instance(); - let band_location = focused - .band_location() - .expect("Failed to start an instance without a focused instance target"); - - self.presenter.present_instance( - band_location, + let presented_instance_path = self.presenter.present_instance( + focused, instance, originating_instance, self.primary_instance_panel_size, &self.scene, )?; - self.interaction.make_foreground( - band_location, - instance, + self.interaction.focus( + presented_instance_path, &self.instance_manager, &mut self.presenter, )?; @@ -264,15 +261,13 @@ impl Desktop { let focused = self.interaction.focused(); // If this instance is currently focused and the new view is primary, make it // foreground so that the view is focused. - // - // Ergonomics: instance() and band_location() are usually used together. - if focused.instance() == Some(instance) - && let Some(band_location) = focused.band_location() + if matches!(focused.last(), Some(DesktopTarget::Instance(..))) + && focused.instance() == Some(instance) && info.role == ViewRole::Primary { - self.interaction.make_foreground( - band_location, - instance, + let view_focus = focused.clone().join(DesktopTarget::View(info.id)); + self.interaction.focus( + view_focus, &self.instance_manager, &mut self.presenter, )?; diff --git a/desktop/src/desktop_interaction.rs b/desktop/src/desktop_interaction.rs index cc1192e6..587d21a8 100644 --- a/desktop/src/desktop_interaction.rs +++ b/desktop/src/desktop_interaction.rs @@ -78,25 +78,6 @@ impl DesktopInteraction { self.camera.value() } - pub fn make_foreground( - &mut self, - band_location: BandLocation, - instance: InstanceId, - instance_manager: &InstanceManager, - presenter: &mut DesktopPresenter, - ) -> Result<()> { - // If the window is not focus, we just focus the instance. - let primary_view = instance_manager.get_view_by_role(instance, ViewRole::Primary)?; - let focus_path = - DesktopFocusPath::from_instance_and_view(band_location, instance, primary_view); - assert_eq!( - self.focus(focus_path, instance_manager, presenter)?, - UserIntent::None, - "Unexpected UserIntent in response to make_foreground" - ); - Ok(()) - } - pub fn focus( &mut self, focus_path: DesktopFocusPath, @@ -138,18 +119,7 @@ impl DesktopInteraction { let hit_test = NavigationHitTester::new(navigation, render_geometry); self.event_router.process(event, &hit_test)? }; - let intent = - presenter.forward_event_transitions(transitions.transitions, instance_manager)?; - - // Robustness: Currently we don't check if the only the instance actually changed. - if let Some(new_focus) = transitions.focus_changed - && let Some(instance) = new_focus.instance() - && let Some(band_location) = new_focus.band_location() - { - self.make_foreground(band_location, instance, instance_manager, presenter)?; - }; - - Ok(intent) + presenter.forward_event_transitions(transitions.transitions, instance_manager) } fn preprocess_keyboard_commands(&self, event: &Event) -> Result { diff --git a/desktop/src/desktop_presenter.rs b/desktop/src/desktop_presenter.rs index 3ba408db..ef786037 100644 --- a/desktop/src/desktop_presenter.rs +++ b/desktop/src/desktop_presenter.rs @@ -1,8 +1,9 @@ use std::time::Duration; -use anyhow::{Result, bail}; +use anyhow::{Result, anyhow, bail}; use derive_more::From; +use log::error; use massive_applications::{InstanceId, ViewCreationInfo, ViewId}; use massive_geometry::{PixelCamera, PointPx, Rect, SizePx}; use massive_layout as layout; @@ -12,7 +13,7 @@ use massive_scene::{Object, ToCamera, ToLocation, Transform}; use massive_shell::Scene; use crate::box_to_rect; -use crate::projects::LaunchProfileId; +use crate::projects::{GroupId, LaunchProfileId}; use crate::{ EventTransition, UserIntent, band_presenter::{BandPresenter, BandTarget}, @@ -40,9 +41,31 @@ pub enum DesktopTarget { // The whole area, covering the top band and Desktop, TopBand, - Band(BandTarget), - // The project itself is a group inside the project (not sure yet if this is a good thing) - Project(ProjectTarget), + + ProjectGroup(GroupId), + ProjectLauncher(LaunchProfileId), + + Instance(InstanceId), + View(ViewId), +} + +impl From for DesktopTarget { + fn from(value: BandTarget) -> Self { + match value { + BandTarget::Instance(instance_id) => Self::Instance(instance_id), + BandTarget::View(view_id) => Self::View(view_id), + } + } +} + +impl From for DesktopTarget { + fn from(value: ProjectTarget) -> Self { + match value { + ProjectTarget::Group(group_id) => Self::ProjectGroup(group_id), + ProjectTarget::Launcher(launch_profile_id) => Self::ProjectLauncher(launch_profile_id), + ProjectTarget::Band(_, band_target) => band_target.into(), + } + } } pub type DesktopFocusPath = FocusPath; @@ -94,27 +117,36 @@ impl DesktopPresenter { pub fn present_instance( &mut self, - target: BandLocation, - instance: InstanceId, + focused: &DesktopFocusPath, + new_instance: InstanceId, originating_from: Option, default_panel_size: SizePx, scene: &Scene, - ) -> Result<()> { - match target { - BandLocation::TopBand => self.top_band.present_instance( - instance, + ) -> Result { + let instance_parent = focused.instance_parent().ok_or(anyhow!( + "Failed to present instance when no parent is focused that can take on a new one" + ))?; + + match instance_parent.last().unwrap() { + DesktopTarget::TopBand => self.top_band.present_instance( + new_instance, originating_from, default_panel_size, scene, - ), - BandLocation::LaunchProfile(launch_profile_id) => self.project.present_instance( - launch_profile_id, - instance, + )?, + DesktopTarget::ProjectLauncher(launch_profile_id) => self.project.present_instance( + *launch_profile_id, + new_instance, originating_from, default_panel_size, scene, - ), + )?, + invalid => { + bail!("Invalid instance parent: {invalid:?}"); + } } + + Ok(instance_parent.join(DesktopTarget::Instance(new_instance))) } pub fn present_view( @@ -196,10 +228,10 @@ impl DesktopPresenter { // Band navigation instances. self.top_band .navigation() - .map_target(DesktopTarget::Band) + .map_target(|bt| bt.into()) .with_target(DesktopTarget::TopBand), // Project navigation. - self.project.navigation().map_target(DesktopTarget::Project), + self.project.navigation().map_target(|pt| pt.into()), ] .into() }) @@ -208,25 +240,50 @@ impl DesktopPresenter { // Camera pub fn camera_for_focus(&self, focus: &DesktopFocusPath) -> Option { - Some(match focus.last()? { + match focus.last()? { // Desktop and TopBand are constrained to their size. - DesktopTarget::Desktop => self.rect.center().to_camera().with_size(self.rect.size()), - DesktopTarget::TopBand => self - .top_band_rect - .center() - .to_camera() - .with_size(self.top_band_rect.size()), - DesktopTarget::Band(BandTarget::Instance(instance_id)) => { + DesktopTarget::Desktop => { + Some(self.rect.center().to_camera().with_size(self.rect.size())) + } + DesktopTarget::TopBand => Some( + self.top_band_rect + .center() + .to_camera() + .with_size(self.top_band_rect.size()), + ), + + DesktopTarget::Instance(instance_id) => { // Architecture: The Band should be responsible for resolving at least the rects, if // not the camera? - self.top_band.instance_transform(*instance_id)?.to_camera() + match focus[focus.len() - 2] { + DesktopTarget::TopBand => { + Some(self.top_band.instance_transform(*instance_id)?.to_camera()) + } + DesktopTarget::ProjectLauncher(_) => self.camera_for_focus(&focus.parent()?), + _ => { + error!("Unexpected parent of instance"); + None + } + } } - DesktopTarget::Band(BandTarget::View(..)) => { - // Forward this to the parent (which is a BandTarget::Instance). - self.camera_for_focus(&focus.parent()?)? + DesktopTarget::View(_) => { + // Forward this to the parent (which mut be a ::Instance). + self.camera_for_focus(&focus.parent()?) } - DesktopTarget::Project(id) => self.project.rect_of(id.clone()).center().to_camera(), - }) + + DesktopTarget::ProjectGroup(group) => Some( + self.project + .rect_of(ProjectTarget::Group(*group)) + .center() + .to_camera(), + ), + DesktopTarget::ProjectLauncher(launcher) => Some( + self.project + .rect_of(ProjectTarget::Launcher(*launcher)) + .center() + .to_camera(), + ), + } } // Event forwarding Architecture: This also seems to be wrong here. Used from the @@ -269,19 +326,30 @@ impl DesktopPresenter { DesktopTarget::TopBand => { band_presenter.process(view_event)?; } - DesktopTarget::Band(BandTarget::Instance(..)) => { + DesktopTarget::Instance(..) => { // Shouldn't we forward this to the band here? } - DesktopTarget::Band(BandTarget::View(view_id)) => { - let Some(instance) = instance_manager.instance_of_view(*view_id) else { + DesktopTarget::View(view_id) => { + let Some(instance) = focus_path.instance() else { bail!("Internal error: Instance of view {view_id:?} not found"); }; instance_manager.send_view_event((instance, *view_id), view_event)?; } - DesktopTarget::Project(project_id) => { + + DesktopTarget::ProjectGroup(group_id) => { // Forward to project presenter - let project_transition = - EventTransition::Directed(vec![project_id.clone()].into(), view_event); + let project_transition = EventTransition::Directed( + vec![ProjectTarget::Group(*group_id)].into(), + view_event, + ); + user_intent = project_presenter.process_transition(project_transition)?; + } + DesktopTarget::ProjectLauncher(launcher_id) => { + // Forward to project presenter + let project_transition = EventTransition::Directed( + vec![ProjectTarget::Launcher(*launcher_id)].into(), + view_event, + ); user_intent = project_presenter.process_transition(project_transition)?; } } @@ -304,74 +372,29 @@ impl DesktopPresenter { // Path utilities impl DesktopFocusPath { - /// Focus the primary view. Currently only on the TopBand. - pub fn from_instance_and_view( - band_location: BandLocation, - instance: InstanceId, - view: impl Into>, - ) -> Self { - match band_location { - BandLocation::TopBand => { - // Ergonomics: what about supporting .join directly on a target? - let instance = Self::new(DesktopTarget::Desktop) - .join(DesktopTarget::TopBand) - .join(DesktopTarget::Band(BandTarget::Instance(instance))); - if let Some(view) = view.into() { - instance.join(DesktopTarget::Band(BandTarget::View(view))) - } else { - instance - } - } - BandLocation::LaunchProfile(launch_profile_id) => { - let instance = Self::new(DesktopTarget::Desktop) - // TODO: Add all the groups that lead to the launcher. - .join(DesktopTarget::Project(ProjectTarget::Launcher( - launch_profile_id, - ))) - .join(DesktopTarget::Project(ProjectTarget::Band( - launch_profile_id, - BandTarget::Instance(instance), - ))); - if let Some(view) = view.into() { - instance.join(DesktopTarget::Band(BandTarget::View(view))) - } else { - instance - } - } - } - } - pub fn instance(&self) -> Option { self.iter().rev().find_map(|t| match t { - // Architecture: We really need a new way to organize paths, this is horrible. - // Perhaps just flatten, but then how to map the ids from BandTargets up? - DesktopTarget::Band(BandTarget::Instance(id)) - | DesktopTarget::Project(ProjectTarget::Band(_, BandTarget::Instance(id))) => Some(*id), - + DesktopTarget::Instance(id) => Some(*id), _ => None, }) } - /// A target that can take on more instances. This defines the locations where new instances can be created. - pub fn band_location(&self) -> Option { - self.iter().rev().find_map(|t| match t { - DesktopTarget::Desktop => { - // This could be useful for spawning a instance in the top band. - None - } - DesktopTarget::TopBand | DesktopTarget::Band(..) => Some(BandLocation::TopBand), - DesktopTarget::Project(ProjectTarget::Launcher(launcher_id)) => { - Some(BandLocation::LaunchProfile(*launcher_id)) - } - DesktopTarget::Project(ProjectTarget::Group(_)) => { - // Idea: Spawn for each member of the group? - None - } - - DesktopTarget::Project(ProjectTarget::Band(..)) => { - // Covered by ProjectTarget::Launcher already. - None - } - }) + /// Is this or a parent something that can be added new instances to? + pub fn instance_parent(&self) -> Option { + self.iter() + .enumerate() + .rev() + .find_map(|(i, t)| match t { + DesktopTarget::Desktop => None, + DesktopTarget::TopBand => Some(i + 1), + DesktopTarget::ProjectGroup(..) => None, + DesktopTarget::ProjectLauncher(..) => Some(i + 1), + DesktopTarget::Instance(..) => Some(i), + DesktopTarget::View(..) => { + assert!(matches!(self[i - 1], DesktopTarget::Instance(..))); + Some(i - 1) + } + }) + .map(|i| self.iter().cloned().take(i).collect::>().into()) } } From 9efc130a2d62082b5d015fb0e288af4135e3a626 Mon Sep 17 00:00:00 2001 From: Armin Date: Tue, 3 Feb 2026 06:50:11 +0100 Subject: [PATCH 3/3] Update desktop/src/desktop_presenter.rs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- desktop/src/desktop_presenter.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/desktop/src/desktop_presenter.rs b/desktop/src/desktop_presenter.rs index ef786037..2c87c054 100644 --- a/desktop/src/desktop_presenter.rs +++ b/desktop/src/desktop_presenter.rs @@ -267,7 +267,7 @@ impl DesktopPresenter { } } DesktopTarget::View(_) => { - // Forward this to the parent (which mut be a ::Instance). + // Forward this to the parent (which must be a ::Instance). self.camera_for_focus(&focus.parent()?) }