aestheticinteractive / Hover-UI-Kit

Create beautiful user interfaces for immersive VR/AR experiences.
Other
789 stars 162 forks source link

TreeUpdate is Glitching Out when using a slider #76

Closed CTVilgalys closed 6 years ago

CTVilgalys commented 6 years ago

We're using Hover-UI-Kit to do, among other things, the audio volume slider. The TreeUpdater component's Update() is hugely slowing down while the slider is selected and the volume is being changed.

I'm trying to work through where the slowdown is occuring, but this just seems like such a crazy amount of work to be doing in Update() and I'm not familiar with the big picture of how this is put together. It looks like every gameObject in the hover hierarchy is starting a recursive search for the root every single frame? And then on the decent another recursive search? This seems very inefficient, although certainly thorough.

Any thoughts on what is eating so many cycles in one frame, and why it would only be happening when using a slider?

treeupdater

zachkinstner commented 6 years ago

Hi @CTVilgalys, sorry you're having this issue!

First, a quick "big picture" overview: the TreeUpdater component is attached to almost every element in Hover UI Kit. It's not ideal, but it was the only way (that I could find) to force Unity to update the elements from a parent element down the hierarchy. This way, for example, if you change the size of a panel, it won't take one or more frames for that size change to bubble down to its child layouts/items.

The huge performance spikes are not normal or expected. Does this only occur during the times that this volume slider is selected and/or changing its values? I should mention that I'm using Hover UI in production code (my EXA project, and others), and I haven't seen a spike like this. Is there anything special or unusual that your code does during volume changes? It also looks like there's a lot of memory allocations happening on these frames.

The TreeUpdater approach does make it more difficult to use the profiler. Instead of seeing the nice breakdown of ComponentA.Update(), ComponentB.Update(), etc., you just see everything lumped into TreeUpdater.Update(). A couple options:

Other things to try:

rvilgalys commented 6 years ago

Thank you so much for the thorough update!

I did find the source of the slowdown, we were running a Save method on the event trigger where the audio volume was changed so both serializing to JSON and writing to disk were causing a massive slowdown. Moving that out of the hot path solved the issue.

The thing that still confuses me about the TreeUpdater is that it has an Update() method that starts to run on every single component and so every single individual component starts to search through the entire hierarchy. As far as I can tell, it only needs to run once a frame at runtime, when you could have each part just find the root at Start() and the begin the process of checking from the root only during Update()? I'm sure you are much more familiar with the reasons for building it like this it than I am, that's just my first impression from trying to track down where the issue was.

We're also using this in production right now for our game Food Fight on Oculus! Overall I've been impressed with it, and it's been working well for our needs. Thanks very very much for all the work you've put into it!

(oh just saw I'm on my other personal account but same guy)

zachkinstner commented 6 years ago

The thing that still confuses me about the TreeUpdater is that it has an Update() method that starts to run on every single component and so every single individual component starts to search through the entire hierarchy.

Ah, the DidTreeUpdateThisFrame property might be missing from your understanding of the code. Let's say there's a simple hierarchy:

Unity's Update() calls can occur for these in any order (unless we have manually set "Script Execution Order" for them). If the first Update() call occurs at, say, level "C", the TreeUpdater climbs to the top of the tree (level "A") via AscendTreeOrBegin(), then begins recursively calling TreeUpdate() on all ITreeUpdater components as it descends the tree.

At each level, each TreeUpdater sets DidTreeUpdateThisFrame to true after completing its tree-based updates. This is important, because Unity will continue calling Update() for level "A", "B", and "D". Since the "C" Update() already triggered the full tree-based update, the other levels will not re-enter this process. Then, on Unity's LateUpdate(), all the DidTreeUpdateThisFrame properties are set back to false.

I do think the version on GitHub has some inefficiencies with re-calculating the tree. I've made some improvements to my local copy used for EXA, but haven't had time to integrate/test them into this main repo.

The reason the tree isn't calculated and cached at Start() is because many interfaces add/remove items, change their arrangement/parent, etc. These tree-based updates also need to work at editor-time, so it's very convenient for the tree to auto-calculate as you make changes.

CTVilgalys commented 6 years ago

That makes a lot more sense now. Thanks so much for taking the time to explain!