-
Notifications
You must be signed in to change notification settings - Fork 143
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
Remove render_to_surface
#803
Conversation
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 am not familiar enough with how all of this works to give an approval, but I didn't see any red flags, and it looks well documented.
bc06a68
to
2723a6a
Compare
2723a6a
to
32bcd15
Compare
render_to_surface
and the async pipelinerender_to_surface
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 think I'm good with this once the clippy and doc stuff is fixed.
This is a step forward and we'll want to iterate more before the next release!
Thanks for pulling out the async changes as that made this easier to understand.
vello/src/lib.rs
Outdated
/// This pattern is supported by the [`util`] module. | ||
/// 2) Call `render_to_texture` directly on the [`SurfaceTexture`][wgpu::SurfaceTexture]'s texture, if | ||
/// it has the right usages. This should generally be avoided, as it's a potential performance pitfall | ||
/// on GPUs where the render pipeline method of writing to surfaces is assumed and optimised for. |
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.
Something about this phrasing is confusing to me.
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've tried to rephrase this.
The async pipeline with |
Replace with blitting/texture utilities in the
utils
module.Fixes #549
Incompatible with #801. I have a follow-up which shows you would do transparency, but my current thinking is that you should drop off the "utils" happy path if you need that. I could be convinced otherwise.