mirror of
https://github.com/penpot/penpot.git
synced 2026-03-25 18:51:16 -04:00
🐛 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 <niwi@niwi.nz>
This commit is contained in:
@@ -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)]
|
||||
|
||||
@@ -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)]
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user