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

Initial implementation of WebGPU API #24708

Merged
merged 3 commits into from Nov 25, 2019
Merged

Initial implementation of WebGPU API #24708

merged 3 commits into from Nov 25, 2019

Conversation

@zakorgy
Copy link
Contributor

zakorgy commented Nov 11, 2019

  • Added the WebIDL bindings for GPU and GPUadapter interfaces.
  • Created a background thread for WebGPU api calls.
  • Established the async communication between the background thread and the WebGPU interfaces.
  • Implemented the requesAdapter function of navigator.gpu

./mach test-tidy reports an error due to the different arrayvec version used in servo and webgpu, so added it to the ignore list in servo-tidy.toml


  • ./mach build -d does not report any errors
  • ./mach test-tidy does not report any errors
  • These changes addresses a part of ##24706

cc @jdm, cc @kvark


This change is Reviewable

@highfive

This comment has been minimized.

Copy link

highfive commented Nov 11, 2019

Heads up! This PR modifies the following files:

  • @asajeffrey: components/script/dom/navigator.rs, components/script/dom/mod.rs, components/script/dom/window.rs, components/script/dom/webidls/Navigator.webidl, components/script/dom/webidls/GPUAdapter.webidl and 12 more
  • @wafflespeanut: python/tidy/servo_tidy/tidy.py
  • @cbrewster: components/constellation/pipeline.rs, components/constellation/Cargo.toml, components/constellation/constellation.rs
  • @KiChjang: components/script/dom/navigator.rs, components/script/dom/mod.rs, components/script/dom/window.rs, components/script_traits/lib.rs, components/script/dom/webidls/Navigator.webidl and 11 more
@highfive

This comment has been minimized.

Copy link

highfive commented Nov 11, 2019

warning Warning warning

  • These commits modify script code, but no tests are modified. Please consider adding a test!
let id = wgpu::wgpu_request_adapter(Some(&options));
let adapter = Arc::new(WebGPUAdapter {
id,
invalid: Mutex::new(false),

This comment has been minimized.

Copy link
@gterzian

gterzian Nov 11, 2019

Collaborator

I'm not sure what you plan to do with the invalid flag, and I can point out that storing the Arc here and then sending a clone over IPC will not result in sharing the same data between the gpu-thread and the script-thread.

This comment has been minimized.

Copy link
@zakorgy

zakorgy Nov 12, 2019

Author Contributor

Thanks for catching this, I've changed the way we could identify invalid adapters in the future, by tracking the invalid IDs in the webgpu thread.

action_receiver.to_opaque(),
Box::new(move |message| {
struct ListenerTask<T: AsyncWGPUListener + DomObject> {
context: Arc<Mutex<WGPUResponse<T>>>,

This comment has been minimized.

Copy link
@gterzian

gterzian Nov 11, 2019

Collaborator

I don't think you need the mutex and the arc, here, because WGPUResponse itself will be Send(see script::dom::bindings::{Trusted, TrustedPromise}), and I don't think you need to make it Sync to use it as you intend in TaskOnce.

This comment has been minimized.

Copy link
@zakorgy

zakorgy Nov 12, 2019

Author Contributor

Looks like the Sync is needed here:

error[E0277]: `*const dom::promise::Promise` cannot be shared between threads safely
  --> components/script/dom/gpu.rs:91:21
   |
91 |             impl<T> TaskOnce for ListenerTask<T>
   |                     ^^^^^^^^ `*const dom::promise::Promise` cannot be shared between threads safely
   |
   = help: within `dom::gpu::WGPUResponse<T>`, the trait `std::marker::Sync` is not implemented for `*const dom::promise::Promise`
   = note: required because it appears within the type `dom::bindings::refcounted::TrustedPromise`
   = note: required because it appears within the type `std::option::Option<dom::bindings::refcounted::TrustedPromise>`
   = note: required because it appears within the type `dom::gpu::WGPUResponse<T>`
   = note: required because of the requirements on the impl of `std::marker::Send` for `std::sync::Arc<dom::gpu::WGPUResponse<T>>`
   = note: required because it appears within the type `dom::gpu::response_async::{{closure}}#0::ListenerTask<T>

This comment has been minimized.

Copy link
@gterzian

gterzian Nov 12, 2019

Collaborator

I think you are getting this error because of the Arc<dom::gpu::WGPUResponse<T>>, which I think is unnecessary, you can try to use the WGPUResponse<T> directly.

So basically, it's the Arc that is complaining about the WGPUResponse not being Sync, and you can just remove the Arc, I think.

This comment has been minimized.

Copy link
@zakorgy

zakorgy Nov 12, 2019

Author Contributor

I think I've figured it out in: 7f4ccba

@zakorgy

This comment has been minimized.

Copy link
Contributor Author

zakorgy commented Nov 12, 2019

@gterzian thanks for the review, I think I've addressed your comments.

Copy link
Member

kvark left a comment

Thank you for the amazing work! Review notes are below:

Reviewed 14 of 14 files at r1, 15 of 15 files at r2, 6 of 6 files at r3, 1 of 1 files at r4, 2 of 2 files at r5, 2 of 2 files at r6, 1 of 1 files at r7.
Reviewable status: all files reviewed, 9 unresolved discussions (waiting on @gterzian, @SimonSapin, and @zakorgy)


components/script/dom/gpu.rs, line 124 at r3 (raw file):

        let promise = Promise::new_in_current_compartment(&self.global(), comp);
        let sender = response_async(&promise, self);
        let options = match options.powerPreference {

let's just produce the wgpu::PowerPreference here instead of the whole options struct


components/script/dom/gpu.rs, line 147 at r3 (raw file):

            None => promise.reject_error(Error::Type("No WebGPU thread...".to_owned())),
        };
        return promise;

return isn't needed here


components/script/dom/gpuadapter.rs, line 24 at r1 (raw file):

impl GPUAdapter {
    pub fn new_inherited(name: DOMString, extensions: Heap<*mut JSObject>) -> GPUAdapter {

The extensions is technically just a struct (or JS dictionary). There was some silly technical reason why the WebGPU API uses an "object" there, but perhaps it's not required on the Servo side?


components/script/dom/webidls/GPUAdapter.webidl, line 7 at r1 (raw file):

// https://gpuweb.github.io/gpuweb/#gpuadapter
[Exposed=(Window, DedicatedWorker), Pref="dom.webgpu.enabled"]
interface GPUAdapter {

Does Servo still require every interface to come with a separate "webidl" file? Neither Gecko or upstream WebIDL has that, and managing that many files/interfaces is going to be a bit of a pain. At the very least, we should consider placing all of them in a gpu subfolder and stripping GPU file prefix, if we have to keep separate files.


components/webgpu/Cargo.toml, line 18 at r3 (raw file):

serde = "1.0"
servo_config = {path = "../config"}
wgpu-native = { version = "0.4.0", features = ["serde","local"] }

This is incorrect, as we discussed on the chat earlier today.
What we'll need is basically wgpu-native providing the Serde feature that affects the relevant structs, and the whole IPC machinery implemented in the Servo's "webgpu" component.


components/webgpu/lib.rs, line 22 at r2 (raw file):

impl WebGPUThread {
    pub fn new() -> Option<WebGPUThread> {

nit: let's use Self where appropriate


docs/ORGANIZATION.md, line 58 at r2 (raw file):

    * In-process server to allow manipulating browser instances via a WebDriver client.
  * webgpu
    * Implementation of the WebGPU thread.

note: WebGPU will likely use more than one thread to operate

@zakorgy

This comment has been minimized.

Copy link
Contributor Author

zakorgy commented Nov 13, 2019


components/script/dom/gpuadapter.rs, line 24 at r1 (raw file):

Previously, kvark (Dzmitry Malyshau) wrote…

The extensions is technically just a struct (or JS dictionary). There was some silly technical reason why the WebGPU API uses an "object" there, but perhaps it's not required on the Servo side?

It's a dictionary and the WebIDL pareser gives the following error if we try to use a dictionary as an attribute

WebIDL.WebIDLError: error: An attribute cannot be of a dictionary type, ../../webidls/GPUAdapter.webidl line 9:13
    readonly attribute GPUExtensions extensions;
             ^

According to https://heycam.github.io/webidl/#idl-dictionaries : Dictionaries must not be used as the type of an attribute or constant.. So I think that's the reason for "object"

@zakorgy

This comment has been minimized.

Copy link
Contributor Author

zakorgy commented Nov 13, 2019


components/script/dom/webidls/GPUAdapter.webidl, line 7 at r1 (raw file):

Previously, kvark (Dzmitry Malyshau) wrote…

Does Servo still require every interface to come with a separate "webidl" file? Neither Gecko or upstream WebIDL has that, and managing that many files/interfaces is going to be a bit of a pain. At the very least, we should consider placing all of them in a gpu subfolder and stripping GPU file prefix, if we have to keep separate files.

Neither of those seems doable for me, I tried both but the WebIDL parser could not resolve the scopes.

@zakorgy

This comment has been minimized.

Copy link
Contributor Author

zakorgy commented Nov 13, 2019


components/script/dom/gpu.rs, line 124 at r3 (raw file):

Previously, kvark (Dzmitry Malyshau) wrote…

let's just produce the wgpu::PowerPreference here instead of the whole options struct

Done.

@zakorgy

This comment has been minimized.

Copy link
Contributor Author

zakorgy commented Nov 13, 2019


components/script/dom/gpu.rs, line 147 at r3 (raw file):

Previously, kvark (Dzmitry Malyshau) wrote…

return isn't needed here

Done.

@zakorgy

This comment has been minimized.

Copy link
Contributor Author

zakorgy commented Nov 13, 2019


components/webgpu/Cargo.toml, line 18 at r3 (raw file):

Previously, kvark (Dzmitry Malyshau) wrote…

This is incorrect, as we discussed on the chat earlier today.
What we'll need is basically wgpu-native providing the Serde feature that affects the relevant structs, and the whole IPC machinery implemented in the Servo's "webgpu" component.

Removed it in e0e5b21

@zakorgy

This comment has been minimized.

Copy link
Contributor Author

zakorgy commented Nov 13, 2019


docs/ORGANIZATION.md, line 58 at r2 (raw file):

Previously, kvark (Dzmitry Malyshau) wrote…

note: WebGPU will likely use more than one thread to operate

Rewrote it according to that

@zakorgy

This comment has been minimized.

Copy link
Contributor Author

zakorgy commented Nov 13, 2019

Thanks for the review @kvark, I've addressed all of your comments. the only place which I'm not sure is are the IDs in request_adapter: https://github.com/servo/servo/pull/24708/files#diff-
118cd3dad97ab1d87b0b12f0b7c2c7bcR86

@kvark

This comment has been minimized.

Copy link
Member

kvark commented Nov 13, 2019

Thanks @zakorgy ! The ids need to be generated (and managed) by the client. You can check https://github.com/gfx-rs/wgpu/blob/ae64e3aa0cf20d9a03ec6a57f6623d56104cfa97/wgpu-remote/src/lib.rs#L93-L127 for the reference on how Gecko does it.

@zakorgy zakorgy force-pushed the szeged:webgpu-base branch from e0e5b21 to 975103f Nov 14, 2019
@zakorgy

This comment has been minimized.

Copy link
Contributor Author

zakorgy commented Nov 14, 2019

@gterzian I've squashed my commits, could you please take a look again? As a note: this PR is just the beginning of the WebGPU related work, and we wan't to ship other parts of the API in different PR-s.

@zakorgy zakorgy requested a review from gterzian Nov 14, 2019
Copy link
Collaborator

gterzian left a comment

Looks good overall, couple of comments and also some higher-level design suggestions, which could be addressed in another PR.

}
}

pub fn response_async<T: AsyncWGPUListener + DomObject + 'static>(

This comment has been minimized.

Copy link
@gterzian

gterzian Nov 15, 2019

Collaborator

I'm wondering if the AsyncWGPUListener trait is necessary. Unless things other than the GPU can be passed to the those methods, I would say you can simply pass it as is and just define a "handle_adapter_response" method on it.

From reading the spec it seems different adapters can be returned(software or GPU), however requesting one seems to go through a single GPU object.

This comment has been minimized.

Copy link
@zakorgy

zakorgy Nov 15, 2019

Author Contributor

Our intention would be using AsyncWGPUListener trait for other WebGPU interfaces in the future, since there are a handful of promise calls in the spec.

This comment has been minimized.

Copy link
@gterzian

gterzian Nov 15, 2019

Collaborator

Ah ok.

Would you have to update the state of those objects at some point based on what's happening in the WebGPU backend(what is currently named "WebGPUThread")?

If so, I think you're going to have to keep track of the various objects passed to the promise, either as Dom<_> or WeakRef<_>, and store those somewhere that is accessible from the object that is handling the incoming IPC message in the route. See for example how MessagePort are tracked with

pub enum MessagePortState {
and how there is only a single route used to manage various incoming messages, at
// Setup a route for IPC, for messages from the constellation to our ports.

I guess for now it's fine to keep it as it is, however I'm not sure that you can scale the response_async pattern to handle all those interfaces. We'll see.

receiver: &T,
) -> IpcSender<WebGPUResponseResult> {
let (action_sender, action_receiver) = ipc::channel().unwrap();
let task_source = receiver.global().networking_task_source();

This comment has been minimized.

Copy link
@gterzian

gterzian Nov 15, 2019

Collaborator

The spec is a bit light on how to exactly implement requestAdapter, I would say you probably want to use the dom_manipulation_task_source here until the spec defines it more precisely.

This comment has been minimized.

Copy link
@zakorgy

zakorgy Nov 19, 2019

Author Contributor

Changed it according to that: 5f06132#diff-7a9976d0e2083724e30717170c8df321R74


let result = task_source.queue_unconditionally(task);
if let Err(err) = result {
error!("Failed to deliver network data: {:?}", err);

This comment has been minimized.

Copy link
@gterzian

gterzian Nov 15, 2019

Collaborator

It would be more appropriate to say something along the lines of "failed to queue GPU listener-task".

This comment has been minimized.

Copy link
@zakorgy
impl<T: AsyncWGPUListener + DomObject> WGPUResponse<T> {
#[allow(unrooted_must_root)]
fn response(self, response: WebGPUResponseResult) {
// JSAutoRealm needs to be manually made.

This comment has been minimized.

Copy link
@gterzian

gterzian Nov 15, 2019

Collaborator

Is this a TODO, or a general note? I don't see the code actually dealing with this.

This comment has been minimized.

Copy link
@zakorgy

zakorgy Nov 19, 2019

Author Contributor

I've found this description above a similar code in the WebBluetooth implementation: https://github.com/servo/servo/blob/master/components/script/dom/bluetooth.rs#L120. Since it's not relevant I removed it.

@@ -268,6 +269,10 @@ pub struct Window {
#[ignore_malloc_size_of = "channels are hard"]
webgl_chan: Option<WebGLChan>,

#[ignore_malloc_size_of = "channels are hard"]
/// A handle for communicating messages to the WebGPU thread.
webgpu_thread: Option<WebGPUThread>,

This comment has been minimized.

Copy link
@gterzian

gterzian Nov 15, 2019

Collaborator

I think it would be clearer to refer to this as WebGPU and webgpu, because the thread will, in multiprocess mode at least, be in a different process, so it's not really a thread from the perspective of script.

(I'm aware there is a precedent for example with webvr_thread, however I don't think that name is right either and we might as well stop referring to components in other processes as "threads").

Not really a big deal, and I don't think it would hurt to just remove the "thread" part from the name.

This comment has been minimized.

Copy link
@zakorgy

zakorgy Nov 19, 2019

Author Contributor

Renamed it in df02f1b

}
}

#[derive(Clone, Copy, Debug, Deserialize, Serialize)]

This comment has been minimized.

Copy link
@gterzian

gterzian Nov 15, 2019

Collaborator

I think you can also derive MallocSizeOf.

This comment has been minimized.

Copy link
@zakorgy

This comment has been minimized.

Copy link
@gterzian

gterzian Nov 20, 2019

Collaborator

I don't understand where this commit is, was it added to this PR?

This comment has been minimized.

Copy link
@zakorgy

zakorgy Nov 20, 2019

Author Contributor

Sorry my phrasing was bad, I meant I've had to impl MallocSizeOf directly since wgpu::AdapterId is a tuple of (u64, PhantomData<T>), where T is a handle to a backend specific Adapter.

name: DOMString,
#[ignore_malloc_size_of = "mozjs"]
extensions: Heap<*mut JSObject>,
#[ignore_malloc_size_of = "Arc"]

This comment has been minimized.

Copy link
@gterzian

gterzian Nov 15, 2019

Collaborator

Since you've removed the arc, I think you can also derive MallocSizeOf.

This comment has been minimized.

Copy link
@zakorgy

zakorgy Nov 19, 2019

Author Contributor

Answered above.

reflector_: Reflector,
name: DOMString,
#[ignore_malloc_size_of = "mozjs"]
extensions: Heap<*mut JSObject>,

This comment has been minimized.

Copy link
@gterzian

gterzian Nov 15, 2019

Collaborator

I'm not sure if that is the best way to store this(although it seems to have been done elsewhere, like GamePad). cc @jdm

@@ -853,6 +854,8 @@ fn create_constellation(

let resource_sender = public_resource_threads.sender();

let webgpu_thread = webgpu::WebGPUThread::new();

This comment has been minimized.

Copy link
@gterzian

gterzian Nov 15, 2019

Collaborator

Couple of high-level design suggestions, which are perhaps best addressed, if at all, in a follow-up:

  1. I wonder if you really want to always start the webgpu component on start-up, rather you might want to start-it when it's first needed.
  2. I wonder if you want any script to just be able to start using WebGPU whenever they want by calling methods on GPU inside script. Instead I think you want to mediate that capability via the constellation. So when script calls RequestAdapter, it should ideally go through the constellation via a ScriptMsg::RequestAdapter(sender, options), and be treated as an untrusted request for a specific capability. It ties into 1 in the sense that the constellation could then start a WebGPU instance, and respond to script with a dedicated means of communication with it(so subsequent messages don't all have to go through the constellation).
  3. The spec doesn't mention it, however I wonder if you don't want some sort of map on the constellation like HashMap<EventLoop, WebGPU>, so that each event-loop(read: agent-cluster) has it's own instance. Perhaps its easier to initially only allow one, meaning reject any ScriptMsg::RequestAdapter if a script is already using WebGPU. While the spec says "Applications can hold onto multiple adapters at once"(https://gpuweb.github.io/gpuweb/#adapters), that doesn't necessarily means different origins or agent-cluster can use adapters at the same time. I assume one "application" is an agent-cluster, which would include the main page and any dedicated workers started by it. So one "application"could use multiple adapters, however I wonder if you want multiple applications running in parallel.
  4. I see a lot of those interfaces in the spec are Serializeable, and I wonder if that also shouldn't be restricted to agent-cluster. Shared array buffers are also restricted to agent-cluster, I believe. See https://html.spec.whatwg.org/#can-share-memory-with and https://tc39.es/ecmascript_sharedmem/shmem.html#WebBrowserEmbedding on the other hand I'm not sure if those objects actually "share memory" or not, or how they would be used concurrently by different agents. That's something that could be clarified in the spec, and to keep in mind as you implement the rest.
  5. It's going to be really interesting to make all these things Serializeable, and it will I think require the changes made in #24123.

cc @jdm

This comment has been minimized.

Copy link
@zakorgy

zakorgy Nov 19, 2019

Author Contributor

Thanks for pointing out these @gterzian . I've added this list to the WebGPU meta issue, so we will not forget about it, and can discuss these things further. As for 1. and 2. we will definitely do that in the near future.

action: WebGPUResponseResult,
}

impl<T> TaskOnce for ListenerTask<T>

This comment has been minimized.

Copy link
@gterzian

gterzian Nov 15, 2019

Collaborator

By the way you can use the task! macro instead, see

task!(process_new_task: move || {

This comment has been minimized.

Copy link
@zakorgy
@zakorgy zakorgy mentioned this pull request Nov 19, 2019
0 of 13 tasks complete
@zakorgy zakorgy requested a review from gterzian Nov 19, 2019
Copy link
Collaborator

gterzian left a comment

Looks good to me, with a few last comments, on the overall structure. Haven't looked in details on the DOM/WebIDL part, although it looks good.

}
}

#[derive(Clone, Copy, Debug, Deserialize, Serialize)]

This comment has been minimized.

Copy link
@gterzian

gterzian Nov 20, 2019

Collaborator

I don't understand where this commit is, was it added to this PR?

@@ -2092,6 +2102,13 @@ where
}
}

if let Some(webgpu) = self.webgpu.as_ref() {
debug!("Exiting WebGPU thread.");
if let Err(e) = webgpu.exit() {

This comment has been minimized.

Copy link
@gterzian

gterzian Nov 20, 2019

Collaborator

I would say that inside exit, you wan to add a IpcSender<()> to the exit message, and then block on the reply.

Because with the current setup, you can know for sure that you've queued the exit message, however there might be any number of other messages in front of it, therefore webgpu could keep running for an undetermined amount of time after the constellation has already exited, which potentially adds to our "thread running after exit count".

This comment has been minimized.

Copy link
@zakorgy

zakorgy Nov 20, 2019

Author Contributor

Addressed this in my latest commit: 40f3781

.spawn(move || {
WGPU::new(receiver).run();
})
.expect("Thread spawning failed");

This comment has been minimized.

Copy link
@gterzian

gterzian Nov 20, 2019

Collaborator

Ok last comment: since this will run in the "main process" we try to not panic there as it would bring down the whole browser. So could you please change any expect or unwrap in this file into a warn!?

This comment has been minimized.

Copy link
@zakorgy

zakorgy Nov 20, 2019

Author Contributor

Reploced thoose in 873a55b

@zakorgy zakorgy force-pushed the szeged:webgpu-base branch from 873a55b to 2744020 Nov 20, 2019
@zakorgy

This comment has been minimized.

Copy link
Contributor Author

zakorgy commented Nov 20, 2019

thanks @gterzian , I've squashed two commits to remove a fixup. Should I squash further?

@gterzian

This comment has been minimized.

Copy link
Collaborator

gterzian commented Nov 20, 2019

Yes it probably makes sense to squash it into one matching the PR title.

@zakorgy zakorgy force-pushed the szeged:webgpu-base branch from 2744020 to 434fa80 Nov 20, 2019
@bors-servo

This comment has been minimized.

Copy link
Contributor

bors-servo commented Nov 24, 2019

💔 Test failed - status-taskcluster

@jdm

This comment has been minimized.

Copy link
Member

jdm commented Nov 24, 2019

@bors-servo r=gterzian,kvark

@bors-servo

This comment has been minimized.

Copy link
Contributor

bors-servo commented Nov 24, 2019

📌 Commit 47e39ec has been approved by gterzian,kvark

@bors-servo

This comment has been minimized.

Copy link
Contributor

bors-servo commented Nov 24, 2019

⌛️ Testing commit 47e39ec with merge e8fcbda...

bors-servo added a commit that referenced this pull request Nov 24, 2019
Initial implementation of WebGPU API

<!-- Please describe your changes on the following line: -->
- Added the WebIDL bindings for GPU and GPUadapter interfaces.
- Created a background thread for WebGPU api calls.
- Established the async communication between the background thread and the WebGPU interfaces.
- Implemented the `requesAdapter` function of `navigator.gpu`

`./mach test-tidy` reports an error due to the different `arrayvec` version used in `servo` and `webgpu`, so added it to the ignore list in `servo-tidy.toml`

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `___` with appropriate data: -->
- [x] `./mach build -d` does not report any errors
- [ ] `./mach test-tidy` does not report any errors
- [ ] These changes addresses a part of ##24706

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

cc @jdm, cc @kvark

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/24708)
<!-- Reviewable:end -->
@bors-servo

This comment has been minimized.

Copy link
Contributor

bors-servo commented Nov 24, 2019

💔 Test failed - status-taskcluster

@jdm

This comment has been minimized.

Copy link
Member

jdm commented Nov 24, 2019

bors-servo added a commit that referenced this pull request Nov 24, 2019
Initial implementation of WebGPU API

<!-- Please describe your changes on the following line: -->
- Added the WebIDL bindings for GPU and GPUadapter interfaces.
- Created a background thread for WebGPU api calls.
- Established the async communication between the background thread and the WebGPU interfaces.
- Implemented the `requesAdapter` function of `navigator.gpu`

`./mach test-tidy` reports an error due to the different `arrayvec` version used in `servo` and `webgpu`, so added it to the ignore list in `servo-tidy.toml`

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `___` with appropriate data: -->
- [x] `./mach build -d` does not report any errors
- [ ] `./mach test-tidy` does not report any errors
- [ ] These changes addresses a part of ##24706

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

cc @jdm, cc @kvark

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/24708)
<!-- Reviewable:end -->
@bors-servo

This comment has been minimized.

Copy link
Contributor

bors-servo commented Nov 24, 2019

⌛️ Testing commit 47e39ec with merge f6aee8b...

@bors-servo

This comment has been minimized.

Copy link
Contributor

bors-servo commented Nov 24, 2019

💔 Test failed - status-taskcluster

@CYBAI

This comment has been minimized.

Copy link
Collaborator

CYBAI commented Nov 25, 2019

@bors-servo retry

@bors-servo

This comment has been minimized.

Copy link
Contributor

bors-servo commented Nov 25, 2019

⌛️ Testing commit 47e39ec with merge ea32495...

bors-servo added a commit that referenced this pull request Nov 25, 2019
Initial implementation of WebGPU API

<!-- Please describe your changes on the following line: -->
- Added the WebIDL bindings for GPU and GPUadapter interfaces.
- Created a background thread for WebGPU api calls.
- Established the async communication between the background thread and the WebGPU interfaces.
- Implemented the `requesAdapter` function of `navigator.gpu`

`./mach test-tidy` reports an error due to the different `arrayvec` version used in `servo` and `webgpu`, so added it to the ignore list in `servo-tidy.toml`

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `___` with appropriate data: -->
- [x] `./mach build -d` does not report any errors
- [ ] `./mach test-tidy` does not report any errors
- [ ] These changes addresses a part of ##24706

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

cc @jdm, cc @kvark

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/24708)
<!-- Reviewable:end -->
@bors-servo

This comment has been minimized.

Copy link
Contributor

bors-servo commented Nov 25, 2019

☀️ Test successful - status-taskcluster
Approved by: gterzian,kvark
Pushing ea32495 to master...

@bors-servo bors-servo merged commit 47e39ec into servo:master Nov 25, 2019
1 of 2 checks passed
1 of 2 checks passed
Community-TC (pull_request) TaskGroup: failure
Details
homu Test successful
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
8 participants
You can’t perform that action at this time.