diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index b4b5ce82..8346add9 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -3,6 +3,31 @@ This file serves as the evolving knowledge base for working with this codebase. Update it whenever you learn something new about the project's patterns, conventions, or receive feedback that should guide future behavior. +## Project Orientation +- This workspace is a Cargo workspace rooted at [Cargo.toml](../Cargo.toml) with core crates under top-level folders like [scene](../scene), [renderer](../renderer), [desktop](../desktop), and [shell](../shell). +- The source-of-truth architecture overview for scene/object lifetime is in [scene/src/lib.rs](../scene/src/lib.rs). Read this before changing scene handle or change-propagation behavior. +- Use [README.md](../README.md) for example entry points and expected demo behavior instead of inferring from code paths. + +## Workspace Boundaries +- Treat [examples/code/rust-analyzer](../examples/code/rust-analyzer) and [examples/markdown/inlyne](../examples/markdown/inlyne) as imported upstream projects; do not modify them unless explicitly asked. +- Prefer changes in first-party crates listed in [Cargo.toml](../Cargo.toml) workspace members. +- Keep changes scoped to the relevant crate. Avoid cross-crate refactors unless the task explicitly requires them. + +## Build, Run, and Validation +- Prefer `cargo build` for broad compile validation. +- Demo runs from [README.md](../README.md): + - `cargo run --release --example code` + - `cargo run --release --example markdown` +- WASM example workflows live in [justfile](../justfile), including `trunk serve --example markdown --port 8888 --open` and release build targets. +- If a task is scoped to one crate, prefer crate-targeted validation before workspace-wide commands. + +## Architecture Anchors +- Scene graph and handle model: [scene/src/lib.rs](../scene/src/lib.rs), [scene/src/handle.rs](../scene/src/handle.rs), [scene/src/change.rs](../scene/src/change.rs) +- Fluent scene ergonomics: [scene/src/ergonomics.rs](../scene/src/ergonomics.rs) +- Desktop orchestration and event routing: [desktop/src/lib.rs](../desktop/src/lib.rs) +- Platform split (native vs wasm): [shell/Cargo.toml](../shell/Cargo.toml), [animation/src/lib.rs](../animation/src/lib.rs) +- Prefer linking to these files in explanations instead of duplicating architectural prose. + ## Code Style - Prefer small, self-contained changes unless explicitly asked for broader refactors. - Match the surrounding code style. diff --git a/animation/src/coordinator.rs b/animation/src/coordinator.rs index 01097f81..22e0d9c7 100644 --- a/animation/src/coordinator.rs +++ b/animation/src/coordinator.rs @@ -81,6 +81,11 @@ impl AnimationCoordinator { self.inner.lock().current_cycle().mode = CycleMode::ApplyAnimations; } + /// `true` if there are active animations right now. + pub fn animations_active(&self) -> bool { + self.inner.lock().animating + } + /// Ends an update cycle. Returns true if animations are active. This resets the current time. pub fn end_cycle(&self) -> bool { let mut inner = self.inner.lock(); diff --git a/applications/src/instance_context.rs b/applications/src/instance_context.rs index 700dfc6d..1b52714b 100644 --- a/applications/src/instance_context.rs +++ b/applications/src/instance_context.rs @@ -1,19 +1,21 @@ //! The context for an instance. -use std::{mem, sync::Arc}; +use std::mem; +use std::sync::Arc; use anyhow::Result; -use massive_scene::ChangeCollector; +use derive_more::Deref; use tokio::sync::mpsc::UnboundedReceiver; use massive_animation::AnimationCoordinator; -use massive_renderer::FontManager; +use massive_renderer::{FontManager, RenderPacing}; +use massive_scene::{HandleChangeReceiver, Location, Ref, SceneChange}; use massive_util::{CoalescingKey, CoalescingReceiver}; +use crate::view_builder::ViewBuilder; use crate::{ - InstanceEnvironment, InstanceId, InstanceParameters, Scene, ViewEvent, ViewExtent, ViewId, - view::{ViewCommand, ViewCreationInfo}, - view_builder::ViewBuilder, + InstanceChange, InstanceEnvironment, InstanceId, InstanceParameters, InstanceSubmission, Scene, + ViewEvent, ViewExtent, ViewId, }; #[derive(Debug, Clone, PartialEq, Eq)] @@ -22,14 +24,31 @@ pub enum CreationMode { Restore, } +// Need a newtype here for the orphan rule. +#[derive(Debug, Default, Deref)] +pub struct InstanceChangeCollector(massive_util::ChangeCollector); + +impl HandleChangeReceiver for InstanceChangeCollector { + fn send(&self, change: SceneChange) { + self.0.collect(InstanceChange::Scene(change)) + } +} + #[derive(Debug)] pub struct InstanceContext { id: InstanceId, creation_mode: CreationMode, environment: InstanceEnvironment, + view_parent: Ref, - /// The `AnimationCoordinator` is here to create new scenes. There is one per instance for now. + /// We currently use one Scene per Context, so that everything is ordered properly. This also + /// contains the AnimationCoordinator, which we need one only per instance anyway. animation_coordinator: AnimationCoordinator, + + /// The current changes of this instance. This includes all Scene changes interleaved with the + /// instance changes (in order). + changes: Arc, + events: CoalescingReceiver, } @@ -38,17 +57,28 @@ impl InstanceContext { id: InstanceId, creation_mode: CreationMode, environment: InstanceEnvironment, + view_parent: Ref, events: UnboundedReceiver, ) -> Self { // ADR: Every instance gets its own animation coordinator and its timestamp is reset as soon // the scene is rendered. This way, consistence can be preserved when animations are applied // in several instances in parallel. Otherwise, timestamps from one instance could affect the // other. + let animation_coordinator = AnimationCoordinator::new(); + + // ADR: Every instance gets its own change collector, because of ordering constraints + // between the commands sent to the desktop and the scene updates (they must be processed in + // order by the desktop, otherwise it could happen that Visual refer to Locations / + // Transforms that are not available anymore). + let changes = InstanceChangeCollector::default(); + Self { id, creation_mode, environment, - animation_coordinator: AnimationCoordinator::new(), + view_parent, + animation_coordinator, + changes: changes.into(), events: events.into(), } } @@ -76,8 +106,12 @@ impl InstanceContext { &self.environment.font_manager } + /// ADR: We share _one_ single scene in all views now, so that we can keep the updates that we + /// send to desktop coordinated. Also, changes can't be submitted independently, all updates + /// from all views need to be submitted at once. pub fn new_scene(&self) -> Scene { - Scene::new(self.animation_coordinator.clone()) + let scene = massive_scene::Scene::new(self.changes.clone()); + Scene::from_parts(scene, self.animation_coordinator.clone()) } pub async fn wait_for_event(&mut self) -> Result { @@ -93,12 +127,35 @@ impl InstanceContext { pub fn view(&self, extent: impl Into) -> ViewBuilder { ViewBuilder::new( - self.environment.command_sender.clone(), - self.id, + self.changes.clone(), + self.view_parent.clone(), extent.into().into(), self.new_scene(), ) } + + pub fn submit(&mut self) -> Result<()> { + // Robustness: To be really thread safe, we would need to collect the changes and end the + // cycle in one go. + let animations_active = self.animation_coordinator.end_cycle(); + + // Empty changes need to end in a submission (we might have done some before, without ending + // the animation cycle) + + let pacing = if animations_active { + RenderPacing::Smooth + } else { + RenderPacing::Fast + }; + + let changes = self.changes.take_all(); + + let submission = InstanceSubmission::new(changes, pacing); + Ok(self + .environment + .submission_sender + .send((self.id, submission))?) + } } #[derive(Debug, Clone)] @@ -109,16 +166,6 @@ pub enum InstanceEvent { ApplyAnimations, } -/// Commands emitted by an instance and handled by the desktop layer. -#[derive(Debug)] -pub enum InstanceCommand { - CreateView(ViewCreationInfo), - // 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), - View(ViewId, ViewCommand), -} - impl CoalescingKey for InstanceEvent { type Key = InstanceEventCoalescingKey; diff --git a/applications/src/instance_environment.rs b/applications/src/instance_environment.rs index 9adce5d3..5189beaa 100644 --- a/applications/src/instance_environment.rs +++ b/applications/src/instance_environment.rs @@ -1,12 +1,16 @@ -use massive_renderer::FontManager; +use derive_more::Constructor; use serde_json::{Map, Value}; use tokio::sync::mpsc::UnboundedSender; -use crate::{InstanceCommand, InstanceId}; +use massive_renderer::{FontManager, RenderPacing}; +use massive_scene::SceneChange; +use massive_util::ChangeSet; + +use crate::{InstanceId, ViewChange, ViewCreationInfo, ViewId}; #[derive(Debug, Clone)] pub struct InstanceEnvironment { - pub(crate) command_sender: UnboundedSender<(InstanceId, InstanceCommand)>, + pub(crate) submission_sender: UnboundedSender<(InstanceId, InstanceSubmission)>, // Robustness: This might change on runtime. pub(crate) primary_monitor_scale_factor: f64, pub(crate) font_manager: FontManager, @@ -17,12 +21,12 @@ pub type InstanceParameters = Map; impl InstanceEnvironment { pub fn new( - requests_tx: UnboundedSender<(InstanceId, InstanceCommand)>, + requests_tx: UnboundedSender<(InstanceId, InstanceSubmission)>, primary_monitor_scale_factor: f64, font_manager: FontManager, ) -> Self { Self { - command_sender: requests_tx, + submission_sender: requests_tx, primary_monitor_scale_factor, font_manager, parameters: Default::default(), @@ -34,3 +38,23 @@ impl InstanceEnvironment { self } } + +#[derive(Debug, Constructor)] +pub struct InstanceSubmission { + changes: ChangeSet, + pacing: RenderPacing, +} + +impl InstanceSubmission { + pub fn into_parts(self) -> (ChangeSet, RenderPacing) { + (self.changes, self.pacing) + } +} + +#[derive(Debug)] +pub enum InstanceChange { + Scene(SceneChange), + CreateView(ViewCreationInfo), + View(ViewId, ViewChange), + DestroyView(ViewId), +} diff --git a/applications/src/scene.rs b/applications/src/scene.rs index 4bf53399..b8a05232 100644 --- a/applications/src/scene.rs +++ b/applications/src/scene.rs @@ -17,8 +17,30 @@ pub struct Scene { impl Scene { pub fn new(animation_coordinator: AnimationCoordinator) -> Self { + Self::new_with_change_collector( + animation_coordinator, + Arc::new(ChangeCollector::default()), + ) + } + + pub fn new_with_change_collector( + animation_coordinator: AnimationCoordinator, + collector: Arc, + ) -> Self { + // Robustness: We shouldn't allow arbitrary free generation of scenes anymore. + let scene = massive_scene::Scene::new(collector); + Self { + inner: scene, + animation_coordinator, + } + } + + pub(crate) fn from_parts( + scene: massive_scene::Scene, + animation_coordinator: AnimationCoordinator, + ) -> Self { Self { - inner: Default::default(), + inner: scene, animation_coordinator, } } @@ -53,10 +75,10 @@ impl Scene { self.animation_coordinator.time_scale() } - /// Accumulate external changes into this scene. - pub fn accumulate_changes(&self, changes: massive_scene::SceneChanges) { - self.inner.push_changes(changes); - } + // Accumulate external changes into this scene. + // pub fn accumulate_changes(&self, changes: massive_scene::SceneChangeSet) { + // self.inner.push_changes(changes); + // } // Render all the current scene changes. // @@ -79,7 +101,7 @@ impl Scene { RenderSubmission::new(self.take_changes(), pacing) } - pub fn into_collector(self) -> Arc { - self.inner.into_collector() - } + // pub fn into_collector(self) -> Arc { + // self.inner.into_collector() + // } } diff --git a/applications/src/view.rs b/applications/src/view.rs index d16a54f5..c0534969 100644 --- a/applications/src/view.rs +++ b/applications/src/view.rs @@ -1,115 +1,63 @@ -use std::mem; +use std::sync::Arc; -use anyhow::{Context, Result}; +use anyhow::Result; use derive_more::{From, Into}; -use log::debug; -use massive_animation::AnimationCoordinator; -use tokio::sync::mpsc::UnboundedSender; -use tokio::sync::mpsc::error::SendError; use uuid::Uuid; use winit::window::CursorIcon; -use massive_geometry::{BoxPx, SizePx}; -use massive_renderer::{RenderSubmission, RenderTarget}; -use massive_scene::{Handle, Location, Object, ToLocation, Transform}; +use massive_geometry::{BoxPx, SizePx, Vector3}; +use massive_scene::{Handle, Location, Object, Ref, ToLocation, Transform}; -use crate::instance_context::InstanceCommand; -use crate::{InstanceId, Scene, ViewId}; +use crate::{InstanceChange, InstanceChangeCollector, Scene, ViewId}; /// 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, + change_collector: Arc, } impl Drop for View { fn drop(&mut self) { - // 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") - } + self.change_collector + .collect(InstanceChange::DestroyView(self.id)); } } impl View { pub(crate) fn new( - command_sender: UnboundedSender<(InstanceId, InstanceCommand)>, - instance: InstanceId, + parent: Ref, extents: BoxPx, scene: Scene, role: ViewRole, + change_collector: Arc, ) -> Result { let id = ViewId(Uuid::new_v4()); - // The parent transform and location to send to the desktop so that it can freely position - // this view. - // - // This is to separate the positioning between this view and the desktop. - // - // Detail: This could be done also in the desktop, but for now we want to keep the local - // location here, so that the desktop can't modify it. - // - // 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); - - // 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 size: SizePx = extents.size().cast(); + let center_x = (size.width / 2) as f64; + let center_y = (size.height / 2) as f64; + let local_transform = + Transform::from_translation(Vector3::new(-center_x, -center_y, 0.0)).enter(&scene); let location = local_transform .to_location() - .relative_to(&desktop_location) + .relative_to(parent) .enter(&scene); - // Bug: - // Architecture: `CreateView` can immediately trigger desktop-side visuals that - // use `desktop_location`. If renderer ingestion of the create changes lags behind this - // command stream, the reference becomes visible before its definition. - // - // Failure mode details: - // - this method creates `desktop_location` and `local_transform` handles in the view scene - // - `CreateView` exposes `desktop_location` to desktop presentation immediately - // - view scene create changes are only uploaded when a later `Render` submission is sent - // - if desktop emits visuals that reference this location before renderer has ingested - // those creates, renderer-side location resolution can index a missing id and panic - // - // Architecture issue: metadata/identity and scene-graph definitions cross subsystem - // boundaries in separate asynchronous messages without an atomic activation step. - // - // Likely long-term fixes: - // - bundle required initial scene creates with `CreateView` - command_sender.send(( - instance, - InstanceCommand::CreateView(ViewCreationInfo { - id, - location: desktop_location.clone(), - role, - extents, - }), - ))?; + change_collector.collect(InstanceChange::CreateView(ViewCreationInfo { + id, + role, + extents, + })); Ok(Self { - command_sender, - instance, scene, id, location, + change_collector, }) } @@ -121,7 +69,7 @@ impl View { /// /// This should not be modified // Architecture: Introduce a kind of Immutable handle or read only Handle. - pub fn transform(&self) -> Handle { + pub fn transform(&self) -> Ref { self.location().value().transform.clone() } @@ -133,42 +81,34 @@ impl View { } #[allow(unused)] - fn resize(&mut self, new_extents: impl Into) -> Result<()> { - self.command_sender - .send(( - self.instance, - InstanceCommand::View(self.id, ViewCommand::Resize(new_extents.into().into())), - )) - .context("Failed to send a resize request") + fn resize(&mut self, new_extents: impl Into) { + self.change_collector.collect(InstanceChange::View( + self.id, + ViewChange::Resize(new_extents.into().into()), + )) } - pub fn set_title(&self, title: impl Into) -> Result<()> { - self.command_sender - .send(( - self.instance, - InstanceCommand::View(self.id, ViewCommand::SetTitle(title.into())), - )) - .context("Failed to send a set title request") + pub fn set_title(&self, title: impl Into) { + self.change_collector.collect(InstanceChange::View( + self.id, + ViewChange::SetTitle(title.into()), + )) } - pub fn set_cursor(&self, icon: CursorIcon) -> Result<()> { - self.command_sender - .send(( - self.instance, - InstanceCommand::View(self.id, ViewCommand::SetCursor(icon)), - )) - .context("Failed to send a set cursor request") + pub fn set_cursor(&self, icon: CursorIcon) { + self.change_collector + .collect(InstanceChange::View(self.id, ViewChange::SetCursor(icon))) } - 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") - } + // 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)] @@ -185,7 +125,6 @@ pub enum ViewRole { #[derive(Debug, Clone)] pub struct ViewCreationInfo { pub id: ViewId, - pub location: Handle, pub role: ViewRole, pub extents: BoxPx, } @@ -197,9 +136,7 @@ impl ViewCreationInfo { } #[derive(Debug)] -pub enum ViewCommand { - /// Detail: Empty changes are possible because animations active might change. - Render { submission: RenderSubmission }, +pub enum ViewChange { /// Feature: This should probably specify a depth too. Resize(BoxPx), /// Set the title of the view. The desktop decides how to display it. @@ -208,16 +145,16 @@ pub enum ViewCommand { SetCursor(CursorIcon), } -impl RenderTarget for View { - fn render(&mut self, submission: RenderSubmission) -> Result<()> { - self.command_sender - .send(( - self.instance, - InstanceCommand::View(self.id, ViewCommand::Render { submission }), - )) - .context("Failed to send a render request") - } -} +// impl RenderTarget for View { +// fn render(&mut self, submission: RenderSubmission) -> Result<()> { +// self.command_sender +// .send(( +// self.instance, +// InstanceCommand::View(self.id, ViewCommand::Render { submission }), +// )) +// .context("Failed to send a render request") +// } +// } #[derive(Debug, From, Into)] pub struct ViewExtent(BoxPx); diff --git a/applications/src/view_builder.rs b/applications/src/view_builder.rs index 05091d5c..066e88e1 100644 --- a/applications/src/view_builder.rs +++ b/applications/src/view_builder.rs @@ -1,18 +1,20 @@ +use std::sync::Arc; + use anyhow::Result; -use tokio::sync::mpsc::UnboundedSender; use massive_geometry::{BoxPx, Color}; +use massive_scene::{Location, Ref}; use crate::{ - InstanceId, Scene, - instance_context::InstanceCommand, + InstanceChangeCollector, Scene, view::{View, ViewRole}, }; #[derive(Debug)] pub struct ViewBuilder { - command_sender: UnboundedSender<(InstanceId, InstanceCommand)>, - instance: InstanceId, + /// The connection to the instance context for submitting changes. + change_collector: Arc, + parent: Ref, extent: BoxPx, scene: Scene, @@ -23,14 +25,14 @@ pub struct ViewBuilder { impl ViewBuilder { pub(crate) fn new( - requests: UnboundedSender<(InstanceId, InstanceCommand)>, - instance: InstanceId, + change_collector: Arc, + parent: Ref, extent: BoxPx, scene: Scene, ) -> Self { Self { - command_sender: requests, - instance, + change_collector, + parent, extent, scene, role: ViewRole::default(), @@ -50,11 +52,11 @@ impl ViewBuilder { pub fn build(self) -> Result { View::new( - self.command_sender, - self.instance, + self.parent, self.extent, self.scene, self.role, + self.change_collector, ) } } diff --git a/desktop/src/desktop.rs b/desktop/src/desktop.rs index 6099340c..00ba5581 100644 --- a/desktop/src/desktop.rs +++ b/desktop/src/desktop.rs @@ -1,16 +1,16 @@ -use std::sync::Arc; -use std::time::Instant; +use std::{sync::Arc, time::Instant}; -use anyhow::{Context, Result, bail}; +use anyhow::{Result, bail}; use log::info; use tokio::sync::mpsc::{UnboundedReceiver, unbounded_channel}; use massive_applications::{ - CreationMode, InstanceCommand, InstanceEnvironment, InstanceEvent, InstanceId, - InstanceParameters, ViewCommand, ViewEvent, + CreationMode, InstanceChange, InstanceEnvironment, InstanceEvent, InstanceId, + InstanceParameters, InstanceSubmission, ViewChange, ViewEvent, }; use massive_input::EventManager; use massive_renderer::RenderPacing; +use massive_scene::ChangeCollector; use massive_shell::{ApplicationContext, FontManager, Scene, ShellEvent}; use massive_shell::{AsyncWindowRenderer, ShellWindow}; @@ -19,7 +19,7 @@ use crate::desktop_system::{ DesktopCommand, DesktopSystem, ProjectCommand, TransactionEffectsMode, }; use crate::event_sourcing::Transaction; -use crate::instance_manager::{InstanceManager, ViewPath}; +use crate::instance_manager::{InstanceManager, InstanceRoot, ViewPath}; use crate::projects::{ LaunchProfile, LaunchProfileId, Launcher, LauncherMode, MatrixPlacement, Project, ProjectConfiguration, ProjectId, ProjectProperties, ProjectSet, @@ -36,7 +36,9 @@ pub struct Desktop { event_manager: EventManager, instance_manager: InstanceManager, - instance_commands: UnboundedReceiver<(InstanceId, InstanceCommand)>, + // May need to move this into the ApplicationContext. + scene_changes: Arc, + instance_submissions: UnboundedReceiver<(InstanceId, InstanceSubmission)>, context: ApplicationContext, } @@ -52,11 +54,12 @@ impl Desktop { let fonts = FontManager::system(); // Create scene early for presenter initialization - let scene = context.new_scene(); + let scene_changes = Arc::new(ChangeCollector::default()); + let scene = context.new_scene_with_change_collector(scene_changes.clone()); - let (requests_tx, mut requests_rx) = unbounded_channel::<(InstanceId, InstanceCommand)>(); + let (submissions_tx, mut submissions_rx) = unbounded_channel(); let environment = InstanceEnvironment::new( - requests_tx, + submissions_tx, context.primary_monitor_scale_factor(), fonts.clone(), ); @@ -74,17 +77,27 @@ impl Desktop { instance_manager.spawn( primary_application, CreationMode::New(InstanceParameters::new()), + InstanceRoot::new(&scene), )?; - // First wait for the initial view that's being created. + // First wait for and interpret the initial submission. + let Some((initial_instance, initial_submission)) = submissions_rx.recv().await else { + bail!("Did not receive the initial submission from the application"); + }; - let Some((primary_instance, InstanceCommand::CreateView(creation_info))) = - requests_rx.recv().await - else { - bail!("Did not or received an unexpected request from the application"); + let initial_interpretation = { + let mut context = SubmissionProcessingContext { + instance_manager: &mut instance_manager, + scene_changes: &scene_changes, + window: None, + }; + Self::interpret_instance_submission(initial_instance, initial_submission, &mut context)? }; + instance_manager.update_instance_pacing(initial_instance, initial_interpretation.pacing)?; - instance_manager.add_view(primary_instance, &creation_info); + let primary_instance = initial_instance; + let primary_view = instance_manager.primary_view(primary_instance)?; + let creation_info = primary_view.creation_info.clone(); // Currently we can't target views directly, the focus system is targeting only instances // and their primary view. @@ -112,18 +125,20 @@ impl Desktop { let project_setup_transaction = project_set_to_transaction(&project_set).map(DesktopCommand::Project); - let primary_view_transaction: Transaction<_> = [ - // Present the primary instance / view - DesktopCommand::PresentInstance { - launcher: primary_project.primary_launcher, - instance: primary_instance, - }, - DesktopCommand::PresentView(primary_instance, creation_info), - ] + let primary_instance_transaction: Transaction<_> = [DesktopCommand::PresentInstance { + launcher: primary_project.primary_launcher, + instance: primary_instance, + }] .into(); + let initial_submission_transaction: Transaction<_> = + initial_interpretation.pending_commands.into(); + system.transact( - primary_project_transaction + project_setup_transaction + primary_view_transaction, + primary_project_transaction + + project_setup_transaction + + primary_instance_transaction + + initial_submission_transaction, &scene, &mut instance_manager, TransactionEffectsMode::Setup, @@ -137,7 +152,8 @@ impl Desktop { system, event_manager, instance_manager, - instance_commands: requests_rx, + scene_changes, + instance_submissions: submissions_rx, context, }) } @@ -145,8 +161,8 @@ impl Desktop { pub async fn run(mut self) -> Result<()> { loop { tokio::select! { - Some((instance_id, request)) = self.instance_commands.recv() => { - self.process_instance_command(instance_id, request)?; + Some((instance_id, submission)) = self.instance_submissions.recv() => { + self.integrate_instance_submission(instance_id, submission)?; } shell_event = self.context.wait_for_shell_event() => { @@ -234,77 +250,127 @@ impl Desktop { } } - fn process_instance_command( + fn integrate_instance_submission( &mut self, instance: InstanceId, - command: InstanceCommand, + submission: InstanceSubmission, ) -> Result<()> { - let effects_mode = if self.system.any_buttons_pressed() { - TransactionEffectsMode::CameraLocked - } else { - TransactionEffectsMode::Normal + let interpretation = { + let mut context = SubmissionProcessingContext { + instance_manager: &mut self.instance_manager, + scene_changes: &self.scene_changes, + // Architecture: We need an indirection here anyway so window updates can target per-view windows. + window: Some(&mut self.window), + }; + + Self::interpret_instance_submission(instance, submission, &mut context)? }; - match command { - InstanceCommand::CreateView(info) => { - self.instance_manager.add_view(instance, &info); - self.system.transact( - DesktopCommand::PresentView(instance, info), - &self.scene, - &mut self.instance_manager, - effects_mode, - )?; + if !interpretation.pending_commands.is_empty() { + let effects_mode = if self.system.any_buttons_pressed() { + TransactionEffectsMode::CameraLocked + } else { + TransactionEffectsMode::Normal + }; + + self.system.transact( + interpretation.pending_commands, + &self.scene, + &mut self.instance_manager, + effects_mode, + )?; + } + + self.instance_manager + .update_instance_pacing(instance, interpretation.pacing)?; + Ok(()) + } + + fn interpret_instance_submission( + instance: InstanceId, + submission: InstanceSubmission, + context: &mut SubmissionProcessingContext, + ) -> Result { + let (changes, pacing) = submission.into_parts(); + let mut interpretation = SubmissionInterpretation { + pacing, + pending_commands: Vec::new(), + }; + + for change in changes.release() { + Self::process_instance_change( + instance, + change, + context, + &mut interpretation.pending_commands, + )?; + } + Ok(interpretation) + } + + fn process_instance_change( + instance: InstanceId, + change: InstanceChange, + context: &mut SubmissionProcessingContext, + pending_commands: &mut Vec, + ) -> Result<()> { + + match change { + InstanceChange::Scene(change) => { + context.scene_changes.collect(change); + } + InstanceChange::CreateView(info) => { + context.instance_manager.add_view(instance, &info); + pending_commands.push(DesktopCommand::PresentView(instance, info)); } - InstanceCommand::DestroyView(id, collector) => { - self.system.transact( - DesktopCommand::HideView((instance, id).into()), - &self.scene, - &mut self.instance_manager, - effects_mode, - )?; - self.instance_manager.remove_view((instance, id).into()); - // Feature: Don't push the remaining changes immediately and fade the remaining - // visuals out. (We do have the root location and should be able to do at least - // alpha blending over that in the future). - self.scene.accumulate_changes(collector.take_all()); - // Now the collector should not have any references. - let refs = Arc::strong_count(&collector); - if refs > 1 { - log::error!( - "Destroyed view's change collector contains {} unexpected references. Are there pending Visuals / Handles?", - refs - 1 - ); - }; + InstanceChange::DestroyView(id) => { + let view_path: ViewPath = (instance, id).into(); + pending_commands.push(DesktopCommand::HideView(view_path)); + context.instance_manager.remove_view(view_path); } - InstanceCommand::View(view_id, command) => { - self.handle_view_command((instance, view_id).into(), command)?; + InstanceChange::View(view_id, command) => { + let window = context.window.as_deref_mut().ok_or_else(|| { + anyhow::anyhow!( + "View command requires a window; initial submission must not contain view commands" + ) + })?; + Self::handle_view_command((instance, view_id).into(), command, window)?; } } Ok(()) } - fn handle_view_command(&mut self, view: ViewPath, command: ViewCommand) -> Result<()> { + fn handle_view_command( + _view: ViewPath, + command: ViewChange, + window: &mut ShellWindow, + ) -> Result<()> { match command { - ViewCommand::Render { submission } => { - self.instance_manager - .update_view_pacing(view, submission.pacing) - .context("render / update_view_pacing")?; - self.scene.accumulate_changes(submission.changes); - } - ViewCommand::Resize(_) => { + ViewChange::Resize(_) => { todo!("Resize is unsupported"); } - ViewCommand::SetTitle(title) => { - self.window.set_title(&title); + ViewChange::SetTitle(title) => { + window.set_title(&title); } - ViewCommand::SetCursor(icon) => { - self.window.set_cursor(icon); + ViewChange::SetCursor(icon) => { + window.set_cursor(icon); } } Ok(()) } } +struct SubmissionProcessingContext<'a> { + instance_manager: &'a mut InstanceManager, + scene_changes: &'a ChangeCollector, + window: Option<&'a mut ShellWindow>, +} + +struct SubmissionInterpretation { + pacing: RenderPacing, + pending_commands: Vec, +} + #[derive(Debug)] struct PrimaryProject { primary_launcher: LaunchProfileId, diff --git a/desktop/src/desktop_system/command_dispatch.rs b/desktop/src/desktop_system/command_dispatch.rs index a6abcc11..053d4a84 100644 --- a/desktop/src/desktop_system/command_dispatch.rs +++ b/desktop/src/desktop_system/command_dispatch.rs @@ -7,7 +7,7 @@ use massive_shell::Scene; use super::effects::{DesktopEffect, Effects}; use super::{DesktopCommand, DesktopSystem, DesktopTarget}; use crate::focus_path::PathResolver; -use crate::instance_manager::InstanceManager; +use crate::instance_manager::{InstanceManager, InstanceRoot}; impl DesktopSystem { // Architecture: The current focus is part of the system, so DesktopInteraction should probably be embedded here. @@ -29,8 +29,11 @@ impl DesktopSystem { .get_named(&self.env.primary_application) .ok_or(anyhow!("Internal error, application not registered"))?; - let instance = - instance_manager.spawn(application, CreationMode::New(parameters))?; + let instance = instance_manager.spawn( + application, + CreationMode::New(parameters), + InstanceRoot::new(scene), + )?; // Robustness: Should this be a real, logged event? // Architecture: Better to start up the primary directly, so that we can remove the PresentInstance command? @@ -84,8 +87,13 @@ impl DesktopSystem { .resolve_path(self.event_router.focused()) .instance(); - let insertion_index = - self.present_instance(launcher, originating_from, instance, scene)?; + let insertion_index = self.present_instance( + launcher, + originating_from, + instance, + instance_manager.instance_root(instance)?, + scene, + )?; let instance_target = DesktopTarget::Instance(instance); diff --git a/desktop/src/desktop_system/presentation.rs b/desktop/src/desktop_system/presentation.rs index 698d8f2f..5110b7a9 100644 --- a/desktop/src/desktop_system/presentation.rs +++ b/desktop/src/desktop_system/presentation.rs @@ -6,7 +6,7 @@ use massive_shell::Scene; use super::DesktopTarget; use super::effects::{DesktopEffect, Effects}; -use crate::instance_manager::ViewPath; +use crate::instance_manager::{InstanceRoot, ViewPath}; use crate::instance_presenter::InstancePresenter; use crate::projects::LaunchProfileId; @@ -18,6 +18,7 @@ impl DesktopSystem { launcher: LaunchProfileId, originating_from: Option, instance: InstanceId, + root: InstanceRoot, scene: &Scene, ) -> Result { let originating_presenter = originating_from @@ -41,6 +42,7 @@ impl DesktopSystem { let presenter = InstancePresenter::new( initial_center_translation, render_instance_background, + root, launcher_location, scene, ); diff --git a/desktop/src/instance_manager.rs b/desktop/src/instance_manager.rs index bc545029..7ee2d39a 100644 --- a/desktop/src/instance_manager.rs +++ b/desktop/src/instance_manager.rs @@ -10,10 +10,11 @@ use uuid::Uuid; use massive_applications::{ CreationMode, InstanceContext, InstanceEnvironment, InstanceEvent, InstanceId, - ViewCreationInfo, ViewEvent, ViewId, + ViewCreationInfo, ViewEvent, ViewId, ViewRole, }; use massive_renderer::RenderPacing; -use massive_shell::Result; +use massive_scene::{Handle, Location, Object, ToLocation, Transform}; +use massive_shell::{Result, Scene}; use crate::application_registry::Application; @@ -25,11 +26,34 @@ pub struct InstanceManager { join_set: JoinSet<(InstanceId, Result<()>)>, } +#[derive(Debug, Clone)] +pub struct InstanceRoot { + transform: Handle, + location: Handle, +} + +impl InstanceRoot { + pub fn new(scene: &Scene) -> Self { + let transform = Transform::IDENTITY.enter(scene); + let location = transform.to_location().enter(scene); + + Self { + transform, + location, + } + } + + pub fn into_parts(self) -> (Handle, Handle) { + (self.transform, self.location) + } +} + #[derive(Debug)] struct RunningInstance { #[allow(unused)] application_name: String, events_tx: UnboundedSender, + root: InstanceRoot, views: HashMap, } @@ -60,6 +84,7 @@ impl InstanceManager { &mut self, application: &Application, creation_mode: CreationMode, + root: InstanceRoot, ) -> Result { let instance_id = InstanceId::from(Uuid::new_v4()); let (events_tx, events_rx) = unbounded_channel(); @@ -68,6 +93,7 @@ impl InstanceManager { instance_id, creation_mode, self.environment.clone(), + root.location.to_ref(), events_rx, ); @@ -86,6 +112,7 @@ impl InstanceManager { RunningInstance { application_name: application.name.clone(), events_tx, + root, views: HashMap::new(), }, ); @@ -169,16 +196,26 @@ impl InstanceManager { } } - pub fn update_view_pacing(&mut self, path: ViewPath, pacing: RenderPacing) -> Result<()> { - let instance = self.mut_instance(path.instance)?; - let view = instance - .views - .get_mut(&path.view) - .ok_or_else(|| anyhow!("View {:?} not found", path.view))?; - view.pacing = pacing; + pub fn update_instance_pacing( + &mut self, + instance_id: InstanceId, + pacing: RenderPacing, + ) -> Result<()> { + let instance = self.mut_instance(instance_id)?; + for view in instance.views.values_mut() { + view.pacing = pacing; + } Ok(()) } + pub fn primary_view(&self, instance_id: InstanceId) -> Result<&ViewInfo> { + self.views() + .filter(|(path, _)| path.instance == instance_id) + .map(|(_, view)| view) + .find(|view| view.role == ViewRole::Primary) + .ok_or_else(|| anyhow!("Instance {:?} has no primary view", instance_id)) + } + #[allow(unused)] pub fn views(&self) -> impl Iterator { self.instances.iter().flat_map(|(instance_id, instance)| { @@ -203,6 +240,10 @@ impl InstanceManager { .map(|ri| ri.application_name.as_str()) } + pub fn instance_root(&self, instance: InstanceId) -> Result { + self.get_instance(instance).map(|ri| ri.root.clone()) + } + fn get_instance(&self, instance: InstanceId) -> Result<&RunningInstance> { self.instances .get(&instance) diff --git a/desktop/src/instance_presenter.rs b/desktop/src/instance_presenter.rs index 5e3211b5..d023629d 100644 --- a/desktop/src/instance_presenter.rs +++ b/desktop/src/instance_presenter.rs @@ -4,11 +4,13 @@ use anyhow::{Result, bail}; use massive_animation::{Animated, Interpolation}; use massive_applications::{ViewCreationInfo, ViewId, ViewRole}; -use massive_geometry::{Color, Point, Rect, SizePx, Transform, Vector3}; -use massive_scene::{At, Handle, Location, Object, ToLocation, Visual}; +use massive_geometry::{Color, Rect, SizePx, Transform, Vector3}; +use massive_scene::{At, Handle, Location, Object, Visual}; use massive_shapes::{self as shapes, Shape}; use massive_shell::Scene; +use crate::instance_manager::InstanceRoot; + pub const STRUCTURAL_ANIMATION_DURATION: Duration = Duration::from_millis(500); const INSTANCE_BACKGROUND_COLOR: Color = Color::rgb_u32(0x282828); @@ -53,14 +55,14 @@ impl InstancePresenter { pub fn new( initial_center_translation: Option, show_background: bool, - location: Handle, + root: InstanceRoot, + parent: Handle, scene: &Scene, ) -> Self { - let instance_transform = Transform::IDENTITY.enter(scene); - let instance_location = instance_transform - .to_location() - .relative_to(&location) - .enter(scene); + let (instance_transform, instance_location) = root.into_parts(); + instance_location.update_if_changed_with(|location| { + location.parent = Some(parent.to_ref()); + }); let background = show_background.then(|| { let visual = background_shapes(false, Rect::ZERO) @@ -99,15 +101,17 @@ impl InstancePresenter { bail!("Only primary views are supported yet"); } - if !matches!(self.state, InstancePresenterState::WaitingForPrimaryView) { - bail!("Primary view is already presenting"); + match self.state { + InstancePresenterState::WaitingForPrimaryView => {} + InstancePresenterState::Presenting { .. } | InstancePresenterState::Disappearing => { + bail!("Primary view is already presenting"); + } } // Blend in. let mut alpha = scene.animated(0.0); { - view_creation_info.location.update_with(|location| { - location.parent = Some(self.instance_location.clone()); + self.instance_location.update_with(|location| { location.alpha = 0.0; }); alpha.animate(1.0, STRUCTURAL_ANIMATION_DURATION, Interpolation::CubicOut); @@ -122,26 +126,11 @@ impl InstancePresenter { if let Some(background) = &mut self.background { background.visual.update_if_changed_with(|visual| { - // Keep background in view space to avoid compounded transform error and reduce - // the depth bias needed for stable layering. - visual.location = view_creation_info.location.clone(); - // We must switch shape coordinates in the same update; otherwise the first frame - // can render with centered-at-origin geometry from the pre-view parent space. - visual.shapes = background_shapes(background.visible, background.local_rect); + visual.location = self.instance_location.to_ref(); + visual.shapes = background_shapes(background.visible, background.centered_rect()); }); } - // Resize is currently unsupported, so centering transform is fixed for this view. - let size = view_creation_info.size(); - let view_center = Point::new((size.width / 2) as f64, (size.height / 2) as f64); - let transform = - Transform::from_translation(Vector3::new(-view_center.x, -view_center.y, 0.0)); - view_creation_info - .location - .value() - .transform - .update_if_changed(transform); - Ok(()) } @@ -194,15 +183,8 @@ impl InstancePresenter { if let Some(background) = &mut self.background { background.visual.update_if_changed_with(|visual| { - // Background geometry basis depends on parent space: - // - no view yet: centered around instance origin - // - view presenting: regular view-local rectangle (top-left origin) - let rect = if self.state.view().is_some() { - background.local_rect - } else { - background.local_rect - background.local_rect.center() - }; - visual.shapes = background_shapes(background.visible, rect); + // Background geometry stays in instance space; views apply their own local offset. + visual.shapes = background_shapes(background.visible, background.centered_rect()); }); } @@ -218,15 +200,20 @@ impl InstancePresenter { let Some(view) = self.state.view_mut() else { return; }; - let location = &view.creation_info.location; let alpha = view.alpha.value(); - location.update_if_changed_with(|location| { + self.instance_location.update_if_changed_with(|location| { location.alpha = *alpha; }); } } +impl InstanceBackground { + fn centered_rect(&self) -> Rect { + self.local_rect - self.local_rect.center() + } +} + impl InstancePresenterState { fn view(&self) -> Option<&PrimaryViewPresenter> { match self { diff --git a/renderer/src/render_submission.rs b/renderer/src/render_submission.rs index 5f01f9f1..ad4c2284 100644 --- a/renderer/src/render_submission.rs +++ b/renderer/src/render_submission.rs @@ -1,7 +1,7 @@ use anyhow::Result; use massive_geometry::PixelCamera; -use massive_scene::SceneChanges; +use massive_scene::SceneChangeSet; use crate::{RenderPacing, RenderTarget}; @@ -13,13 +13,13 @@ use crate::{RenderPacing, RenderTarget}; #[must_use] #[derive(Debug, Default)] pub struct RenderSubmission { - pub changes: SceneChanges, + pub changes: SceneChangeSet, pub pacing: RenderPacing, pub camera_update: Option, } impl RenderSubmission { - pub fn new(changes: SceneChanges, pacing: RenderPacing) -> Self { + pub fn new(changes: SceneChangeSet, pacing: RenderPacing) -> Self { Self { changes, pacing, diff --git a/scene/src/ergonomics.rs b/scene/src/ergonomics.rs index 44409a24..1f9c0709 100644 --- a/scene/src/ergonomics.rs +++ b/scene/src/ergonomics.rs @@ -8,7 +8,7 @@ use std::sync::Arc; use massive_geometry::{PixelCamera, Point, PointPx, Rect, Transform}; use massive_shapes::Shape; -use crate::{Handle, Location, Visual}; +use crate::{Handle, Location, Ref, Visual}; // This should probably be moved to massive_geometry: @@ -100,21 +100,21 @@ impl VisualWithoutLocation { } } - pub fn at(self, location: impl Into>) -> Visual { + pub fn at(self, location: impl Into>) -> Visual { Visual::new(location.into(), self.shapes) } } // Everything that can is positioned can be converted to a visual. pub trait At { - fn at(self, location: impl Into>) -> Visual; + fn at(self, location: impl Into>) -> Visual; } impl At for T where T: IntoVisual, { - fn at(self, location: impl Into>) -> Visual { + fn at(self, location: impl Into>) -> Visual { self.into_visual().at(location) } } diff --git a/scene/src/handle.rs b/scene/src/handle.rs index d2433887..90f1b069 100644 --- a/scene/src/handle.rs +++ b/scene/src/handle.rs @@ -1,8 +1,8 @@ -use std::{fmt, hash, sync::Arc}; +use std::{fmt, hash, ops::Deref, sync::Arc}; use parking_lot::{Mutex, MutexGuard}; -use crate::{Change, ChangeCollector, Id, Scene, SceneChange}; +use crate::{Change, HandleChangeReceiver, Id, Scene, SceneChange}; /// A handle is a mutable representation of an object staged on a scene. /// @@ -53,14 +53,14 @@ impl Handle where SceneChange: From>, { - pub(crate) fn new(id: Id, value: T, change_collector: Arc) -> Self { + pub(crate) fn new(id: Id, value: T, change_collector: Arc) -> Self { let uploaded = T::to_change(&value); - change_collector.collect(Change::Create(id, uploaded)); + change_collector.send(Change::Create(id, uploaded).into()); Self { inner: InnerHandle { id, - change_tracker: change_collector, + change_collector, value: value.into(), } .into(), @@ -71,14 +71,17 @@ where self.inner.id } + pub fn to_ref(&self) -> Ref { + Ref { + inner: self.inner.clone(), + } + } + pub fn update_if_changed(&self, update: T) where T: PartialEq, { - // Robustness: This locks twice. - if update != *self.value() { - self.update(update) - } + self.inner.update_if_changed(update) } /// Update the value of the handle. @@ -86,40 +89,122 @@ where self.inner.update(update) } - // Performance: May use replace_with? pub fn update_with(&self, f: impl FnOnce(&mut T)) { - // Performance: This locks twice. - f(&mut *self.value_mut()); - self.inner.updated(); + self.inner.update_with(f); } - // Performance: May use replace_with? pub fn update_if_changed_with(&self, f: impl FnOnce(&mut T)) where T: Clone + PartialEq, { - // Robustness: This locks twice if changed. - // - // Detail: Need to separate the lock range here clearly, otherwise the mutex stays locked - // until self.inner.updated() - let changed = { - let mut v = self.value_mut(); - let before = v.clone(); - f(&mut *v); - *v != before - }; - if changed { - self.inner.updated(); + self.inner.update_if_changed_with(f); + } + + pub fn value(&self) -> HandleValue<'_, T> { + HandleValue { + value: self.inner.value.lock(), + } + } +} + +/// A read-only handle to an object staged on a scene. +#[derive(Debug)] +pub struct Ref +where + SceneChange: From>, +{ + inner: Arc>, +} + +impl Clone for Ref +where + SceneChange: From>, +{ + fn clone(&self) -> Self { + Self { + inner: self.inner.clone(), } } +} + +impl PartialEq for Ref +where + SceneChange: From>, +{ + fn eq(&self, other: &Self) -> bool { + self.inner.id.eq(&other.inner.id) + } +} + +impl Eq for Ref where SceneChange: From> {} + +impl hash::Hash for Ref +where + SceneChange: From>, +{ + fn hash(&self, state: &mut H) { + self.inner.id.hash(state); + } +} + +impl From> for Ref +where + SceneChange: From>, +{ + fn from(value: Handle) -> Self { + value.to_ref() + } +} + +impl From<&Handle> for Ref +where + SceneChange: From>, +{ + fn from(value: &Handle) -> Self { + value.to_ref() + } +} + +impl From<&Ref> for Ref +where + SceneChange: From>, +{ + fn from(value: &Ref) -> Self { + value.clone() + } +} + +impl Ref +where + SceneChange: From>, +{ + pub fn id(&self) -> Id { + self.inner.id + } - pub fn value(&self) -> MutexGuard<'_, T> { - self.inner.value.lock() + pub fn value(&self) -> HandleValue<'_, T> { + HandleValue { + value: self.inner.value.lock(), + } } +} + +#[derive(Debug)] +pub struct HandleValue<'a, T: Object> +where + SceneChange: From>, +{ + value: MutexGuard<'a, T>, +} + +impl Deref for HandleValue<'_, T> +where + SceneChange: From>, +{ + type Target = T; - // Internal access. - fn value_mut(&self) -> MutexGuard<'_, T> { - self.inner.value.lock() + fn deref(&self) -> &Self::Target { + &self.value } } @@ -131,7 +216,7 @@ where { id: Id, /// This is effectively the connection to the scene it was staged in. - change_tracker: Arc, + change_collector: Arc, // Optimization: Some values might be too large to be duplicated between the application and the // renderer. value: Mutex, @@ -141,16 +226,56 @@ impl InnerHandle where SceneChange: From>, { + // Invariant: mutate the value and enqueue its update while holding the same lock so change + // ordering matches committed state under concurrent writers. pub fn update(&self, value: T) { - let change = T::to_change(&value); - self.change_tracker.collect(Change::Update(self.id, change)); + self.update_locked(|current| { + *current = value; + true + }); + } + + pub fn update_if_changed(&self, value: T) + where + T: PartialEq, + { + self.update_locked(|current| { + if *current == value { + return false; + } + + *current = value; + true + }); + } + + pub fn update_with(&self, f: impl FnOnce(&mut T)) { + self.update_locked(|current| { + f(current); + true + }); + } - *self.value.lock() = value; + pub fn update_if_changed_with(&self, f: impl FnOnce(&mut T)) + where + T: Clone + PartialEq, + { + self.update_locked(|current| { + let before = current.clone(); + f(current); + *current != before + }); } - pub fn updated(&self) { - let change = T::to_change(&*self.value.lock()); - self.change_tracker.collect(Change::Update(self.id, change)); + fn update_locked(&self, mutate: impl FnOnce(&mut T) -> bool) { + let mut current = self.value.lock(); + if !mutate(&mut *current) { + return; + } + + let change = T::to_change(&*current); + self.change_collector + .send(Change::Update(self.id, change).into()); } } @@ -159,7 +284,7 @@ where SceneChange: From>, { fn drop(&mut self) { - self.change_tracker.collect(Change::Delete(self.id)); + self.change_collector.send(Change::Delete(self.id).into()); } } diff --git a/scene/src/lib.rs b/scene/src/lib.rs index 87499902..7f62e833 100644 --- a/scene/src/lib.rs +++ b/scene/src/lib.rs @@ -23,6 +23,7 @@ //! the renderer minimizes allocations and can trivially associate arbitrary additional data like //! buffers or caches that are needed to render the objects fast and with a low memory //! footprint and allocations. +use std::fmt; mod change; mod change_surface; @@ -44,11 +45,49 @@ pub use scene::Scene; pub use transform_resolver::*; pub use type_id_generator::id_generator; -use massive_util as util; +use massive_util::{self as util}; + +// Re-exports +pub use massive_geometry::Transform; pub type ChangeCollector = util::ChangeCollector; -pub type SceneChanges = util::Changes; +pub type SceneChangeSet = util::ChangeSet; -// Re-exports +/// This receiver trait acts as the receiver the `Handle` type needs to propagate its changes and +/// drops. +/// +/// The trait indirection is here so that other layers can interleave scene changes into their +/// specific collector. +pub trait HandleChangeReceiver: fmt::Debug + Send + Sync { + fn send(&self, change: SceneChange); -pub use massive_geometry::Transform; + fn take_changes(&self) -> SceneChangeSet { + panic!("HandleChangeReceiver::take_changes is not supported by this receiver"); + } +} + +impl HandleChangeReceiver for ChangeCollector { + fn send(&self, change: SceneChange) { + self.collect(change); + } + + fn take_changes(&self) -> SceneChangeSet { + self.take_all() + } +} + +// A receiver for all changes in a scene. +// pub trait SceneChangeReceiver: HandleChangeReceiver { +// fn collect_many(&self, changes: ChangeSet); +// fn take_all(&self) -> SceneChangeSet; +// } + +// impl SceneChangeReceiver for ChangeCollector { +// fn collect_many(&self, changes: ChangeSet) { +// self.collect_many(changes); +// } + +// fn take_all(&self) -> SceneChangeSet { +// self.take_all() +// } +// } diff --git a/scene/src/objects.rs b/scene/src/objects.rs index 7e77341d..83781742 100644 --- a/scene/src/objects.rs +++ b/scene/src/objects.rs @@ -3,7 +3,7 @@ use std::sync::Arc; use massive_geometry::{Bounds, Transform}; use massive_shapes::{GlyphRun, Shape}; -use crate::{Change, Handle, Id, Object, SceneChange}; +use crate::{Change, Handle, Id, Object, Ref, SceneChange}; /// A visual represents a set of shapes that have a common position / location in the space. /// @@ -13,7 +13,7 @@ use crate::{Change, Handle, Id, Object, SceneChange}; /// Detail: `Clone` was added for `Handle::update_with_if_changed()`. #[derive(Debug, Clone, PartialEq)] pub struct Visual { - pub location: Handle, + pub location: Ref, /// Optional decal ordering value for this visual. /// /// If set, the renderer treats this visual as a decal and renders it in decal order using the @@ -41,9 +41,9 @@ pub struct Visual { } impl Visual { - pub fn new(location: Handle, shapes: impl Into>) -> Self { + pub fn new(location: impl Into>, shapes: impl Into>) -> Self { Self { - location, + location: location.into(), decal_order: None, clip_bounds: None, shapes: shapes.into(), @@ -106,8 +106,8 @@ impl Object for Visual { #[derive(Debug, Clone, PartialEq)] pub struct Location { - pub parent: Option>, - pub transform: Handle, + pub parent: Option>, + pub transform: Ref, pub alpha: f32, } @@ -115,14 +115,14 @@ impl From> for Location { fn from(transform: Handle) -> Self { Self { parent: None, - transform, + transform: transform.into(), alpha: 1.0, } } } impl Location { - pub fn new(parent: Option>, transform: impl Into>) -> Self { + pub fn new(parent: Option>, transform: impl Into>) -> Self { Self { parent, transform: transform.into(), @@ -130,7 +130,7 @@ impl Location { } } - pub fn relative_to(mut self, parent: impl Into>) -> Self { + pub fn relative_to(mut self, parent: impl Into>) -> Self { self.parent = Some(parent.into()); self } @@ -192,11 +192,12 @@ impl Object for Transform { #[cfg(test)] mod tests { use super::*; - use crate::Scene; + use crate::{ChangeCollector, Scene}; #[test] fn location_new_defaults_to_opaque_alpha() { - let scene = Scene::new(); + let receiver = Arc::new(ChangeCollector::default()); + let scene = Scene::new(receiver); let transform = Transform::IDENTITY.enter(&scene); let location = Location::new(None, transform); @@ -206,7 +207,8 @@ mod tests { #[test] fn location_alpha_is_normalized_when_set_and_uploaded() { - let scene = Scene::new(); + let receiver = Arc::new(ChangeCollector::default()); + let scene = Scene::new(receiver); let transform = Transform::IDENTITY.enter(&scene); assert_eq!( diff --git a/scene/src/scene.rs b/scene/src/scene.rs index db23aeb9..26754ad1 100644 --- a/scene/src/scene.rs +++ b/scene/src/scene.rs @@ -1,25 +1,24 @@ use std::sync::Arc; use crate::id_generator; -use crate::{Change, ChangeCollector, Handle, Object, SceneChange, SceneChanges}; +use crate::{Change, Handle, HandleChangeReceiver, Object, SceneChange, SceneChangeSet}; -/// A scene is the only direct connection of actual contents to the renderer. It tracks all the -/// changes to scene graph and uploads it when an update cycle ends. +/// A scene is an indepent collector and instantiator of changes that are meant to send to the +/// renderer in a later submission. /// -/// A scene does not have direct observable changes, so it can always be shared and used for staging -/// objects onto it. -#[derive(Debug, Default)] +/// It is used primarily for instantiating new `Handle` objects. +#[derive(Debug)] pub struct Scene { // This tracks all changes from staging, changing the values in the handles, and dropping // them. // // Shared because handles need to push changes when dropped. - change_collector: Arc, + change_receiver: Arc, } impl Scene { - pub fn new() -> Self { - Self::default() + pub fn new(change_receiver: Arc) -> Self { + Self { change_receiver } } /// Put an object on the stage. @@ -28,20 +27,20 @@ impl Scene { SceneChange: From>, { let id = id_generator::acquire::(); - Handle::new(id, value, self.change_collector.clone()) + Handle::new(id, value, self.change_receiver.clone()) } - /// Push external changes into this scene. - pub fn push_changes(&self, changes: SceneChanges) { - self.change_collector.collect_many(changes); - } - - // Take the changes that need to be sent to the renderer. - pub fn take_changes(&self) -> SceneChanges { - self.change_collector.take_all() - } + // Push external changes into this scene. + // + // This can be useful for combining changes from lower layers. + // + // Safety: The changes need to occupy the same identity space (i.e. use the same id generator). + // pub fn push_changes(&self, changes: SceneChangeSet) { + // self.change_receiver.collect_many(changes); + // } - pub fn into_collector(self) -> Arc { - self.change_collector + // Take all the changes. + pub fn take_changes(&self) -> SceneChangeSet { + self.change_receiver.take_changes() } } diff --git a/scene/src/transform_resolver.rs b/scene/src/transform_resolver.rs index 41b56133..22c09782 100644 --- a/scene/src/transform_resolver.rs +++ b/scene/src/transform_resolver.rs @@ -2,16 +2,16 @@ use std::collections::HashMap; use massive_geometry::Transform; -use crate::{Handle, Location}; +use crate::{Location, Ref}; /// Resolve final transforms from a set of locations. #[derive(Debug, Default)] pub struct TransformResolver { - map: HashMap, Transform>, + map: HashMap, Transform>, } impl TransformResolver { - pub fn resolve(&mut self, location: &Handle) -> Transform { + pub fn resolve(&mut self, location: &Ref) -> Transform { if let Some(&transform) = self.map.get(location) { return transform; } diff --git a/shell/src/application_context.rs b/shell/src/application_context.rs index 731add3a..464098fc 100644 --- a/shell/src/application_context.rs +++ b/shell/src/application_context.rs @@ -1,5 +1,6 @@ use anyhow::{Result, anyhow}; use massive_geometry::SizePx; +use massive_scene::ChangeCollector; use tokio::sync::mpsc::{UnboundedReceiver, WeakUnboundedSender}; use tokio::sync::oneshot; use winit::dpi::PhysicalSize; @@ -61,6 +62,11 @@ impl ApplicationContext { Scene::new(self.animation_coordinator.clone()) } + /// Creates a new scene with a caller-provided change collector. + pub fn new_scene_with_change_collector(&self, collector: std::sync::Arc) -> Scene { + Scene::new_with_change_collector(self.animation_coordinator.clone(), collector) + } + /// Creates a new window. /// /// Async because it needs to communicate with the application's main thread on which the window diff --git a/shell/src/window_renderer.rs b/shell/src/window_renderer.rs index 76c63e9c..90b6bd4f 100644 --- a/shell/src/window_renderer.rs +++ b/shell/src/window_renderer.rs @@ -12,7 +12,7 @@ use winit::window::WindowId; use massive_geometry::{Color, Matrix4, SizePx}; use massive_renderer::{PresentationMode, RenderPacing, Renderer}; -use massive_scene::{SceneChanges, id_generator}; +use massive_scene::{SceneChangeSet, id_generator}; use massive_util::message_filter; use crate::{ShellEvent, shell_window::ShellWindowShared}; @@ -218,7 +218,7 @@ impl WindowRenderer { PresentationMode::new(present_mode, maximum_frame_latency) } - fn apply_scene_changes(&mut self, changes: SceneChanges) -> Result<()> { + fn apply_scene_changes(&mut self, changes: SceneChangeSet) -> Result<()> { #[cfg(feature = "metrics")] let time_of_oldest_change = changes.time_of_oldest_change(); @@ -302,7 +302,7 @@ pub enum RendererMessage { /// An extended accumulable submission structure that contains everything the renderer needs to know. #[derive(Debug)] pub struct RenderThreadSubmission { - pub changes: SceneChanges, + pub changes: SceneChangeSet, pub pacing: RenderPacing, pub view_projection: Matrix4, } @@ -310,7 +310,7 @@ pub struct RenderThreadSubmission { impl RenderThreadSubmission { pub fn new(view_projection: Matrix4) -> Self { Self { - changes: SceneChanges::default(), + changes: SceneChangeSet::default(), view_projection, pacing: RenderPacing::Fast, } diff --git a/util/src/change_collector.rs b/util/src/change_collector.rs index ad4d69fd..83b48ff9 100644 --- a/util/src/change_collector.rs +++ b/util/src/change_collector.rs @@ -7,13 +7,13 @@ use parking_lot::Mutex; #[derive(Debug)] pub struct ChangeCollector { - changes: Mutex>, + changes: Mutex>, } impl Default for ChangeCollector { fn default() -> Self { Self { - changes: Mutex::new(Changes::default()), + changes: Mutex::new(ChangeSet::default()), } } } @@ -24,19 +24,21 @@ impl ChangeCollector { self.changes.lock().push(change); } - pub fn collect_many(&self, changes: impl Into>) { + pub fn collect_many(&self, changes: impl Into>) { let changes = changes.into(); self.changes.lock().accumulate(changes); } - pub fn take_all(&self) -> Changes { + /// Temporary API: this exists while scenes can still be created without + /// requiring a separately owned change collector. + pub fn take_all(&self) -> ChangeSet { // Performance: Preserve capacity here? mem::take(&mut self.changes.lock()) } } #[derive(Debug, Deref)] -pub struct Changes { +pub struct ChangeSet { #[deref] pub changes: Vec, @@ -44,7 +46,7 @@ pub struct Changes { pub time_of_oldest_change: Option, } -impl Default for Changes { +impl Default for ChangeSet { fn default() -> Self { Self { changes: Vec::new(), @@ -55,7 +57,7 @@ impl Default for Changes { } } -impl Drop for Changes { +impl Drop for ChangeSet { fn drop(&mut self) { if !self.changes.is_empty() { log::error!("{} changes were not processed", self.changes.len()); @@ -63,7 +65,7 @@ impl Drop for Changes { } } -impl Changes { +impl ChangeSet { pub fn push(&mut self, change: C) { #[cfg(feature = "metrics")] { @@ -75,7 +77,7 @@ impl Changes { self.changes.push(change); } - pub fn accumulate(&mut self, mut changes: Changes) { + pub fn accumulate(&mut self, mut changes: ChangeSet) { if self.changes.is_empty() { // Performance: Capacity *self = changes; @@ -94,6 +96,19 @@ impl Changes { } } + pub fn map(mut self, map_change: impl FnMut(C) -> M) -> ChangeSet { + ChangeSet { + // take is needed because of Drop. + changes: mem::take(&mut self.changes) + .into_iter() + .map(map_change) + .collect(), + + #[cfg(feature = "metrics")] + time_of_oldest_change: self.time_of_oldest_change, + } + } + pub fn release(mut self) -> Vec { mem::take(&mut self.changes) }