From b91c42e186ad0054d9d23ebc05af955858cb150a Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Mon, 21 Mar 2022 09:25:19 +0100 Subject: [PATCH] :zap: Add performance improvements to file thumbnails Mainly addresing unnecesary object transmission. The new code strips unnecesary data to be transferred from back to front. Additionally it removes some legacy code and simplifies other parts of code. --- backend/src/app/rpc/mutations/files.clj | 2 +- backend/src/app/rpc/queries/files.clj | 91 +++++++++++------------ common/src/app/common/spec/shape.cljc | 4 +- frontend/src/app/main/data/workspace.cljs | 32 +++----- frontend/src/app/main/render.cljs | 25 ------- frontend/src/app/worker/thumbnails.cljs | 36 ++++----- 6 files changed, 72 insertions(+), 118 deletions(-) diff --git a/backend/src/app/rpc/mutations/files.clj b/backend/src/app/rpc/mutations/files.clj index d3c605534..19a00a404 100644 --- a/backend/src/app/rpc/mutations/files.clj +++ b/backend/src/app/rpc/mutations/files.clj @@ -320,7 +320,7 @@ _ (mtx/run! metrics {:id :update-file-changes :inc (count changes)}) ts (dt/now) - file (-> (files/retrieve-data cfg file) + file (-> file (update :revn inc) (update :data (fn [data] ;; Trace the length of bytes of processed data diff --git a/backend/src/app/rpc/queries/files.clj b/backend/src/app/rpc/queries/files.clj index 30e349e8f..9706c9e17 100644 --- a/backend/src/app/rpc/queries/files.clj +++ b/backend/src/app/rpc/queries/files.clj @@ -16,11 +16,9 @@ [app.rpc.queries.projects :as projects] [app.rpc.queries.share-link :refer [retrieve-share-link]] [app.rpc.queries.teams :as teams] - [app.storage.impl :as simpl] [app.util.blob :as blob] [app.util.services :as sv] - [clojure.spec.alpha :as s] - [promesa.core :as p])) + [clojure.spec.alpha :as s])) (declare decode-row) (declare decode-row-xf) @@ -186,25 +184,12 @@ ;; --- Query: File (By ID) -(defn- retrieve-data* - [{:keys [storage] :as cfg} file] - (p/do - (when-let [backend (simpl/resolve-backend storage (:data-backend file))] - (simpl/get-object-bytes backend file)))) - -(defn retrieve-data - [cfg file] - (if (bytes? (:data file)) - file - (p/->> (retrieve-data* cfg file) - (assoc file :data)))) - (defn retrieve-file [{:keys [pool] :as cfg} id] - (p/->> (db/get-by-id pool :file id) - (retrieve-data cfg) + (let [item (db/get-by-id pool :file id)] + (->> item (decode-row) - (pmg/migrate-file))) + (pmg/migrate-file)))) (s/def ::file (s/keys :req-un [::profile-id ::id])) @@ -214,8 +199,8 @@ [{:keys [pool] :as cfg} {:keys [profile-id id] :as params}] (let [perms (get-permissions pool profile-id id)] (check-read-permissions! perms) - (p/-> (retrieve-file cfg id) - (assoc :permissions perms)))) + (-> (retrieve-file cfg id) + (assoc :permissions perms)))) (declare trim-file-data) @@ -233,9 +218,9 @@ [{:keys [pool] :as cfg} {:keys [profile-id id] :as params}] (let [perms (get-permissions pool profile-id id)] (check-read-permissions! perms) - (p/-> (retrieve-file cfg id) - (trim-file-data params) - (assoc :permissions perms)))) + (-> (retrieve-file cfg id) + (trim-file-data params) + (assoc :permissions perms)))) (defn- trim-file-data [file {:keys [page-id object-id]}] @@ -248,9 +233,12 @@ (update :data assoc :pages-index {page-id page}) (update :data assoc :pages [page-id])))) +;; --- FILE THUMBNAIL + (declare strip-frames-with-thumbnails) (declare extract-file-thumbnail) (declare get-first-page-data) +(declare get-thumbnail-data) (s/def ::strip-frames-with-thumbnails ::us/boolean) @@ -258,6 +246,17 @@ (s/keys :req-un [::profile-id ::file-id] :opt-un [::strip-frames-with-thumbnails])) +(sv/defmethod ::page + "Retrieves the first page of the file. Used mainly for render + thumbnails on dashboard. + + DEPRECATED: still here for backward compatibility." + [{:keys [pool] :as cfg} {:keys [profile-id file-id] :as props}] + (check-read-permissions! pool profile-id file-id) + (let [file (retrieve-file cfg file-id) + data (get-first-page-data file props)] + data)) + (s/def ::file-data-for-thumbnail (s/keys :req-un [::profile-id ::file-id] :opt-un [::strip-frames-with-thumbnails])) @@ -267,20 +266,29 @@ thumbnails on dashboard." [{:keys [pool] :as cfg} {:keys [profile-id file-id] :as props}] (check-read-permissions! pool profile-id file-id) - (p/let [file (retrieve-file cfg file-id) - data (get-first-page-data file props) - file-thumbnail (extract-file-thumbnail (get-in file [:data :pages-index]))] + (let [file (retrieve-file cfg file-id)] + (get-thumbnail-data file props))) - (assoc data :file-thumbnail file-thumbnail))) +(defn get-thumbnail-data + [{:keys [data] :as file} props] + (if-let [[page frame] (first + (for [page (-> data :pages-index vals) + frame (-> page :objects cph/get-frames) + :when (:file-thumbnail frame)] + [page frame]))] + (let [objects (->> (cph/get-children-with-self (:objects page) (:id frame)) + (d/index-by :id))] + (cond-> (assoc page :objects objects) + (:strip-frames-with-thumbnails props) + (strip-frames-with-thumbnails) -(sv/defmethod ::page - "Retrieves the first page of the file. Used mainly for render - thumbnails on dashboard." - [{:keys [pool] :as cfg} {:keys [profile-id file-id] :as props}] - (check-read-permissions! pool profile-id file-id) - (p/let [file (retrieve-file cfg file-id) - data (get-first-page-data file props)] - data)) + :always + (assoc :thumbnail-frame frame))) + + (let [page-id (-> data :pages first)] + (cond-> (get-in data [:pages-index page-id]) + (:strip-frames-with-thumbnails props) + (strip-frames-with-thumbnails))))) (defn get-first-page-data [file props] @@ -318,16 +326,6 @@ (update data :objects update-objects))) -(defn extract-file-thumbnail - "Extract the frame marked as file-thumbnail" - [pages] - (->> pages - vals - (mapcat :objects) - vals - (filter :file-thumbnail) - first)) - ;; --- Query: Shared Library Files (def ^:private sql:team-shared-files @@ -384,7 +382,6 @@ [{:keys [pool] :as cfg} is-indirect file-id] (let [xform (comp (map #(assoc % :is-indirect is-indirect)) - (map #(retrieve-data cfg %)) (map decode-row))] (into #{} xform (db/exec! pool [sql:file-libraries file-id])))) diff --git a/common/src/app/common/spec/shape.cljc b/common/src/app/common/spec/shape.cljc index 422c95f1e..e2732a0f4 100644 --- a/common/src/app/common/spec/shape.cljc +++ b/common/src/app/common/spec/shape.cljc @@ -53,6 +53,7 @@ (s/def ::hide-fill-on-export boolean?) +(s/def ::file-thumbnail boolean?) (s/def ::masked-group? boolean?) (s/def ::font-family string?) (s/def ::font-size ::us/safe-integer) @@ -301,7 +302,8 @@ (defmethod shape-spec :frame [_] (s/and ::shape-attrs - (s/keys :opt-un [::hide-fill-on-export]))) + (s/keys :opt-un [::file-thumbnail + ::hide-fill-on-export]))) (s/def ::shape (s/and (s/multi-spec shape-spec :type) diff --git a/frontend/src/app/main/data/workspace.cljs b/frontend/src/app/main/data/workspace.cljs index ada2bcc73..8d85661df 100644 --- a/frontend/src/app/main/data/workspace.cljs +++ b/frontend/src/app/main/data/workspace.cljs @@ -957,35 +957,23 @@ (let [selected (wsh/lookup-selected state)] (rx/of (dch/update-shapes selected #(update % :blocked not))))))) -(defn extract-file-thumbnails-from-page - [state selected page] - (let [extract-frames (fn [page-id] - (let [objects (wsh/lookup-page-objects state page-id)] - (cph/get-frames objects))) - page-id (key page) - frames-with-thumbnail (->> (extract-frames page-id) - (filter (comp true? :file-thumbnail)) - (map :id) - (remove #(some #{%} selected)) - (map #(into {} {:id % :page-id page-id})))] - (when frames-with-thumbnail frames-with-thumbnail))) - - (defn toggle-file-thumbnail-selected [] (ptk/reify ::toggle-file-thumbnail-selected ptk/WatchEvent (watch [_ state _] (let [selected (wsh/lookup-selected state) - pages (get-in state [:workspace-data - :pages-index]) - file-thumbnails (->> pages - (mapcat #(extract-file-thumbnails-from-page state selected %)))] + pages (-> state :workspace-data :pages-index vals) + extract (fn [{:keys [objects id] :as page}] + (->> (cph/get-frames objects) + (filter :file-thumbnail) + (map :id) + (remove selected) + (map (fn [frame-id] [id frame-id]))))] (rx/concat - (rx/from - (for [ft file-thumbnails] - (dch/update-shapes [(:id ft)] #(update % :file-thumbnail not) (:page-id ft) nil))) - (rx/of (dch/update-shapes selected #(update % :file-thumbnail not)))))))) + (rx/from (for [[page-id frame-id] (mapcat extract pages)] + (dch/update-shapes [frame-id] #(dissoc % :file-thumbnail) page-id nil))) + (rx/of (dch/update-shapes selected #(assoc % :file-thumbnail true)))))))) ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; ;; Navigation diff --git a/frontend/src/app/main/render.cljs b/frontend/src/app/main/render.cljs index 7970bb173..34eebbabc 100644 --- a/frontend/src/app/main/render.cljs +++ b/frontend/src/app/main/render.cljs @@ -214,31 +214,6 @@ [:& shape-wrapper {:shape item :key (:id item)}])))]]])) -(mf/defc file-thumbnail-svg - {::mf/wrap [mf/memo]} - [{:keys [data embed? include-metadata?] :as props - :or {embed? false include-metadata? false}}] - (let [data (assoc data :x 0 :y 0) - vbox (format-viewbox {:width (:width data 0) :height (:height data 0)}) - background-color (get-in data [:options :background] default-color)] - - [:& (mf/provider embed/context) {:value embed?} - [:& (mf/provider export/include-metadata-ctx) {:value include-metadata?} - [:svg {:view-box vbox - :version "1.1" - :xmlns "http://www.w3.org/2000/svg" - :xmlnsXlink "http://www.w3.org/1999/xlink" - :xmlns:penpot (when include-metadata? "https://penpot.app/xmlns") - :style {:width "100%" - :height "100%" - :background background-color}} - - (when include-metadata? - [:& export/export-page {:options (:options data)}]) - - [:> shape-container {:shape data} - [:& frame/frame-thumbnail {:shape data}]]]]])) - (mf/defc frame-svg {::mf/wrap [mf/memo]} [{:keys [objects frame zoom show-thumbnails?] :or {zoom 1} :as props}] diff --git a/frontend/src/app/worker/thumbnails.cljs b/frontend/src/app/worker/thumbnails.cljs index f49e94b1e..5524c087d 100644 --- a/frontend/src/app/worker/thumbnails.cljs +++ b/frontend/src/app/worker/thumbnails.cljs @@ -31,36 +31,28 @@ (defn- request-thumbnail [file-id] - (let [uri (u/join (cfg/get-public-uri) "api/rpc/query/file-data-for-thumbnail") - params {:file-id file-id - :strip-frames-with-thumbnails true}] - (->> (http/send! - {:method :get - :uri uri - :credentials "include" - :query params}) + (let [uri (u/join (cfg/get-public-uri) "api/rpc/query/file-data-for-thumbnail") + params {:file-id file-id + :strip-frames-with-thumbnails true} + request {:method :get + :uri uri + :credentials "include" + :query params}] + (->> (http/send! request) (rx/map http/conditional-decode-transit) (rx/mapcat handle-response)))) -(defonce cache (atom {})) - (defn render-frame - [data ckey] - (let [prev (get @cache ckey)] - (if (= (:data prev) data) - (:result prev) - (let [file-thumbnail (:file-thumbnail data) - elem (if file-thumbnail - (mf/element render/file-thumbnail-svg #js {:data file-thumbnail :width "290" :height "150"}) - (mf/element render/page-svg #js {:data data :width "290" :height "150" :thumbnails? true})) - result (rds/renderToStaticMarkup elem)] - (swap! cache assoc ckey {:data data :result result}) - result)))) + [data] + (let [elem (if-let [frame (:thumbnail-frame data)] + (mf/element render/frame-svg #js {:objects (:objects data) :frame frame}) + (mf/element render/page-svg #js {:data data :width "290" :height "150" :thumbnails? true}))] + (rds/renderToStaticMarkup elem))) (defmethod impl/handler :thumbnails/generate [{:keys [file-id] :as message}] (->> (request-thumbnail file-id) (rx/map (fn [data] - {:svg (render-frame data #{file-id}) + {:svg (render-frame data) :fonts @fonts/loaded}))))