From 866661092039ac9d2affbad6657555e5af5e78e5 Mon Sep 17 00:00:00 2001 From: Aitor Moreno Date: Wed, 29 Apr 2026 12:12:27 +0200 Subject: [PATCH] :recycle: Change how we deal with the render_loop --- frontend/src/app/render_wasm/api.cljs | 25 +++++++++++------ frontend/src/app/render_wasm/wasm.cljs | 32 +++++++++++++++------ render-wasm/src/js/wapi.js | 14 --------- render-wasm/src/main.rs | 29 ++++--------------- render-wasm/src/render.rs | 39 +++++++++++--------------- render-wasm/src/state.rs | 17 ++++------- render-wasm/src/wapi.rs | 39 -------------------------- 7 files changed, 67 insertions(+), 128 deletions(-) diff --git a/frontend/src/app/render_wasm/api.cljs b/frontend/src/app/render_wasm/api.cljs index aba6fc7970..05379ed2c6 100644 --- a/frontend/src/app/render_wasm/api.cljs +++ b/frontend/src/app/render_wasm/api.cljs @@ -268,11 +268,20 @@ (declare request-render) (declare set-shape-vertical-align fonts-from-text-content) -;; This should never be called from the outside. +;; Performs a render (defn- render + ([] + (render (js/performance.now))) + ([timestamp] + (let [queue-frame (h/call wasm/internal-module "_render" timestamp)] + (when (= queue-frame 1) + (request-render "queue-frame"))))) + +;; This should never be called from the outside. +(defn- on-render [timestamp] (when (and wasm/context-initialized? (not @wasm/context-lost?)) - (h/call wasm/internal-module "_render" timestamp) + (render timestamp) ;; Update text editor blink (so cursor toggles) using the same timestamp (try @@ -297,14 +306,12 @@ (catch :default e (js/console.error "text-editor overlay/update failed:" e))) - (set! wasm/internal-frame-id nil) (ug/dispatch! (ug/event "penpot:wasm:render")))) (defn render-sync [] (when (and wasm/context-initialized? (not @wasm/context-lost?)) - (h/call wasm/internal-module "_render_sync") - (set! wasm/internal-frame-id nil))) + (h/call wasm/internal-module "_render_sync"))) (defn render-sync-shape [id] @@ -314,8 +321,7 @@ (aget buffer 0) (aget buffer 1) (aget buffer 2) - (aget buffer 3)) - (set! wasm/internal-frame-id nil)))) + (aget buffer 3))))) (defonce shapes-loading? (atom false)) (defonce deferred-render? (atom false)) @@ -327,7 +333,7 @@ (not @wasm/disable-request-render?)) (if @shapes-loading? (reset! deferred-render? true) - (wasm/request-frame render)))) + (wasm/request-frame)))) (defn- begin-shapes-loading! [] @@ -1081,7 +1087,7 @@ ;; completes in the first frame. For zoom, interest- ;; area tiles (~3 tile margin) don't block the main ;; thread. - (h/call wasm/internal-module "_render" 0)))] + (render)))] (fns/debounce do-render DEBOUNCE_DELAY_MS))) (defn set-view-box @@ -1965,6 +1971,7 @@ (p/fmap (fn [default] (set! wasm/internal-module default) + (wasm/set-frame-function on-render) true)) (p/merr (fn [cause] diff --git a/frontend/src/app/render_wasm/wasm.cljs b/frontend/src/app/render_wasm/wasm.cljs index d5d6b9aeaf..dd409e2678 100644 --- a/frontend/src/app/render_wasm/wasm.cljs +++ b/frontend/src/app/render_wasm/wasm.cljs @@ -8,8 +8,14 @@ (:require ["./api/shared.js" :as shared])) (defonce internal-frame-id nil) +(defonce internal-frame-fn nil) + +(defonce internal-stats #js {:requests 0 :cancelled 0 :queued 0 :frames 0}) + (defonce internal-module #js {}) +(unchecked-set js/globalThis "internalStats" internal-stats) + ;; Is a frame requested? (defn frame-requested? "Returns true if a frame was requested" @@ -20,25 +26,33 @@ (defn cancel-frame "Cancels the current requested frame" [] - (when-not (nil? internal-frame-id) + (when (frame-requested?) + (unchecked-set internal-stats "cancelled" (inc (unchecked-get internal-stats "cancelled"))) (js/cancelAnimationFrame internal-frame-id) (set! internal-frame-id nil) true) false) -(defn create-frame-delegate - "Creates a new frame delegate" +(defn- on-frame + "The frame function" + [timestamp] + (let [frame-id internal-frame-id] + (set! internal-frame-id nil) + (unchecked-set internal-stats "frames" (inc (unchecked-get internal-stats "frames"))) + (internal-frame-fn timestamp frame-id))) + +(defn set-frame-function [f] - (fn frame-delegate [timestamp] - (let [frame-id internal-frame-id] - (set! internal-frame-id nil) - (f timestamp frame-id)))) + (set! internal-frame-fn f)) ;; Requests a frame (defn request-frame "Requests a new frame" - [f] - (set! internal-frame-id (js/requestAnimationFrame (create-frame-delegate f)))) + [] + (unchecked-set internal-stats "requests" (inc (unchecked-get internal-stats "requests"))) + (when-not (frame-requested?) + (unchecked-set internal-stats "queued" (inc (unchecked-get internal-stats "queued"))) + (set! internal-frame-id (js/requestAnimationFrame on-frame)))) ;; Reference to the HTML canvas element. (defonce canvas nil) diff --git a/render-wasm/src/js/wapi.js b/render-wasm/src/js/wapi.js index 13f3fcb698..90f6bccf6d 100644 --- a/render-wasm/src/js/wapi.js +++ b/render-wasm/src/js/wapi.js @@ -1,18 +1,4 @@ addToLibrary({ - wapi_requestAnimationFrame: function wapi_requestAnimationFrame() { - if (typeof WorkerGlobalScope !== 'undefined' && self instanceof WorkerGlobalScope) { - setTimeout(Module._process_animation_frame); - } else { - return window.requestAnimationFrame(Module._process_animation_frame); - } - }, - wapi_cancelAnimationFrame: function wapi_cancelAnimationFrame(frameId) { - if (typeof WorkerGlobalScope !== 'undefined' && self instanceof WorkerGlobalScope) { - clearTimeout(frameId); - } else { - return window.cancelAnimationFrame(frameId); - } - }, wapi_notifyTilesRenderComplete: function wapi_notifyTilesRenderComplete() { // The corresponding listener lives on `document` (main thread), so in a // worker context we simply skip the dispatch instead of crashing. diff --git a/render-wasm/src/main.rs b/render-wasm/src/main.rs index 962462216e..61dffb7b36 100644 --- a/render-wasm/src/main.rs +++ b/render-wasm/src/main.rs @@ -18,6 +18,8 @@ use std::collections::HashMap; #[allow(unused_imports)] use crate::error::{Error, Result}; +use crate::render::QueueFrame; + use macros::wasm_error; use math::{Bounds, Matrix}; use mem::SerializableResult; @@ -123,12 +125,6 @@ pub extern "C" fn set_browser(browser: u8) -> Result<()> { #[no_mangle] #[wasm_error] pub extern "C" fn clean_up() -> Result<()> { - with_state_mut!(state, { - // Cancel the current animation frame if it exists so - // it won't try to render without context - let render_state = state.render_state_mut(); - render_state.cancel_animation_frame(); - }); unsafe { STATE = None } mem::free_bytes()?; Ok(()) @@ -222,7 +218,7 @@ pub extern "C" fn set_canvas_background(raw_color: u32) -> Result<()> { #[no_mangle] #[wasm_error] -pub extern "C" fn render(_: i32) -> Result<()> { +pub extern "C" fn render(_: i32) -> Result { with_state_mut!(state, { state.rebuild_touched_tiles(); // Drain the throttled modifier-tile invalidation accumulated @@ -236,11 +232,10 @@ pub extern "C" fn render(_: i32) -> Result<()> { state.rebuild_modifier_tiles(ids)?; } } - state - .start_render_loop(performance::get_time()) - .map_err(|_| Error::RecoverableError("Error rendering".to_string()))?; + return state + .start_render_loop(performance::get_time()) + .map_err(|_| Error::RecoverableError("Error rendering".to_string())); }); - Ok(()) } #[no_mangle] @@ -342,16 +337,6 @@ pub extern "C" fn render_loading_overlay() -> Result<()> { Ok(()) } -#[no_mangle] -#[wasm_error] -pub extern "C" fn process_animation_frame(timestamp: i32) -> Result<()> { - let result = with_state_mut!(state, { state.process_animation_frame(timestamp) }); - if let Err(err) = result { - eprintln!("process_animation_frame error: {}", err); - } - Ok(()) -} - #[no_mangle] #[wasm_error] pub extern "C" fn reset_canvas() -> Result<()> { @@ -411,7 +396,6 @@ pub extern "C" fn set_view_end() -> Result<()> { with_state_mut!(state, { performance::begin_measure!("set_view_end"); state.render_state.options.set_fast_mode(false); - state.render_state.cancel_animation_frame(); let scale = state.render_state.get_scale(); state @@ -471,7 +455,6 @@ pub extern "C" fn set_modifiers_end() -> Result<()> { performance::begin_measure!("set_modifiers_end"); state.render_state.options.set_fast_mode(false); state.render_state.options.set_interactive_transform(false); - state.render_state.cancel_animation_frame(); performance::end_measure!("set_modifiers_end"); }); Ok(()) diff --git a/render-wasm/src/render.rs b/render-wasm/src/render.rs index c1f08e76e6..b93ac98b79 100644 --- a/render-wasm/src/render.rs +++ b/render-wasm/src/render.rs @@ -39,6 +39,12 @@ pub use images::*; type ClipStack = Vec<(Rect, Option, Matrix)>; +#[repr(u8)] +pub enum QueueFrame { + Yes = 1, + No = 0, +} + #[derive(Debug)] pub struct NodeRenderState { pub id: Uuid, @@ -336,8 +342,6 @@ pub(crate) struct RenderState { pub cached_viewbox: Viewbox, pub images: ImageStore, pub background_color: skia::Color, - // Identifier of the current requestAnimationFrame call, if any. - pub render_request_id: Option, // Indicates whether the rendering process has pending frames. pub render_in_progress: bool, // Stack of nodes pending to be rendered. @@ -508,7 +512,6 @@ impl RenderState { cached_viewbox: Viewbox::new(0., 0.), images: ImageStore::new(gpu_state.context.clone()), background_color: skia::Color::TRANSPARENT, - render_request_id: None, render_in_progress: false, pending_nodes: vec![], current_tile: None, @@ -1608,14 +1611,6 @@ impl RenderState { self.surfaces.update_render_context(self.render_area, scale); } - pub fn cancel_animation_frame(&mut self) { - if self.render_in_progress { - if let Some(frame_id) = self.render_request_id { - wapi::cancel_animation_frame!(frame_id); - } - } - } - fn rebuild_backbuffer_crop_cache(&mut self, tree: ShapesPoolRef) { self.backbuffer_crop_cache.clear(); @@ -1864,7 +1859,7 @@ impl RenderState { performance::end_measure!("render_from_cache"); performance::end_timed_log!("render_from_cache", _start); - + } pub fn start_render_loop( &mut self, @@ -1872,7 +1867,7 @@ impl RenderState { tree: ShapesPoolRef, timestamp: i32, sync_render: bool, - ) -> Result<()> { + ) -> Result { #[cfg(feature = "stats")] self.stats.clear(); @@ -1965,10 +1960,11 @@ impl RenderState { self.apply_drawing_to_render_canvas(None, SurfaceId::Current); + let mut result = QueueFrame::No; if sync_render { self.render_shape_tree_sync(base_object, tree, timestamp)?; } else { - self.process_animation_frame(base_object, tree, timestamp)?; + result = self.render_loop_step(base_object, tree, timestamp)?; // Update cached_viewbox after visible tiles render // synchronously so that render_from_cache uses the correct // zoom ratio even if interest-area tiles are still rendering @@ -1983,7 +1979,7 @@ impl RenderState { performance::end_measure!("start_render_loop"); performance::end_timed_log!("start_render_loop", _start); - Ok(()) + Ok(result) } fn compute_document_bounds( @@ -2017,12 +2013,12 @@ impl RenderState { acc } - pub fn process_animation_frame( + pub fn render_loop_step( &mut self, base_object: Option<&Uuid>, tree: ShapesPoolRef, timestamp: i32, - ) -> Result<()> { + ) -> Result { performance::begin_measure!("process_animation_frame"); if self.render_in_progress { if tree.len() != 0 { @@ -2043,8 +2039,7 @@ impl RenderState { } if self.render_in_progress { - self.cancel_animation_frame(); - self.render_request_id = Some(wapi::request_animation_frame!()); + return Ok(QueueFrame::Yes); } else { // A full-quality frame is now complete. Refresh Backbuffer and regenerate // the per-shape crop cache so interactive drags can reuse pixels. @@ -2057,7 +2052,7 @@ impl RenderState { } } performance::end_measure!("process_animation_frame"); - Ok(()) + Ok(QueueFrame::No) } pub fn render_shape_tree_sync( @@ -2065,12 +2060,12 @@ impl RenderState { base_object: Option<&Uuid>, tree: ShapesPoolRef, timestamp: i32, - ) -> Result<()> { + ) -> Result { if tree.len() != 0 { self.render_shape_tree_partial(base_object, tree, timestamp, false)?; } self.flush_and_submit(); - Ok(()) + Ok(QueueFrame::No) } pub fn render_shape_pixels( diff --git a/render-wasm/src/state.rs b/render-wasm/src/state.rs index 0f666fe0b0..e9d30a0d2f 100644 --- a/render-wasm/src/state.rs +++ b/render-wasm/src/state.rs @@ -7,13 +7,11 @@ pub use shapes_pool::{ShapesPool, ShapesPoolMutRef, ShapesPoolRef}; pub use text_editor::*; use crate::error::{Error, Result}; -use crate::render::RenderState; -use crate::shapes::Shape; +use crate::render::{RenderState, QueueFrame}; +use crate::shapes::{Shape, modifiers::grid_layout::grid_cell_data}; use crate::tiles; use crate::uuid::Uuid; -use crate::shapes::modifiers::grid_layout::grid_cell_data; - /// This struct holds the state of the Rust application between JS calls. /// /// It is created by [init] and passed to the other exported functions. @@ -93,12 +91,12 @@ impl State { self.render_state.render_from_cache(&self.shapes); } - pub fn render_sync(&mut self, timestamp: i32) -> Result<()> { + pub fn render_sync(&mut self, timestamp: i32) -> Result { self.render_state .start_render_loop(None, &self.shapes, timestamp, true) } - pub fn render_sync_shape(&mut self, id: &Uuid, timestamp: i32) -> Result<()> { + pub fn render_sync_shape(&mut self, id: &Uuid, timestamp: i32) -> Result { self.render_state .start_render_loop(Some(id), &self.shapes, timestamp, true) } @@ -113,7 +111,7 @@ impl State { .render_shape_pixels(id, &self.shapes, scale, timestamp) } - pub fn start_render_loop(&mut self, timestamp: i32) -> Result<()> { + pub fn start_render_loop(&mut self, timestamp: i32) -> Result { // If zoom changed (e.g. interrupted zoom render followed by pan), the // tile index may be stale for the new viewport position. Rebuild the // index so shapes are mapped to the correct tiles. We use @@ -128,11 +126,6 @@ impl State { .start_render_loop(None, &self.shapes, timestamp, false) } - pub fn process_animation_frame(&mut self, timestamp: i32) -> Result<()> { - self.render_state - .process_animation_frame(None, &self.shapes, timestamp) - } - pub fn clear_focus_mode(&mut self) { self.render_state.clear_focus_mode(); } diff --git a/render-wasm/src/wapi.rs b/render-wasm/src/wapi.rs index f9e7e65769..5d4ef780db 100644 --- a/render-wasm/src/wapi.rs +++ b/render-wasm/src/wapi.rs @@ -1,40 +1,3 @@ -#[macro_export] -macro_rules! request_animation_frame { - () => {{ - #[cfg(target_arch = "wasm32")] - unsafe extern "C" { - pub fn wapi_requestAnimationFrame() -> i32; - } - - #[cfg(target_arch = "wasm32")] - let result = unsafe { wapi_requestAnimationFrame() }; - #[cfg(not(target_arch = "wasm32"))] - let result = 0; - - result - }}; -} - -#[macro_export] -macro_rules! cancel_animation_frame { - ($frame_id:expr) => { - #[cfg(target_arch = "wasm32")] - unsafe extern "C" { - pub fn wapi_cancelAnimationFrame(frame_id: i32); - } - - { - let frame_id = $frame_id; - #[cfg(target_arch = "wasm32")] - unsafe { - wapi_cancelAnimationFrame(frame_id) - }; - #[cfg(not(target_arch = "wasm32"))] - let _ = frame_id; - } - }; -} - #[macro_export] macro_rules! notify_tiles_render_complete { () => {{ @@ -50,6 +13,4 @@ macro_rules! notify_tiles_render_complete { }}; } -pub use cancel_animation_frame; pub use notify_tiles_render_complete; -pub use request_animation_frame;