diff --git a/frontend/src/app/main/constants.cljs b/frontend/src/app/main/constants.cljs index d473e621c6..40390e2ee4 100644 --- a/frontend/src/app/main/constants.cljs +++ b/frontend/src/app/main/constants.cljs @@ -341,3 +341,9 @@ (def ^:const resize-sample-time default-sample-time) (def ^:const rotation-sample-time default-sample-time) (def ^:const move-sample-time default-sample-time) + +(def ^:const sidebar-transform-sample-time + "Time in ms for coalescing sidebar measures-panel transform commits: at + most one full commit per window during a burst, plus a trailing flush + with the exact final value." + 50) diff --git a/frontend/src/app/main/data/workspace.cljs b/frontend/src/app/main/data/workspace.cljs index cedfad1d96..d337381cb4 100644 --- a/frontend/src/app/main/data/workspace.cljs +++ b/frontend/src/app/main/data/workspace.cljs @@ -1551,9 +1551,11 @@ (dm/export dwt/trigger-bounding-box-cloaking) (dm/export dwt/start-resize) (dm/export dwt/update-dimensions) +(dm/export dwt/update-dimensions-coalesced) (dm/export dwt/change-orientation) (dm/export dwt/start-rotate) (dm/export dwt/increase-rotation) +(dm/export dwt/increase-rotation-coalesced) (dm/export dwt/start-move-selected) (dm/export dwt/move-selected) (dm/export dwt/update-position) diff --git a/frontend/src/app/main/data/workspace/transforms.cljs b/frontend/src/app/main/data/workspace/transforms.cljs index d0e612b493..f53cf8b2ee 100644 --- a/frontend/src/app/main/data/workspace/transforms.cljs +++ b/frontend/src/app/main/data/workspace/transforms.cljs @@ -1125,19 +1125,169 @@ :ignore-touched (:ignore-touched options) :ignore-snap-pixel true})))))))) +;; -- Sidebar measures transform coalescing ---------------------------- + +;; The sidebar measures panel numeric inputs emit one event per DOM +;; gesture tick (held arrow keys, mouse wheel, scrub drags). Committing +;; each tick would run a full `apply-modifiers` per DOM event and starve +;; the renderer (React error #185). The events in this section coalesce +;; those bursts at the data layer: the first event of a burst commits +;; immediately (leading edge, so single edits stay synchronous), further +;; ticks commit at most once per `mconst/sidebar-transform-sample-time` +;; (throttle), and a trailing debounced flush guarantees the exact final +;; value lands. All payloads are absolute values, so keeping only the +;; latest queued value per shape/attribute is lossless. + +(defn- sidebar-commit-events + "Build the real commit events for a drained pending entry of `kind`, + skipping shapes that no longer exist on the queued page." + [state kind entry] + (let [options (:options entry) + page-id (or (:page-id options) (:current-page-id state)) + objects (dsh/lookup-page-objects state page-id) + options (assoc options :page-id page-id) + live-ids (fn [ids] (into [] (filter #(contains? objects %)) ids))] + (case kind + ::positions + (keep (fn [[id position]] + (when (contains? objects id) + (update-position id position options))) + (:positions entry)) + + ::dimensions + (let [ids (live-ids (:ids entry))] + (when (seq ids) + (map (fn [[attr value]] + (update-dimensions ids attr value options)) + (:values entry)))) + + ::rotation + (let [ids (live-ids (:ids entry))] + (when (seq ids) + [(increase-rotation ids (:value entry) nil :page-id page-id)]))))) + +(defn- flush-sidebar-transforms + "Internal: atomically drain the pending sidebar transform payloads and + emit their commit events. No-op when nothing is pending." + [] + (ptk/reify ::flush-sidebar-transforms + ptk/UpdateEvent + (update [_ state] + (let [pending (::pending-sidebar-transforms state)] + (-> state + (dissoc ::pending-sidebar-transforms) + (assoc ::flushing-sidebar-transforms pending)))) + + ptk/WatchEvent + (watch [_ state _] + (let [pending (::flushing-sidebar-transforms state)] + (rx/concat + (if (empty? pending) + (rx/empty) + (->> pending + (mapcat (fn [[kind entry]] (sidebar-commit-events state kind entry))) + (rx/from))) + (rx/of (fn [state] (dissoc state ::flushing-sidebar-transforms)))))))) + +(defn- queue-sidebar-transform + "Internal: accumulate the latest payload of `kind` with `update-entry` + (a fn from the previous pending entry to the new one). + + The very first queued event of the workspace session also installs the + drain stream that commits pending payloads: a leading flush for the + first event, at most one flush per + `mconst/sidebar-transform-sample-time` while a burst is ongoing + (throttle), and a trailing flush (debounce) that guarantees the exact + final value lands. The drain stream lives until the workspace is + finalized, so subsequent bursts reuse it." + [kind update-entry] + (let [cur-event (js/Symbol)] + (ptk/reify ::queue-sidebar-transform + ptk/UpdateEvent + (update [_ state] + (let [state (update-in state [::pending-sidebar-transforms kind] + (fn [entry] (update-entry (or entry {}))))] + (if (nil? (::sidebar-transform-drain state)) + (assoc state ::sidebar-transform-drain cur-event) + state))) + + ptk/WatchEvent + (watch [_ state stream] + (if (= cur-event (::sidebar-transform-drain state)) + (let [stopper (->> stream (rx/filter (ptk/type? :app.main.data.workspace/finalize)))] + (rx/merge + ;; Leading edge: commit the payload this first event queued. + (rx/of (flush-sidebar-transforms)) + ;; At most one commit per window while a burst is ongoing. + (->> stream + (rx/filter (ptk/type? ::queue-sidebar-transform)) + (rx/throttle mconst/sidebar-transform-sample-time) + (rx/map (fn [_] (flush-sidebar-transforms))) + (rx/take-until stopper)) + ;; Trailing edge: guarantee the exact final value lands. + (->> stream + (rx/filter (ptk/type? ::queue-sidebar-transform)) + (rx/debounce mconst/sidebar-transform-sample-time) + (rx/map (fn [_] (flush-sidebar-transforms))) + (rx/take-until stopper)))) + (rx/empty)))))) + (defn update-positions - "Move multiple shapes to a new position." + "Move multiple shapes to a new position, from the sidebar options form. + + Burst-coalesced (see `queue-sidebar-transform`): rapid successive calls + from the sidebar numeric inputs commit at most once per + `mconst/sidebar-transform-sample-time`, and the trailing flush commits + the exact final position. A single call still commits synchronously." ([ids position] (update-positions ids position nil)) ([ids position options] (assert (every? uuid? ids) "expected valid coll of uuids") (assert (map? position) "expected a valid map for `position`") - (ptk/reify ::update-positions - ptk/WatchEvent - (watch [_ _ _] - (->> ids - (map (fn [id] (update-position id position options))) - (rx/from)))))) + (queue-sidebar-transform + ::positions + (fn [entry] + (-> entry + (update :positions + (fn [positions] + (reduce (fn [positions id] + (update positions id merge position)) + (or positions {}) + ids))) + (assoc :options options)))))) + +(defn update-dimensions-coalesced + "Like `update-dimensions`, but burst-coalesced (see + `queue-sidebar-transform`); used by the sidebar measures panel numeric + inputs. The latest queued value per attribute wins." + ([ids attr value] (update-dimensions-coalesced ids attr value nil)) + ([ids attr value options] + (assert (number? value)) + (assert (every? uuid? ids) + "expected valid coll of uuids") + (assert (contains? #{:width :height} attr) + "expected valid attr") + (queue-sidebar-transform + ::dimensions + (fn [entry] + (-> entry + (assoc-in [:values attr] value) + (assoc :ids ids :options options)))))) + +(defn increase-rotation-coalesced + "Like `increase-rotation` with an absolute rotation value, but + burst-coalesced (see `queue-sidebar-transform`); used by the sidebar + measures panel rotation input. The latest queued absolute value wins; + the delta is recomputed from the current rotation when the burst + commits." + [ids rotation] + (assert (every? uuid? ids) + "expected valid coll of uuids") + (assert (number? rotation)) + (queue-sidebar-transform + ::rotation + (fn [entry] + (assoc entry :value rotation :ids ids :options nil)))) (defn position-shapes [shapes] diff --git a/frontend/src/app/main/ui/workspace/sidebar/options/menus/measures.cljs b/frontend/src/app/main/ui/workspace/sidebar/options/menus/measures.cljs index f4a454af54..c9b3190574 100644 --- a/frontend/src/app/main/ui/workspace/sidebar/options/menus/measures.cljs +++ b/frontend/src/app/main/ui/workspace/sidebar/options/menus/measures.cljs @@ -374,7 +374,7 @@ (fn [value attr] (if (or (string? value) (number? value)) (st/emit! (udw/trigger-bounding-box-cloaking ids) - (udw/update-dimensions ids attr value)) + (udw/update-dimensions-coalesced ids attr value)) (st/emit! (udw/trigger-bounding-box-cloaking ids) (dwta/apply-token-from-input {:token (first value) :attrs #{attr} @@ -408,7 +408,7 @@ (if (or (string? value) (number? value)) (let [value (fixed-decimal-value value)] (st/emit! (udw/trigger-bounding-box-cloaking ids)) - (st/emit! (udw/increase-rotation ids value))) + (st/emit! (udw/increase-rotation-coalesced ids value))) (st/emit! (udw/trigger-bounding-box-cloaking ids) (dwta/apply-token-from-input {:token (first value) :attrs #{:rotation} diff --git a/frontend/test/frontend_tests/logic/sidebar_transform_coalescing_test.cljs b/frontend/test/frontend_tests/logic/sidebar_transform_coalescing_test.cljs new file mode 100644 index 0000000000..40c174d9cf --- /dev/null +++ b/frontend/test/frontend_tests/logic/sidebar_transform_coalescing_test.cljs @@ -0,0 +1,226 @@ +;; 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.logic.sidebar-transform-coalescing-test + "Regression tests for the sidebar measures panel transform coalescing + (React error #185): a burst of numeric-input gestures (held arrow key, + wheel, scrub) must collapse to a handful of commits, and the trailing + flush must land the exact final value." + (:require + [app.common.geom.rect :as grc] + [app.common.test-helpers.compositions :as ctho] + [app.common.test-helpers.files :as cthf] + [app.common.test-helpers.shapes :as cths] + [app.main.data.workspace :as dw] + [app.main.data.workspace.transforms :as-alias dwt] + [beicon.v2.core :as rx] + [cljs.test :as t :include-macros true] + [frontend-tests.helpers.pages :as thp] + [frontend-tests.helpers.state :as ths] + [frontend-tests.helpers.wasm :as thw] + [potok.v2.core :as ptk])) + +(t/use-fixtures :each + {:before (fn [] (thp/reset-idmap!) (thw/setup-wasm-mocks!)) + :after (fn [] (thw/teardown-wasm-mocks!))}) + +(def ^:private flush-wait-ms + "How long to keep the store running after a burst so the 50 ms + trailing flush fires before checking the final state." + 150) + +(defn- count-events + "Return an atom counting how many events of `type` get emitted on the + store input stream (i.e. the real commits triggered by the coalescer)." + [store type] + (let [counter (atom 0)] + (->> (ptk/input-stream store) + (rx/filter (ptk/type? type)) + (rx/tap (fn [_] (swap! counter inc))) + (rx/subs! (fn [_] nil))) + counter)) + +(defn- run-store-timed + "Like `ths/run-store`, but emits `:the/end` `wait-ms` after `events` + so the timer-based coalescing (throttle/debounce) gets to fire." + [store done events wait-ms completed-cb] + (->> (ptk/input-stream store) + (rx/filter #(= :the/end %)) + (rx/take 1) + (rx/tap (fn [_] (completed-cb @store))) + (rx/subs! (fn [_] nil) + (fn [cause] + (done) + (t/do-report {:type :error :message "Stream error" :actual cause})) + (fn [_] (done)))) + (doseq [event events] + (ptk/emit! store event)) + (js/setTimeout (fn [] (ptk/emit! store :the/end)) wait-ms)) + +(defn- burst + "A burst of `n` events built with `make-event`, like the stream of + calls a held arrow key or a scrub gesture produces." + [n make-event] + (mapv make-event (range 1 (inc n)))) + +;; --- Positions (update-positions, coalesced in place) ----------------- + +(t/deftest update-positions-burst-commits-exact-final-value-wasm + (t/async + done + (let [file (-> (cthf/sample-file :file1) + (ctho/add-frame :frame1 :x 0 :y 0 :width 100 :height 100)) + store (ths/setup-store file) + frame1 (cths/get-shape file :frame1) + commits (count-events store ::dwt/update-position) + events (burst 20 (fn [i] (dw/update-positions [(:id frame1)] {:x (+ 100 i)})))] + (run-store-timed + store done events flush-wait-ms + (fn [new-state] + (let [file' (ths/get-file-from-state new-state) + frame1' (cths/get-shape file' :frame1) + x (-> frame1' :points grc/points->rect :x)] + ;; The trailing flush lands the exact final value... + (t/is (= 120 x)) + ;; ...and the 20-event burst collapsed to a handful of commits. + (t/is (<= @commits 3)))))))) + +(t/deftest update-positions-burst-commits-exact-final-value-svg + (t/async + done + (let [file (-> (cthf/sample-file :file1) + (ctho/add-frame :frame1 :x 0 :y 0 :width 100 :height 100)) + store (ths/setup-store file {:renderer :svg}) + frame1 (cths/get-shape file :frame1) + commits (count-events store ::dwt/update-position) + events (burst 20 (fn [i] (dw/update-positions [(:id frame1)] {:x (+ 100 i)})))] + (run-store-timed + store done events flush-wait-ms + (fn [new-state] + (let [file' (ths/get-file-from-state new-state) + frame1' (cths/get-shape file' :frame1) + x (-> frame1' :points grc/points->rect :x)] + (t/is (= 120 x)) + (t/is (<= @commits 3)))))))) + +(t/deftest update-positions-burst-merges-x-and-y + (t/async + done + (let [file (-> (cthf/sample-file :file1) + (ctho/add-frame :frame1 :x 0 :y 0 :width 100 :height 100)) + store (ths/setup-store file) + frame1 (cths/get-shape file :frame1) + commits (count-events store ::dwt/update-position) + events (into (burst 10 (fn [i] (dw/update-positions [(:id frame1)] {:x (+ 100 i)}))) + (burst 10 (fn [i] (dw/update-positions [(:id frame1)] {:y (+ 200 i)}))))] + (run-store-timed + store done events flush-wait-ms + (fn [new-state] + (let [file' (ths/get-file-from-state new-state) + frame1' (cths/get-shape file' :frame1) + rect (-> frame1' :points grc/points->rect)] + ;; Partial position maps of the same shape merge, so the last + ;; value of each attribute lands. + (t/is (= 110 (:x rect))) + (t/is (= 210 (:y rect))) + (t/is (<= @commits 3)))))))) + +;; --- Dimensions (update-dimensions-coalesced) -------------------------- + +(t/deftest update-dimensions-burst-commits-exact-final-value-wasm + (t/async + done + (let [file (-> (cthf/sample-file :file1) + (ctho/add-rect :rect1 :x 0 :y 0 :width 100 :height 100)) + store (ths/setup-store file) + rect1 (cths/get-shape file :rect1) + commits (count-events store ::dwt/update-dimensions) + events (burst 20 (fn [i] (dw/update-dimensions-coalesced [(:id rect1)] :width (+ 100 i))))] + (run-store-timed + store done events flush-wait-ms + (fn [new-state] + (let [file' (ths/get-file-from-state new-state) + rect1' (cths/get-shape file' :rect1) + width (-> rect1' :points grc/points->rect :width)] + (t/is (= 120 width)) + (t/is (<= @commits 3)))))))) + +(t/deftest update-dimensions-burst-commits-exact-final-value-svg + (t/async + done + (let [file (-> (cthf/sample-file :file1) + (ctho/add-rect :rect1 :x 0 :y 0 :width 100 :height 100)) + store (ths/setup-store file {:renderer :svg}) + rect1 (cths/get-shape file :rect1) + commits (count-events store ::dwt/update-dimensions) + events (burst 20 (fn [i] (dw/update-dimensions-coalesced [(:id rect1)] :width (+ 100 i))))] + (run-store-timed + store done events flush-wait-ms + (fn [new-state] + (let [file' (ths/get-file-from-state new-state) + rect1' (cths/get-shape file' :rect1) + width (-> rect1' :points grc/points->rect :width)] + (t/is (= 120 width)) + (t/is (<= @commits 3)))))))) + +(t/deftest update-dimensions-burst-merges-width-and-height + (t/async + done + (let [file (-> (cthf/sample-file :file1) + (ctho/add-rect :rect1 :x 0 :y 0 :width 100 :height 100)) + store (ths/setup-store file) + rect1 (cths/get-shape file :rect1) + commits (count-events store ::dwt/update-dimensions) + events (into (burst 10 (fn [i] (dw/update-dimensions-coalesced [(:id rect1)] :width (+ 100 i)))) + (burst 10 (fn [i] (dw/update-dimensions-coalesced [(:id rect1)] :height (+ 200 i)))))] + (run-store-timed + store done events flush-wait-ms + (fn [new-state] + (let [file' (ths/get-file-from-state new-state) + rect1' (cths/get-shape file' :rect1) + rect (-> rect1' :points grc/points->rect)] + ;; Each attribute keeps its own latest queued value. + (t/is (= 110 (:width rect))) + (t/is (= 210 (:height rect))) + ;; At most 3 flushes; the trailing one commits both pending + ;; attributes, hence 4 commit events. + (t/is (<= @commits 4)))))))) + +;; --- Rotation (increase-rotation-coalesced) ---------------------------- + +(t/deftest increase-rotation-burst-commits-exact-final-value-wasm + (t/async + done + (let [file (-> (cthf/sample-file :file1) + (ctho/add-rect :rect1 :x 0 :y 0 :width 100 :height 100)) + store (ths/setup-store file) + rect1 (cths/get-shape file :rect1) + commits (count-events store ::dwt/increase-rotation) + events (burst 20 (fn [i] (dw/increase-rotation-coalesced [(:id rect1)] (* i 3))))] + (run-store-timed + store done events flush-wait-ms + (fn [new-state] + (let [file' (ths/get-file-from-state new-state) + rect1' (cths/get-shape file' :rect1)] + (t/is (= 60 (:rotation rect1'))) + (t/is (<= @commits 3)))))))) + +(t/deftest increase-rotation-burst-commits-exact-final-value-svg + (t/async + done + (let [file (-> (cthf/sample-file :file1) + (ctho/add-rect :rect1 :x 0 :y 0 :width 100 :height 100)) + store (ths/setup-store file {:renderer :svg}) + rect1 (cths/get-shape file :rect1) + commits (count-events store ::dwt/increase-rotation) + events (burst 20 (fn [i] (dw/increase-rotation-coalesced [(:id rect1)] (* i 3))))] + (run-store-timed + store done events flush-wait-ms + (fn [new-state] + (let [file' (ths/get-file-from-state new-state) + rect1' (cths/get-shape file' :rect1)] + (t/is (= 60 (:rotation rect1'))) + (t/is (<= @commits 3)))))))) diff --git a/frontend/test/frontend_tests/logic/update_position_test.cljs b/frontend/test/frontend_tests/logic/update_position_test.cljs index 8c55dacd43..93b51f95d6 100644 --- a/frontend/test/frontend_tests/logic/update_position_test.cljs +++ b/frontend/test/frontend_tests/logic/update_position_test.cljs @@ -12,7 +12,12 @@ [app.common.test-helpers.shapes :as cths] [app.main.data.workspace :as dw] [cljs.test :as t :include-macros true] - [frontend-tests.helpers.state :as ths])) + [frontend-tests.helpers.state :as ths] + [frontend-tests.helpers.wasm :as thw])) + +(t/use-fixtures :each + {:before (fn [] (thw/setup-wasm-mocks!)) + :after (fn [] (thw/teardown-wasm-mocks!))}) (t/deftest test-update-positions-multiple-ids (t/async diff --git a/frontend/test/frontend_tests/runner.cljs b/frontend/test/frontend_tests/runner.cljs index 21c7d23c06..a4a7b378ea 100644 --- a/frontend/test/frontend_tests/runner.cljs +++ b/frontend/test/frontend_tests/runner.cljs @@ -34,6 +34,8 @@ [frontend-tests.logic.groups-test] [frontend-tests.logic.nudge-selected-shapes-test] [frontend-tests.logic.pasting-in-containers-test] + [frontend-tests.logic.sidebar-transform-coalescing-test] + [frontend-tests.logic.update-position-test] [frontend-tests.main-errors-test] [frontend-tests.plugins.comments-test] [frontend-tests.plugins.context-shapes-test] @@ -126,6 +128,8 @@ 'frontend-tests.logic.nudge-selected-shapes-test 'frontend-tests.logic.pasting-in-containers-test 'frontend-tests.main-errors-test + 'frontend-tests.logic.sidebar-transform-coalescing-test + 'frontend-tests.logic.update-position-test 'frontend-tests.plugins.comments-test 'frontend-tests.plugins.context-shapes-test 'frontend-tests.plugins.file-test