From 108cdcecbb58be6ad4d8bbda4b58da749263558c Mon Sep 17 00:00:00 2001 From: Alejandro Alonso Date: Mon, 5 Sep 2022 11:56:51 +0200 Subject: [PATCH] :tada: Improve viewer performance degradation because of fixed position --- .../styles/main/partials/viewer.scss | 14 ++++ frontend/src/app/main/ui/shapes/group.cljs | 12 +--- frontend/src/app/main/ui/shapes/mask.cljs | 6 -- frontend/src/app/main/ui/viewer.cljs | 48 ++++++------- .../app/main/ui/viewer/handoff/render.cljs | 2 +- .../src/app/main/ui/viewer/interactions.cljs | 67 ++++++++++++++----- frontend/src/app/main/ui/viewer/shapes.cljs | 27 +------- 7 files changed, 91 insertions(+), 85 deletions(-) diff --git a/frontend/resources/styles/main/partials/viewer.scss b/frontend/resources/styles/main/partials/viewer.scss index 4fa9b3de1..0f1ddcdf9 100644 --- a/frontend/resources/styles/main/partials/viewer.scss +++ b/frontend/resources/styles/main/partials/viewer.scss @@ -171,6 +171,20 @@ } .viewport-container { + clip-path: inset(0 0 0 0); grid-column: 1 / 1; grid-row: 1 / 1; + + .not-fixed { + position: absolute; + } + + .fixed { + position: fixed; + pointer-events: none; + + .frame-children g { + pointer-events: auto; + } + } } diff --git a/frontend/src/app/main/ui/shapes/group.cljs b/frontend/src/app/main/ui/shapes/group.cljs index 664c4e834..bf792757e 100644 --- a/frontend/src/app/main/ui/shapes/group.cljs +++ b/frontend/src/app/main/ui/shapes/group.cljs @@ -42,21 +42,13 @@ (if masked-group? ["g" (-> (obj/create) (obj/set! "mask" (mask-url render-id mask)))] - [mf/Fragment nil]) - - ;; This factory is generic, it's used for viewer, workspace and handoff. - ;; These props are generated in viewer wrappers only, in the rest of the - ;; cases these props will be nil, not affecting the code. - delta (unchecked-get props "delta") - fixed? (unchecked-get props "fixed?")] + [mf/Fragment nil])] [:> clip-wrapper clip-props [:> mask-wrapper mask-props (when masked-group? [:> render-mask #js {:mask mask - :objects objects - :delta delta - :fixed? fixed?}]) + :objects objects}]) (for [item childs] [:& shape-wrapper {:shape item diff --git a/frontend/src/app/main/ui/shapes/mask.cljs b/frontend/src/app/main/ui/shapes/mask.cljs index c43b12a60..a06098173 100644 --- a/frontend/src/app/main/ui/shapes/mask.cljs +++ b/frontend/src/app/main/ui/shapes/mask.cljs @@ -50,13 +50,7 @@ render-id (mf/use-ctx muc/render-id) svg-text? (and (= :text (:type mask)) (some? (:position-data mask))) - ;; This factory is generic, it's used for viewer, workspace and handoff. - ;; These props are generated in viewer wrappers only, in the rest of the - ;; cases these props will be nil, not affecting the code. - fixed? (unchecked-get props "fixed?") - delta (unchecked-get props "delta") mask-bb (-> (gsh/transform-shape mask) - (cond-> fixed? (gsh/move delta)) (:points)) mask-bb-rect (gsh/points->rect mask-bb)] diff --git a/frontend/src/app/main/ui/viewer.cljs b/frontend/src/app/main/ui/viewer.cljs index 5eef0464d..872bb5345 100644 --- a/frontend/src/app/main/ui/viewer.cljs +++ b/frontend/src/app/main/ui/viewer.cljs @@ -255,7 +255,6 @@ fullscreen? (mf/deref header/fullscreen-ref) overlays (mf/deref current-overlays-ref) - scroll (mf/use-state nil) orig-frame (mf/with-memo [current-animations] @@ -309,11 +308,6 @@ size (dom/get-client-size viewer-section)] (st/emit! (dv/set-viewport-size {:size size}))))) - on-scroll - (mf/use-fn - (fn [event] - (reset! scroll (dom/get-target-scroll event)))) - on-wheel (mf/use-fn (fn [event] @@ -354,12 +348,10 @@ (mf/with-effect [] (dom/set-html-theme-color clr/gray-50 "dark") (let [key1 (events/listen js/window "click" on-click) - key2 (events/listen (mf/ref-val viewer-section-ref) "scroll" on-scroll #js {"passive" true}) - key3 (events/listen (mf/ref-val viewer-section-ref) "wheel" on-wheel #js {"passive" false})] + key2 (events/listen (mf/ref-val viewer-section-ref) "wheel" on-wheel #js {"passive" false})] (fn [] (events/unlistenByKey key1) - (events/unlistenByKey key2) - (events/unlistenByKey key3)))) + (events/unlistenByKey key2)))) (mf/use-layout-effect (fn [] @@ -499,24 +491,24 @@ :index index :viewer-pagination viewer-pagination}] - [:& (mf/provider ctx/current-scroll) {:value @scroll} - [:& (mf/provider ctx/current-zoom) {:value zoom} - [:& viewer-wrapper - {:wrapper-size wrapper-size - :orig-frame orig-frame - :orig-viewport-ref orig-viewport-ref - :orig-size orig-size - :page page - :file file - :users users - :current-viewport-ref current-viewport-ref - :size size - :frame frame - :interactions-mode interactions-mode - :overlays overlays - :zoom zoom - :section section - :index index}]]]))]]])) + + [:& (mf/provider ctx/current-zoom) {:value zoom} + [:& viewer-wrapper + {:wrapper-size wrapper-size + :orig-frame orig-frame + :orig-viewport-ref orig-viewport-ref + :orig-size orig-size + :page page + :file file + :users users + :current-viewport-ref current-viewport-ref + :size size + :frame frame + :interactions-mode interactions-mode + :overlays overlays + :zoom zoom + :section section + :index index}]]))]]])) ;; --- Component: Viewer Page diff --git a/frontend/src/app/main/ui/viewer/handoff/render.cljs b/frontend/src/app/main/ui/viewer/handoff/render.cljs index 61d982910..1b90da72c 100644 --- a/frontend/src/app/main/ui/viewer/handoff/render.cljs +++ b/frontend/src/app/main/ui/viewer/handoff/render.cljs @@ -189,7 +189,7 @@ (mf/defc render-frame-svg [{:keys [page frame local size]}] (let [objects (mf/with-memo [page frame size] - (prepare-objects page frame size)) + (prepare-objects frame size (:objects page))) ;; Retrieve frame again with correct modifier frame (get objects (:id frame)) diff --git a/frontend/src/app/main/ui/viewer/interactions.cljs b/frontend/src/app/main/ui/viewer/interactions.cljs index aa479c9cb..6260f32be 100644 --- a/frontend/src/app/main/ui/viewer/interactions.cljs +++ b/frontend/src/app/main/ui/viewer/interactions.cljs @@ -12,6 +12,7 @@ [app.common.geom.point :as gpt] [app.common.pages.helpers :as cph] [app.common.types.page :as ctp] + [app.common.uuid :as uuid] [app.main.data.comments :as dcm] [app.main.data.viewer :as dv] [app.main.refs :as refs] @@ -27,8 +28,8 @@ [rumext.alpha :as mf])) (defn prepare-objects - [page frame size] - (let [objects (:objects page) + [frame size objects] + (let [ frame-id (:id frame) modifier (-> (gpt/point (:x size) (:y size)) (gpt/negate) @@ -52,26 +53,60 @@ vbox (:vbox size) - objects (mf/with-memo [page frame size] - (prepare-objects page frame size)) + fixed-ids (filter :fixed-scroll (vals (:objects page))) - wrapper (mf/with-memo [objects] - (shapes/frame-container-factory objects)) + ;; we have con consider the children if the fixed element is a group + fixed-children-ids (into #{} (mapcat #(cph/get-children-ids (:objects page) (:id %)) fixed-ids)) + + parent-children-ids (->> fixed-ids + (mapcat #(cons (:id %) (cph/get-parent-ids (:objects page) (:id %)))) + (remove #(= % uuid/zero))) + + fixed-ids (concat fixed-children-ids parent-children-ids) + + not-fixed-ids (->> (remove (set fixed-ids) (keys (:objects page))) + (remove #(= % uuid/zero))) + + calculate-objects (fn [ids] (->> ids + (map (d/getf (:objects page))) + (concat [frame]) + (d/index-by :id) + (prepare-objects frame size))) + + wrapper-fixed (mf/with-memo [page frame size] + (shapes/frame-container-factory (calculate-objects fixed-ids))) + + objects-not-fixed (mf/with-memo [page frame size] + (calculate-objects not-fixed-ids)) + + wrapper-not-fixed (mf/with-memo [objects-not-fixed] + (shapes/frame-container-factory objects-not-fixed)) ;; Retrieve frames again with correct modifier - frame (get objects (:id frame)) - base (get objects (:id base))] + frame (get objects-not-fixed (:id frame)) + base (get objects-not-fixed (:id base))] [:& (mf/provider shapes/base-frame-ctx) {:value base} [:& (mf/provider shapes/frame-offset-ctx) {:value offset} - [:svg {:view-box vbox - :width (:width size) - :height (:height size) - :version "1.1" - :xmlnsXlink "http://www.w3.org/1999/xlink" - :xmlns "http://www.w3.org/2000/svg" - :fill "none"} - [:& wrapper {:shape frame :view-box vbox}]]]])) + ;; We have two different svgs for fixed and not fixed elements so we can emulate the sticky css attribute in svg + [:svg.not-fixed {:view-box vbox + :width (:width size) + :height (:height size) + :version "1.1" + :xmlnsXlink "http://www.w3.org/1999/xlink" + :xmlns "http://www.w3.org/2000/svg" + :fill "none"} + [:& wrapper-not-fixed {:shape frame :view-box vbox}]] + [:svg.fixed {:view-box vbox + :width (:width size) + :height (:height size) + :version "1.1" + :xmlnsXlink "http://www.w3.org/1999/xlink" + :xmlns "http://www.w3.org/2000/svg" + :fill "none" + :style {:width (:width size) + :height (:height size)}} + [:& wrapper-fixed {:shape (dissoc frame :fills) :view-box vbox}]]]])) (mf/defc viewport {::mf/wrap [mf/memo] diff --git a/frontend/src/app/main/ui/viewer/shapes.cljs b/frontend/src/app/main/ui/viewer/shapes.cljs index d0609383f..f2c65a450 100644 --- a/frontend/src/app/main/ui/viewer/shapes.cljs +++ b/frontend/src/app/main/ui/viewer/shapes.cljs @@ -7,14 +7,12 @@ (ns app.main.ui.viewer.shapes "The main container for a frame in viewer mode" (:require - [app.common.data :as d] [app.common.geom.shapes :as gsh] [app.common.pages.helpers :as cph] [app.common.types.shape.interactions :as ctsi] [app.main.data.viewer :as dv] [app.main.refs :as refs] [app.main.store :as st] - [app.main.ui.context :as ctx] [app.main.ui.shapes.bool :as bool] [app.main.ui.shapes.circle :as circle] [app.main.ui.shapes.frame :as frame] @@ -223,8 +221,6 @@ childs (unchecked-get props "childs") frame (unchecked-get props "frame") objects (unchecked-get props "objects") - fixed? (unchecked-get props "fixed?") - delta (unchecked-get props "delta") base-frame (mf/use-ctx base-frame-ctx) frame-offset (mf/use-ctx frame-offset-ctx) @@ -269,9 +265,7 @@ :frame frame :childs childs :is-child-selected? true - :objects objects - :fixed? fixed? - :delta delta}] + :objects objects}] [:& interaction {:shape shape :interactions interactions @@ -393,20 +387,6 @@ (let [shape (unchecked-get props "shape") frame (unchecked-get props "frame") - ;; TODO: this watch of scroll position is killing - ;; performance of the viewer. - scroll (mf/use-ctx ctx/current-scroll) - zoom (mf/use-ctx ctx/current-zoom) - - fixed? (mf/with-memo [shape objects] - (->> (cph/get-parent-ids objects (:id shape)) - (map (d/getf objects)) - (concat [shape]) - (some :fixed-scroll))) - - delta {:x (/ (:scroll-left scroll) zoom) - :y (/ (:scroll-top scroll) zoom)} - group-container (mf/with-memo [objects] (group-container-factory objects)) @@ -426,8 +406,7 @@ ] (when (and shape (not (:hidden shape))) (let [shape (-> (gsh/transform-shape shape) - (gsh/translate-to-frame frame) - (cond-> fixed? (gsh/move delta))) + (gsh/translate-to-frame frame)) opts #js {:shape shape :objects objects}] @@ -438,6 +417,6 @@ :path [:> path-wrapper opts] :image [:> image-wrapper opts] :circle [:> circle-wrapper opts] - :group [:> group-container {:shape shape :frame frame :objects objects :fixed? fixed? :delta delta}] + :group [:> group-container {:shape shape :frame frame :objects objects}] :bool [:> bool-container {:shape shape :frame frame :objects objects}] :svg-raw [:> svg-raw-container {:shape shape :frame frame :objects objects}])))))))