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] createRender/ComputePipelineAsync are unimplemented #11118

Conversation

mwyrzykowski
Copy link
Contributor

@mwyrzykowski mwyrzykowski commented Mar 6, 2023

b794bac

[WebGPU] createRender/ComputePipelineAsync are unimplemented
https://bugs.webkit.org/show_bug.cgi?id=253404
<radar://106244188>

Reviewed by Myles C. Maxfield.

Add GPUPipelineError and use it to implement createRenderPipelineAsync
and createComputePipelineAsync.

* Source/WebCore/CMakeLists.txt:
* Source/WebCore/DerivedSources-input.xcfilelist:
* Source/WebCore/DerivedSources-output.xcfilelist:
* Source/WebCore/DerivedSources.make:
* Source/WebCore/Headers.cmake:
Add files.

* Source/WebCore/Modules/WebGPU/GPUDevice.cpp:
(WebCore::GPUDevice::createComputePipelineAsync):
(WebCore::GPUDevice::createRenderPipelineAsync):
Reject the promise on failure as specified in the specification.

* Source/WebCore/Modules/WebGPU/GPUPipelineError.cpp: Added.
(WebCore::GPUPipelineError::GPUPipelineError):
* Source/WebCore/Modules/WebGPU/GPUPipelineError.h: Added.
* Source/WebCore/Modules/WebGPU/GPUPipelineError.idl: Added.
* Source/WebCore/Modules/WebGPU/GPUPipelineErrorInit.h: Added.
* Source/WebCore/Modules/WebGPU/GPUPipelineErrorInit.idl: Added.
* Source/WebCore/Modules/WebGPU/GPUPipelineErrorReason.h: Added.
* Source/WebCore/Modules/WebGPU/GPUPipelineErrorReason.idl: Added.
New files.

* Source/WebCore/Modules/mediastream/libwebrtc/LibWebRTCRtpTransformableFrame.cpp:
Unified build fix.

* Source/WebCore/PAL/pal/graphics/WebGPU/Impl/WebGPUDeviceImpl.cpp:
(PAL::WebGPU::DeviceImpl::createComputePipelineAsync):
(PAL::WebGPU::DeviceImpl::createRenderPipelineAsync):
Return nullptr on failure.

* Source/WebCore/PAL/pal/graphics/WebGPU/Impl/WebGPUDeviceImpl.h:
* Source/WebCore/PAL/pal/graphics/WebGPU/WebGPUDevice.h:
* Source/WebCore/Sources.txt:
* Source/WebCore/WebCore.xcodeproj/project.pbxproj:
New files.

* Source/WebCore/bindings/js/WebCoreBuiltinNames.h:
New IDLInterface type.

* Source/WebGPU/WebGPU/ComputePipeline.mm:
(WebGPU::Device::createComputePipelineAsync):
* Source/WebGPU/WebGPU/RenderPipeline.mm:
(WebGPU::Device::createRenderPipelineAsync):
* Source/WebKit/GPUProcess/graphics/WebGPU/RemoteDevice.cpp:
(WebKit::RemoteDevice::createComputePipelineAsync):
(WebKit::RemoteDevice::createRenderPipelineAsync):
* Source/WebKit/GPUProcess/graphics/WebGPU/RemoteDevice.h:
* Source/WebKit/GPUProcess/graphics/WebGPU/RemoteDevice.messages.in:
* Source/WebKit/WebProcess/GPU/graphics/WebGPU/RemoteDeviceProxy.cpp:
(WebKit::WebGPU::RemoteDeviceProxy::createComputePipelineAsync):
(WebKit::WebGPU::RemoteDeviceProxy::createRenderPipelineAsync):
* Source/WebKit/WebProcess/GPU/graphics/WebGPU/RemoteDeviceProxy.h:
Return false or nullptr on failure from the callback.

* Source/WebCore/Modules/streams/WritableStream.cpp:
(WebCore::WritableStream::lock):
(WebCore::WritableStream::locked const):
(WebCore::WritableStream::internalWritableStream):
* Source/WebCore/Modules/streams/WritableStream.h:
(WebCore::WritableStream::lock): Deleted.
(WebCore::WritableStream::locked const): Deleted.
(WebCore::WritableStream::internalWritableStream): Deleted.
Unified build fixes.

Canonical link: https://commits.webkit.org/261415@main

ba9f13b

Misc iOS, tvOS & watchOS macOS Linux Windows
βœ… πŸ§ͺ style βœ… πŸ›  ios βœ… πŸ›  mac βœ… πŸ›  wpe βœ… πŸ›  wincairo
βœ… πŸ§ͺ bindings βœ… πŸ›  ios-sim βœ… πŸ›  mac-AS-debug   πŸ§ͺ wpe-wk2
βœ… πŸ§ͺ webkitperl βœ… πŸ§ͺ ios-wk2 βœ… πŸ§ͺ api-mac βœ… πŸ›  gtk
βœ… πŸ§ͺ api-ios βœ… πŸ§ͺ mac-wk1 βœ… πŸ§ͺ gtk-wk2
βœ… πŸ›  tv βœ… πŸ§ͺ mac-wk2 ❌ πŸ§ͺ api-gtk
βœ… πŸ›  tv-sim βœ… πŸ§ͺ mac-AS-debug-wk2
βœ… πŸ›  watch βœ… πŸ§ͺ mac-wk2-stress
βœ… πŸ›  πŸ§ͺ merge βœ… πŸ›  watch-sim

@mwyrzykowski mwyrzykowski self-assigned this Mar 6, 2023
@mwyrzykowski mwyrzykowski added the WebGPU For bugs in WebGPU label Mar 6, 2023
@@ -29,9 +29,11 @@

ALLOW_UNUSED_PARAMETERS_BEGIN
ALLOW_DEPRECATED_DECLARATIONS_BEGIN
ALLOW_COMMA_BEGIN
Copy link
Contributor Author

Choose a reason for hiding this comment

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

unified build fixes

Copy link
Contributor

Choose a reason for hiding this comment

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

Really? Usually unified build fixes are just #includes....

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In file included from /Volumes/Data/OpenSource/WebKitBuild/Debug/usr/local/include/absl/memory/memory.h:33:
/Volumes/Data/OpenSource/WebKitBuild/Debug/usr/local/include/absl/base/macros.h:134:37: note: expanded from macro 'ABSL_HARDENING_ASSERT'
#define ABSL_HARDENING_ASSERT(expr) ABSL_ASSERT(expr)
/Volumes/Data/OpenSource/WebKitBuild/Debug/usr/local/include/absl/base/macros.h:98:3: note: expanded from macro 'ABSL_ASSERT'
  (ABSL_PREDICT_TRUE((expr)) ? static_cast<void>(0) \
  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from /Volumes/Data/OpenSource/WebKitBuild/Debug/DerivedSources/WebCore/unified-sources/UnifiedSource56.cpp:1:
In file included from /Volumes/Data/OpenSource/Source/WebCore/Modules/mediastream/libwebrtc/LibWebRTCRtpTransformableFrame.cpp:33:
In file included from /Volumes/Data/OpenSource/WebKitBuild/Debug/usr/local/include/webrtc/api/frame_transformer_interface.h:18:
In file included from /Volumes/Data/OpenSource/WebKitBuild/Debug/usr/local/include/webrtc/api/video/encoded_frame.h:18:
In file included from /Volumes/Data/OpenSource/WebKitBuild/Debug/usr/local/include/webrtc/api/units/timestamp.h:21:
In file included from /Volumes/Data/OpenSource/WebKitBuild/Debug/usr/local/include/webrtc/api/units/time_delta.h:22:
In file included from /Volumes/Data/OpenSource/WebKitBuild/Debug/usr/local/include/webrtc/rtc_base/units/unit_base.h:20:
In file included from /Volumes/Data/OpenSource/WebKitBuild/Debug/usr/local/include/webrtc/rtc_base/checks.h:58:
/Volumes/Data/OpenSource/WebKitBuild/Debug/usr/local/include/absl/strings/string_view.h:320:43: error: possible misuse of comma operator here [-Werror,-Wcomma]
    return ABSL_HARDENING_ASSERT(!empty()), ptr_[size() - 1];
/Volumes/Data/OpenSource/WebKitBuild/Debug/usr/local/include/absl/strings/string_view.h:320:12: note: cast expression to void to silence warning
    return ABSL_HARDENING_ASSERT(!empty()), ptr_[size() - 1];
In file included from /Volumes/Data/OpenSource/WebKitBuild/Debug/DerivedSources/WebCore/unified-sources/UnifiedSource56.cpp:1:
In file included from /Volumes/Data/OpenSource/Source/WebCore/Modules/mediastream/libwebrtc/LibWebRTCRtpTransformableFrame.cpp:33:
In file included from /Volumes/Data/OpenSource/WebKitBuild/Debug/usr/local/include/webrtc/api/frame_transformer_interface.h:18:
In file included from /Volumes/Data/OpenSource/WebKitBuild/Debug/usr/local/include/webrtc/api/video/encoded_frame.h:17:
In file included from /Volumes/Data/OpenSource/WebKitBuild/Debug/usr/local/include/absl/types/optional.h:67:
In file included from /Volumes/Data/OpenSource/WebKitBuild/Debug/usr/local/include/absl/types/internal/optional.h:24:

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It depends on who includes Debug/usr/local/include/absl/strings/string_view.h first with the warning ignored

@mwyrzykowski mwyrzykowski force-pushed the eng/WebGPU-createRenderComputePipelineAsync-are-unimplemented branch from d861fb9 to 37f1adc Compare March 6, 2023 21:46
@mwyrzykowski mwyrzykowski force-pushed the eng/WebGPU-createRenderComputePipelineAsync-are-unimplemented branch from 37f1adc to f895641 Compare March 6, 2023 22:29
@mwyrzykowski mwyrzykowski force-pushed the eng/WebGPU-createRenderComputePipelineAsync-are-unimplemented branch from f895641 to 2bf8c24 Compare March 6, 2023 22:51
@mwyrzykowski mwyrzykowski force-pushed the eng/WebGPU-createRenderComputePipelineAsync-are-unimplemented branch from 2bf8c24 to cbb4078 Compare March 7, 2023 01:01
@mwyrzykowski mwyrzykowski force-pushed the eng/WebGPU-createRenderComputePipelineAsync-are-unimplemented branch from cbb4078 to bd0c564 Compare March 7, 2023 01:18
@mwyrzykowski mwyrzykowski force-pushed the eng/WebGPU-createRenderComputePipelineAsync-are-unimplemented branch from bd0c564 to e4656bf Compare March 7, 2023 01:42
Copy link
Contributor

@litherum litherum left a comment

Choose a reason for hiding this comment

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

Thanks for keeping the patch fairly mechanical πŸ‘

@@ -29,9 +29,11 @@

ALLOW_UNUSED_PARAMETERS_BEGIN
ALLOW_DEPRECATED_DECLARATIONS_BEGIN
ALLOW_COMMA_BEGIN
Copy link
Contributor

Choose a reason for hiding this comment

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

Really? Usually unified build fixes are just #includes....

UNUSED_PARAM(descriptor);
instance().scheduleWork([strongThis = Ref { *this }, callback = WTFMove(callback)]() mutable {
callback(WGPUCreatePipelineAsyncStatus_InternalError, ComputePipeline::createInvalid(strongThis), { });
auto pipeline = createComputePipeline(descriptor);
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this is fine for now, but we should at least file a bug and add a FIXME with a link to the bug about doing this actually asynchronously

UNUSED_PARAM(descriptor);
instance().scheduleWork([strongThis = Ref { *this }, callback = WTFMove(callback)]() mutable {
callback(WGPUCreatePipelineAsyncStatus_InternalError, RenderPipeline::createInvalid(strongThis), { });
auto pipeline = createRenderPipeline(descriptor);
Copy link
Contributor

Choose a reason for hiding this comment

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

Ditto.

objectHeap->addObject(identifier, remoteComputePipeline);
callback();
m_backing->createComputePipelineAsync(*convertedDescriptor, [callback = WTFMove(callback), objectHeap = Ref { m_objectHeap }, streamConnection = m_streamConnection.copyRef(), identifier] (RefPtr<PAL::WebGPU::ComputePipeline>&& computePipeline) mutable {
bool result = computePipeline.get();
Copy link
Contributor

Choose a reason for hiding this comment

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

This is so we know whether or not to reject the promise, I assume?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

that's correct

@mwyrzykowski mwyrzykowski force-pushed the eng/WebGPU-createRenderComputePipelineAsync-are-unimplemented branch from e4656bf to cdf1a4c Compare March 8, 2023 02:01
@mwyrzykowski mwyrzykowski force-pushed the eng/WebGPU-createRenderComputePipelineAsync-are-unimplemented branch from cdf1a4c to ba9f13b Compare March 8, 2023 16:50
@@ -45,6 +46,23 @@ ExceptionOr<Ref<WritableStream>> WritableStream::create(JSC::JSGlobalObject& glo
return create(globalObject, underlyingSinkValue, strategyValue);
}

WritableStream::~WritableStream() = default;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

cc @youennf to verify these unified build fixes are acceptable. They are appearing in #11154 too, I think these changes will fix them there as well

Copy link
Contributor

Choose a reason for hiding this comment

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

This seems fine, JSDOMGlobalObject.h is now included in WritableStream.h so no need to include it in WritableStream.cpp

@mwyrzykowski mwyrzykowski added the merge-queue Applied to send a pull request to merge-queue label Mar 9, 2023
https://bugs.webkit.org/show_bug.cgi?id=253404
<radar://106244188>

Reviewed by Myles C. Maxfield.

Add GPUPipelineError and use it to implement createRenderPipelineAsync
and createComputePipelineAsync.

* Source/WebCore/CMakeLists.txt:
* Source/WebCore/DerivedSources-input.xcfilelist:
* Source/WebCore/DerivedSources-output.xcfilelist:
* Source/WebCore/DerivedSources.make:
* Source/WebCore/Headers.cmake:
Add files.

* Source/WebCore/Modules/WebGPU/GPUDevice.cpp:
(WebCore::GPUDevice::createComputePipelineAsync):
(WebCore::GPUDevice::createRenderPipelineAsync):
Reject the promise on failure as specified in the specification.

* Source/WebCore/Modules/WebGPU/GPUPipelineError.cpp: Added.
(WebCore::GPUPipelineError::GPUPipelineError):
* Source/WebCore/Modules/WebGPU/GPUPipelineError.h: Added.
* Source/WebCore/Modules/WebGPU/GPUPipelineError.idl: Added.
* Source/WebCore/Modules/WebGPU/GPUPipelineErrorInit.h: Added.
* Source/WebCore/Modules/WebGPU/GPUPipelineErrorInit.idl: Added.
* Source/WebCore/Modules/WebGPU/GPUPipelineErrorReason.h: Added.
* Source/WebCore/Modules/WebGPU/GPUPipelineErrorReason.idl: Added.
New files.

* Source/WebCore/Modules/mediastream/libwebrtc/LibWebRTCRtpTransformableFrame.cpp:
Unified build fix.

* Source/WebCore/PAL/pal/graphics/WebGPU/Impl/WebGPUDeviceImpl.cpp:
(PAL::WebGPU::DeviceImpl::createComputePipelineAsync):
(PAL::WebGPU::DeviceImpl::createRenderPipelineAsync):
Return nullptr on failure.

* Source/WebCore/PAL/pal/graphics/WebGPU/Impl/WebGPUDeviceImpl.h:
* Source/WebCore/PAL/pal/graphics/WebGPU/WebGPUDevice.h:
* Source/WebCore/Sources.txt:
* Source/WebCore/WebCore.xcodeproj/project.pbxproj:
New files.

* Source/WebCore/bindings/js/WebCoreBuiltinNames.h:
New IDLInterface type.

* Source/WebGPU/WebGPU/ComputePipeline.mm:
(WebGPU::Device::createComputePipelineAsync):
* Source/WebGPU/WebGPU/RenderPipeline.mm:
(WebGPU::Device::createRenderPipelineAsync):
* Source/WebKit/GPUProcess/graphics/WebGPU/RemoteDevice.cpp:
(WebKit::RemoteDevice::createComputePipelineAsync):
(WebKit::RemoteDevice::createRenderPipelineAsync):
* Source/WebKit/GPUProcess/graphics/WebGPU/RemoteDevice.h:
* Source/WebKit/GPUProcess/graphics/WebGPU/RemoteDevice.messages.in:
* Source/WebKit/WebProcess/GPU/graphics/WebGPU/RemoteDeviceProxy.cpp:
(WebKit::WebGPU::RemoteDeviceProxy::createComputePipelineAsync):
(WebKit::WebGPU::RemoteDeviceProxy::createRenderPipelineAsync):
* Source/WebKit/WebProcess/GPU/graphics/WebGPU/RemoteDeviceProxy.h:
Return false or nullptr on failure from the callback.

* Source/WebCore/Modules/streams/WritableStream.cpp:
(WebCore::WritableStream::lock):
(WebCore::WritableStream::locked const):
(WebCore::WritableStream::internalWritableStream):
* Source/WebCore/Modules/streams/WritableStream.h:
(WebCore::WritableStream::lock): Deleted.
(WebCore::WritableStream::locked const): Deleted.
(WebCore::WritableStream::internalWritableStream): Deleted.
Unified build fixes.

Canonical link: https://commits.webkit.org/261415@main
@webkit-commit-queue webkit-commit-queue force-pushed the eng/WebGPU-createRenderComputePipelineAsync-are-unimplemented branch from ba9f13b to b794bac Compare March 9, 2023 09:00
@webkit-commit-queue
Copy link
Collaborator

Committed 261415@main (b794bac): https://commits.webkit.org/261415@main

Reviewed commits have been landed. Closing PR #11118 and removing active labels.

@webkit-commit-queue webkit-commit-queue merged commit b794bac into WebKit:main Mar 9, 2023
@webkit-commit-queue webkit-commit-queue removed the merge-queue Applied to send a pull request to merge-queue label Mar 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
WebGPU For bugs in WebGPU
Projects
None yet
5 participants