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

webgpu continued #946

Merged
merged 6 commits into from
Aug 8, 2023
Merged

webgpu continued #946

merged 6 commits into from
Aug 8, 2023

Conversation

edevil
Copy link
Contributor

@edevil edevil commented Jul 31, 2023

Implemented more webgpu methods and objects.

@edevil edevil force-pushed the w4 branch 9 times, most recently from edb3884 to 5b9583e Compare August 4, 2023 13:55
: device_(d), async_(kj::refcounted<AsyncRunner>(d)) {
device_.SetLoggingCallback(
[](WGPULoggingType type, char const *message, void *userdata) {
KJ_LOG(INFO, "WebGPU logging", kj::str(type), message);
Copy link
Member

Choose a reason for hiding this comment

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

What's the purpose of the KJ_LOG statements? We do not generate INFO logs in production so if these are meant to be seen in production they need to use KJ_LOG(WARNING, ...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is purely for debugging purposes when someone is using --verbose. I don't yet have a place to send these.

Would it be possible/advisable to simulate console.log()s so that the script author could see these?

@edevil edevil force-pushed the w4 branch 4 times, most recently from 2308d3a to 278841b Compare August 4, 2023 18:15
Implemented more webgpu methods and objects.
edevil added 3 commits August 7, 2023 14:31
Ensure `lost` promise is fulfilled when the callback is triggered or
the device is destroyed.
Correctly handle case where there are issues finding a GPU.
If we store a local reference to the detach key it can happen
that at detach time it won't match the key stored on the arraybuffer,
therefore failing the detach operation.
@edevil
Copy link
Contributor Author

edevil commented Aug 7, 2023

Any additional comments @jasnell ?

? buffer_.GetMappedRange(o, s)
: const_cast<void *>(buffer_.GetConstMappedRange(o, s));
: const_cast<void*>(buffer_.GetConstMappedRange(o, s));
Copy link
Member

Choose a reason for hiding this comment

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

What's the expected behavior here if offset and size are outside the allocated range? Will this return nullptr? For instance, if the range is only 10 and I specify an offset of 5 and a size of 10, will this return nullptr?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Exactly, ptr will be nullptr and JSG_REQUIRE will bail out.

Use a kj::Array instead of a std::vector.
}

kj::Maybe<GPUFeatureName> getFeatureName(wgpu::FeatureName& feature) {
switch (feature) {
Copy link
Member

Choose a reason for hiding this comment

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

You might consider implementing KJ_STRINGIFY for these enum structures. The implementation would be largely the same but it would allow you to do kj::str(feature), rather than kj::str(getFeatureName(feature)), etc. Search the code base for examples of where KJ_STRINGIFY has been implemented.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I didn't use KJ_STRINGIFY because in this case I want to return an optional value. We may receive unknown features and I don't want to blow up but just ignore those features. KJ_STRINGIFY AFAICT, forces me to return some string.

Remove unique_ptr usage with V8Refs.
@edevil edevil merged commit 9f56120 into main Aug 8, 2023
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