github/penpot
1
0
Fork 0
mirror of https://github.com/penpot/penpot.git synced 2026-07-28 00:48:22 +00:00
Code Issues Packages Projects Releases Wiki Activity

🐛 Fix problems with async font loading (#10081)

Browse Source
This commit is contained in:
Alonso Torres 2026-06-10 14:51:46 +02:00 committed by GitHub
parent 3ffc76a552
commit ccd34a2705
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
6 changed files with 214 additions and 10 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

20
frontend/src/app/main/data/changes.cljs
View File

@ -92,15 +92,17 @@
(effect [_ state _]
(when (and wasm/context-initialized?
(not @wasm/context-lost?))
(let [objects (dsh/lookup-page-objects state)]
(doseq [{:keys [type id parent-id]} redo-changes
:when (contains? wasm-structural-change-types type)
:let [shape-id (case type
:add-obj id
:mov-objects parent-id)
shape (get objects shape-id)]
:when shape]
(wasm.api/process-object shape))
(let [objects (dsh/lookup-page-objects state)
shapes
(into []
(keep (fn [{:keys [type id parent-id]}]
(when (contains? wasm-structural-change-types type)
(get objects (case type
:add-obj id
:mov-objects parent-id)))))
redo-changes)]
(wasm.api/process-objects shapes)
(wasm.api/request-render "sync-wasm-structural-changes"))))))
(defn- apply-changes-localy

3
frontend/src/app/main/data/workspace/clipboard.cljs
View File

@ -743,7 +743,8 @@
(update :fills translate-fills)
(update :strokes translate-strokes)
(d/update-when :content #(txt/transform-nodes process-text-node %))
(d/update-when :position-data #(mapv process-text-node %)))))
;; Removes the position-data so it's regenerated
(dissoc :position-data))))
;; Analyze the rchange and replace staled media and
;; references to the new uploaded media-objects.

18
frontend/src/app/render_wasm/api.cljs
View File

@ -1418,6 +1418,24 @@
(let [{:keys [thumbnails full]} (set-object shape)]
(process-pending [shape] thumbnails full noop-fn)))
(defn process-objects
"Like process-object but for multiple shapes at once. Accumulates all
pending font/image callbacks before calling process-pending, so that
update-text-layouts fires for all text shapes after fonts load — not
just the first shape that triggered the fetch."
[shapes]
(let [total-shapes (count shapes)
{:keys [thumbnails full]}
(loop [index 0 thumbnails-acc (transient []) full-acc (transient [])]
(if (< index total-shapes)
(let [shape (nth shapes index)
{:keys [thumbnails full]} (set-object shape)]
(recur (inc index)
(reduce conj! thumbnails-acc thumbnails)
(reduce conj! full-acc full)))
{:thumbnails (persistent! thumbnails-acc) :full (persistent! full-acc)}))]
(process-pending shapes thumbnails full noop-fn)))
(defn- process-shapes-chunk
"Process shapes starting at `start-index` until the time budget is exhausted.
Returns {:thumbnails [...] :full [...] :next-index n}"

71
frontend/test/frontend_tests/logic/copying_and_duplicating_test.cljs
View File

@ -562,3 +562,74 @@
;; Total = 1 + 2 + 3 + 4 + 2 + 3 + 4 + 4 + 3 + 2 + 1 = 29
(t/is (= (count (:objects page')) 29)))))))))
;; ---------------------------------------------------------------------------
;; Tests for issue-14302
;; Pasting a board with text shapes must clear stale position-data so the
;; workspace can remeasure the text and avoid incorrect line breaks.
;; ---------------------------------------------------------------------------
(defn- setup-board-with-texts
"Two-page file. Page 1 has a board with two text shapes that carry stale
position-data (simulating a shape that was already laid out on a
different page). Page 2 is empty. Current page is page-1."
[]
(let [stale-pd [{:x 999 :y 999 :width 999 :height 20 :fills [] :text "stale"}]]
(-> (cthf/sample-file :file1 :page-label :page-1)
(ctho/add-frame :frame-1 :name "board-with-texts")
(cths/add-sample-shape :text-a :type :text :parent-label :frame-1 :name "text-a")
(cths/update-shape :text-a :position-data stale-pd)
(cths/add-sample-shape :text-b :type :text :parent-label :frame-1 :name "text-b")
(cths/update-shape :text-b :position-data stale-pd)
(cthf/add-sample-page :page-2)
(cthf/switch-to-page :page-1))))
(t/deftest paste-to-empty-page-clears-text-position-data
"Regression for issue-14302: copying a board with text shapes and pasting
onto an empty page produced incorrect line breaks because stale
position-data from the source page was preserved."
(t/async done
(with-redefs [uuid/next cthi/next-uuid]
(let [file (setup-board-with-texts)
store (ths/setup-store file)
;; copy-paste-shape handles initialize-page + select-shape (needed
;; for calculate-paste-position) + paste-shapes + return to page-1
events (copy-paste-shape :frame-1 file
:target-page-label :page-2
:target-container-id uuid/zero)]
(ths/run-store
store done events
(fn [new-state]
(let [file' (ths/get-file-from-state new-state)
page-2 (cthf/get-page file' :page-2)
pasted-texts (->> (vals (:objects page-2))
(filter #(= :text (:type %))))]
(t/is (= 2 (count pasted-texts))
"Both text shapes are pasted onto the empty page")
(t/is (every? #(nil? (:position-data %)) pasted-texts)
"Pasted text shapes have nil position-data so they get remeasured"))))))))
(t/deftest paste-to-same-page-clears-text-position-data
"Position-data is stripped on paste regardless of destination page."
(t/async done
(with-redefs [uuid/next cthi/next-uuid]
(let [file (setup-board-with-texts)
store (ths/setup-store file)
events (copy-paste-shape :frame-1 file
:target-container-id uuid/zero)]
(ths/run-store
store done events
(fn [new-state]
(let [file' (ths/get-file-from-state new-state)
page-1' (cthf/get-page file' :page-1)
;; Pasted copies have IDs not registered in idmap — they show
;; up as "<no-label …>" strings from cthi/label.
pasted-texts (->> (vals (:objects page-1'))
(filter #(= :text (:type %)))
(remove #(keyword? (cthi/label (:id %)))))]
(t/is (= 2 (count pasted-texts))
"Two new text shapes are pasted onto the same page")
(t/is (every? #(nil? (:position-data %)) pasted-texts)
"Pasted text shapes have nil position-data"))))))))

110
frontend/test/frontend_tests/render_wasm/process_objects_test.cljs Normal file
View File

@ -0,0 +1,110 @@
;; This Source Code Form is subject to the terms of the Mozilla Public
;; License, v. 2.0. If a copy of the MPL was not distributed with this
;; file, You can obtain one at http://mozilla.org/MPL/2.0/.
;;
;; Copyright (c) KALEIDOS INC Sucursal en España SL
(ns frontend-tests.render-wasm.process-objects-test
"Unit tests for wasm.api/process-objects.
The key invariant: process-objects must call set-object for every shape
and forward ALL shapes to a single process-pending invocation.
Without this batching the async font-load callback only covers the shape
that triggered the font fetch. Subsequent text shapes that share the same
font URL get no callback (fetch-font returns nil when the URL is already
in :fetching) and are permanently stuck with fallback-font layout metrics."
(:require
[app.render-wasm.api :as wasm.api]
[beicon.v2.core :as rx]
[cljs.test :as t :include-macros true]))
;; ---------------------------------------------------------------------------
;; Helpers
;; ---------------------------------------------------------------------------
(defn- make-shape [type]
{:id (random-uuid) :type type})
(defn- with-mocks*
"Temporarily replaces wasm.api/set-object and wasm.api/process-pending,
calls (thunk), then restores the originals."
[mock-set-object mock-process-pending thunk]
(let [orig-set-object wasm.api/set-object
orig-process-pending wasm.api/process-pending]
(set! wasm.api/set-object mock-set-object)
(set! wasm.api/process-pending mock-process-pending)
(try
(thunk)
(finally
(set! wasm.api/set-object orig-set-object)
(set! wasm.api/process-pending orig-process-pending)))))
;; ---------------------------------------------------------------------------
;; Tests
;; ---------------------------------------------------------------------------
(t/deftest process-objects-calls-set-object-for-every-shape
"Each shape in the input must go through set-object exactly once."
(let [shapes [(make-shape :text) (make-shape :text) (make-shape :rect)]
visited-ids (atom [])
mock-set (fn [s] (swap! visited-ids conj (:id s)) {:thumbnails [] :full []})
mock-pend (fn [_sh _t _f _cb] nil)]
(with-mocks* mock-set mock-pend #(wasm.api/process-objects shapes))
(t/is (= (mapv :id shapes) @visited-ids)
"set-object called once per shape in order")))
(t/deftest process-objects-calls-process-pending-once-with-all-shapes
"process-pending must receive ALL shapes in a single call.
This is the invariant that makes the async font-load callback update text
layouts for every text shape, not just the first one that triggered the
font fetch."
(let [shapes [(make-shape :text) (make-shape :text)]
captured (atom nil)
mock-set (fn [_s] {:thumbnails [] :full []})
mock-pend (fn [sh t f cb] (reset! captured {:shapes sh :thumbnails t :full f :cb cb}))]
(with-mocks* mock-set mock-pend #(wasm.api/process-objects shapes))
(t/is (some? @captured)
"process-pending was called")
(t/is (= 2 (count (:shapes @captured)))
"process-pending received all shapes")
(t/is (= (set (map :id shapes))
(set (map :id (:shapes @captured))))
"process-pending received the correct shape objects")))
(t/deftest process-objects-accumulates-callbacks-across-shapes
"Pending font/image callbacks from all shapes must be merged into the single
process-pending call. This covers the deduplication scenario: when a second
text shape shares the same font (fetch-font returns nil because the URL is
already in :fetching), the callback from the first shape still covers the
second shape because both are in the :shapes list passed to process-pending."
(let [shape-a (make-shape :text)
shape-b (make-shape :text)
shapes [shape-a shape-b]
font-cb (fn [] (rx/of true))
captured (atom nil)
;; Simulate deduplication: shape-a triggers a real font fetch,
;; shape-b gets an empty pending list (same URL already in :fetching).
mock-set
(fn [s]
(if (= (:id s) (:id shape-a))
{:thumbnails [{:key "font-url" :callback font-cb}] :full []}
{:thumbnails [] :full []}))
mock-pend
(fn [sh t f _cb] (reset! captured {:shapes sh :thumbnails t :full f}))]
(with-mocks* mock-set mock-pend #(wasm.api/process-objects shapes))
;; The single font callback from shape-a is present...
(t/is (= 1 (count (:thumbnails @captured)))
"The font callback from shape-a is forwarded")
;; ...and BOTH shapes are in the list, so when the font loads and
;; process-pending fires update-text-layouts, it covers shape-b too.
(t/is (= 2 (count (:shapes @captured)))
"Both shapes are in process-pending so font-load covers all of them")))

2
frontend/test/frontend_tests/runner.cljs
View File

@ -28,6 +28,7 @@
[frontend-tests.plugins.parser-test]
[frontend-tests.plugins.tokens-test]
[frontend-tests.plugins.utils-test]
[frontend-tests.render-wasm.process-objects-test]
[frontend-tests.svg-fills-test]
[frontend-tests.tokens.import-export-test]
[frontend-tests.tokens.logic.token-actions-test]
@ -85,6 +86,7 @@
frontend-tests.tokens.token-errors-test
frontend-tests.tokens.workspace-tokens-remap-test
frontend-tests.ui.ds-controls-numeric-input-test
frontend-tests.render-wasm.process-objects-test
frontend-tests.util-object-test
frontend-tests.util-range-tree-test
frontend-tests.util-simple-math-test