fix(svg): Android crash, UI stalls, iOS stale frames, hsl() colours and <style> rules

crates/canvas-svg-android/src/lib.rs

@@ -414,19 +414,18 @@ pub extern "system" fn Java_org_nativescript_canvas_svg_NSCSVG_nativeRenderThrea
         return 0;
     };
     let ptr = window.ptr().as_ptr() as *mut std::ffi::c_void;
-    // Held until `nativeRenderThreadDestroy` has joined the thread.
+    // Released by the render thread once `nativeRenderThreadDestroy` has detached the view.
     std::mem::forget(window);
     let thread = canvas_svg_c::gpu::canvas_native_svg_render_thread_create(ptr, width, height, backend);
     if thread.is_null() {
         release_window(ptr);
         return 0;
     }
-    Box::into_raw(Box::new(ThreadHandle { thread, window: ptr })) as jlong
+    Box::into_raw(Box::new(ThreadHandle { thread })) as jlong
 }
 
 struct ThreadHandle {
     thread: *mut canvas_svg_c::gpu::thread::RenderThread,
-    window: *mut std::ffi::c_void,
 }
 
 /// Records on the calling thread; rasterizing happens on the render thread without blocking.
@@ -492,7 +491,11 @@ pub extern "system" fn Java_org_nativescript_canvas_svg_NSCSVG_nativeRenderThrea
         return;
     }
     let handle = unsafe { Box::from_raw(handle as *mut ThreadHandle) };
-    // Blocks until joined and the surface is gone; only then may the window be released.
-    canvas_svg_c::gpu::canvas_native_svg_render_thread_destroy(handle.thread);
-    release_window(handle.window);
+    // Doesn't wait: the render thread releases the window itself once the surface is gone, so a
+    // detaching view doesn't hold the UI thread behind contexts being built for other views.
+    canvas_svg_c::gpu::canvas_native_svg_render_thread_release(handle.thread, release_window_raw);
+}
+
+unsafe extern "C" fn release_window_raw(ptr: *mut std::ffi::c_void) {
+    release_window(ptr);
 }

crates/canvas-svg-c/src/gpu/mod.rs

@@ -209,24 +209,38 @@ impl SvgGpuSurface {
 
     pub fn render(&mut self, doc: &mut SvgDocument, scale: f32) -> FrameStatus {
         let doc = std::cell::RefCell::new(doc);
-        self.paint(&|canvas, width, height| {
-            doc.borrow_mut().0.draw(canvas, width, height, scale)
-        })
+        self.paint(
+            &|canvas, width, height| doc.borrow_mut().0.draw(canvas, width, height, scale),
+            &|| true,
+        )
     }
 
     /// Render-thread path: replays an immutable display list, never touching the document.
-    pub fn present(&mut self, frame: &canvas_svg::RecordedFrame) -> FrameStatus {
-        self.paint(&|canvas, _, _| frame.replay(canvas))
+    /// A failed frame is only rebuilt from while `alive` holds: the owner may have detached
+    /// mid-frame, and a context built against its abandoned window is a wasted one.
+    pub fn present(
+        &mut self,
+        frame: &canvas_svg::RecordedFrame,
+        alive: &dyn Fn() -> bool,
+    ) -> FrameStatus {
+        self.paint(&|canvas, _, _| frame.replay(canvas), alive)
     }
 
-    fn paint(&mut self, paint: &dyn Fn(&skia_safe::Canvas, i32, i32)) -> FrameStatus {
+    fn paint(
+        &mut self,
+        paint: &dyn Fn(&skia_safe::Canvas, i32, i32),
+        alive: &dyn Fn() -> bool,
+    ) -> FrameStatus {
         match self.render_once(paint) {
             Frame::Presented => {
                 self.recoveries = 0;
                 FrameStatus::Presented
             }
             Frame::Skipped => FrameStatus::Skipped,
             Frame::Lost => {
+                if !alive() {
+                    return FrameStatus::Lost;
+                }
                 log::warn!("svg gpu: context lost, rebuilding");
                 if !self.rebuild() {
                     return FrameStatus::Lost;
@@ -414,6 +428,18 @@ pub extern "C" fn canvas_native_svg_render_thread_status(render: *mut RenderThre
     }
 }
 
+/// Detaches without blocking: the thread tears the surface down, then passes the window to
+/// `release` on its own thread, so the caller must not release the window itself.
+#[unsafe(no_mangle)]
+pub extern "C" fn canvas_native_svg_render_thread_release(
+    render: *mut RenderThread,
+    release: thread::ReleaseWindow,
+) {
+    if !render.is_null() {
+        unsafe { Box::from_raw(render) }.release(release);
+    }
+}
+
 /// Blocks until the thread has torn down its surface, since the caller releases the window next.
 #[unsafe(no_mangle)]
 pub extern "C" fn canvas_native_svg_render_thread_destroy(render: *mut RenderThread) {

crates/canvas-svg-c/src/gpu/thread.rs

@@ -1,6 +1,8 @@
 //! One render thread per process replays every threaded view's display lists. Each view's GPU
 //! surface lives only on this thread, since EGL contexts and `VkQueue`s are single-thread.
 
+use std::collections::VecDeque;
+use std::sync::atomic::{AtomicBool, Ordering};
 use std::sync::{Arc, Condvar, Mutex, OnceLock};
 
 use canvas_svg::{FrameSlot, RecordedFrame};
@@ -15,6 +17,9 @@ unsafe impl Send for Window {}
 struct Target {
     slot: FrameSlot,
     state: Mutex<TargetState>,
+    /// Set by the owner as it detaches, so the thread neither builds nor presents to a surface
+    /// whose window is going away.
+    removed: AtomicBool,
 }
 
 #[derive(Default)]
@@ -36,11 +41,22 @@ struct Add {
 /// Set by the render thread once the view's surface is gone.
 type Removed = Arc<(Mutex<bool>, Condvar)>;
 
+/// Hands a view's window back once its surface is gone. Called on the render thread.
+pub type ReleaseWindow = unsafe extern "C" fn(*mut std::ffi::c_void);
+
+/// How the render thread tells a view's owner that its surface is gone.
+enum Removal {
+    /// The owner is blocked in `drop` until this is set.
+    Wait(Removed),
+    /// The owner has moved on, so the thread releases the window itself.
+    Release(Window, ReleaseWindow),
+}
+
 #[derive(Default)]
 struct Queue {
     next_id: u64,
     adds: Vec<Add>,
-    removes: Vec<(u64, Removed)>,
+    removes: Vec<(u64, Removal)>,
     /// Anything to do since the thread last looked: a frame, a resize, an add or a remove.
     dirty: bool,
 }
@@ -81,8 +97,11 @@ fn worker() -> Option<Arc<Worker>> {
 /// A view's registration on the shared render thread.
 pub struct RenderThread {
     id: u64,
+    window: *mut std::ffi::c_void,
     target: Arc<Target>,
     worker: Arc<Worker>,
+    /// Set by [`Self::release`], which has already queued the removal.
+    released: bool,
 }
 
 impl RenderThread {
@@ -98,6 +117,7 @@ impl RenderThread {
         let target = Arc::new(Target {
             slot: FrameSlot::new(),
             state: Mutex::new(TargetState::default()),
+            removed: AtomicBool::new(false),
         });
         let id = {
             let mut queue = worker.queue.lock().ok()?;
@@ -114,7 +134,30 @@ impl RenderThread {
             id
         };
         worker.wake();
-        Some(Self { id, target, worker })
+        Some(Self {
+            id,
+            window,
+            target,
+            worker,
+            released: false,
+        })
+    }
+
+    /// Detaches without waiting: the thread tears the surface down and then passes the window
+    /// to `release`, so the caller must not release it as well. Dropping instead blocks until
+    /// the thread gets to the removal, which can be behind a context being built for every
+    /// other view that just appeared: seconds of frozen UI when a page of them is swapped out.
+    pub fn release(mut self, release: ReleaseWindow) {
+        self.target.removed.store(true, Ordering::Release);
+        self.queue_removal(Removal::Release(Window(self.window), release));
+        self.released = true;
+    }
+
+    fn queue_removal(&self, removal: Removal) {
+        if let Ok(mut queue) = self.worker.queue.lock() {
+            queue.removes.push((self.id, removal));
+        }
+        self.worker.wake();
     }
 
     pub fn commit(&self, frame: RecordedFrame) {
@@ -137,11 +180,12 @@ impl RenderThread {
 
 impl Drop for RenderThread {
     fn drop(&mut self) {
-        let removed: Removed = Arc::new((Mutex::new(false), Condvar::new()));
-        if let Ok(mut queue) = self.worker.queue.lock() {
-            queue.removes.push((self.id, Arc::clone(&removed)));
+        if self.released {
+            return;
         }
-        self.worker.wake();
+        self.target.removed.store(true, Ordering::Release);
+        let removed: Removed = Arc::new((Mutex::new(false), Condvar::new()));
+        self.queue_removal(Removal::Wait(Arc::clone(&removed)));
         // Wait, not detach: GPU teardown after the caller releases the window crashes.
         let (done, signal) = &*removed;
         if let Ok(mut done) = done.lock() {
@@ -173,47 +217,85 @@ fn run(worker: Arc<Worker>) {
             queue.dirty = false;
             (std::mem::take(&mut queue.adds), std::mem::take(&mut queue.removes))
         };
+        let mut adds = VecDeque::from(adds);
+        remove(removes, &mut adds, &mut views);
 
-        // Adds first: a view dropped before its add was seen is in the same batch.
-        for add in adds {
+        // One at a time, each presented as soon as it exists: every surface is a whole GPU
+        // context, and a page of views would otherwise sit blank until the last one is up.
+        // Removals queued meanwhile are taken between them rather than after the lot.
+        loop {
+            remove(take_removes(&worker), &mut adds, &mut views);
+            let Some(add) = adds.pop_front() else { break };
+            if add.target.removed.load(Ordering::Acquire) {
+                // Detached while queued; its removal is already on its way.
+                continue;
+            }
             let surface = SvgGpuSurface::new(add.window.0, add.width, add.height, add.backend);
             if surface.is_none() {
                 // Nobody waits on startup, so report failure as a loss or the view stays blank.
                 if let Ok(mut state) = add.target.state.lock() {
                     state.status = Some(FrameStatus::Lost);
                 }
             }
-            views.push(View {
+            let mut view = View {
                 id: add.id,
                 target: add.target,
                 surface,
-            });
+            };
+            present(&mut view);
+            views.push(view);
         }
 
-        for (id, removed) in removes {
-            // Dropping the view tears its surface down before the owner releases the window.
-            views.retain(|view| view.id != id);
-            let (done, signal) = &*removed;
-            if let Ok(mut done) = done.lock() {
-                *done = true;
-            }
-            signal.notify_all();
+        for view in views.iter_mut() {
+            present(view);
         }
+    }
+}
 
-        for view in views.iter_mut() {
-            let Some(surface) = view.surface.as_mut() else {
-                continue;
-            };
-            let resize = view.target.state.lock().ok().and_then(|mut s| s.resize.take());
-            if let Some((width, height)) = resize {
-                surface.resize(width, height);
-            }
-            if let Some(frame) = view.target.slot.take() {
-                let status = surface.present(&frame);
-                if let Ok(mut state) = view.target.state.lock() {
-                    state.status = Some(status);
+fn take_removes(worker: &Worker) -> Vec<(u64, Removal)> {
+    worker
+        .queue
+        .lock()
+        .map(|mut queue| std::mem::take(&mut queue.removes))
+        .unwrap_or_default()
+}
+
+fn remove(removes: Vec<(u64, Removal)>, adds: &mut VecDeque<Add>, views: &mut Vec<View>) {
+    for (id, removal) in removes {
+        // A view gone before its add was seen never gets a surface.
+        adds.retain(|add| add.id != id);
+        // Dropping the view tears its surface down before the window is released.
+        views.retain(|view| view.id != id);
+        match removal {
+            Removal::Wait(removed) => {
+                let (done, signal) = &*removed;
+                if let Ok(mut done) = done.lock() {
+                    *done = true;
                 }
+                signal.notify_all();
             }
+            Removal::Release(window, release) => unsafe { release(window.0) },
+        }
+    }
+}
+
+fn present(view: &mut View) {
+    // Its window may already be abandoned, and a failed present would rebuild the context.
+    if view.target.removed.load(Ordering::Acquire) {
+        return;
+    }
+    let Some(surface) = view.surface.as_mut() else {
+        return;
+    };
+    let resize = view.target.state.lock().ok().and_then(|mut s| s.resize.take());
+    if let Some((width, height)) = resize {
+        surface.resize(width, height);
+    }
+    if let Some(frame) = view.target.slot.take() {
+        let target = &view.target;
+        let status = surface.present(&frame, &|| !target.removed.load(Ordering::Acquire));
+        if let Ok(mut state) = view.target.state.lock() {
+            state.status = Some(status);
         }
     }
 }
@@ -255,6 +337,27 @@ mod tests {
         assert!(start.elapsed() < Duration::from_secs(5));
     }
 
+    static RELEASED: std::sync::atomic::AtomicUsize = std::sync::atomic::AtomicUsize::new(0);
+
+    unsafe extern "C" fn count_release(_window: *mut std::ffi::c_void) {
+        RELEASED.fetch_add(1, Ordering::SeqCst);
+    }
+
+    #[test]
+    fn release_returns_at_once_and_the_thread_releases_every_window() {
+        let before = RELEASED.load(Ordering::SeqCst);
+        for _ in 0..50 {
+            RenderThread::new(std::ptr::null_mut(), 10, 10, Backend::Auto)
+                .unwrap()
+                .release(count_release);
+        }
+        let deadline = Instant::now() + Duration::from_secs(5);
+        while RELEASED.load(Ordering::SeqCst) < before + 50 && Instant::now() < deadline {
+            std::thread::sleep(Duration::from_millis(5));
+        }
+        assert!(RELEASED.load(Ordering::SeqCst) >= before + 50);
+    }
+
     #[test]
     fn commits_and_resizes_for_a_dead_view_are_ignored() {
         let view = RenderThread::new(std::ptr::null_mut(), 10, 10, Backend::Auto).unwrap();

crates/canvas-svg-napi/__test__/svg.test.mjs

@@ -40,7 +40,7 @@ test('installs global.SVGModule with the V8 bindings\' members', () => {
 test('parses a document and renders it as RGBA, or BGRA on request', () => {
 	const document = SVGModule.createSVGDocument(RED_SQUARE);
 	assert.equal(document.root().tagName(), 'svg');
-	assert.ok(document.nativePointer() > 0);
+	assert.ok(BigInt(document.nativePointer()) > 0n);
 	const rgba = render(document, 20, 10);
 	assert.deepEqual(rgba(5, 5), [255, 0, 0, 255]);
 	assert.deepEqual(rgba(15, 5), [0, 0, 255, 255]);

crates/canvas-svg-napi/src/lib.rs

@@ -228,10 +228,11 @@ impl SVGDocument {
     canvas_svg_c::canvas_native_svg_document_invalidate_frames(self.document);
   }
 
-  /// The canvas-svg-c document pointer, as a number (a native renderer takes it).
+  /// The canvas-svg-c document pointer (a native renderer takes it), as a decimal string like
+  /// the V8 bindings: a double can't hold every 64-bit pointer.
   #[napi]
-  pub fn native_pointer(&self) -> f64 {
-    self.document as usize as f64
+  pub fn native_pointer(&self) -> String {
+    (self.document as usize).to_string()
   }
 
   /// Renders the current frame into `buffer` (any typed array, `width` x `height` premultiplied

crates/canvas-svg/Cargo.toml

@@ -7,6 +7,7 @@ edition = "2021"
 skia-safe = { workspace = true, features = ["svg", "textlayout"] }
 canvas-2d = { workspace = true, default-features = false }
 quick-xml = { workspace = true }
+csscolorparser = "0.7.1"
 
 [dev-dependencies]
 skia-safe = { workspace = true, features = ["svg", "textlayout"] }

crates/canvas-svg/src/attr.rs

@@ -248,6 +248,12 @@ fn get_specific_attribute(node: &TypedNode, name: &str) -> Option<String> {
 }
 
 pub fn set_attribute(node: &mut TypedNode, name: &str, value: &str) -> bool {
+    set_normalized_attribute(node, name, &crate::css_color::normalize(value))
+}
+
+/// For values that can't hold a colour Skia rejects: parsed markup and SMIL frames, which are
+/// rewritten when the document loads. Keeps the rewrite off the per-frame path.
+pub(crate) fn set_normalized_attribute(node: &mut TypedNode, name: &str, value: &str) -> bool {
     if name == "id" {
         // Tracked by `SvgDocument`.
         return true;