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

feat(deno): surface support #3265

Merged
merged 11 commits into from
Jan 9, 2023
Merged

feat(deno): surface support #3265

merged 11 commits into from
Jan 9, 2023

Conversation

crowlKats
Copy link
Collaborator

@crowlKats crowlKats commented Dec 6, 2022

WGPU has various incompatibilities with the spec with no clear solutions:

@codecov-commenter
Copy link

codecov-commenter commented Dec 6, 2022

Codecov Report

Merging #3265 (59398c0) into master (3cc6621) will decrease coverage by 1.16%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #3265      +/-   ##
==========================================
- Coverage   65.66%   64.49%   -1.17%     
==========================================
  Files          82       86       +4     
  Lines       39489    42696    +3207     
==========================================
+ Hits        25930    27537    +1607     
- Misses      13559    15159    +1600     
Impacted Files Coverage Δ
wgpu-types/src/lib.rs 88.06% <ø> (-0.24%) ⬇️
wgpu-core/src/track/texture.rs 59.46% <0.00%> (-21.52%) ⬇️
wgpu-core/src/track/buffer.rs 84.86% <0.00%> (-7.28%) ⬇️
wgpu-hal/src/auxil/dxgi/result.rs 73.07% <0.00%> (-3.85%) ⬇️
wgpu/src/lib.rs 51.14% <0.00%> (-0.95%) ⬇️
wgpu-hal/src/dx12/adapter.rs 81.38% <0.00%> (-0.38%) ⬇️
wgpu-core/src/command/transfer.rs 75.14% <0.00%> (-0.36%) ⬇️
wgpu-core/src/instance.rs 65.67% <0.00%> (-0.35%) ⬇️
wgpu-core/src/device/queue.rs 70.18% <0.00%> (-0.34%) ⬇️
wgpu-hal/src/gles/adapter.rs 83.72% <0.00%> (-0.24%) ⬇️
... and 32 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@crowlKats crowlKats marked this pull request as ready for review December 19, 2022 05:40
# Conflicts:
#	deno_webgpu/Cargo.toml
Copy link
Member

@cwfitzgerald cwfitzgerald left a comment

Choose a reason for hiding this comment

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

This isn't really my code to check, so as long as deno is happy with everything, consider this g2g

Copy link
Contributor

@littledivy littledivy left a comment

Choose a reason for hiding this comment

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

LGTM

@crowlKats crowlKats merged commit 2252b12 into gfx-rs:master Jan 9, 2023
@crowlKats crowlKats deleted the surface branch January 9, 2023 15:44
@jbatez
Copy link

jbatez commented Jan 23, 2023

@crowlKats What's the use-case you have in-mind for this? Is there a current Deno window system support effort? Last week I decided to try tackling it myself (see https://github.com/denogdev/denog) and didn't realize your surface extension existed since I originally forked off deno/v1.29.

My version's still very much a work-in-progress. I decided to call the WebGPU type GPUSurface instead of GPUCanvasContext because, as you noted, it's hard to get the interfaces to match exactly, and it's not clear if we'd even want to. I'm happy to use whatever you decide, though; I'm more interested in working on the window creation / configuration / event handling APIs.

Also, to any Deno devs who see this: if you have time, please take a look at my fork and let me know if it's a direction upstream Deno would be interested in pursuing. I think it's something that needs to be built into the Deno CLI itself because of how window event loops like hijack the main thread.

@crowlKats
Copy link
Collaborator Author

crowlKats commented Jan 23, 2023

@jbatez this is a low priority side project of mine, which wouldnt be part of the CLI (we dont want a windowing manager built into the CLI). I have an internal prototype already, which does window creation, configuration & event handling.

@jbatez
Copy link

jbatez commented Jan 23, 2023

@crowlKats Got it. Is your prototype based on Winit? If so, what do you do about EventLoop::run wanting to take over the main thread? If not, what are you doing instead?

I find the idea of an all-in-one tool for JavaScript gamedev without having to resort to native code extremely appealing. Built-in window system integration for Deno doesn't seem like much of a stretch since WebGPU's already there, but I understand if you consider it out-of-scope.

@crowlKats
Copy link
Collaborator Author

crowlKats commented Jan 23, 2023

Yes, we are using winit. spawning separate thread for deno.

The CLI is meant to be headless, which is why we dont have any APIs like notification or clipboard.

This quarter I am quite busy, but potentially next quarter I will have time to continue on this project and potentially have something to release.

@jbatez
Copy link

jbatez commented Jan 24, 2023

Oh, you're probably creating a whole new binary and using deno_runtime as a library. I was stuck wondering how to hijack the main thread of a deno run --allow-ffi process. Using deno_runtime as a library makes more sense.

The Deno CLI still has a lot of functionality I'd want in an all-in-one tool that isn't available in deno_runtime, but I suppose such a tool could shell out to the Deno CLI under-the-hood for most of that.

@joubertnel
Copy link

This quarter I am quite busy, but potentially next quarter I will have time to continue on this project and potentially have something to release.

This was exciting to read. Now that Deno supports WebGPU again, I'm curious to hear whether there's any update on Windowing support? It would be pretty awesome to use Deno for graphics apps!

@quinton-ashley
Copy link

Is windowing support still planned for WebGPU? Could be a cool way to make game apps without Electron.

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.

8 participants