From b17afb0d5ebcf792d5395aaeb92c1d45805c20fa Mon Sep 17 00:00:00 2001 From: Elena Torro Date: Mon, 20 Apr 2026 13:24:25 +0200 Subject: [PATCH] :wrench: Fix thumbnail and shape generation after tokens propagation --- frontend/src/app/main/data/changes.cljs | 20 ++- .../app/main/data/workspace/libraries.cljs | 8 +- .../data/workspace/tokens/propagation.cljs | 83 +++++++++-- .../test/frontend_tests/helpers/wasm.cljs | 22 +++ .../logic/components_and_tokens.cljs | 130 ++++++++++-------- render-wasm/src/render/images.rs | 4 + 6 files changed, 193 insertions(+), 74 deletions(-) diff --git a/frontend/src/app/main/data/changes.cljs b/frontend/src/app/main/data/changes.cljs index 7cae1add0f..4ff9d2b6f4 100644 --- a/frontend/src/app/main/data/changes.cljs +++ b/frontend/src/app/main/data/changes.cljs @@ -27,6 +27,12 @@ (def page-change? #{:add-page :mod-page :del-page :mov-page}) + +;; Flag to temporarily pause WASM synchronization during batch operations +(defonce wasm-sync-paused? (atom false)) +(defn pause-wasm-sync! [] (reset! wasm-sync-paused? true)) +(defn resume-wasm-sync! [] (reset! wasm-sync-paused? false)) + (def update-layout-attr? #{:hidden}) @@ -103,7 +109,9 @@ pids (into #{} xf:map-page-id redo-changes)] (reduce #(ctst/update-object-indices %1 %2) fdata pids)))] - (if (and (not ignore-wasm?) (features/active-feature? state "render-wasm/v1")) + (if (and (not ignore-wasm?) + (not @wasm-sync-paused?) + (features/active-feature? state "render-wasm/v1")) ;; Update the wasm model (let [shape-changes (volatile! {}) @@ -116,13 +124,14 @@ state) - ;; wasm renderer deactivated + ;; wasm renderer deactivated (or paused) (update-in state [:files file-id :data] apply-changes)))))) (defn commit "Create a commit event instance" [{:keys [commit-id redo-changes undo-changes origin save-undo? features - file-id file-revn file-vern undo-group tags stack-undo? source ignore-wasm?]}] + file-id file-revn file-vern undo-group tags stack-undo? source + ignore-wasm? skip-component-changes?]}] (assert (cpc/check-changes redo-changes) "expect valid vector of changes for redo-changes") @@ -148,7 +157,8 @@ :undo-group undo-group :tags tags :stack-undo? stack-undo? - :ignore-wasm? ignore-wasm?}] + :ignore-wasm? ignore-wasm? + :skip-component-changes? skip-component-changes?}] (ptk/reify ::commit cljs.core/IDeref @@ -214,6 +224,8 @@ (assoc :file-id file-id) (assoc :file-revn (resolve-file-revn state file-id)) (assoc :file-vern (resolve-file-vern state file-id)) + (cond-> @wasm-sync-paused? + (assoc :skip-component-changes? true)) (assoc :undo-changes uchg) (assoc :redo-changes rchg) (commit)))))))) diff --git a/frontend/src/app/main/data/workspace/libraries.cljs b/frontend/src/app/main/data/workspace/libraries.cljs index 28057cfd09..07ca6285cf 100644 --- a/frontend/src/app/main/data/workspace/libraries.cljs +++ b/frontend/src/app/main/data/workspace/libraries.cljs @@ -955,9 +955,8 @@ root-id (:main-instance-id component)] (if (and (= tag "component") (features/active-feature? state "render-wasm/v1")) - ;; WASM: render immediately, UI only — server persist happens - ;; on the debounced path (update-component-thumbnail) - (dwt.wasm/render-thumbnail file-id page-id root-id) + (when-not @dch/wasm-sync-paused? + (dwt.wasm/render-thumbnail file-id page-id root-id)) (dwt/update-thumbnail file-id page-id root-id tag "update-component-thumbnail-sync")))) (defn update-component-sync @@ -1372,6 +1371,9 @@ (rx/filter dch/commit?) (rx/map deref) (rx/filter #(= :local (:source %))) + ;; Filter out propagation commits that are originated by component changes, + ;; to avoid infinite loops. + (rx/filter #(not (:skip-component-changes? %))) (rx/observe-on :async)) check-changes diff --git a/frontend/src/app/main/data/workspace/tokens/propagation.cljs b/frontend/src/app/main/data/workspace/tokens/propagation.cljs index d20b095e98..9d0d5bd9b6 100644 --- a/frontend/src/app/main/data/workspace/tokens/propagation.cljs +++ b/frontend/src/app/main/data/workspace/tokens/propagation.cljs @@ -10,16 +10,21 @@ [app.common.files.helpers :as cfh] [app.common.logging :as l] [app.common.time :as ct] + [app.common.types.components-list :as ctkl] [app.common.types.token :as ctt] [app.common.types.tokens-lib :as ctob] [app.config :as cf] + [app.main.data.changes :as dch] [app.main.data.helpers :as dsh] [app.main.data.style-dictionary :as sd] [app.main.data.tokenscript :as ts] [app.main.data.workspace.shapes :as dwsh] [app.main.data.workspace.thumbnails :as dwt] + [app.main.data.workspace.thumbnails-wasm :as dwt.wasm] [app.main.data.workspace.tokens.application :as dwta] [app.main.data.workspace.undo :as dwu] + [app.main.features :as features] + [app.render-wasm.api :as wasm.api] [beicon.v2.core :as rx] [clojure.data :as data] [clojure.set :as set] @@ -144,6 +149,7 @@ (rx/of current-page-id) (->> (rx/from (:pages fdata)) (rx/filter (fn [id] (not= id current-page-id))))) + (rx/observe-on :async) ;; Do not block UI on token propagation (rx/mapcat (fn [page-id] (let [page @@ -170,8 +176,14 @@ (->> (rx/from frame-ids) (rx/mapcat (fn [frame-id] - (rx/of (dwt/clear-thumbnail file-id page-id frame-id "frame") - (dwt/clear-thumbnail file-id page-id frame-id "component"))))) + (rx/concat + (rx/of (dwt/clear-thumbnail file-id page-id frame-id "frame")) + ;; WASM: skip component thumbnails — they are rendered + ;; lazily when the assets panel is opened, so clearing + ;; them here would trigger a flood of render-shape-pixels calls. + (if (features/active-feature? state "render-wasm/v1") + (rx/empty) + (rx/of (dwt/clear-thumbnail file-id page-id frame-id "component"))))))) (when (not= page-id current-page-id) ;; Texts in the current page have already their position-data regenerated (rx/of (dwsh/update-shapes text-ids ;; after change. But those on other pages need to be specifically reset. (fn [shape] @@ -183,6 +195,37 @@ (let [elapsed (tpoint)] (l/inf :status "END" :hint "propagate-tokens" :elapsed elapsed))))))) +(defn- reload-wasm-current-page + "Bulk-reload all current-page shapes into WASM after token + propagation has updated the data model without per-shape sync. + After reload, refreshes all component thumbnails progressively, + current-page components first, then components on other pages." + [] + (ptk/reify ::reload-wasm-current-page + ptk/WatchEvent + (watch [_ state _] + (let [objects (dsh/lookup-page-objects state) + file-id (get state :current-file-id) + page-id (get state :current-page-id) + fdata (dsh/lookup-file-data state file-id) + components (ctkl/components-seq fdata) + ;; Current-page components first (already loaded in WASM), + ;; then other pages (render-thumbnail loads their subtree) + sorted (sort-by #(if (= (:main-instance-page %) page-id) 0 1) components)] + + (->> (rx/create + (fn [subs] + (wasm.api/set-objects objects + (fn [] + (wasm.api/request-render "token-propagation") + (rx/push! subs true) + (rx/end! subs))))) + (rx/mapcat + (fn [_] + (->> (rx/from sorted) + (rx/map (fn [{:keys [main-instance-page main-instance-id]}] + (dwt.wasm/render-thumbnail file-id main-instance-page main-instance-id :persist? true))))))))))) + (defn propagate-workspace-tokens [] (ptk/reify ::propagate-workspace-tokens @@ -191,13 +234,29 @@ (when-let [tokens-tree (-> (dsh/lookup-file-data state) (get :tokens-lib) (ctob/get-tokens-in-active-sets))] - (->> (if (contains? cf/flags :tokenscript) - (rx/of (-> (ts/resolve-tokens tokens-tree) - (d/update-vals #(update % :resolved-value ts/tokenscript-symbols->penpot-unit)))) - (sd/resolve-tokens tokens-tree)) - (rx/mapcat (fn [sd-tokens] - (let [undo-id (js/Symbol)] - (rx/concat - (rx/of (dwu/start-undo-transaction undo-id :timeout false)) - (propagate-tokens state sd-tokens) - (rx/of (dwu/commit-undo-transaction undo-id))))))))))) + (let [wasm? (features/active-feature? state "render-wasm/v1")] + ;; Pause per-shape WASM sync so token propagation commits + ;; only update the data model, bulk-reload WASM at the end. + (when wasm? + (dch/pause-wasm-sync!)) + (->> ;; Yield one frame so the UI can repaint (close dropdown, + ;; show loading state, etc.) before the heavy work starts. + (rx/of nil) + (rx/observe-on :async) + (rx/mapcat + (fn [_] + (if (contains? cf/flags :tokenscript) + (rx/of (-> (ts/resolve-tokens tokens-tree) + (d/update-vals #(update % :resolved-value ts/tokenscript-symbols->penpot-unit)))) + (sd/resolve-tokens tokens-tree)))) + (rx/mapcat (fn [sd-tokens] + (let [undo-id (js/Symbol)] + (rx/concat + (rx/of (dwu/start-undo-transaction undo-id :timeout false)) + (propagate-tokens state sd-tokens) + (rx/of (dwu/commit-undo-transaction undo-id)) + (when wasm? + (rx/of (reload-wasm-current-page))))))) + (rx/finalize (fn [] + (when wasm? + (dch/resume-wasm-sync!)))))))))) diff --git a/frontend/test/frontend_tests/helpers/wasm.cljs b/frontend/test/frontend_tests/helpers/wasm.cljs index f64d7b7d59..928bed057b 100644 --- a/frontend/test/frontend_tests/helpers/wasm.cljs +++ b/frontend/test/frontend_tests/helpers/wasm.cljs @@ -119,6 +119,22 @@ (track! :get-content-fonts) []) +(defn- mock-set-objects + "Mock for `wasm.api/set-objects`. + Accepts the same arities as the real function. Does NOT invoke + the render-callback — the real one touches WASM/Skia internals." + ([_objects] + (track! :set-objects)) + ([_objects _render-callback] + (track! :set-objects)) + ([_objects _render-callback _on-shapes-ready] + (track! :set-objects))) + +(defn- mock-request-render + [_requester] + (track! :request-render) + nil) + ;; --- Persistent mock installation via `set!` -------------------------- ;; ;; These use `set!` to directly mutate the module-level JS vars, making @@ -145,6 +161,8 @@ :set-shape-text-content wasm.api/set-shape-text-content :set-shape-text-images wasm.api/set-shape-text-images :get-text-dimensions wasm.api/get-text-dimensions + :set-objects wasm.api/set-objects + :request-render wasm.api/request-render :font-stored? wasm.fonts/font-stored? :make-font-data wasm.fonts/make-font-data :get-content-fonts wasm.fonts/get-content-fonts}) @@ -156,6 +174,8 @@ (set! wasm.api/set-shape-text-content mock-set-shape-text-content) (set! wasm.api/set-shape-text-images mock-set-shape-text-images) (set! wasm.api/get-text-dimensions mock-get-text-dimensions) + (set! wasm.api/set-objects mock-set-objects) + (set! wasm.api/request-render mock-request-render) (set! wasm.fonts/font-stored? mock-font-stored?) (set! wasm.fonts/make-font-data mock-make-font-data) (set! wasm.fonts/get-content-fonts mock-get-content-fonts)) @@ -171,6 +191,8 @@ (set! wasm.api/set-shape-text-content (:set-shape-text-content orig)) (set! wasm.api/set-shape-text-images (:set-shape-text-images orig)) (set! wasm.api/get-text-dimensions (:get-text-dimensions orig)) + (set! wasm.api/set-objects (:set-objects orig)) + (set! wasm.api/request-render (:request-render orig)) (set! wasm.fonts/font-stored? (:font-stored? orig)) (set! wasm.fonts/make-font-data (:make-font-data orig)) (set! wasm.fonts/get-content-fonts (:get-content-fonts orig))) diff --git a/frontend/test/frontend_tests/logic/components_and_tokens.cljs b/frontend/test/frontend_tests/logic/components_and_tokens.cljs index 2ec37b8db1..028eb9b046 100644 --- a/frontend/test/frontend_tests/logic/components_and_tokens.cljs +++ b/frontend/test/frontend_tests/logic/components_and_tokens.cljs @@ -217,27 +217,39 @@ :type :border-radius :value 66})] + ;; propagate-workspace-tokens defers work via rx/timer 0, + ;; so we use the timeout-based stopper to let it complete, + ;; then sync-file in a subsequent step. step2 (fn [_] - (let [events2 [(dwtp/propagate-workspace-tokens) - (dwl/sync-file (:id file) (:id file))]] - (tohs/run-store-async - store done events2 - (fn [new-state] - (let [;; ==== Get - file' (ths/get-file-from-state new-state) - c-frame1' (cths/get-shape file' :c-frame1) - tokens-frame1' (:applied-tokens c-frame1')] + (tohs/run-store-async + store + (fn [_] + (tohs/run-store-async + store done + [(dwl/sync-file (:id file) (:id file))] + (fn [new-state] + (let [;; ==== Get + file' (ths/get-file-from-state new-state) + c-frame1' (cths/get-shape file' :c-frame1) + tokens-frame1' (:applied-tokens c-frame1')] - ;; ==== Check - (t/is (= (count tokens-frame1') 4)) - (t/is (= (get tokens-frame1' :r1) "test-token-1")) - (t/is (= (get tokens-frame1' :r2) "test-token-1")) - (t/is (= (get tokens-frame1' :r3) "test-token-1")) - (t/is (= (get tokens-frame1' :r4) "test-token-1")) - (t/is (= (get c-frame1' :r1) 66)) - (t/is (= (get c-frame1' :r2) 66)) - (t/is (= (get c-frame1' :r3) 66)) - (t/is (= (get c-frame1' :r4) 66)))))))] + ;; ==== Check + (t/is (= (count tokens-frame1') 4)) + (t/is (= (get tokens-frame1' :r1) "test-token-1")) + (t/is (= (get tokens-frame1' :r2) "test-token-1")) + (t/is (= (get tokens-frame1' :r3) "test-token-1")) + (t/is (= (get tokens-frame1' :r4) "test-token-1")) + (t/is (= (get c-frame1' :r1) 66)) + (t/is (= (get c-frame1' :r2) 66)) + (t/is (= (get c-frame1' :r3) 66)) + (t/is (= (get c-frame1' :r4) 66)))))) + [(dwtp/propagate-workspace-tokens)] + identity + ;; Use timeout-based stopper: propagation defers + ;; work via rx/timer, so the normal ::end stopper + ;; fires too early. The 200ms timeout lets deferred + ;; work complete before moving to sync-file. + (tohs/stop-on ::propagation-done)))] (tohs/run-store-async store step2 events identity)))) @@ -395,45 +407,53 @@ :value 200})] step2 (fn [_] - (let [events2 [(dwtp/propagate-workspace-tokens) - (dwl/sync-file (:id file) (:id file))]] - (tohs/run-store-async - store done events2 - (fn [new-state] - (let [;; ==== Get - file' (ths/get-file-from-state new-state) - c-frame1' (cths/get-shape file' :c-frame1) - tokens-frame1' (:applied-tokens c-frame1')] + (tohs/run-store-async + store + (fn [_] + (tohs/run-store-async + store done + [(dwl/sync-file (:id file) (:id file))] + (fn [new-state] + (let [;; ==== Get + file' (ths/get-file-from-state new-state) + c-frame1' (cths/get-shape file' :c-frame1) + tokens-frame1' (:applied-tokens c-frame1')] - ;; ==== Check - (t/is (= (count tokens-frame1') 11)) - (t/is (= (get tokens-frame1' :r1) "token-radius")) - (t/is (= (get tokens-frame1' :r2) "token-radius")) - (t/is (= (get tokens-frame1' :r3) "token-radius")) - (t/is (= (get tokens-frame1' :r4) "token-radius")) - (t/is (= (get tokens-frame1' :rotation) "token-rotation")) - (t/is (= (get tokens-frame1' :opacity) "token-opacity")) - (t/is (= (get tokens-frame1' :stroke-width) "token-stroke-width")) - (t/is (= (get tokens-frame1' :stroke-color) "token-color")) - (t/is (= (get tokens-frame1' :fill) "token-color")) - (t/is (= (get tokens-frame1' :width) "token-dimensions")) - (t/is (= (get tokens-frame1' :height) "token-dimensions")) - (t/is (= (get c-frame1' :r1) 30)) - (t/is (= (get c-frame1' :r2) 30)) - (t/is (= (get c-frame1' :r3) 30)) - (t/is (= (get c-frame1' :r4) 30)) - (t/is (= (get c-frame1' :rotation) 45)) - (t/is (= (get c-frame1' :opacity) 0.9)) - (t/is (= (get-in c-frame1' [:strokes 0 :stroke-width]) 8)) - (t/is (= (get-in c-frame1' [:strokes 0 :stroke-color]) "#ff0000")) - (t/is (= (-> c-frame1' :fills (nth 0) :fill-color) "#ff0000")) - (t/is (mth/close? (get c-frame1' :width) 200)) - (t/is (mth/close? (get c-frame1' :height) 200)) + ;; ==== Check + (t/is (= (count tokens-frame1') 11)) + (t/is (= (get tokens-frame1' :r1) "token-radius")) + (t/is (= (get tokens-frame1' :r2) "token-radius")) + (t/is (= (get tokens-frame1' :r3) "token-radius")) + (t/is (= (get tokens-frame1' :r4) "token-radius")) + (t/is (= (get tokens-frame1' :rotation) "token-rotation")) + (t/is (= (get tokens-frame1' :opacity) "token-opacity")) + (t/is (= (get tokens-frame1' :stroke-width) "token-stroke-width")) + (t/is (= (get tokens-frame1' :stroke-color) "token-color")) + (t/is (= (get tokens-frame1' :fill) "token-color")) + (t/is (= (get tokens-frame1' :width) "token-dimensions")) + (t/is (= (get tokens-frame1' :height) "token-dimensions")) + (t/is (= (get c-frame1' :r1) 30)) + (t/is (= (get c-frame1' :r2) 30)) + (t/is (= (get c-frame1' :r3) 30)) + (t/is (= (get c-frame1' :r4) 30)) + (t/is (= (get c-frame1' :rotation) 45)) + (t/is (= (get c-frame1' :opacity) 0.9)) + (t/is (= (get-in c-frame1' [:strokes 0 :stroke-width]) 8)) + (t/is (= (get-in c-frame1' [:strokes 0 :stroke-color]) "#ff0000")) + (t/is (= (-> c-frame1' :fills (nth 0) :fill-color) "#ff0000")) + (t/is (mth/close? (get c-frame1' :width) 200)) + (t/is (mth/close? (get c-frame1' :height) 200)) - (t/is (empty? (:touched c-frame1'))) + (t/is (empty? (:touched c-frame1'))) - (t/testing "WASM mocks were exercised" - (t/is (pos? (thw/call-count :propagate-modifiers)))))))))] + (t/testing "WASM mocks were exercised" + (t/is (pos? (thw/call-count :propagate-modifiers)))))))) + [(dwtp/propagate-workspace-tokens)] + identity + ;; Use timeout-based stopper: propagation defers + ;; work via rx/timer, so the normal ::end stopper + ;; fires too early. + (tohs/stop-on ::propagation-done)))] (tohs/run-store-async store step2 events identity)))) diff --git a/render-wasm/src/render/images.rs b/render-wasm/src/render/images.rs index e1c66b2a51..e2d7cc90d3 100644 --- a/render-wasm/src/render/images.rs +++ b/render-wasm/src/render/images.rs @@ -158,6 +158,8 @@ impl ImageStore { ) -> crate::error::Result<()> { let key = (id, is_thumbnail); + // During bulk reload (set-objects) images may already be cached + // from different components if self.images.contains_key(&key) { return Ok(()); } @@ -185,6 +187,8 @@ impl ImageStore { ) -> Result<()> { let key = (id, is_thumbnail); + // During bulk reload (set-objects) images may already be cached + // from different components if self.images.contains_key(&key) { return Ok(()); }