app/win: move to serialized draw task in Window which makes drawing strongly atomic eliminating all races

bin/app/src/app/mod.rs

@@ -27,10 +27,10 @@ use crate::android;
 use crate::plugin::PluginSettings;
 use crate::{
     error::Error,
-    gfx::{gfxtag, EpochIndex, GraphicsEventPublisherPtr, Renderer},
+    gfx::{EpochIndex, GraphicsEventPublisherPtr, Renderer},
     prop::{PropertyAtomicGuard, PropertyValue, Role},
     scene::{Pimpl, SceneNode, SceneNodePtr, SceneNodeType},
-    ui::Window,
+    ui::{RedrawTrigger, Window},
     util::i18n::I18nBabelFish,
     ExecutorPtr,
 };
@@ -59,11 +59,25 @@ pub struct App {
     pub renderer: Renderer,
     pub tasks: SyncMutex<Vec<Task<()>>>,
     pub ex: ExecutorPtr,
+    /// Handle for requesting a serialized draw pass from the window's
+    /// draw loop. Passed to `Window::new` (with the receiver) and to
+    /// widget constructors during migration to the draw-pass model.
+    pub redraw_trigger: RedrawTrigger,
+    /// Receiver side of the redraw queue, handed to the window in `setup()`.
+    redraw_rx: async_channel::Receiver<()>,
 }
 
 impl App {
     pub fn new(sg_root: SceneNodePtr, renderer: Renderer, ex: ExecutorPtr) -> Arc<Self> {
-        Arc::new(Self { sg_root, ex, renderer, tasks: SyncMutex::new(vec![]) })
+        let (redraw_trigger, redraw_rx) = RedrawTrigger::new();
+        Arc::new(Self {
+            sg_root,
+            ex,
+            renderer,
+            tasks: SyncMutex::new(vec![]),
+            redraw_trigger,
+            redraw_rx,
+        })
     }
 
     /// Does not require miniquad to be init. Created the scene graph tree / schema and all
@@ -110,7 +124,14 @@ impl App {
         }
         let window = window
             .setup(|me| {
-                Window::new(me, self.renderer.clone(), i18n_fish.clone(), setting_root.clone())
+                Window::new(
+                    me,
+                    self.renderer.clone(),
+                    i18n_fish.clone(),
+                    setting_root.clone(),
+                    self.redraw_trigger.clone(),
+                    self.redraw_rx.clone(),
+                )
             })
             .await;
 
@@ -188,9 +209,10 @@ impl App {
 
         // Access drawable in window node and call draw()
         self.init();
-        //if epoch == 1 {
-        self.trigger_draw().await;
-        //}
+        // Enqueue a draw pass on the window's serialized draw loop.
+        // The bounded(1) queue buffers this until the listener task in
+        // Window::start() is running, so calling before start is safe.
+        self.redraw_trigger.trigger();
 
         self.start_procs(event_pub).await;
         i!("App started");
@@ -212,14 +234,6 @@ impl App {
         }
     }
 
-    async fn trigger_draw(&self) {
-        let atom = &mut self.renderer.make_guard(gfxtag!("App::trigger_draw"));
-        let window_node = self.sg_root.lookup_node("/window").expect("no window attached!");
-        match window_node.pimpl() {
-            Pimpl::Window(win) => win.draw(atom).await,
-            _ => panic!("wrong pimpl"),
-        }
-    }
     async fn start_procs(&self, event_pub: GraphicsEventPublisherPtr) {
         let window_node = self.sg_root.lookup_node("/window").unwrap();
         match window_node.pimpl() {

bin/app/src/ui/mod.rs

@@ -68,6 +68,37 @@ pub use win::{GestureAction, Window, WindowPtr};
 macro_rules! e { ($($arg:tt)*) => { error!(target: "scene::on_modify", $($arg)*); } }
 macro_rules! t { ($($arg:tt)*) => { trace!(target: "scene::on_modify", $($arg)*); } }
 
+/// Handle for requesting a redraw pass from the root window's draw loop.
+/// Cheap to clone. The underlying queue is bounded(1), so triggers sent
+/// while a pass is running or pending are coalesced into a single
+/// additional pass. State mutations must happen before calling `trigger()`
+/// so the resulting pass observes them.
+#[derive(Clone)]
+pub struct RedrawTrigger(async_channel::Sender<()>);
+
+impl RedrawTrigger {
+    /// Create the trigger handle and the receiver consumed by the draw loop.
+    pub fn new() -> (Self, async_channel::Receiver<()>) {
+        let (tx, rx) = async_channel::bounded(1);
+        (Self(tx), rx)
+    }
+
+    /// Request a draw pass. Never blocks. A trigger is only dropped when
+    /// another is already queued, which is equivalent: the queued token
+    /// guarantees a pass that starts after this call, and since callers
+    /// mutate state before triggering, that pass observes the mutation.
+    ///
+    /// Correctness relies on the draw loop draining exactly one token per
+    /// iteration *before* drawing. Do not change the loop to recv after
+    /// the draw or to drain multiple tokens per pass: a full channel means
+    /// a pass is guaranteed, and that guarantee is what makes dropped
+    /// triggers safe. Blocking here would also self-deadlock, since draws
+    /// can trigger further passes.
+    pub fn trigger(&self) {
+        let _ = self.0.try_send(());
+    }
+}
+
 #[async_trait]
 pub trait UIObject: Sync {
     fn priority(&self) -> u32;

bin/app/src/ui/win/mod.rs

@@ -40,7 +40,7 @@ use crate::{
 #[cfg(target_os = "android")]
 use crate::{android, prop::PropertyRect};
 
-use super::{get_children_ordered, get_ui_object3, get_ui_object_ptr, OnModify};
+use super::{get_children_ordered, get_ui_object3, get_ui_object_ptr, OnModify, RedrawTrigger};
 
 mod gesture;
 pub use gesture::{GestureAction, GestureProcessor};
@@ -70,6 +70,10 @@ pub struct Window {
     insets: PropertyRect,
     /// Gesture processor for recognizing gestures
     gesture_proc: SyncMutex<GestureProcessor>,
+    /// Sender side used by window-internal triggers to request a draw pass.
+    redraw_tx: RedrawTrigger,
+    /// Receiver consumed by the single draw-pass listener task in `start()`.
+    redraw_rx: async_channel::Receiver<()>,
 }
 
 impl Window {
@@ -78,6 +82,8 @@ impl Window {
         renderer: Renderer,
         i18n_fish: I18nBabelFish,
         _setting_root: SceneNodePtr,
+        redraw_tx: RedrawTrigger,
+        redraw_rx: async_channel::Receiver<()>,
     ) -> Pimpl {
         let node_ref = &node.upgrade().unwrap();
         let locale = PropertyStr::wrap(node_ref, Role::Internal, "locale", 0).unwrap();
@@ -96,6 +102,8 @@ impl Window {
             #[cfg(target_os = "android")]
             insets: PropertyRect::wrap(node_ref, Role::Internal, "insets").unwrap(),
             gesture_proc: SyncMutex::new(GestureProcessor::new()),
+            redraw_tx,
+            redraw_rx,
         });
 
         Pimpl::Window(self_)
@@ -133,7 +141,26 @@ impl Window {
                 let atom = &mut self_.renderer.make_guard(gfxtag!("Window::resize_task"));
                 // Now update the properties
                 screen_size2.set(atom, size);
+                drop(atom);
 
+                self_.redraw_tx.trigger();
+            }
+        });
+
+        // The serialized draw pass. Single consumer: one pass runs at a
+        // time. Triggers arriving during (or pending at the end of) a pass
+        // are coalesced by the bounded(1) queue into one trailing pass.
+        let me2 = me.clone();
+        let redraw_rx = self.redraw_rx.clone();
+        let redraw_task = ex.spawn(async move {
+            loop {
+                if redraw_rx.recv().await.is_err() {
+                    t!("Redraw trigger queue closed");
+                    break
+                }
+
+                let Some(self_) = me2.upgrade() else { break };
+                let atom = &mut self_.renderer.make_guard(gfxtag!("Window::draw_pass"));
                 self_.draw(atom).await;
             }
         });
@@ -199,17 +226,16 @@ impl Window {
             let atom = &mut batch.spawn();
             self_.reload_locale(atom).await;
         }
-        async fn redraw(self_: Arc<Window>, batch: BatchGuardPtr) {
-            let atom = &mut batch.spawn();
-            self_.draw(atom).await;
-        }
 
         let mut on_modify = OnModify::new(ex.clone(), self.node.clone(), me.clone());
         on_modify.when_change(self.locale.prop(), reload_locale);
-        on_modify.when_change(self.scale.prop(), redraw);
+        on_modify.when_change(self.scale.prop(), |self_, _| async move {
+            self_.redraw_tx.trigger();
+        });
 
         let mut tasks = vec![
             resize_task,
+            redraw_task,
             char_task,
             key_down_task,
             key_up_task,