ScenicFramework / scenic

Core Scenic library
Apache License 2.0
1.99k stars 137 forks source link

Proposal: embed graph within scene struct #260

Closed bcardarella closed 2 years ago

bcardarella commented 2 years ago

I'd like to formally propose that the graph itself be part of the scene struct:

defmodule Scenic.Scene
  defstruct viewport: nil,
            pid: nil,
            module: nil,
            theme: nil,
            id: nil,
            parent: nil,
            children: nil,
            child_supervisor: nil,
            assigns: %{},
            supervisor: nil,
            stop_pid: nil,
            graph: nil

This can change Scenic.Scene.push_graph/3 to:

def push_graph(scene, id) do
   ...
   ViewPort.push_graph(viewport, id, scene.graph)
end

Considering the graph actually carries state, with the position, shape, and transformation of given primitives I believe that state should be reflected in the GenServer's state with is the scene

boydm commented 2 years ago

I'll think about this but... scene's can push more than one graph, and there are some legitimate use cases where this is done. I've tried coupling the graph to the scene struct before, and that failed for multiple reasons.

Also, yes, the graph carries state, but it is not necessarily state that is tied to any specific Scene. You could, for instance, serve graphs out of a central location and have scene's pick them up.

But, mainly, the cases where a scene maintains and pushes more than one graph at a time kind of messes this up. That is why the ID parameter is in push_graph. For example, you can use a ref as an ID. The script that is generated from the graph then has that ID as well. You can reference that script, with the ref based ID from another Graph and it all works.

The closest the built in components come to do this is Checkbox, but that is just using a hand-rolled script.

Multiple graphs becomes very important when building compositors for remote rendering.

bcardarella commented 2 years ago

But, mainly, the cases where a scene maintains and pushes more than one graph at a time kind of messes this up.

Do you have an example of this? I can see how multiple graphs presents problems on which is stored, but I'm not conceptualizing the need for more than one graph in the same scene.

JediLuke commented 2 years ago

Well, I'm not claiming this is the greatest use of Scenic but - I have a root scene, which routinely swaps in and out graphs in my app. Maybe I should make better use of Scenes, swapping them in and out at the viewport layer, but I've found it convenient to treat graphs as just another piece of state. For example, when swapping between different "apps", I don't actually change the scene in the ViewPort, I usually just update the graph in my root scene...

My 2c is that it's fine as is, I don't personally have any issue managing graphs as a piece of state. If you want to keep track of the graph, just assign it to a scene

Edit: I can see how, in a way, this could be seen as an argument for having the graph in the state of a scene... I'm just not sold on what that gives us, versus the relative ease of just keeping the graph in the state via assigns

JediLuke commented 2 years ago

This is my main Scenic app: https://github.com/JediLuke/flamelex

To illustrate perhaps what might be a scene that swaps graphs all the time - here https://github.com/JediLuke/flamelex/blob/franklin_dev/lib/flamelex/fluxus/supervision_tree/radix_store.ex#L58

I have one central store of state, including the "main graph" being drawn. Whenever I want to swap what's being drawn on the screen, I just update this graph, which then gets propagated out.

Maybe this isn't what you were talking about though, but I'm just throwing it out there

JediLuke commented 2 years ago

We can chat about it in Slack if you have an questions, but basically - I calculate the graph & state of the root scene completely external to my scenic app, Scenic is a fairly "light" layer in my app, responsible for only the UI... when the real internal state of my UI changes, I calculate the new graph & push it, but this all happens outside any normal Scenic.Scene or other structure. So I only have one root scene, which accepts new Graphs to draw & just immediately pushes them

bcardarella commented 2 years ago

I'll withdraw this request