Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Experimental] Render Graph v1 #574

Closed
wants to merge 13 commits into from
Closed

Conversation

DasLixou
Copy link
Collaborator

@DasLixou DasLixou commented May 11, 2024

Draft PR? yeah, this is not directly for merge. Instead, this is a research of how a Render Graph for vello could look like. My thoughts are below, please share yours.

Other than most of the other graphical application who drive a render graph, vello needs to know the size (for a buffer) or width,heigth (for an image) for a resource, while it creates them dynamically in the render nodes/stages. So a simple "just create a static buffer of that size and give it to this node" doesn't quite work. What I have done is somewhat quite overengineered/overkill I think, so I would love to hear simpler ideas from you. Basically, every node has an output struct and when linking the nodes, you can already specify how the outcome of a dependency will be used for the node. Then with some type erasure magic I store all the outputs of the nodes and when all dependencies are resolved, we cast them back for the node to get its inputs.

I haven't removed the old code in render.rs, but it isn't used anymore. Also, I just now see how many resources we create, I wonder how long we are able to run before we run out of the u64 (probably loooong, but still feels itchy).

Also, I implemented some cool helpers for recording such as that now, dispatch doesn't get an array, but rather a tuple where we can now pass different impl Into<ResourceProxy> structs like ImageProxy, BufferProxy, ResourceProxy at once without having to call .into() on the different ones.

All special stuff like reusing resources isn't implemented yet (where I also wonder how we would do that exactly... because even with "the same buffer" (being the same usage) could just be bigger on the next iteration.. hmm)

So yeah just a basic prototype, which at least runs, to get one example of how it could look and gather feedback. :>

@DJMcNab DJMcNab mentioned this pull request Jun 10, 2024
3 tasks
@TheNachoBIT
Copy link
Contributor

YOOOOOOOO THIS IS INSAAAAAAAANE
My mind rn:

crocodile-explosion.mp4

Amazing work! Can't wait to see some examples for this :D

@DJMcNab
Copy link
Member

DJMcNab commented Dec 9, 2024

Thanks for looking into this. Apologies that we haven't given feedback here earlier; the image filters feature has not been a high priority for us, which has meant this has languished.
It isn't clear to me how this implementation provided the necessary groundwork for image filters. As currently written, this seems to largely be a sideways step of abstraction on top of the rendering abstraction, without any additional features on top implemented. In order to see this land, we'd need to either see some new features, or a clear path towards where those would be implemented (e.g. a trivial hue-shift filter integration).

I also don't think that this use of unsafe code is necessary. The use of unsafe was a factor in me not feeling motivated to review this earlier.

I am inclined to close this PR currently. I am currently working on my own design for image filters, and it would be nice to chat with you to see what you think about it. Feel free to book in an office hours (on my GitHub profile)

@DasLixou
Copy link
Collaborator Author

DasLixou commented Dec 9, 2024

Well the problem I was facing was that I wanted to mostly retain the current render logic, and the texture sizes are not fixed so can't be created on a graph level but in a node itself. I didn't have that much experience (and still don't really have) about rendering, so that's just what I've come up with.
But it seems like dagga as stated on zulip might be a better choice. And your own implementation surely might also be interesting. I'm ok with closing this.

@DJMcNab DJMcNab closed this Dec 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants