-
-
Notifications
You must be signed in to change notification settings - Fork 916
Add implementation of rasterize node using vello #3232
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
base: master
Are you sure you want to change the base?
Conversation
Performance Benchmark Results
|
5b7e4a7
to
8c45949
Compare
Performance Benchmark Results
|
|
||
// Render data to Vello scene | ||
let render_params = graphene_svg_renderer::RenderParams { | ||
footprint: Footprint::default(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't this included in ..Default::default()
?
|
||
// Convert GPU raster to CPU raster using Convert trait | ||
use graphene_core::ops::Convert; | ||
let cpu_raster = gpu_raster.convert(Footprint::default(), wgpu_executor).await; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why don't we return a GPU texture here? Vector -> Rasterize -> Invert GPU and similar usecases.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wanted this to match the CPU version also to make this usable for the led artwork
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But we should make the two nodes compatible with each other so opening a document created in web also works on desktop
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we make two nodes one with CPU and one with GPU output, both using the same utility function to render?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd rather wait until we have proper automatic type conversion, this should ideally not be complexity we expose to the user, if we can avoid it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We already do with all the oder GPU nodes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Which is something I want to fix asap
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Feel free to merge this. I will at some point create a follow-up PR with a GPU node.
No description provided.