Branch: refs/heads/main
Home: https://github.com/WebKit/WebKit
Commit: 6c4c981002fe98d371b03ab862b589120661a63d
https://github.com/WebKit/WebKit/commit/6c4c981002fe98d371b03ab862b589120661a63d
Author: Myles C. Maxfield <[email protected]>
Date: 2023-02-04 (Sat, 04 Feb 2023)
Changed paths:
M Source/WebCore/PAL/PAL.xcodeproj/project.pbxproj
M Source/WebCore/PAL/pal/CMakeLists.txt
R Source/WebCore/PAL/pal/graphics/WebGPU/Impl/WebGPUDeviceHolderImpl.cpp
R Source/WebCore/PAL/pal/graphics/WebGPU/Impl/WebGPUDeviceHolderImpl.h
M Source/WebCore/PAL/pal/graphics/WebGPU/Impl/WebGPUDeviceImpl.cpp
M Source/WebCore/PAL/pal/graphics/WebGPU/Impl/WebGPUDeviceImpl.h
A Source/WebCore/PAL/pal/graphics/WebGPU/Impl/WebGPUDeviceWrapper.cpp
A Source/WebCore/PAL/pal/graphics/WebGPU/Impl/WebGPUDeviceWrapper.h
M
Source/WebCore/PAL/pal/graphics/WebGPU/Impl/WebGPUPresentationContextImpl.cpp
M
Source/WebCore/PAL/pal/graphics/WebGPU/Impl/WebGPUPresentationContextImpl.h
M Source/WebCore/PAL/pal/graphics/WebGPU/Impl/WebGPUQueueImpl.cpp
M Source/WebCore/PAL/pal/graphics/WebGPU/Impl/WebGPUQueueImpl.h
A Source/WebCore/PAL/pal/graphics/WebGPU/Impl/WebGPUSwapChainWrapper.cpp
A Source/WebCore/PAL/pal/graphics/WebGPU/Impl/WebGPUSwapChainWrapper.h
M Source/WebCore/PAL/pal/graphics/WebGPU/Impl/WebGPUTextureImpl.cpp
M Source/WebCore/PAL/pal/graphics/WebGPU/Impl/WebGPUTextureImpl.h
Log Message:
-----------
[WebGPU] Non-owning getters have the wrong lifetime
https://bugs.webkit.org/show_bug.cgi?id=250958
rdar://104518638
Reviewed by Dean Jackson.
There are 2 places in WebGPU where objects have getter methods that return
internally-retained objects:
1. Device::getQueue() is supposed to return the same queue object every time
you call it, and
2. PresentationContext::getCurrentTexture() is supposed to return the same
texture object every time you call it within
the same frame.
Let's call this pattern "Owner" and "Owned." The Owner is supposed to retain
its Owned. Easy peasy, right?
Well, it gets trickier because:
1. We have a corresponding set of Impl objects in PAL, each of which is
supposed to maintain a strong reference to its
corresponding object in WebGPU.framework.
2. Objects exposed by WebGPU.framework are not reference counted.
So, naively, we would have:
+-----------+
| OwnerImpl |
+-----------+
| \
| \
| \
| V
| +-----------+
| | OwnedImpl |
| +-----------+
| |
~~~~~~~~|~~~~~~~~~~~~~|~~~~~~~~~~ WebGPU.framework boundary
V |
+-------+ |
| Owner | |
+-------+ |
\ |
\ |
\ |
V V BANG!!! EXPLOSION!!!
+-------+
| Owned |
+-------+
The above design can't actually work, because Owned isn't reference counted.
So, instead, we can introduce a reference-
counted facade on top of Owner, to look like this:
+-----------+
| OwnerImpl |
+-----------+
| \
| \
| \
| V
| +-----------+
| | OwnedImpl |
| +-----------+
| |
| |
V V
+--------------+
| OwnerWrapper |
+--------------+
|
|
~~~~~~~~|~~~~~~~~~~~~~~~~~~~~~~~~ WebGPU.framework boundary
V
+-------+
| Owner |
+-------+
\
\
\
V
+-------+
| Owned |
+-------+
This design has all the properties we want:
1. All WebGPU.framework objects have a single owner
2. Any strong reference to the OwnerImpl keeps the Owner alive
3. Any strong reference to the OwnedImpl keeps the Owned alive
4. There are no reference cycles
The OwnedImpl doesn't actually call any functions on the OwnerWrapper; the only
reason it refs it is to make the above
requirements hold.
There are 2 other possible designs which satisfy the requirements: 1. Have
OwnedImpl delegate its ref() and deref()
calls to the OwnerImpl, and 2. Make WebGPU.framework objects reference counted.
I chose this patch's design over (1)
because (1) is significantly more complicated and I'm more likely to make a
mistake with that design. I chose this
patch's design over (2) because I didn't want to change the semantic behavior
of the WebGPU.h objects.
* Source/WebCore/PAL/PAL.xcodeproj/project.pbxproj:
* Source/WebCore/PAL/pal/CMakeLists.txt:
* Source/WebCore/PAL/pal/graphics/WebGPU/Impl/WebGPUDeviceImpl.cpp:
(PAL::WebGPU::DeviceImpl::DeviceImpl):
(PAL::WebGPU::DeviceImpl::destroy):
(PAL::WebGPU::DeviceImpl::createBuffer):
(PAL::WebGPU::DeviceImpl::createTexture):
(PAL::WebGPU::DeviceImpl::createSurfaceTexture):
(PAL::WebGPU::DeviceImpl::createSampler):
(PAL::WebGPU::DeviceImpl::createBindGroupLayout):
(PAL::WebGPU::DeviceImpl::createPipelineLayout):
(PAL::WebGPU::DeviceImpl::createBindGroup):
(PAL::WebGPU::DeviceImpl::createShaderModule):
(PAL::WebGPU::DeviceImpl::createComputePipeline):
(PAL::WebGPU::DeviceImpl::createRenderPipeline):
(PAL::WebGPU::DeviceImpl::createComputePipelineAsync):
(PAL::WebGPU::DeviceImpl::createRenderPipelineAsync):
(PAL::WebGPU::DeviceImpl::createCommandEncoder):
(PAL::WebGPU::DeviceImpl::createRenderBundleEncoder):
(PAL::WebGPU::DeviceImpl::createQuerySet):
(PAL::WebGPU::DeviceImpl::pushErrorScope):
(PAL::WebGPU::DeviceImpl::popErrorScope):
(PAL::WebGPU::DeviceImpl::setLabelInternal):
* Source/WebCore/PAL/pal/graphics/WebGPU/Impl/WebGPUDeviceImpl.h:
* Source/WebCore/PAL/pal/graphics/WebGPU/Impl/WebGPUDeviceWrapper.cpp: Copied
from Source/WebCore/PAL/pal/graphics/WebGPU/Impl/WebGPUDeviceHolderImpl.cpp.
(PAL::WebGPU::DeviceWrapper::DeviceWrapper):
(PAL::WebGPU::DeviceWrapper::~DeviceWrapper):
* Source/WebCore/PAL/pal/graphics/WebGPU/Impl/WebGPUDeviceWrapper.h: Copied
from Source/WebCore/PAL/pal/graphics/WebGPU/Impl/WebGPUDeviceHolderImpl.h.
* Source/WebCore/PAL/pal/graphics/WebGPU/Impl/WebGPUPresentationContextImpl.cpp:
(PAL::WebGPU::PresentationContextImpl::~PresentationContextImpl):
(PAL::WebGPU::PresentationContextImpl::configure):
(PAL::WebGPU::PresentationContextImpl::unconfigure):
* Source/WebCore/PAL/pal/graphics/WebGPU/Impl/WebGPUPresentationContextImpl.h:
* Source/WebCore/PAL/pal/graphics/WebGPU/Impl/WebGPUQueueImpl.cpp:
(PAL::WebGPU::QueueImpl::QueueImpl):
(PAL::WebGPU::QueueImpl::~QueueImpl):
(PAL::WebGPU::QueueImpl::submit):
(PAL::WebGPU::QueueImpl::onSubmittedWorkDone):
(PAL::WebGPU::QueueImpl::writeBuffer):
(PAL::WebGPU::QueueImpl::writeTexture):
(PAL::WebGPU::QueueImpl::setLabelInternal):
* Source/WebCore/PAL/pal/graphics/WebGPU/Impl/WebGPUQueueImpl.h:
* Source/WebCore/PAL/pal/graphics/WebGPU/Impl/WebGPUSwapChainWrapper.cpp:
Renamed from
Source/WebCore/PAL/pal/graphics/WebGPU/Impl/WebGPUDeviceHolderImpl.cpp.
(PAL::WebGPU::SwapChainWrapper::SwapChainWrapper):
(PAL::WebGPU::SwapChainWrapper::~SwapChainWrapper):
* Source/WebCore/PAL/pal/graphics/WebGPU/Impl/WebGPUSwapChainWrapper.h: Renamed
from Source/WebCore/PAL/pal/graphics/WebGPU/Impl/WebGPUDeviceHolderImpl.h.
* Source/WebCore/PAL/pal/graphics/WebGPU/Impl/WebGPUTextureImpl.cpp:
(PAL::WebGPU::TextureImpl::TextureImpl):
(PAL::WebGPU::TextureImpl::~TextureImpl):
* Source/WebCore/PAL/pal/graphics/WebGPU/Impl/WebGPUTextureImpl.h:
Canonical link: https://commits.webkit.org/259867@main
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
