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

Move to piet-wgsl for rendering #12

Merged
merged 2 commits into from
Nov 26, 2022
Merged

Move to piet-wgsl for rendering #12

merged 2 commits into from
Nov 26, 2022

Conversation

dfrg
Copy link
Contributor

@dfrg dfrg commented Nov 26, 2022

Removes the piet-gpu and piet-gpu-hal dependencies and updates the rendering code to use piet-wgsl.

Removes the piet-gpu and piet-gpu-hal dependencies and updates the rendering code to use piet-wgsl.
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.

Works on my machine, and looks reasonable.

I like that there's now no unsafe in this repository, although I don't like that this is because of piet-wgsl being 'unsound'-ish.

)
.unwrap(),
);
self.surface = Some(self.render_cx.create_surface(handle, width, height));
Copy link
Member

Choose a reason for hiding this comment

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

create_surface laundering the unsafety of RawWindowHandle isn't ideal.

Really the ecosystem needs to get their act together and find a safe way to access swapchains from wgpu.
I don't really want to block on it, because afaik the safety comment on literally every call to instance.create_surface is along the lines of "hope this works :)"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I suppose the assumption is that if you're sound on both ends of RawWindowHandle then you're good. Otherwise, there be dragons. Still, it's a pretty nifty solution to this problem that avoids the need to bless a particular windowing library or marshal these types around yourself.

Copy link
Member

Choose a reason for hiding this comment

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

I think the issue is that the current raw-window-handle doesn't guarantee that the window will be sound to create a swapchain for, whilst wgpu assumes it is.
I don't know when this breaks down, but that's the real concern. It's not just about soundness on each side, it's about the windows having the expected capabilities.

Choose a reason for hiding this comment

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

Relevant wgpu issue: gfx-rs/wgpu#1463

@dfrg dfrg merged commit fa4d632 into main Nov 26, 2022
@dfrg dfrg deleted the piet-wgsl-render branch November 26, 2022 22:56
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