heronarts / LX

Core library for 3D LED lighting engines
https://chromatik.co/
Other
41 stars 25 forks source link

Views created in UI aren't saved to LXP file (disappear next time project is opened) #99

Closed andrewlook closed 1 year ago

andrewlook commented 1 year ago

I'm loving the new views feature, but I notice that when I define on in the "Model" pane, the view isn't there when I re-open the same LXP file.

For example, after defining a view like this:

image

I click "save" in the top of the UI, then close the project and reopen it, and the view list is empty again:

image

mcslee commented 1 year ago

Fixed here: https://github.com/heronarts/LX/commit/27ced33324721ee05743f12d8b105f2098a10e60

CC @jkbelcher as we should maybe chat about how you guys want to handle this. The cause of this was the fact that you guys are using a pre-constructed hardcoded LXModel in code. In this case, the LXStructure system doesn't really do any saving/loading.

It's an open question whether you guys will want to use different sets of views in different projects, which will mean dynamically constructing them again each time you have a project change. Or whether you guys want to hardcode your views along with your immutable model and just have them persist across project loads, rather than editing them via this normal project-based UI workflow.

In the long run if it's possible to get TE onto the normal, non-immutable model system, that will probably minimize the number of edge case gotchas like this. But it's also totally reasonable to not want to spin any extra CPU/memory reconstructing the same models/views over and over, so I'm open to how you guys want to proceed here.

jkbelcher commented 1 year ago

Good call. We are loading the TE views from text file for the old system, and could insert those definitions in the global list at the same time.

@andrewlook done here: #415

andrewlook commented 1 year ago

Thanks @mcslee for explaining the root cause of this, and to @jkbelcher for the quick fix 🙏

mcslee commented 1 year ago

Great - fix on the TE side looks reasonable. But I think I should probably revert this change then, so that LX just leaves the views alone entirely in the case of an immutable externally managed model.

With this fix left in, it's going to tear down and rebuild your custom views every time a project file is loaded. That seems like useless churn.

@jkbelcher just confirming are you agreed w/ removing this fix from the LX-core side?

jkbelcher commented 1 year ago

Agreed, views are part of the structure and should come from structure objects. No need to persist them across file opens in an immutable model, when they'll get lost anyway on a full restart.