Proper shutdown

animation/src/coordinator.rs

@@ -74,7 +74,7 @@ impl AnimationCoordinator {
     /// Upgrade the current cycle to an apply animations cycle.
     ///
     /// If the cycle has not been started yet, it's started now.
-    pub fn upgrade_to_apply_animations(&self) {
+    pub fn upgrade_to_apply_animations_cycle(&self) {
         let mut inner = self.inner.lock();
         // Be sure there is a current cycle.
         let cycle = inner.current_cycle();

applications/src/instance_context.rs

@@ -1,8 +1,9 @@
 //! The context for an instance.
 
-use std::mem;
+use std::{mem, sync::Arc};
 
 use anyhow::Result;
+use massive_scene::ChangeCollector;
 use tokio::sync::mpsc::UnboundedReceiver;
 use winit::event::DeviceId;
 
@@ -84,7 +85,8 @@ impl InstanceContext {
         let event = self.events.recv().await?;
 
         if matches!(event, InstanceEvent::ApplyAnimations) {
-            self.animation_coordinator.upgrade_to_apply_animations();
+            self.animation_coordinator
+                .upgrade_to_apply_animations_cycle();
         }
 
         Ok(event)
@@ -95,6 +97,7 @@ impl InstanceContext {
             self.environment.command_sender.clone(),
             self.id,
             extent.into().into(),
+            self.new_scene(),
         )
     }
 }
@@ -110,7 +113,9 @@ pub enum InstanceEvent {
 #[derive(Debug)]
 pub enum InstanceCommand {
     CreateView(ViewCreationInfo),
-    DestroyView(ViewId),
+    // Detail: We pass the change collector up to the desktop, so it can make all Handles are destroyed and
+    // pending changes are sent to the renderer.
+    DestroyView(ViewId, Arc<ChangeCollector>),
     View(ViewId, ViewCommand),
 }
 

applications/src/scene.rs

@@ -1,11 +1,12 @@
 //! A wrapper around a regular Scene that adds animation support.
-use std::time::Duration;
+use std::{sync::Arc, time::Duration};
 
 use anyhow::Result;
 use derive_more::Deref;
 
 use massive_animation::{Animated, AnimationCoordinator, Interpolatable, Interpolation, TimeScale};
 use massive_renderer::{RenderPacing, RenderSubmission, RenderTarget};
+use massive_scene::ChangeCollector;
 
 #[derive(Debug, Deref)]
 pub struct Scene {
@@ -77,4 +78,8 @@ impl Scene {
 
         RenderSubmission::new(self.take_changes(), pacing)
     }
+
+    pub fn into_collector(self) -> Arc<ChangeCollector> {
+        self.inner.into_collector()
+    }
 }

applications/src/view.rs

@@ -1,6 +1,9 @@
+use std::mem;
+
 use anyhow::{Context, Result};
 use derive_more::{From, Into};
 use log::debug;
+use massive_animation::AnimationCoordinator;
 use tokio::sync::mpsc::{UnboundedSender, error::SendError};
 
 use uuid::Uuid;
@@ -12,32 +15,41 @@ use massive_scene::{Handle, Location, Object, ToLocation, Transform};
 
 use crate::{InstanceId, Scene, ViewId, instance_context::InstanceCommand};
 
+/// ADR: Decided to let the View own the Scene, so that we do have a lifetime restriction on the
+/// Scene and can properly clean up and detect dangling handles in this scene in the Desktop.
 #[derive(Debug)]
 pub struct View {
+    command_sender: UnboundedSender<(InstanceId, InstanceCommand)>,
     instance: InstanceId,
+    scene: Scene,
     id: ViewId,
     location: Handle<Location>,
-    command_sender: UnboundedSender<(InstanceId, InstanceCommand)>,
 }
 
 impl Drop for View {
     fn drop(&mut self) {
-        if let Err(SendError { .. }) = self
-            .command_sender
-            .send((self.instance, InstanceCommand::DestroyView(self.id)))
-        {
+        // Detail: Quite an expensive hack, but we need to take out the scene and send it up to the
+        // desktop.
+        //
+        // Architecture: This also means that users can create default scenes.
+        let scene = mem::replace(&mut self.scene, Scene::new(AnimationCoordinator::new()));
+
+        if let Err(SendError { .. }) = self.command_sender.send((
+            self.instance,
+            InstanceCommand::DestroyView(self.id, scene.into_collector()),
+        )) {
             debug!("Ignored DestroyView command because the command receiver is gone")
         }
     }
 }
 
 impl View {
     pub(crate) fn new(
-        instance: InstanceId,
         command_sender: UnboundedSender<(InstanceId, InstanceCommand)>,
-        role: ViewRole,
+        instance: InstanceId,
         extents: BoxPx,
-        scene: &Scene,
+        scene: Scene,
+        role: ViewRole,
     ) -> Result<Self> {
         let id = ViewId(Uuid::new_v4());
 
@@ -51,18 +63,18 @@ impl View {
         //
         // Detail: The identity transform here is incorrect but will be adjusted by the desktop
         // based on extents.
-        let desktop_transform = Transform::IDENTITY.enter(scene);
-        let desktop_location = desktop_transform.to_location().enter(scene);
+        let desktop_transform = Transform::IDENTITY.enter(&scene);
+        let desktop_location = desktop_transform.to_location().enter(&scene);
 
         // The local transform is the basic center transform.
         //
         // Architecture: Do we need a local location anymore, if it does not make sense for the view
         // to modify it now that a full extents can be provided?
-        let local_transform = Transform::IDENTITY.enter(scene);
+        let local_transform = Transform::IDENTITY.enter(&scene);
         let location = local_transform
             .to_location()
             .relative_to(&desktop_location)
-            .enter(scene);
+            .enter(&scene);
 
         command_sender.send((
             instance,
@@ -75,13 +87,18 @@ impl View {
         ))?;
 
         Ok(Self {
+            command_sender,
             instance,
+            scene,
             id,
             location,
-            command_sender,
         })
     }
 
+    pub fn scene(&self) -> &Scene {
+        &self.scene
+    }
+
     /// The location's transform.
     ///
     /// This should not be modified
@@ -124,6 +141,16 @@ impl View {
             ))
             .context("Failed to send a set cursor request")
     }
+
+    pub fn render(&self) -> Result<()> {
+        let submission = self.scene.begin_frame();
+        self.command_sender
+            .send((
+                self.instance,
+                InstanceCommand::View(self.id, ViewCommand::Render { submission }),
+            ))
+            .context("Failed to send a render request")
+    }
 }
 
 #[derive(Debug, Copy, Clone, PartialEq, Eq, Default)]

applications/src/view_builder.rs

@@ -13,10 +13,11 @@ use crate::{
 pub struct ViewBuilder {
     command_sender: UnboundedSender<(InstanceId, InstanceCommand)>,
     instance: InstanceId,
+    extent: BoxPx,
+    scene: Scene,
 
     role: ViewRole,
 
-    extent: BoxPx,
     background_color: Option<Color>,
 }
 
@@ -25,12 +26,14 @@ impl ViewBuilder {
         requests: UnboundedSender<(InstanceId, InstanceCommand)>,
         instance: InstanceId,
         extent: BoxPx,
+        scene: Scene,
     ) -> Self {
         Self {
             command_sender: requests,
             instance,
-            role: ViewRole::default(),
             extent,
+            scene,
+            role: ViewRole::default(),
             background_color: None,
         }
     }
@@ -45,13 +48,13 @@ impl ViewBuilder {
         self
     }
 
-    pub fn build(self, scene: &Scene) -> Result<View> {
+    pub fn build(self) -> Result<View> {
         View::new(
-            self.instance,
             self.command_sender,
-            self.role,
+            self.instance,
             self.extent,
-            scene,
+            self.scene,
+            self.role,
         )
     }
 }

desktop/src/band_presenter.rs

@@ -2,16 +2,17 @@ use std::{collections::HashMap, time::Duration};
 
 use anyhow::{Result, bail};
 
+use log::{info, warn};
 use massive_applications::{InstanceId, ViewCreationInfo, ViewEvent, ViewId, ViewRole};
 use massive_geometry::{RectPx, SizePx};
 use massive_layout::{self as layout, LayoutAxis};
 use massive_scene::Transform;
 use massive_shell::Scene;
 
 use crate::{
+    instance_manager::ViewPath,
     instance_presenter::{InstancePresenter, InstancePresenterState, PrimaryViewPresenter},
-    navigation,
-    navigation::NavigationNode,
+    navigation::{self, NavigationNode},
 };
 
 #[derive(Debug, Default)]
@@ -93,7 +94,7 @@ impl BandPresenter {
             originating_from.and_then(|originating_from| self.instances.get(&originating_from));
 
         let presenter = InstancePresenter {
-            state: InstancePresenterState::Appearing,
+            state: InstancePresenterState::WaitingForPrimaryView,
             panel_size: originating_presenter
                 .map(|p| p.panel_size)
                 .unwrap_or(default_panel_size),
@@ -123,24 +124,21 @@ impl BandPresenter {
         Ok(())
     }
 
-    #[allow(unused)]
     pub fn hide_instance(&mut self, instance: InstanceId) -> Result<()> {
+        info!("Hiding instance: {instance:?}");
         let Some(presenter) = self.instances.get_mut(&instance) else {
             bail!("Instance not found");
         };
 
         match &presenter.state {
-            InstancePresenterState::Presenting { view } => {
-                let view = PrimaryViewPresenter {
-                    creation_info: view.creation_info.clone(),
-                };
-                presenter.state = InstancePresenterState::Disappearing { view };
+            InstancePresenterState::WaitingForPrimaryView => {
+                bail!("Cannot hide instance that is still appearing")
             }
-            InstancePresenterState::Disappearing { .. } => {
-                bail!("Instance is already disappearing")
+            InstancePresenterState::Presenting { .. } => {
+                presenter.state = InstancePresenterState::Disappearing;
             }
-            InstancePresenterState::Appearing => {
-                bail!("Cannot hide instance that is still appearing")
+            InstancePresenterState::Disappearing => {
+                bail!("Instance is already disappearing")
             }
         }
 
@@ -162,7 +160,10 @@ impl BandPresenter {
             bail!("Instance not found");
         };
 
-        if !matches!(instance_presenter.state, InstancePresenterState::Appearing) {
+        if !matches!(
+            instance_presenter.state,
+            InstancePresenterState::WaitingForPrimaryView
+        ) {
             bail!("Primary view is already presenting");
         }
 
@@ -177,8 +178,33 @@ impl BandPresenter {
         Ok(())
     }
 
-    pub fn hide_view(&mut self, _id: ViewId) -> Result<()> {
-        bail!("Hiding views is not supported yet");
+    pub fn hide_view(&mut self, path: ViewPath) -> Result<()> {
+        let Some(instance_presenter) = self.instances.get_mut(&path.instance) else {
+            warn!("Can't hide view: Instance for view not found");
+            // Robustness: Decide if this should return an error.
+            return Ok(());
+        };
+
+        match &instance_presenter.state {
+            InstancePresenterState::WaitingForPrimaryView => {
+                bail!(
+                    "A view needs to be hidden, but instance presenter waits for a view with a primary role."
+                )
+            }
+            InstancePresenterState::Presenting { view } => {
+                if view.creation_info.id == path.view {
+                    // Feature: this should initiate a disappearing animation?
+                    instance_presenter.state = InstancePresenterState::Disappearing;
+                    Ok(())
+                } else {
+                    bail!("Invalid view: It's not related to anything we present");
+                }
+            }
+            InstancePresenterState::Disappearing => {
+                // ignored, we are already disappearing.
+                Ok(())
+            }
+        }
     }
 
     pub fn layout(&self) -> layout::Layout<InstanceId, 2> {