From c10c7b08aaf693a907de30d0f5d704477c619a39 Mon Sep 17 00:00:00 2001 From: AK <144495202+AKnassa@users.noreply.github.com> Date: Fri, 24 Jul 2026 07:29:37 -0400 Subject: [PATCH] :bug: Fix radial gradient handles on rotated ellipses (#10666) The handle transform composed rotation incorrectly, so handles blew out in size when an ellipse was rotated. Fixes #10069 Signed-off-by: Akshit Nassa Co-authored-by: Akshit Nassa Co-authored-by: Andrey Antukh --- .../main/ui/workspace/viewport/gradients.cljs | 51 +++++++---- frontend/test/frontend_tests/runner.cljs | 2 + .../ui/gradient_handlers_test.cljs | 90 +++++++++++++++++++ 3 files changed, 127 insertions(+), 16 deletions(-) create mode 100644 frontend/test/frontend_tests/ui/gradient_handlers_test.cljs diff --git a/frontend/src/app/main/ui/workspace/viewport/gradients.cljs b/frontend/src/app/main/ui/workspace/viewport/gradients.cljs index bc16b89045..7f1abe92e9 100644 --- a/frontend/src/app/main/ui/workspace/viewport/gradients.cljs +++ b/frontend/src/app/main/ui/workspace/viewport/gradients.cljs @@ -447,12 +447,38 @@ :r (/ 4 zoom) :fill "var(--app-white)"}]))])) +;; The gradient geometry is defined in objectBoundingBox units, so the +;; perpendicular of the gradient vector has to be taken in that normalized +;; space and only afterwards mapped to the shape dimensions. Doing it the other +;; way around blows the handler up by the shape aspect ratio (see #10069). + +(defn radial-width-point + "Position, in shape local coordinates, of the radial gradient width handler." + [{:keys [x y width height]} gradient] + (let [{:keys [start-x start-y end-x end-y] gwidth :width} gradient + vx (- end-x start-x) + vy (- end-y start-y)] + (gpt/point (+ x (* width (+ start-x (* gwidth vy)))) + (+ y (* height (- start-y (* gwidth vx))))))) + +(defn point->gradient-width + "Inverse of `radial-width-point`: the radial gradient `:width` factor that + places its width handler on the given shape local coordinates point." + [{:keys [x y width height]} gradient point] + (let [{:keys [start-x start-y end-x end-y]} gradient + dx (- (/ (- (:x point) x) width) start-x) + dy (- (/ (- (:y point) y) height) start-y)] + (/ (mth/hypot dx dy) + (mth/hypot (- end-x start-x) (- end-y start-y))))) + (mf/defc gradient-handlers-impl* [{:keys [zoom stops gradient editing shape]}] (let [transform (gsh/transform-matrix shape) transform-inverse (gsh/inverse-transform-matrix shape) - {:keys [x y width height] :as sr} (:selrect shape) + selrect (:selrect shape) + + {:keys [x y width height]} selrect from-p (-> (gpt/point (+ x (* width (:start-x gradient))) (+ y (* height (:start-y gradient)))) @@ -461,15 +487,9 @@ (+ y (* height (:end-y gradient)))) (gpt/transform transform)) - gradient-vec (gpt/to-vec from-p to-p) - gradient-length (gpt/length gradient-vec) - - width-v (-> gradient-vec - (gpt/normal-right) - (gpt/multiply (gpt/point (* (:width gradient) (/ gradient-length (/ height 2))))) - (gpt/multiply (gpt/point (/ width 2)))) - - width-p (gpt/add from-p width-v) + width-p (when (= :radial (:type gradient)) + (-> (radial-width-point selrect gradient) + (gpt/transform transform))) change! (mf/use-fn @@ -496,19 +516,18 @@ on-change-width (mf/use-fn - (mf/deps gradient-length width height) + (mf/deps transform-inverse selrect gradient) (fn [point] - (let [scale-factor-y (/ gradient-length (/ height 2)) - norm-dist (/ (gpt/distance point from-p) - (* (/ width 2) scale-factor-y))] - (when (and norm-dist (d/num? norm-dist)) + (let [point (gpt/transform point transform-inverse) + norm-dist (point->gradient-width selrect gradient point)] + (when (d/num? norm-dist) (change! {:width norm-dist})))))] [:> gradient-handler-transformed* {:editing editing :from-p from-p :to-p to-p - :width-p (when (= :radial (:type gradient)) width-p) + :width-p width-p :stops stops :zoom zoom :on-change-start on-change-start diff --git a/frontend/test/frontend_tests/runner.cljs b/frontend/test/frontend_tests/runner.cljs index ee116cf11f..7b85bfdcf7 100644 --- a/frontend/test/frontend_tests/runner.cljs +++ b/frontend/test/frontend_tests/runner.cljs @@ -60,6 +60,7 @@ [frontend-tests.ui.comments-clustering-test] [frontend-tests.ui.comments-position-modifier-test] [frontend-tests.ui.ds-controls-numeric-input-test] + [frontend-tests.ui.gradient-handlers-test] [frontend-tests.ui.layout-container-multiple-test] [frontend-tests.ui.measures-menu-props-test] [frontend-tests.ui.settings-password-schema-test] @@ -140,6 +141,7 @@ 'frontend-tests.ui.comments-clustering-test 'frontend-tests.ui.comments-position-modifier-test 'frontend-tests.ui.ds-controls-numeric-input-test + 'frontend-tests.ui.gradient-handlers-test 'frontend-tests.ui.layout-container-multiple-test 'frontend-tests.ui.measures-menu-props-test 'frontend-tests.ui.settings-password-schema-test diff --git a/frontend/test/frontend_tests/ui/gradient_handlers_test.cljs b/frontend/test/frontend_tests/ui/gradient_handlers_test.cljs new file mode 100644 index 0000000000..c20deaca39 --- /dev/null +++ b/frontend/test/frontend_tests/ui/gradient_handlers_test.cljs @@ -0,0 +1,90 @@ +;; 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.ui.gradient-handlers-test + (:require + [app.common.geom.point :as gpt] + [app.common.math :as mth] + [app.main.ui.workspace.viewport.gradients :refer [radial-width-point point->gradient-width]] + [cljs.test :as t :include-macros true])) + +;; A 4:1 ellipse. The bug (#10069) only shows on non-square shapes. +(def ^:private wide-sr {:x 0 :y 0 :width 400 :height 100}) +(def ^:private square-sr {:x 0 :y 0 :width 100 :height 100}) + +(defn- radial + [start-x start-y end-x end-y width] + {:type :radial + :start-x start-x + :start-y start-y + :end-x end-x + :end-y end-y + :width width}) + +(defn- close? + [a b] + (< (mth/abs (- a b)) 0.0001)) + +(defn- point-close? + [p1 p2] + (and (close? (:x p1) (:x p2)) + (close? (:y p1) (:y p2)))) + +(t/deftest test-radial-width-point + (t/testing "default vertical gradient is not affected" + ;; No regression allowed here: the handler already sits at the correct + ;; place when the gradient vector is axis aligned. + (let [gradient (radial 0.5 0.5 0.5 1.0 1.0)] + (t/is (point-close? (gpt/point 400 50) + (radial-width-point wide-sr gradient))))) + + (t/testing "rotated gradient keeps the handler inside the shape" + ;; The gradient is defined in objectBoundingBox units, so the half-width + ;; of a 90 degrees rotated gradient must be half the shape height (50), + ;; not half the shape width scaled again by the aspect ratio (800). + (let [gradient (radial 0.5 0.5 1.0 0.5 1.0)] + (t/is (point-close? (gpt/point 200 0) + (radial-width-point wide-sr gradient))))) + + (t/testing "oblique gradient is anisotropically scaled" + (let [gradient (radial 0.5 0.5 1.0 1.0 1.0)] + (t/is (point-close? (gpt/point 400 0) + (radial-width-point wide-sr gradient))))) + + (t/testing "gradient width factor scales the handler distance" + (let [gradient (radial 0.5 0.5 1.0 0.5 0.5)] + (t/is (point-close? (gpt/point 200 25) + (radial-width-point wide-sr gradient))))) + + (t/testing "square shapes are isotropic" + (let [gradient (radial 0.5 0.5 1.0 0.5 1.0)] + (t/is (point-close? (gpt/point 50 0) + (radial-width-point square-sr gradient))))) + + (t/testing "translated selrect" + (let [gradient (radial 0.5 0.5 1.0 0.5 1.0)] + (t/is (point-close? (gpt/point 210 15) + (radial-width-point (assoc wide-sr :x 10 :y 15) gradient)))))) + +(t/deftest test-point->gradient-width + (t/testing "is the inverse of radial-width-point" + (doseq [gradient [(radial 0.5 0.5 0.5 1.0 1.0) + (radial 0.5 0.5 1.0 0.5 1.0) + (radial 0.5 0.5 1.0 1.0 0.5) + (radial 0.2 0.3 0.9 0.7 2.0)] + selrect [wide-sr square-sr]] + (t/is (close? (:width gradient) + (point->gradient-width selrect gradient + (radial-width-point selrect gradient)))))) + + (t/testing "handler on the start point means zero width" + (let [gradient (radial 0.5 0.5 1.0 0.5 1.0)] + (t/is (close? 0 (point->gradient-width wide-sr gradient (gpt/point 200 50)))))) + + (t/testing "degenerate gradient does not produce a usable width" + (let [gradient (radial 0.5 0.5 0.5 0.5 1.0)] + (t/is (not (js/Number.isFinite + (point->gradient-width wide-sr gradient (gpt/point 200 0))))))))