From e0970c19d52dc359bc237440dd88f2247b21a326 Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Tue, 24 Mar 2026 15:04:27 +0000 Subject: [PATCH] :bug: Fix zero-dimension selrect crash in change-dimensions-modifiers When a text shape is decoded from the server via map->Rect (which bypasses make-rect's 0.01 minimum enforcement), its selrect can have width or height of exactly 0. change-dimensions-modifiers and change-size were dividing by these values, producing Infinity scale factors that propagated through the transform pipeline until calculate-selrect / center->rect returned nil, causing gpt/point to throw 'invalid arguments (on pointer constructor)'. Fix: before computing scale factors, guard sr-width / sr-height (and old-width / old-height in change-size) against zero/negative and non-finite values. When degenerate, fall back to the shape's own top-level :width/:height so the denominator and proportion-lock base remain consistent. Also simplify apply-text-modifier's delta calculation now that the transform pipeline is guaranteed to produce a valid selrect, and update the test suite to test the exact degenerate-selrect scenario that triggered the original crash. Signed-off-by: Andrey Antukh --- common/src/app/common/types/modifiers.cljc | 26 ++++-- .../src/app/main/data/workspace/texts.cljs | 9 +- .../data/workspace_texts_test.cljs | 90 ++++++++++++++----- 3 files changed, 92 insertions(+), 33 deletions(-) diff --git a/common/src/app/common/types/modifiers.cljc b/common/src/app/common/types/modifiers.cljc index 2f79e6483f..63ac4f90c6 100644 --- a/common/src/app/common/types/modifiers.cljc +++ b/common/src/app/common/types/modifiers.cljc @@ -12,6 +12,7 @@ [app.common.files.helpers :as cfh] [app.common.geom.matrix :as gmt] [app.common.geom.point :as gpt] + [app.common.geom.rect :as grc] [app.common.geom.shapes.common :as gco] [app.common.geom.shapes.corners :as gsc] [app.common.geom.shapes.effects :as gse] @@ -117,6 +118,16 @@ (or (not ^boolean (mth/almost-zero? (- (dm/get-prop vector :x) 1))) (not ^boolean (mth/almost-zero? (- (dm/get-prop vector :y) 1))))) +(defn- safe-size-rect + [{:keys [selrect points]}] + (let [{selrect-width :width selrect-height :height} selrect] + (if (and (d/num? selrect-width selrect-height) + (pos? selrect-width) + (pos? selrect-height)) + selrect + (or (grc/points->rect points) + selrect)))) + (defn- mergeable-move? [op1 op2] (let [type-op1 (dm/get-prop op1 :type) @@ -447,9 +458,10 @@ (scale-content value))) (defn change-size - [{:keys [selrect points transform transform-inverse] :as shape} width height] - (let [old-width (-> selrect :width) - old-height (-> selrect :height) + [{:keys [points transform transform-inverse] :as shape} width height] + (let [size-rect (safe-size-rect shape) + old-width (:width size-rect) + old-height (:height size-rect) width (or width old-width) height (or height old-height) origin (first points) @@ -472,7 +484,11 @@ value) {:keys [proportion proportion-lock]} shape - size (select-keys (:selrect shape) [:width :height]) + size-rect (safe-size-rect shape) + sr-width (:width size-rect) + sr-height (:height size-rect) + + size {:width sr-width :height sr-height} new-size (if-not (and (not ignore-lock?) proportion-lock) (assoc size attr value) (if (= attr :width) @@ -486,8 +502,6 @@ width (:width new-size) height (:height new-size) - {sr-width :width sr-height :height} (:selrect shape) - origin (-> shape :points first) scalex (/ width sr-width) scaley (/ height sr-height)] diff --git a/frontend/src/app/main/data/workspace/texts.cljs b/frontend/src/app/main/data/workspace/texts.cljs index 97ffd135d9..9db51462f7 100644 --- a/frontend/src/app/main/data/workspace/texts.cljs +++ b/frontend/src/app/main/data/workspace/texts.cljs @@ -633,14 +633,9 @@ (some? position-data) (assoc :position-data position-data)) - selrect-new (:selrect new-shape) - selrect-old (:selrect shape) - delta-move - (if (and (some? selrect-new) (some? selrect-old)) - (gpt/subtract (gpt/point selrect-new) - (gpt/point selrect-old)) - (gpt/point 0 0)) + (gpt/subtract (gpt/point (:selrect new-shape)) + (gpt/point (:selrect shape))) new-shape (update new-shape :position-data gsh/move-position-data delta-move)] diff --git a/frontend/test/frontend_tests/data/workspace_texts_test.cljs b/frontend/test/frontend_tests/data/workspace_texts_test.cljs index 95deabf104..0d8a7ae5b0 100644 --- a/frontend/test/frontend_tests/data/workspace_texts_test.cljs +++ b/frontend/test/frontend_tests/data/workspace_texts_test.cljs @@ -6,7 +6,7 @@ (ns frontend-tests.data.workspace-texts-test (:require - [app.common.geom.point :as gpt] + [app.common.geom.rect :as grc] [app.common.types.shape :as cts] [app.main.data.workspace.texts :as dwt] [cljs.test :as t :include-macros true])) @@ -27,6 +27,24 @@ (some? position-data) (assoc :position-data position-data))) +(defn- make-degenerate-text-shape + "Simulate a text shape decoded from the server via map->Rect (which bypasses + make-rect's 0.01 minimum enforcement), giving it a zero-width / zero-height + selrect. This is the exact condition that triggered the original crash: + change-dimensions-modifiers divided by sr-width (== 0), producing an Infinity + scale factor that propagated through the transform pipeline until + calculate-selrect / center->rect returned nil, and then gpt/point threw + 'invalid arguments (on pointer constructor)'." + [& {:keys [x y width height] + :or {x 10 y 20 width 0 height 0}}] + (-> (make-text-shape :x x :y y :width 100 :height 50) + ;; Bypass make-rect by constructing the Rect record directly, the same + ;; way decode-rect does during JSON deserialization from the backend. + (assoc :selrect (grc/map->Rect {:x x :y y + :width width :height height + :x1 x :y1 y + :x2 (+ x width) :y2 (+ y height)})))) + (defn- sample-position-data "Return a minimal position-data vector with the supplied coords." [x y] @@ -145,28 +163,60 @@ (t/is (nil? (:position-data result)))))) ;; --------------------------------------------------------------------------- -;; Tests: shape with nil selrect (defensive guard for gpt/point delta path) +;; Tests: degenerate selrect (zero width or height decoded from the server) +;; +;; Root cause of the original crash: +;; change-dimensions-modifiers divided by (:width selrect) or (:height selrect) +;; which is 0 when the shape was decoded via map->Rect (bypassing make-rect's +;; 0.01 minimum), producing Infinity → transform pipeline returned nil selrect +;; → gpt/point threw "invalid arguments (on pointer constructor)". ;; --------------------------------------------------------------------------- -(t/deftest apply-text-modifier-nil-selrect-nil-modifier-does-not-throw - (t/testing "nil selrect + nil modifier returns shape unchanged without throwing" - ;; The nil-modifier guard fires first, so even a stripped shape is safe. - (let [shape (-> (make-text-shape) - (dissoc :selrect)) - result (dwt/apply-text-modifier shape nil)] - (t/is (= shape result))))) - -(t/deftest apply-text-modifier-only-position-data-nil-selrects-safe - (t/testing "position-data-only modifier with nil selrects does not throw" - ;; When only position-data is set, transform-shape is NOT called, so - ;; the selrect-nil guard in the delta calculation is exercised directly. - (let [pd (sample-position-data 5 10) - shape (-> (make-text-shape :x 0 :y 0 :width 100 :height 50) - (dissoc :selrect)) - modifier {:position-data pd} +(t/deftest apply-text-modifier-zero-width-selrect-does-not-throw + (t/testing "width modifier on a shape with zero selrect width does not throw" + ;; Simulates a shape received from the server whose selrect has width=0 + ;; (map->Rect bypasses the 0.01 floor of make-rect). + (let [shape (make-degenerate-text-shape :x 0 :y 0 :width 0 :height 50) + modifier {:width 200} result (dwt/apply-text-modifier shape modifier)] - ;; Should not throw; position-data is updated on the result - (t/is (= pd (:position-data result)))))) + (t/is (some? result)) + (t/is (some? (:selrect result)))))) + +(t/deftest apply-text-modifier-zero-height-selrect-does-not-throw + (t/testing "height modifier on a shape with zero selrect height does not throw" + (let [shape (make-degenerate-text-shape :x 0 :y 0 :width 100 :height 0) + modifier {:height 80} + result (dwt/apply-text-modifier shape modifier)] + (t/is (some? result)) + (t/is (some? (:selrect result)))))) + +(t/deftest apply-text-modifier-zero-width-and-height-selrect-does-not-throw + (t/testing "both modifiers on a fully-degenerate selrect do not throw" + (let [shape (make-degenerate-text-shape :x 0 :y 0 :width 0 :height 0) + modifier {:width 150 :height 60} + result (dwt/apply-text-modifier shape modifier)] + (t/is (some? result)) + (t/is (some? (:selrect result)))))) + +(t/deftest apply-text-modifier-zero-width-selrect-result-has-correct-width + (t/testing "applying width modifier to a zero-width shape yields the requested width" + (let [shape (make-degenerate-text-shape :x 0 :y 0 :width 0 :height 50) + modifier {:width 200} + result (dwt/apply-text-modifier shape modifier)] + (t/is (= 200.0 (-> result :selrect :width)))))) + +(t/deftest apply-text-modifier-zero-height-selrect-result-has-correct-height + (t/testing "applying height modifier to a zero-height shape yields the requested height" + (let [shape (make-degenerate-text-shape :x 0 :y 0 :width 100 :height 0) + modifier {:height 80} + result (dwt/apply-text-modifier shape modifier)] + (t/is (= 80.0 (-> result :selrect :height)))))) + +(t/deftest apply-text-modifier-nil-modifier-on-degenerate-shape-returns-unchanged + (t/testing "nil modifier on a zero-selrect shape returns the same shape" + (let [shape (make-degenerate-text-shape :x 0 :y 0 :width 0 :height 0) + result (dwt/apply-text-modifier shape nil)] + (t/is (identical? shape result))))) ;; --------------------------------------------------------------------------- ;; Tests: shape origin is preserved when there is no dimension change