graph: Refactor graph item management into new graph_manager object

The graph widgets management (watching a glib receiver, adding and removing
Nodes, Ports and Links) currently done in the `Application` and `GraphView`
objects has been extracted into a new GraphManager object, which watches the
receiver instead, pushes changes directly to the widgets, and reacts to their signals.

This seperates widget logic and management logic cleanly instead of both
being mixed into the GraphView, and also reduces the code size for the
Application object.

src/application.rs

@@ -14,18 +14,15 @@
 //
 // SPDX-License-Identifier: GPL-3.0-only
 
-use std::cell::RefCell;
-
 use gtk::{
     gio,
-    glib::{self, clone, Continue, Receiver},
+    glib::{self, clone, Receiver},
     prelude::*,
     subclass::prelude::*,
 };
-use log::info;
-use pipewire::{channel::Sender, spa::Direction};
+use pipewire::channel::Sender;
 
-use crate::{ui, GtkMessage, MediaType, NodeType, PipewireLink, PipewireMessage};
+use crate::{graph_manager::GraphManager, ui, GtkMessage, PipewireMessage};
 
 static STYLE: &str = include_str!("style.css");
 
@@ -37,7 +34,7 @@ mod imp {
     #[derive(Default)]
     pub struct Application {
         pub(super) graphview: ui::graph::GraphView,
-        pub(super) pw_sender: OnceCell<RefCell<Sender<GtkMessage>>>,
+        pub(super) graph_manager: OnceCell<GraphManager>,
     }
 
     #[glib::object_subclass]
@@ -51,6 +48,7 @@ mod imp {
     impl ApplicationImpl for Application {
         fn activate(&self) {
             let app = &*self.obj();
+
             let scrollwindow = gtk::ScrolledWindow::builder()
                 .child(&self.graphview)
                 .build();
@@ -117,11 +115,10 @@ impl Application {
             .build();
 
         let imp = app.imp();
-        imp.pw_sender
-            .set(RefCell::new(pw_sender))
-            // Discard the returned sender, as it does not implement `Debug`.
-            .map_err(|_| ())
-            .expect("pw_sender field was already set");
+
+        imp.graph_manager
+            .set(GraphManager::new(&imp.graphview, pw_sender, gtk_receiver))
+            .expect("Should be able to set graph manager");
 
         // Add <Control-Q> shortcut for quitting the application.
         let quit = gtk::gio::SimpleAction::new("quit", None);
@@ -131,138 +128,6 @@ impl Application {
         app.set_accels_for_action("app.quit", &["<Control>Q"]);
         app.add_action(&quit);
 
-        // React to messages received from the pipewire thread.
-        gtk_receiver.attach(
-            None,
-            clone!(
-                @weak app => @default-return Continue(true),
-                move |msg| {
-                    match msg {
-                        PipewireMessage::NodeAdded{ id, name, node_type } => app.add_node(id, name.as_str(), node_type),
-                        PipewireMessage::PortAdded{ id, node_id, name, direction, media_type } => app.add_port(id, name.as_str(), node_id, direction, media_type),
-                        PipewireMessage::LinkAdded{ id, node_from, port_from, node_to, port_to, active} => app.add_link(id, node_from, port_from, node_to, port_to, active),
-                        PipewireMessage::LinkStateChanged { id, active } => app.link_state_changed(id, active), // TODO
-                        PipewireMessage::NodeRemoved { id } => app.remove_node(id),
-                        PipewireMessage::PortRemoved { id, node_id } => app.remove_port(id, node_id),
-                        PipewireMessage::LinkRemoved { id } => app.remove_link(id)
-                    };
-                    Continue(true)
-                }
-            ),
-        );
-
         app
     }
-
-    /// Add a new node to the view.
-    fn add_node(&self, id: u32, name: &str, node_type: Option<NodeType>) {
-        info!("Adding node to graph: id {}", id);
-
-        self.imp()
-            .graphview
-            .add_node(id, ui::graph::Node::new(name, id), node_type);
-    }
-
-    /// Add a new port to the view.
-    fn add_port(
-        &self,
-        id: u32,
-        name: &str,
-        node_id: u32,
-        direction: Direction,
-        media_type: Option<MediaType>,
-    ) {
-        info!("Adding port to graph: id {}", id);
-
-        let port = ui::graph::Port::new(id, name, direction, media_type);
-
-        // Create or delete a link if the widget emits the "port-toggled" signal.
-        port.connect_local(
-            "port_toggled",
-            false,
-            clone!(@weak self as app => @default-return None, move |args| {
-                // Args always look like this: &[widget, id_port_from, id_port_to]
-                let port_from = args[1].get::<u32>().unwrap();
-                let port_to = args[2].get::<u32>().unwrap();
-
-                app.toggle_link(port_from, port_to);
-
-                None
-            }),
-        );
-
-        self.imp().graphview.add_port(node_id, id, port);
-    }
-
-    /// Add a new link to the view.
-    fn add_link(
-        &self,
-        id: u32,
-        node_from: u32,
-        port_from: u32,
-        node_to: u32,
-        port_to: u32,
-        active: bool,
-    ) {
-        info!("Adding link to graph: id {}", id);
-
-        // FIXME: Links should be colored depending on the data they carry (video, audio, midi) like ports are.
-
-        // Update graph to contain the new link.
-        self.imp().graphview.add_link(
-            id,
-            PipewireLink {
-                node_from,
-                port_from,
-                node_to,
-                port_to,
-            },
-            active,
-        );
-    }
-
-    fn link_state_changed(&self, id: u32, active: bool) {
-        info!(
-            "Link state changed: Link (id={}) is now {}",
-            id,
-            if active { "active" } else { "inactive" }
-        );
-
-        self.imp().graphview.set_link_state(id, active);
-    }
-
-    // Toggle a link between the two specified ports on the remote pipewire server.
-    fn toggle_link(&self, port_from: u32, port_to: u32) {
-        let sender = self
-            .imp()
-            .pw_sender
-            .get()
-            .expect("pw_sender not set")
-            .borrow_mut();
-        sender
-            .send(GtkMessage::ToggleLink { port_from, port_to })
-            .expect("Failed to send message");
-    }
-
-    /// Remove the node with the specified id from the view.
-    fn remove_node(&self, id: u32) {
-        info!("Removing node from graph: id {}", id);
-
-        self.imp().graphview.remove_node(id);
-    }
-
-    /// Remove the port with the id `id` from the node with the id `node_id`
-    /// from the view.
-    fn remove_port(&self, id: u32, node_id: u32) {
-        info!("Removing port from graph: id {}, node_id: {}", id, node_id);
-
-        self.imp().graphview.remove_port(id, node_id);
-    }
-
-    /// Remove the link with the specified id from the view.
-    fn remove_link(&self, id: u32) {
-        info!("Removing link from graph: id {}", id);
-
-        self.imp().graphview.remove_link(id);
-    }
 }

src/graph_manager.rs

@@ -0,0 +1,283 @@
+// Copyright 2021 Tom A. Wagner <tom.a.wagner@protonmail.com>
+//
+// This program is free software: you can redistribute it and/or modify
+// it under the terms of the GNU General Public License version 3 as published by
+// the Free Software Foundation.
+//
+// This program is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+//
+// You should have received a copy of the GNU General Public License
+// along with this program.  If not, see <http://www.gnu.org/licenses/>.
+//
+// SPDX-License-Identifier: GPL-3.0-only
+
+use gtk::{glib, prelude::*, subclass::prelude::*};
+
+use pipewire::channel::Sender as PwSender;
+
+use crate::{ui::graph::GraphView, GtkMessage, PipewireMessage};
+
+mod imp {
+    use super::*;
+
+    use std::{cell::RefCell, collections::HashMap};
+
+    use once_cell::unsync::OnceCell;
+
+    use crate::{ui::graph, MediaType, NodeType};
+
+    #[derive(Default, glib::Properties)]
+    #[properties(wrapper_type = super::GraphManager)]
+    pub struct GraphManager {
+        #[property(get, set, construct_only)]
+        pub graph: OnceCell<crate::ui::graph::GraphView>,
+
+        pub pw_sender: OnceCell<PwSender<crate::GtkMessage>>,
+        pub items: RefCell<HashMap<u32, glib::Object>>,
+    }
+
+    #[glib::object_subclass]
+    impl ObjectSubclass for GraphManager {
+        const NAME: &'static str = "HelvumGraphManager";
+        type Type = super::GraphManager;
+        type ParentType = glib::Object;
+    }
+
+    impl ObjectImpl for GraphManager {
+        fn properties() -> &'static [glib::ParamSpec] {
+            Self::derived_properties()
+        }
+
+        fn property(&self, id: usize, pspec: &glib::ParamSpec) -> glib::Value {
+            Self::derived_property(self, id, pspec)
+        }
+
+        fn set_property(&self, id: usize, value: &glib::Value, pspec: &glib::ParamSpec) {
+            Self::derived_set_property(self, id, value, pspec)
+        }
+    }
+
+    impl GraphManager {
+        pub fn attach_receiver(&self, receiver: glib::Receiver<crate::PipewireMessage>) {
+            receiver.attach(None, glib::clone!(
+                @weak self as imp => @default-return Continue(true),
+                move |msg| {
+                    match msg {
+                        PipewireMessage::NodeAdded{ id, name, node_type } => imp.add_node(id, name.as_str(), node_type),
+                        PipewireMessage::PortAdded{ id, node_id, name, direction, media_type } => imp.add_port(id, name.as_str(), node_id, direction, media_type),
+                        PipewireMessage::LinkAdded{ id, port_from, port_to, active} => imp.add_link(id, port_from, port_to, active),
+                        PipewireMessage::LinkStateChanged { id, active } => imp.link_state_changed(id, active),
+                        PipewireMessage::NodeRemoved { id } => imp.remove_node(id),
+                        PipewireMessage::PortRemoved { id, node_id } => imp.remove_port(id, node_id),
+                        PipewireMessage::LinkRemoved { id } => imp.remove_link(id)
+                    };
+                    Continue(true)
+                }
+            ));
+        }
+
+        /// Add a new node to the view.
+        fn add_node(&self, id: u32, name: &str, node_type: Option<NodeType>) {
+            log::info!("Adding node to graph: id {}", id);
+
+            let node = graph::Node::new(name, id);
+
+            self.items.borrow_mut().insert(id, node.clone().upcast());
+
+            self.obj().graph().add_node(node, node_type);
+        }
+
+        /// Remove the node with the specified id from the view.
+        fn remove_node(&self, id: u32) {
+            log::info!("Removing node from graph: id {}", id);
+
+            let Some(node) = self.items.borrow_mut().remove(&id) else {
+                log::warn!("Unknown node (id={id}) removed from graph");
+                return;
+            };
+            let Ok(node) = node.dynamic_cast::<graph::Node>() else {
+                log::warn!("Graph Manager item under node id {id} is not a node");
+                return;
+            };
+
+            self.obj().graph().remove_node(&node);
+        }
+
+        /// Add a new port to the view.
+        fn add_port(
+            &self,
+            id: u32,
+            name: &str,
+            node_id: u32,
+            direction: pipewire::spa::Direction,
+            media_type: Option<MediaType>,
+        ) {
+            log::info!("Adding port to graph: id {}", id);
+
+            let mut items = self.items.borrow_mut();
+
+            let Some(node) = items.get(&node_id) else {
+                log::warn!("Node (id: {node_id}) for port (id: {id}) not found in graph manager");
+                return;
+            };
+            let Ok(node) = node.clone().dynamic_cast::<graph::Node>() else {
+                log::warn!("Graph Manager item under node id {node_id} is not a node");
+                return;
+            };
+
+            let port = graph::Port::new(id, name, direction, media_type);
+
+            // Create or delete a link if the widget emits the "port-toggled" signal.
+            port.connect_local(
+                "port_toggled",
+                false,
+                glib::clone!(@weak self as app => @default-return None, move |args| {
+                    // Args always look like this: &[widget, id_port_from, id_port_to]
+                    let port_from = args[1].get::<u32>().unwrap();
+                    let port_to = args[2].get::<u32>().unwrap();
+
+                    app.toggle_link(port_from, port_to);
+
+                    None
+                }),
+            );
+
+            items.insert(id, port.clone().upcast());
+
+            node.add_port(port);
+        }
+
+        /// Remove the port with the id `id` from the node with the id `node_id`
+        /// from the view.
+        fn remove_port(&self, id: u32, node_id: u32) {
+            log::info!("Removing port from graph: id {}, node_id: {}", id, node_id);
+
+            let mut items = self.items.borrow_mut();
+
+            let Some(node) = items.get(&node_id) else {
+                log::warn!("Node (id: {node_id}) for port (id: {id}) not found in graph manager");
+                return;
+            };
+            let Ok(node) = node.clone().dynamic_cast::<graph::Node>() else {
+                log::warn!("Graph Manager item under node id {node_id} is not a node");
+                return;
+            };
+            let Some(port) = items.remove(&id) else {
+                log::warn!("Unknown Port (id: {id}) removed from graph");
+                return;
+            };
+            let Ok(port) = port.dynamic_cast::<graph::Port>() else {
+                log::warn!("Graph Manager item under port id {id} is not a port");
+                return;
+            };
+
+            node.remove_port(&port);
+        }
+
+        /// Add a new link to the view.
+        fn add_link(&self, id: u32, output_port_id: u32, input_port_id: u32, active: bool) {
+            log::info!("Adding link to graph: id {}", id);
+
+            let mut items = self.items.borrow_mut();
+
+            let Some(output_port) = items.get(&output_port_id) else {
+                log::warn!("Output port (id: {output_port_id}) for link (id: {id}) not found in graph manager");
+                return;
+            };
+            let Ok(output_port) = output_port.clone().dynamic_cast::<graph::Port>() else {
+                log::warn!("Graph Manager item under port id {output_port_id} is not a port");
+                return;
+            };
+            let Some(input_port) = items.get(&input_port_id) else {
+                log::warn!("Output port (id: {input_port_id}) for link (id: {id}) not found in graph manager");
+                return;
+            };
+            let Ok(input_port) = input_port.clone().dynamic_cast::<graph::Port>() else {
+                log::warn!("Graph Manager item under port id {input_port_id} is not a port");
+                return;
+            };
+
+            let link = graph::Link::new();
+            link.set_output_port(Some(&output_port));
+            link.set_input_port(Some(&input_port));
+            link.set_active(active);
+
+            items.insert(id, link.clone().upcast());
+
+            // Update graph to contain the new link.
+            self.graph
+                .get()
+                .expect("graph should be set")
+                .add_link(link);
+        }
+
+        fn link_state_changed(&self, id: u32, active: bool) {
+            log::info!(
+                "Link state changed: Link (id={id}) is now {}",
+                if active { "active" } else { "inactive" }
+            );
+
+            let items = self.items.borrow();
+
+            let Some(link) = items.get(&id) else {
+                log::warn!("Link state changed on unknown link (id={id})");
+                return;
+            };
+            let Some(link) = link.dynamic_cast_ref::<graph::Link>() else {
+                log::warn!("Graph Manager item under link id {id} is not a link");
+                return;
+            };
+
+            link.set_active(active);
+        }
+
+        // Toggle a link between the two specified ports on the remote pipewire server.
+        fn toggle_link(&self, port_from: u32, port_to: u32) {
+            let sender = self.pw_sender.get().expect("pw_sender shoud be set");
+            sender
+                .send(crate::GtkMessage::ToggleLink { port_from, port_to })
+                .expect("Failed to send message");
+        }
+
+        /// Remove the link with the specified id from the view.
+        fn remove_link(&self, id: u32) {
+            log::info!("Removing link from graph: id {}", id);
+
+            let Some(link) = self.items.borrow_mut().remove(&id) else {
+                log::warn!("Unknown Link (id={id}) removed from graph");
+                return;
+            };
+            let Ok(link) = link.dynamic_cast::<graph::Link>() else {
+                log::warn!("Graph Manager item under link id {id} is not a link");
+                return;
+            };
+
+            self.obj().graph().remove_link(&link);
+        }
+    }
+}
+
+glib::wrapper! {
+    pub struct GraphManager(ObjectSubclass<imp::GraphManager>);
+}
+
+impl GraphManager {
+    pub fn new(
+        graph: &GraphView,
+        sender: PwSender<GtkMessage>,
+        receiver: glib::Receiver<PipewireMessage>,
+    ) -> Self {
+        let res: Self = glib::Object::builder().property("graph", graph).build();
+
+        res.imp().attach_receiver(receiver);
+        assert!(
+            res.imp().pw_sender.set(sender).is_ok(),
+            "Should be able to set pw_sender)"
+        );
+
+        res
+    }
+}

src/main.rs

@@ -15,6 +15,7 @@
 // SPDX-License-Identifier: GPL-3.0-only
 
 mod application;
+mod graph_manager;
 mod pipewire_connection;
 mod ui;
 
@@ -24,7 +25,7 @@ use pipewire::spa::Direction;
 
 /// Messages sent by the GTK thread to notify the pipewire thread.
 #[derive(Debug, Clone)]
-enum GtkMessage {
+pub enum GtkMessage {
     /// Toggle a link between the two specified ports.
     ToggleLink { port_from: u32, port_to: u32 },
     /// Quit the event loop and let the thread finish.
@@ -33,7 +34,7 @@ enum GtkMessage {
 
 /// Messages sent by the pipewire thread to notify the GTK thread.
 #[derive(Debug, Clone)]
-enum PipewireMessage {
+pub enum PipewireMessage {
     NodeAdded {
         id: u32,
         name: String,
@@ -48,9 +49,7 @@ enum PipewireMessage {
     },
     LinkAdded {
         id: u32,
-        node_from: u32,
         port_from: u32,
-        node_to: u32,
         port_to: u32,
         active: bool,
     },

src/pipewire_connection.rs

@@ -243,9 +243,7 @@ fn handle_link(
                 // TODO -- check other values that might have changed
             } else {
                 // First time we get info. We can now notify the gtk thread of a new link.
-                let node_from = info.output_node_id();
                 let port_from = info.output_port_id();
-                let node_to = info.input_node_id();
                 let port_to = info.input_port_id();
 
                 state.insert(id, Item::Link {
@@ -254,9 +252,7 @@ fn handle_link(
 
                 sender.send(PipewireMessage::LinkAdded {
                     id,
-                    node_from,
                     port_from,
-                    node_to,
                     port_to,
                     active: matches!(info.state(), LinkState::Active)
                 }).expect(

src/ui/graph/graph_view.rs

@@ -22,9 +22,8 @@ use gtk::{
     prelude::*,
     subclass::prelude::*,
 };
-use log::{error, warn};
 
-use std::{cmp::Ordering, collections::HashMap};
+use std::cmp::Ordering;
 
 use super::{Link, Node, Port};
 use crate::NodeType;
@@ -35,6 +34,7 @@ mod imp {
     use super::*;
 
     use std::cell::{Cell, RefCell};
+    use std::collections::{HashMap, HashSet};
 
     use gtk::{
         gdk::{self, RGBA},
@@ -56,9 +56,9 @@ mod imp {
     #[derive(Default)]
     pub struct GraphView {
         /// Stores nodes and their positions.
-        pub(super) nodes: RefCell<HashMap<u32, (Node, Point)>>,
+        pub(super) nodes: RefCell<HashMap<Node, Point>>,
         /// Stores the link and whether it is currently active.
-        pub(super) links: RefCell<HashMap<u32, Link>>,
+        pub(super) links: RefCell<HashSet<Link>>,
         pub hadjustment: RefCell<Option<gtk::Adjustment>>,
         pub vadjustment: RefCell<Option<gtk::Adjustment>>,
         pub zoom_factor: Cell<f64>,
@@ -95,7 +95,7 @@ mod imp {
         fn dispose(&self) {
             self.nodes
                 .borrow()
-                .values()
+                .iter()
                 .for_each(|(node, _)| node.unparent())
         }
 
@@ -152,7 +152,7 @@ mod imp {
         fn size_allocate(&self, _width: i32, _height: i32, baseline: i32) {
             let widget = &*self.obj();
 
-            for (node, point) in self.nodes.borrow().values() {
+            for (node, point) in self.nodes.borrow().iter() {
                 let (_, natural_size) = node.preferred_size();
 
                 let transform = self
@@ -184,7 +184,7 @@ mod imp {
             // Draw all visible children
             self.nodes
                 .borrow()
-                .values()
+                .iter()
                 // Cull nodes from rendering when they are outside the visible canvas area
                 .filter(|(node, _)| alloc.intersect(&node.allocation()).is_some())
                 .for_each(|(node, _)| widget.snapshot_child(node, snapshot));
@@ -430,7 +430,7 @@ mod imp {
                 rgba.alpha().into(),
             );
 
-            for link in self.links.borrow().values() {
+            for link in self.links.borrow().iter() {
                 // TODO: Do not draw links when they are outside the view
                 if let Some((from_x, from_y, to_x, to_y)) = self.get_link_coordinates(link) {
                     link_cr.move_to(from_x, from_y);
@@ -605,7 +605,7 @@ impl GraphView {
         self.set_property("zoom-factor", zoom_factor);
     }
 
-    pub fn add_node(&self, id: u32, node: Node, node_type: Option<NodeType>) {
+    pub fn add_node(&self, node: Node, node_type: Option<NodeType>) {
         let imp = self.imp();
         node.set_parent(self);
 
@@ -622,7 +622,7 @@ impl GraphView {
         let y = imp
             .nodes
             .borrow()
-            .values()
+            .iter()
             .map(|node| {
                 // Map nodes to their locations
                 let point = self.node_position(&node.0.clone().upcast()).unwrap();
@@ -638,68 +638,33 @@ impl GraphView {
             })
             .map_or(20_f32, |(_x, y)| y + 120.0);
 
-        imp.nodes.borrow_mut().insert(id, (node, Point::new(x, y)));
+        imp.nodes.borrow_mut().insert(node, Point::new(x, y));
     }
 
-    pub fn remove_node(&self, id: u32) {
+    pub fn remove_node(&self, node: &Node) {
         let mut nodes = self.imp().nodes.borrow_mut();
-        if let Some((node, _)) = nodes.remove(&id) {
+
+        if nodes.remove(node).is_some() {
             node.unparent();
         } else {
-            warn!("Tried to remove non-existant node (id={}) from graph", id);
+            log::warn!("Tried to remove non-existant node widget from graph");
         }
     }
 
-    pub fn add_port(&self, node_id: u32, port_id: u32, port: Port) {
-        if let Some((node, _)) = self.imp().nodes.borrow_mut().get_mut(&node_id) {
-            node.add_port(port_id, port);
-        } else {
-            error!(
-                "Node with id {} not found when trying to add port with id {} to graph",
-                node_id, port_id
-            );
-        }
-    }
-
-    pub fn remove_port(&self, id: u32, node_id: u32) {
-        let nodes = self.imp().nodes.borrow();
-        if let Some((node, _)) = nodes.get(&node_id) {
-            node.remove_port(id);
-        }
-    }
-
-    pub fn add_link(&self, link_id: u32, link: crate::PipewireLink, active: bool) {
-        let nodes = self.imp().nodes.borrow();
-
-        let output_port = nodes
-            .get(&link.node_from)
-            .and_then(|(node, _)| node.get_port(link.port_from));
-
-        let input_port = nodes
-            .get(&link.node_to)
-            .and_then(|(node, _)| node.get_port(link.port_to));
-
-        let link = Link::new();
-        link.set_input_port(input_port.as_ref());
-        link.set_output_port(output_port.as_ref());
-        link.set_active(active);
-
-        self.imp().links.borrow_mut().insert(link_id, link);
+    pub fn add_link(&self, link: Link) {
+        link.connect_notify_local(
+            Some("active"),
+            glib::clone!(@weak self as graph => move |_, _| {
+                graph.queue_draw();
+            }),
+        );
+        self.imp().links.borrow_mut().insert(link);
         self.queue_draw();
     }
 
-    pub fn set_link_state(&self, link_id: u32, active: bool) {
-        if let Some(link) = self.imp().links.borrow_mut().get_mut(&link_id) {
-            link.set_active(active);
-            self.queue_draw();
-        } else {
-            warn!("Link state changed on unknown link (id={})", link_id);
-        }
-    }
-
-    pub fn remove_link(&self, id: u32) {
+    pub fn remove_link(&self, link: &Link) {
         let mut links = self.imp().links.borrow_mut();
-        links.remove(&id);
+        links.remove(link);
 
         self.queue_draw();
     }
@@ -708,30 +673,22 @@ impl GraphView {
     ///
     /// The returned position is in canvas-space (non-zoomed, (0, 0) fixed in the middle of the canvas).
     pub(super) fn node_position(&self, node: &Node) -> Option<Point> {
-        self.imp()
-            .nodes
-            .borrow()
-            .get(&node.pipewire_id())
-            .map(|(_, point)| *point)
+        self.imp().nodes.borrow().get(node).copied()
     }
 
     pub(super) fn move_node(&self, widget: &Node, point: &Point) {
         let mut nodes = self.imp().nodes.borrow_mut();
-        let node = nodes
-            .get_mut(&widget.pipewire_id())
-            .expect("Node is not on the graph");
+        let node_point = nodes.get_mut(widget).expect("Node is not on the graph");
 
         // Clamp the new position to within the graph, so a node can't be moved outside it and be lost.
-        node.1 = Point::new(
-            point.x().clamp(
-                -(CANVAS_SIZE / 2.0) as f32,
-                (CANVAS_SIZE / 2.0) as f32 - widget.width() as f32,
-            ),
-            point.y().clamp(
-                -(CANVAS_SIZE / 2.0) as f32,
-                (CANVAS_SIZE / 2.0) as f32 - widget.height() as f32,
-            ),
-        );
+        node_point.set_x(point.x().clamp(
+            -(CANVAS_SIZE / 2.0) as f32,
+            (CANVAS_SIZE / 2.0) as f32 - widget.width() as f32,
+        ));
+        node_point.set_y(point.y().clamp(
+            -(CANVAS_SIZE / 2.0) as f32,
+            (CANVAS_SIZE / 2.0) as f32 - widget.height() as f32,
+        ));
 
         self.queue_allocate();
     }

src/ui/graph/node.rs

@@ -17,14 +17,15 @@
 use gtk::{glib, prelude::*, subclass::prelude::*};
 use pipewire::spa::Direction;
 
-use std::collections::HashMap;
-
 use super::Port;
 
 mod imp {
     use super::*;
 
-    use std::cell::{Cell, RefCell};
+    use std::{
+        cell::{Cell, RefCell},
+        collections::HashSet,
+    };
 
     #[derive(glib::Properties)]
     #[properties(wrapper_type = super::Node)]
@@ -41,7 +42,7 @@ mod imp {
             }
         )]
         pub(super) label: gtk::Label,
-        pub(super) ports: RefCell<HashMap<u32, Port>>,
+        pub(super) ports: RefCell<HashSet<Port>>,
         pub(super) num_ports_in: Cell<i32>,
         pub(super) num_ports_out: Cell<i32>,
     }
@@ -74,7 +75,7 @@ mod imp {
                 pipewire_id: Cell::new(0),
                 grid,
                 label,
-                ports: RefCell::new(HashMap::new()),
+                ports: RefCell::new(HashSet::new()),
                 num_ports_in: Cell::new(0),
                 num_ports_out: Cell::new(0),
             }
@@ -120,7 +121,7 @@ impl Node {
             .build()
     }
 
-    pub fn add_port(&mut self, id: u32, port: Port) {
+    pub fn add_port(&self, port: Port) {
         let imp = self.imp();
 
         match port.direction() {
@@ -134,22 +135,20 @@ impl Node {
             }
         }
 
-        imp.ports.borrow_mut().insert(id, port);
+        imp.ports.borrow_mut().insert(port);
     }
 
-    pub fn get_port(&self, id: u32) -> Option<Port> {
-        self.imp().ports.borrow_mut().get(&id).cloned()
-    }
-
-    pub fn remove_port(&self, id: u32) {
+    pub fn remove_port(&self, port: &Port) {
         let imp = self.imp();
-        if let Some(port) = imp.ports.borrow_mut().remove(&id) {
+        if imp.ports.borrow_mut().remove(port) {
             match port.direction() {
                 Direction::Input => imp.num_ports_in.set(imp.num_ports_in.get() - 1),
                 Direction::Output => imp.num_ports_in.set(imp.num_ports_out.get() - 1),
             }
 
             port.unparent();
+        } else {
+            log::warn!("Tried to remove non-existant port widget from node");
         }
     }
 }