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

Add transparency support to render_to_surface. #801

Closed
wants to merge 1 commit into from

Conversation

TheNachoBIT
Copy link
Contributor

This tiny PR adds the feature suggested in #549.

It adds transparency support for render_to_surface, with the addition of being able to render scenes on top of already rendered stuff with this function by giving you the option to choose if you want to clear the screen before drawing via an extra argument called "clear". If the check is true, it clears the screen before drawing the scene; if it is false, it doesn't.

Copy link
Member

@DJMcNab DJMcNab left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd like a slight change in the API, but these changes look reasonable

@@ -502,6 +502,7 @@ impl Renderer {
scene: &Scene,
surface: &SurfaceTexture,
params: &RenderParams,
clear: bool,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A bare boolean parameter is very rarely understandable. I think the best solution here would be to move clear into being a field of RendererOptions. (We'd need to also then make it a field of Renderer).

Clearly that's not ideal in all cases. I hope that we can remove render_to_surface entirely soon. But that's been planned for probably a year, so...

Copy link
Contributor Author

@TheNachoBIT TheNachoBIT Jan 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I personally think that render_to_surface shouldn't be removed, mostly because it is a function that i depend a lot on to display to the screen directly, without having to set it up manually.

And moving the boolean to the Renderer kills a bit of flexibility, since by using this bool inside of the function, it allows you to split scenes in separate draw calls if you need to, and still being able to use the same renderer.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still, if you guys remove it at some point, it's totally fine! But, i think this function works pretty well, and this fix should allow the surface to have transparency for now c:

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

render_to_surface is a really terrible API, for a number of reasons. It's definitely going.
With the new wgpu release, you will be able to use TextureBlitter, which will make render_to_surface almost trivial.

I don't know if we'll end up writing our own utility for it.

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.

2 participants