DougHamil / threeagent

ClojureScript library for building Three.js apps in a reagent-like fashion
MIT License
134 stars 10 forks source link

form three component? #40

Closed joinr closed 2 years ago

joinr commented 2 years ago

In my explorations, I decided to ape stuff I did with Vega and implement a form 3 component that wraps the setup from the examples. The hope was to manage all of my web page via reagent (and the scene with threeagent), with the threeagent merely being a component handled outside of react. I am not a reagent genius though (mildly experienced, able to copy and extend). My solution worked okay:

(defn three-canvas [name f on-before-render]
  (r/create-class
   {:display-name (str name)
    :reagent-render (fn [] [:canvas])
    :component-did-mount
    (fn [this]
       (threehelp/render f (rdom/dom-node this) {:render-params {:antialias true :resize true}
                                                     :on-before-render on-before-render}))
    :component-did-update
    (fn [this]
      (threehelp/render f (rdom/dom-node this) {:render-params {:antialias true :resize true}
                                                     :on-before-render on-before-render}))}))

Used

(defn app [ratom]
  [:div.header {:style {:display "flex" :flex-direction "column" :width "100%" :height "100%"}}
   [:div {:id "chart-root" :style {:display "flex"}}
    [:div {:style {:flex "1" :width "100%"}}
     [v/vega-chart "ltn-plot" v/ltn-spec]]]
   [:div  {:style {:display "flex" :width "100%" :height  "auto"  :class "fullSize" :overflow "hidden"
                   :justify-content "space-between"}}
;;here....
    [three-canvas "root-scene" scene (fn [dt] (swap! ratom tick-scene))]]
   [:div.header  {:style {:display "flex" :width "100%" :height  "auto"  :class "fullSize" :overflow "hidden"
                          :justify-content "space-between"
                          :font-size "xxx-large"}}
    [:p {:id "c-day" :style {:margin "0 auto" :text-align "center" }}
     ;;no idea why this causes a slow memory leak!
     (str "C-Day:"  (int @c-day))
     ]]])

This works great, except when I have a dynamic text element (the C-Day label in the hiccup above) that is updating, I get awful performance and an eventual memory leak. If the text data doesn't change, performance is great. I am suspecting there is something going on with lifecycle methods and the like.

full janky source here https://github.com/joinr/threeagentdemo/blob/master/src/threeagentdemo/core.cljs The threehelp ns is just a wrapper around the original threeagent.impl.scene to allow some stuff like setting up pxiel ratios and other rendering setup (maybe unnecessary now), also trying to work on responsive resizing of the canvas (pending).

Any ideas? I am revisiting the reagent docs to get more skilled on form 3 components. I noticed that in all the existing threeagent examples, you specify different divs in the index.html, and then have different reagent mounting points for the reagent scene and the ui. My attempted design tries to unify them; maybe you went with the separation for a reason.

joinr commented 2 years ago

[comment updated to fix formatting]

DougHamil commented 2 years ago

I hadn't tried embedding threeagent in a reagent component like this before, but it seems to work.

I put together a simple example to try to reproduce the memory leak issue:

(ns app
  (:require [threeagent.core :as th]
            [reagent.dom :as rdom]
            [reagent.core :as r]))

(defonce state (r/atom 0))

(defn- tick [delta-time]
  (swap! state + delta-time))

(defn- threeagent-root []
  [:object
   [:ambient-light {:intensity 1.0}]
   [:box {:position [0 0 -5]
          :rotation [0 @state 0]}]])

(defn- threeagent-scene [root-fn]
  (r/create-class
   {:display-name "threeagent"
    :reagent-render (fn [] [:canvas])
    :component-did-mount
    (fn [this]
      (th/render root-fn (rdom/dom-node this) {:on-before-render tick}))
    :component-did-update
    (fn [this]
      (th/render root-fn (rdom/dom-node this) {:on-before-render tick}))}))

(defn root []
  [:div
   [:h1 "Embedded Threeagent Scene"]
   [:p "Tick:" @state]
   [threeagent-scene threeagent-root]])

(defn ^:dev/after-load init []
  (rdom/render [root] (.getElementById js/document "root")))

I didn't see any obvious memory leaks. There is definitely a lot of garbage generated per frame (I'm seeing about 30KB/frame) but it's all being reclaimed during the minor GC (this is on Chrome). I haven't spent much time reducing the garbage generated by threeagent, but there's certainly room for improvement.

So, I don't think using threeagent in this way is causing the memory leak. If you can put together a smaller example that reproduces the issue, that would help a lot.

joinr commented 2 years ago

It could be something I introduced in the utils I wrote. I will strip down my example and see if I can get an isolated case. Thanks.

joinr commented 2 years ago

I traced a memory profile and it went to component-did-update invoking render every frame for some reason. If I elide that from the implementation, memory leak disappears and performance is great. Note: I am using sprites in my stuff too (another degree of freedom) which your minimal example does not. Very curious.

DougHamil commented 2 years ago

Oh interesting, when I put together my example it was not running component-did-update on each frame. It might be due to the (fn [dt] (swap! ratom tick-scene)) parameter, as it's being recreated each frame due to the @c-day deref. Reagent is probably sees this new fn as a different parameter, so it forces the three-canvas component to re-render.

You could try passing the ratom into the three-canvas component and have it define and use that function.

DougHamil commented 2 years ago

There is likely a memory leak somewhere in threeagent from the multiple threeagent/render calls, as that usually should not be called multiple times (except after a hot-reload).

joinr commented 2 years ago

You could try passing the ratom into the three-canvas component and have it define and use that function.

I ended up just wrapping the app state in a form-2 component where the render function is computed outside the reagent form:

(defn app [ratom]
  (let [render-scene! (fn [dt] (swap! ratom tick-scene))]
      (fn [] 
         ...the-page...)))

I can't tell if this has made a drastic difference though, performance was pretty good after the last fix.

tytrdev commented 2 years ago

I'd recommend updating this issue to reflect that both reagent and threeagent are being used

DougHamil commented 2 years ago

Sorry for not getting back to this sooner. I improved the docs in the latest release and added an FAQ entry for how to embed threeagent within a Reagent app.

Please re-open or create a new issue if you are running into issues with this approach. Thank you!