commit ec8a6a54a0472cb37ca23161ec9bf30ebc6e057f
parent 8d929c6b779e762ad4a54593258c58720f8c23ab
Author: Teodor Tanasoaia <ttanasoaia@mozilla.com>
Date: Fri, 24 Oct 2025 07:25:58 +0000
Bug 1973591 - Avoid freeing swapchain buffer IDs too early. r=webgpu-reviewers,aleiserson
Differential Revision: https://phabricator.services.mozilla.com/D269784
Diffstat:
8 files changed, 51 insertions(+), 33 deletions(-)
diff --git a/dom/webgpu/CanvasContext.cpp b/dom/webgpu/CanvasContext.cpp
@@ -153,15 +153,8 @@ void CanvasContext::Configure(const dom::GPUCanvasConfiguration& aConfig,
}
#endif
- // buffer count doesn't matter much, will be created on demand
- const size_t maxBufferCount = 10;
- for (size_t i = 0; i < maxBufferCount; ++i) {
- mBufferIds.AppendElement(
- ffi::wgpu_client_make_buffer_id(aConfig.mDevice->GetClient()));
- }
-
mCurrentTexture = aConfig.mDevice->InitSwapChain(
- mConfiguration.get(), mRemoteTextureOwnerId.ref(), mBufferIds,
+ mConfiguration.get(), mRemoteTextureOwnerId.ref(),
mUseSharedTextureInSwapChain, mGfxFormat, mCanvasSize);
if (!mCurrentTexture) {
Unconfigure();
@@ -183,12 +176,7 @@ void CanvasContext::Unconfigure() {
auto txn_id = layers::ToRemoteTextureTxnId(mFwdTransactionTracker);
ffi::wgpu_client_swap_chain_drop(
mChild->GetClient(), mRemoteTextureOwnerId->mId, txn_type, txn_id);
-
- for (auto& id : mBufferIds) {
- ffi::wgpu_client_free_buffer_id(mChild->GetClient(), id);
- }
}
- mBufferIds.Clear();
mRemoteTextureOwnerId = Nothing();
mFwdTransactionTracker = nullptr;
mChild = nullptr;
diff --git a/dom/webgpu/CanvasContext.h b/dom/webgpu/CanvasContext.h
@@ -113,7 +113,6 @@ class CanvasContext final : public nsICanvasRenderingContextInternal,
Maybe<layers::RemoteTextureId> mLastRemoteTextureId;
Maybe<layers::RemoteTextureOwnerId> mRemoteTextureOwnerId;
- nsTArray<RawId> mBufferIds;
RefPtr<layers::FwdTransactionTracker> mFwdTransactionTracker;
bool mUseSharedTextureInSwapChain = false;
bool mNewTextureRequested = false;
diff --git a/dom/webgpu/Device.cpp b/dom/webgpu/Device.cpp
@@ -933,8 +933,8 @@ already_AddRefed<dom::Promise> Device::CreateRenderPipelineAsync(
already_AddRefed<Texture> Device::InitSwapChain(
const dom::GPUCanvasConfiguration* const aConfig,
const layers::RemoteTextureOwnerId aOwnerId,
- mozilla::Span<RawId const> aBufferIds, bool aUseSharedTextureInSwapChain,
- gfx::SurfaceFormat aFormat, gfx::IntSize aCanvasSize) {
+ bool aUseSharedTextureInSwapChain, gfx::SurfaceFormat aFormat,
+ gfx::IntSize aCanvasSize) {
MOZ_ASSERT(aConfig);
// Check that aCanvasSize and aFormat will generate a texture stride
@@ -948,8 +948,7 @@ already_AddRefed<Texture> Device::InitSwapChain(
ffi::wgpu_client_create_swap_chain(
GetClient(), GetId(), mQueue->GetId(), rgbDesc.size().Width(),
- rgbDesc.size().Height(), (int8_t)rgbDesc.format(),
- {aBufferIds.Elements(), aBufferIds.Length()}, aOwnerId.mId,
+ rgbDesc.size().Height(), (int8_t)rgbDesc.format(), aOwnerId.mId,
aUseSharedTextureInSwapChain);
// TODO: `mColorSpace`: <https://bugzilla.mozilla.org/show_bug.cgi?id=1846608>
diff --git a/dom/webgpu/Device.h b/dom/webgpu/Device.h
@@ -107,8 +107,8 @@ class Device final : public DOMEventTargetHelper,
already_AddRefed<Texture> InitSwapChain(
const dom::GPUCanvasConfiguration* const aConfig,
const layers::RemoteTextureOwnerId aOwnerId,
- mozilla::Span<RawId const> aBufferIds, bool aUseSharedTextureInSwapChain,
- gfx::SurfaceFormat aFormat, gfx::IntSize aCanvasSize);
+ bool aUseSharedTextureInSwapChain, gfx::SurfaceFormat aFormat,
+ gfx::IntSize aCanvasSize);
bool CheckNewWarning(const nsACString& aMessage);
void TrackBuffer(Buffer* aBuffer);
diff --git a/dom/webgpu/ipc/WebGPUParent.cpp b/dom/webgpu/ipc/WebGPUParent.cpp
@@ -1545,9 +1545,19 @@ void WebGPUParent::SwapChainDrop(const layers::RemoteTextureOwnerId& aOwnerId,
for (const auto bid : data->mAvailableBufferIds) {
ffi::wgpu_server_buffer_drop(mContext.get(), bid);
+ data->mUnassignedBufferIds.push_back(bid);
}
for (const auto bid : data->mQueuedBufferIds) {
ffi::wgpu_server_buffer_drop(mContext.get(), bid);
+ data->mUnassignedBufferIds.push_back(bid);
+ }
+
+ ipc::ByteBuf bb;
+ ffi::wgpu_server_pack_free_swap_chain_buffer_ids(
+ ToFFI(&bb),
+ {data->mUnassignedBufferIds.data(), data->mUnassignedBufferIds.size()});
+ if (!SendServerMessage(std::move(bb))) {
+ NS_ERROR("SendServerMessage failed");
}
}
diff --git a/gfx/wgpu_bindings/src/client.rs b/gfx/wgpu_bindings/src/client.rs
@@ -21,6 +21,7 @@ use parking_lot::Mutex;
use nsstring::{nsACString, nsCString, nsString};
+use std::array;
use std::fmt::Write;
use std::{borrow::Cow, ptr};
@@ -516,16 +517,6 @@ pub extern "C" fn wgpu_client_request_device(
}
#[no_mangle]
-pub extern "C" fn wgpu_client_make_buffer_id(client: &Client) -> id::BufferId {
- client.identities.lock().buffers.process()
-}
-
-#[no_mangle]
-pub extern "C" fn wgpu_client_free_buffer_id(client: &Client, id: id::BufferId) {
- client.identities.lock().buffers.free(id)
-}
-
-#[no_mangle]
pub extern "C" fn wgpu_client_make_render_pass_encoder_id(
client: &Client,
) -> id::RenderPassEncoderId {
@@ -826,6 +817,13 @@ pub extern "C" fn wgpu_client_receive_server_message(client: &Client, byte_buf:
ServerMessage::QueueOnSubmittedWorkDoneResponse(queue_id) => unsafe {
wgpu_child_resolve_on_submitted_work_done_promise(client.owner, queue_id);
},
+
+ ServerMessage::FreeSwapChainBufferIds(buffer_ids) => {
+ let identities = client.identities.lock();
+ for id in buffer_ids {
+ identities.buffers.free(id);
+ }
+ }
}
}
@@ -881,17 +879,21 @@ pub extern "C" fn wgpu_client_create_swap_chain(
width: i32,
height: i32,
format: crate::SurfaceFormat,
- buffers: FfiSlice<'_, id::BufferId>,
remote_texture_owner_id: crate::RemoteTextureOwnerId,
use_shared_texture_in_swap_chain: bool,
) {
+ let identities = client.identities.lock();
+ let buffer_ids: [id::BufferId; crate::MAX_SWAPCHAIN_BUFFER_COUNT] =
+ array::from_fn(|_| identities.buffers.process());
+ drop(identities);
+
let message = Message::CreateSwapChain {
device_id,
queue_id,
width,
height,
format,
- buffer_ids: Cow::Borrowed(unsafe { buffers.as_slice() }),
+ buffer_ids,
remote_texture_owner_id,
use_shared_texture_in_swap_chain,
};
diff --git a/gfx/wgpu_bindings/src/lib.rs b/gfx/wgpu_bindings/src/lib.rs
@@ -227,6 +227,8 @@ pub enum QueueWriteDataSource {
Shmem(usize),
}
+const MAX_SWAPCHAIN_BUFFER_COUNT: usize = 10;
+
#[derive(serde::Serialize, serde::Deserialize)]
enum Message<'a> {
RequestAdapter {
@@ -280,7 +282,7 @@ enum Message<'a> {
width: i32,
height: i32,
format: SurfaceFormat,
- buffer_ids: Cow<'a, [id::BufferId]>,
+ buffer_ids: [id::BufferId; MAX_SWAPCHAIN_BUFFER_COUNT],
remote_texture_owner_id: RemoteTextureOwnerId,
use_shared_texture_in_swap_chain: bool,
},
@@ -448,6 +450,11 @@ enum ServerMessage<'a> {
CreateShaderModuleResponse(id::ShaderModuleId, Vec<ShaderModuleCompilationMessage>),
BufferMapResponse(id::BufferId, BufferMapResult<'a>),
QueueOnSubmittedWorkDoneResponse(id::QueueId),
+
+ /// This message tells the client when we are done with swapchain readback buffers.
+ /// Freeing a swapchain buffer too early may result in an ID resolution panic or
+ /// an error submitting swapchain-related commands to the GPU.
+ FreeSwapChainBufferIds([id::BufferId; MAX_SWAPCHAIN_BUFFER_COUNT]),
}
#[repr(C)]
diff --git a/gfx/wgpu_bindings/src/server.rs b/gfx/wgpu_bindings/src/server.rs
@@ -2543,6 +2543,19 @@ pub unsafe extern "C" fn wgpu_server_pack_work_done(bb: &mut ByteBuf, queue_id:
*bb = make_byte_buf(&ServerMessage::QueueOnSubmittedWorkDoneResponse(queue_id));
}
+/// # Panics
+///
+/// If the size of `buffer_ids` is not [`crate::MAX_SWAPCHAIN_BUFFER_COUNT`].
+#[no_mangle]
+pub unsafe extern "C" fn wgpu_server_pack_free_swap_chain_buffer_ids(
+ bb: &mut ByteBuf,
+ buffer_ids: FfiSlice<'_, id::BufferId>,
+) {
+ *bb = make_byte_buf(&ServerMessage::FreeSwapChainBufferIds(
+ buffer_ids.as_slice().try_into().unwrap(),
+ ));
+}
+
#[no_mangle]
pub unsafe extern "C" fn wgpu_server_messages(
global: &Global,
