diff --git a/frontend/src/app/main/data/workspace/transforms.cljs b/frontend/src/app/main/data/workspace/transforms.cljs index 5eb90ca433..48cf42d014 100644 --- a/frontend/src/app/main/data/workspace/transforms.cljs +++ b/frontend/src/app/main/data/workspace/transforms.cljs @@ -1023,6 +1023,9 @@ (rx/concat (rx/merge (->> modif-stream + ;; Sample at a fixed cadence to cap re-renders, mirroring the + ;; drag/resize/rotation paths throttled in #10560. + (rx/sample mconst/move-sample-time) (rx/map #(dwm/set-wasm-modifiers % {:ignore-snap-pixel true}))) (->> modif-stream @@ -1031,17 +1034,28 @@ (rx/of (nudge-selected-shapes direction shift?))) (rx/of (finish-transform)))) - (rx/concat - (rx/merge - (->> move-events - (rx/scan #(gpt/add %1 mov-vec) (gpt/point 0 0)) - (rx/map #(dwm/create-modif-tree selected (ctm/move-modifiers %))) - (rx/map #(dwm/set-modifiers % false true)) - (rx/take-until stopper)) - (rx/of (nudge-selected-shapes direction shift?))) + (let [modif-stream + (->> move-events + (rx/scan #(gpt/add %1 mov-vec) (gpt/point 0 0)) + (rx/map #(dwm/create-modif-tree selected (ctm/move-modifiers %))) + (rx/take-until stopper))] + (rx/concat + (rx/merge + (->> modif-stream + ;; Sample at a fixed cadence to cap re-renders, mirroring the + ;; drag/resize/rotation paths throttled in #10560. + (rx/sample mconst/move-sample-time) + (rx/map #(dwm/set-modifiers % false true))) + ;; Un-sampled final write ensures the modifiers atom holds the + ;; exact cumulative position before `apply-modifiers` commits, + ;; even if `sample` drops the tail value on completion. + (->> modif-stream + (rx/last) + (rx/map #(dwm/set-modifiers % false true))) + (rx/of (nudge-selected-shapes direction shift?))) - (rx/of (dwm/apply-modifiers) - (finish-transform))))) + (rx/of (dwm/apply-modifiers) + (finish-transform)))))) (rx/empty)))))) (defn move-selected diff --git a/frontend/test/frontend_tests/helpers/state.cljs b/frontend/test/frontend_tests/helpers/state.cljs index 90e4c995d0..eb7914b72c 100644 --- a/frontend/test/frontend_tests/helpers/state.cljs +++ b/frontend/test/frontend_tests/helpers/state.cljs @@ -33,17 +33,20 @@ (pprint (sm/humanize-explain data)))) (defn setup-store - [file] - (let [state (-> initial-state - (assoc :current-file-id (:id file) - :current-page-id (cthf/current-page-id file) - :permissions {:can-edit true} - :files {(:id file) file})) - store (ptk/store {:state state :on-error on-error})] - ;; Unit tests skip team/workspace bootstrap; mirror team init so - ;; :features is populated the same way as features/initialize does in app. - (ptk/emit! store (features/initialize #{})) - store)) + ([file] (setup-store file nil)) + ([file {:keys [renderer] :as _opts}] + (let [state (-> initial-state + (assoc :current-file-id (:id file) + :current-page-id (cthf/current-page-id file) + :permissions {:can-edit true} + :files {(:id file) file}) + (cond-> (some? renderer) + (assoc-in [:profile :props :renderer] renderer))) + store (ptk/store {:state state :on-error on-error})] + ;; Unit tests skip team/workspace bootstrap; mirror team init so + ;; :features is populated the same way as features/initialize does in app. + (ptk/emit! store (features/initialize #{})) + store))) (defn run-store ([store done events completed-cb] diff --git a/frontend/test/frontend_tests/helpers/wasm.cljs b/frontend/test/frontend_tests/helpers/wasm.cljs index 497b4c22f1..c4ff7b5ca4 100644 --- a/frontend/test/frontend_tests/helpers/wasm.cljs +++ b/frontend/test/frontend_tests/helpers/wasm.cljs @@ -86,6 +86,20 @@ (track! :set-shape-grow-type) nil) +(defn- mock-set-modifiers-start + [] + (track! :set-modifiers-start) + nil) + +(defn- mock-set-modifiers-end + [] + (track! :set-modifiers-end) + nil) + +(defn- mock-get-selection-rect + ([_ids] (track! :get-selection-rect) nil) + ([_ids _] (track! :get-selection-rect) nil)) + (defn- mock-initialized? [] (track! :initialized?) @@ -181,6 +195,9 @@ :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-modifiers-start wasm.api/set-modifiers-start + :set-modifiers-end wasm.api/set-modifiers-end + :get-selection-rect wasm.api/get-selection-rect :font-stored? wasm.fonts/font-stored? :make-font-data wasm.fonts/make-font-data :get-content-fonts wasm.fonts/get-content-fonts}) @@ -197,6 +214,9 @@ (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-modifiers-start mock-set-modifiers-start) + (set! wasm.api/set-modifiers-end mock-set-modifiers-end) + (set! wasm.api/get-selection-rect mock-get-selection-rect) (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)) @@ -217,6 +237,9 @@ (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-modifiers-start (:set-modifiers-start orig)) + (set! wasm.api/set-modifiers-end (:set-modifiers-end orig)) + (set! wasm.api/get-selection-rect (:get-selection-rect 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/nudge_selected_shapes_test.cljs b/frontend/test/frontend_tests/logic/nudge_selected_shapes_test.cljs new file mode 100644 index 0000000000..0d182e5fd3 --- /dev/null +++ b/frontend/test/frontend_tests/logic/nudge_selected_shapes_test.cljs @@ -0,0 +1,172 @@ +;; 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.nudge-selected-shapes-test + "Regression tests for the keyboard-nudge transform stream. + + Holding an arrow key on a selection drives `nudge-selected-shapes` + through OS key-repeat. Before the throttle introduced for issue #10726, + every key-repeat was mapped 1:1 into a `set-modifiers`/`set-wasm-modifiers` + store write, which could starve the renderer and trip React error #185 + (Maximum update depth exceeded). + + These tests do NOT reproduce the render storm (that requires real React + renders and real OS key-repeat timing the unit harness cannot simulate). + They guard the invariant the throttle must preserve: after a burst of + `move-selected` events, the final committed shape position equals exactly + `(event-count) * nudge-step` — i.e. the throttle drops no displacement. + The legacy (non-WASM) branch case specifically guards the new un-sampled + `rx/last` commit substream against `sample` dropping the tail value on + completion." + (: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.ids-map :as cthi] + [app.common.test-helpers.shapes :as cths] + [app.main.data.workspace.selection :as dws] + [app.main.data.workspace.transforms :as dwt] + [beicon.v2.core :as rx] + [cljs.test :as t :include-macros true] + [frontend-tests.helpers.state :as ths] + [frontend-tests.helpers.wasm :as thw])) + +(t/use-fixtures :each + {:before (fn [] + (cthi/reset-idmap!) + ;; The Node test environment has no real WASM binary, so the + ;; WASM-branch cases below would throw inside `wasm.api/*` + ;; calls. Install lightweight mocks that let the CLJS-side + ;; logic (`apply-wasm-modifiers`'s `dwsh/update-shapes`) + ;; run normally while stubbing the WASM heap boundary. + (thw/setup-wasm-mocks!)) + :after (fn [] + ;; Teardown runs after the async test's `done` fires. + (thw/teardown-wasm-mocks!))}) + +(def ^:private nudge-event-count + "Number of `move-selected` events emitted in a burst. Chosen large enough + that, with a 16 ms `sample` cadence and the unit harness's synchronous + dispatch, the throttled live-preview substream would coalesce emissions + were it not for the final `rx/last` commit honoring the exact cumulative + position." + 20) + +(def ^:private run-store-timeout-ms + "Delay before `run-store` invokes its completion callback. + + `nudge-selected-shapes` does not complete its transform streams on source + completion alone; its internal stopper fires either after 1000 ms of idle + time or 250 ms after a key-up event (see `transforms.cljs`). The unit + harness emits no keyboard events, so only the 1000 ms idle timer can fire. + We let the harness wait long enough for that timer to fire and run the + final `apply-modifiers`/`finish-transform` before reading the store." + 1500) + +(defn- run-nudge-store + "Like `ths/run-store` but the outer subscription completes on a fixed + timer instead of immediately on `:the/end`, giving the nudge's idle + stopper time to fire and commit the cumulative displacement." + [store done events completed-cb] + (ths/run-store store done events completed-cb (fn [_stream] (rx/timer run-store-timeout-ms)))) + +(defn- shape-x + "Read a shape's committed x position from its points-derived rect." + [file label] + (let [shape (cths/get-shape file label) + rect (grc/points->rect (:points shape))] + (:x rect))) + +(defn- shape-y + "Read a shape's committed y position from its points-derived rect." + [file label] + (let [shape (cths/get-shape file label) + rect (grc/points->rect (:points shape))] + (:y rect))) + +(defn- make-file + [] + (-> (cthf/sample-file :file1) + (ctho/add-rect :rect1 :x 100 :y 200 :width 50 :height 50))) + +(defn- burst-move-events + "Builds the event sequence: select rect, then `n` `move-selected` events + in the given `direction` (with `shift?` controlling the nudge big/small + step size — false = 1 px, true = 10 px)." + [file direction shift? n] + (let [rect (cths/get-shape file :rect1)] + (into [(dws/select-shape (:id rect))] + (repeat n (dwt/move-selected direction shift?))))) + +;; --------------------------------------------------------------------------- +;; Default (WASM) branch — `render-wasm/v1` active (helpers/state default). +;; --------------------------------------------------------------------------- + +(t/deftest nudge-burst-wasm-branches-commits-exact-final-position-right-small + (t/async + done + (let [file (make-file) + store (ths/setup-store file) + events (burst-move-events file :right false nudge-event-count)] + (run-nudge-store + store done events + (fn [new-state] + (let [file' (ths/get-file-from-state new-state) + final-x (shape-x file' :rect1)] + ;; nudge small = 1 px, direction :right => +x. + (t/is (= (+ 100 nudge-event-count) final-x) + "WASM branch: final x equals original + N * small-nudge after a throttled burst"))))))) + +(t/deftest nudge-burst-wasm-branches-commits-exact-final-position-up-big + (t/async + done + (let [file (make-file) + store (ths/setup-store file) + events (burst-move-events file :up true nudge-event-count)] + (run-nudge-store + store done events + (fn [new-state] + (let [file' (ths/get-file-from-state new-state) + final-y (shape-y file' :rect1)] + ;; nudge big = 10 px, direction :up => -y. + (t/is (= (- 200 (* 10 nudge-event-count)) final-y) + "WASM branch: final y equals original - N * big-nudge after a throttled burst"))))))) + +;; --------------------------------------------------------------------------- +;; Legacy (non-WASM) branch — :renderer :svg disables render-wasm/v1. +;; This case specifically guards the new `rx/last` commit substream added +;; alongside the live-preview sample: if `sample` drops the tail value on +;; completion, the un-sampled `rx/last` write still delivers the exact +;; cumulative position to `apply-modifiers`. +;; --------------------------------------------------------------------------- + +(t/deftest nudge-burst-legacy-branch-commits-exact-final-position-right-small + (t/async + done + (let [file (make-file) + store (ths/setup-store file {:renderer :svg}) + events (burst-move-events file :right false nudge-event-count)] + (run-nudge-store + store done events + (fn [new-state] + (let [file' (ths/get-file-from-state new-state) + final-x (shape-x file' :rect1)] + (t/is (= (+ 100 nudge-event-count) final-x) + "Legacy branch: final x equals original + N * small-nudge after a throttled burst"))))))) + +(t/deftest nudge-burst-legacy-branch-commits-exact-final-position-up-big + (t/async + done + (let [file (make-file) + store (ths/setup-store file {:renderer :svg}) + events (burst-move-events file :up true nudge-event-count)] + (run-nudge-store + store done events + (fn [new-state] + (let [file' (ths/get-file-from-state new-state) + final-y (shape-y file' :rect1)] + (t/is (= (- 200 (* 10 nudge-event-count)) final-y) + "Legacy branch: final y equals original - N * big-nudge after a throttled burst"))))))) \ No newline at end of file diff --git a/frontend/test/frontend_tests/runner.cljs b/frontend/test/frontend_tests/runner.cljs index f21b124277..128ce0c216 100644 --- a/frontend/test/frontend_tests/runner.cljs +++ b/frontend/test/frontend_tests/runner.cljs @@ -26,6 +26,7 @@ [frontend-tests.logic.copying-and-duplicating-test] [frontend-tests.logic.frame-guides-test] [frontend-tests.logic.groups-test] + [frontend-tests.logic.nudge-selected-shapes-test] [frontend-tests.logic.pasting-in-containers-test] [frontend-tests.main-errors-test] [frontend-tests.plugins.comments-test] @@ -99,6 +100,7 @@ 'frontend-tests.logic.copying-and-duplicating-test 'frontend-tests.logic.frame-guides-test 'frontend-tests.logic.groups-test + 'frontend-tests.logic.nudge-selected-shapes-test 'frontend-tests.logic.pasting-in-containers-test 'frontend-tests.plugins.context-shapes-test 'frontend-tests.plugins.comments-test