From c28534555baa023647d460ca01e662ca575e117f Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Mon, 14 Nov 2022 11:08:15 +0100 Subject: [PATCH] :paperclip: Add minor microptimizations and tests to points->rect --- common/src/app/common/data.cljc | 34 ++++++++++++------- common/src/app/common/geom/shapes/rect.cljc | 23 +++++++++---- common/src/app/common/math.cljc | 5 ++- common/test/common_tests/data_test.cljc | 14 ++++++++ .../test/common_tests/geom_shapes_test.cljc | 23 +++++++++++-- 5 files changed, 76 insertions(+), 23 deletions(-) diff --git a/common/src/app/common/data.cljc b/common/src/app/common/data.cljc index ff7d98f28..03fe97ca3 100644 --- a/common/src/app/common/data.cljc +++ b/common/src/app/common/data.cljc @@ -555,22 +555,32 @@ (defn num? "Checks if a value `val` is a number but not an Infinite or NaN" - ([val] - (and (number? val) - (mth/finite? val) - (not (mth/nan? val)))) - - ([val & vals] - (and (num? val) - (->> vals (every? num?))))) + ([a] + (mth/finite? a)) + ([a b] + (and (mth/finite? a) + (mth/finite? b))) + ([a b c] + (and (mth/finite? a) + (mth/finite? b) + (mth/finite? c))) + ([a b c d] + (and (mth/finite? a) + (mth/finite? b) + (mth/finite? c) + (mth/finite? d))) + ([a b c d & others] + (and (mth/finite? a) + (mth/finite? b) + (mth/finite? c) + (mth/finite? d) + (every? mth/finite? others)))) (defn check-num "Function that checks if a number is nil or nan. Will return 0 when not valid and the number otherwise." - ([v] - (check-num v 0)) - ([v default] - (if (num? v) v default))) + ([v] (mth/finite v 0)) + ([v default] (mth/finite v default))) (defn any-key? [element & rest] (some #(contains? element %) rest)) diff --git a/common/src/app/common/geom/shapes/rect.cljc b/common/src/app/common/geom/shapes/rect.cljc index 672928a61..057687e1e 100644 --- a/common/src/app/common/geom/shapes/rect.cljc +++ b/common/src/app/common/geom/shapes/rect.cljc @@ -83,13 +83,22 @@ (defn points->rect [points] - (when (d/not-empty? points) - (let [minx (transduce (keep :x) min ##Inf points) - miny (transduce (keep :y) min ##Inf points) - maxx (transduce (keep :x) max ##-Inf points) - maxy (transduce (keep :y) max ##-Inf points)] - (when (d/num? minx miny maxx maxy) - (make-rect minx miny (- maxx minx) (- maxy miny)))))) + (when-let [points (seq points)] + (loop [minx ##Inf + miny ##Inf + maxx ##-Inf + maxy ##-Inf + pts points] + (if-let [pt (first pts)] + (let [x (d/get-prop pt :x) + y (d/get-prop pt :y)] + (recur (min minx x) + (min miny y) + (max maxx x) + (max maxy y) + (rest pts))) + (when (d/num? minx miny maxx maxy) + (make-rect minx miny (- maxx minx) (- maxy miny))))))) (defn bounds->rect [[{ax :x ay :y} {bx :x by :y} {cx :x cy :y} {dx :x dy :y}]] diff --git a/common/src/app/common/math.cljc b/common/src/app/common/math.cljc index e5483b118..eee928bd9 100644 --- a/common/src/app/common/math.cljc +++ b/common/src/app/common/math.cljc @@ -19,10 +19,13 @@ #?(:cljs (js/isNaN v) :clj (Double/isNaN v))) +;; NOTE: on cljs we don't need to check for `number?` so we explicitly +;; ommit it for performance reasons. + (defn finite? [v] #?(:cljs (and (not (nil? v)) (js/isFinite v)) - :clj (and (not (nil? v)) (Double/isFinite v)))) + :clj (and (not (nil? v)) (number? v) (Double/isFinite v)))) (defn finite [v default] diff --git a/common/test/common_tests/data_test.cljc b/common/test/common_tests/data_test.cljc index 5fd35b920..0c0521003 100644 --- a/common/test/common_tests/data_test.cljc +++ b/common/test/common_tests/data_test.cljc @@ -51,3 +51,17 @@ (t/is (= [1 10 100 2 20 200 3 30 300] (d/join [1 2 3] [1 10 100] *)))) +(t/deftest num-predicate + (t/is (not (d/num? ##NaN))) + (t/is (not (d/num? nil))) + (t/is (d/num? 1)) + (t/is (d/num? -0.3)) + (t/is (not (d/num? {})))) + +(t/deftest check-num-helper + (t/is (= 1 (d/check-num 1 0))) + (t/is (= 0 (d/check-num ##NaN 0))) + (t/is (= 0 (d/check-num {} 0))) + (t/is (= 0 (d/check-num [] 0))) + (t/is (= 0 (d/check-num :foo 0))) + (t/is (= 0 (d/check-num nil 0)))) diff --git a/common/test/common_tests/geom_shapes_test.cljc b/common/test/common_tests/geom_shapes_test.cljc index 864bf6b8c..a9d1e2df7 100644 --- a/common/test/common_tests/geom_shapes_test.cljc +++ b/common/test/common_tests/geom_shapes_test.cljc @@ -21,7 +21,8 @@ {:command :curve-to :params {:x 40 :y 40 :c1x 35 :c1y 35 :c2x 45 :c2y 45}} {:command :close-path}]) -(defn add-path-data [shape] +(defn add-path-data + [shape] (let [content (:content shape default-path) selrect (gsh/content->selrect content) points (gsh/rect->points selrect)] @@ -30,7 +31,8 @@ :selrect selrect :points points))) -(defn add-rect-data [shape] +(defn add-rect-data + [shape] (let [shape (-> shape (assoc :width 20 :height 20)) selrect (gsh/rect->selrect shape) @@ -49,7 +51,7 @@ (not= type :path) (add-rect-data))))) -(t/deftest transform-shape-tests +(t/deftest transform-shapes (t/testing "Shape without modifiers should stay the same" (t/are [type] (let [shape-before (create-test-shape type) @@ -181,3 +183,18 @@ :path {:x 0.0 :y 0.0 :x1 0.0 :y1 0.0 :x2 ##Inf :y2 ##Inf :width ##Inf :height ##Inf} :rect nil :path nil))) + +(t/deftest points-to-selrect + (let [points [(gpt/point 0.5 0.5) + (gpt/point -1 -2) + (gpt/point 20 65.2) + (gpt/point 12 -10)] + result (gsh/points->rect points) + expect {:x -1, :y -10, :width 21, :height 75.2}] + + (t/is (= (:x expect) (:x result))) + (t/is (= (:y expect) (:y result))) + (t/is (= (:width expect) (:width result))) + (t/is (= (:height expect) (:height result))) + )) +