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

Remove render_to_surface #803

Merged
merged 4 commits into from
Feb 11, 2025
Merged

Remove render_to_surface #803

merged 4 commits into from
Feb 11, 2025

Conversation

DJMcNab
Copy link
Member

@DJMcNab DJMcNab commented Jan 29, 2025

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.

Copy link

@jaredoconnell jaredoconnell left a 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.

vello/src/lib.rs Outdated Show resolved Hide resolved
@DJMcNab DJMcNab changed the title Remove render_to_surface and the async pipeline Remove render_to_surface Feb 11, 2025
Copy link
Collaborator

@waywardmonkeys waywardmonkeys left a 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.
Copy link
Collaborator

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.

Copy link
Member Author

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.

@DJMcNab
Copy link
Member Author

DJMcNab commented Feb 11, 2025

The async pipeline with debug_layers is not working in this PR. However, this is not really a regression from main, as it also doesn't work there.

@DJMcNab DJMcNab added this pull request to the merge queue Feb 11, 2025
Merged via the queue into linebender:main with commit 782fb5a Feb 11, 2025
17 checks passed
@DJMcNab DJMcNab deleted the remove_surface branch February 11, 2025 14:43
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.

Rendering to a surface and transparency
3 participants