Auto merge of #27180 - kunalmohan:segfault, r=kvark

Remove segfaults in WebGPU threads

<!-- Please describe your changes on the following line: -->
I have also increased the number of staging buffers for presentation.
Segfault occurred at 2 places-
1. RenderPipeline descriptor.
2. BufferMapAsync callback.

r?@kvark

---
<!-- 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
- [X] `./mach test-tidy` does not report any errors
- [ ] These changes fix #___ (GitHub issue number if applicable)

<!-- Either: -->
- [ ] There are tests for these changes OR
- [ ] These changes do not require tests because ___

<!-- 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. -->
This commit is contained in:
bors-servo 2020-07-06 17:45:09 -04:00 committed by GitHub
commit ec9308199e
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
2 changed files with 44 additions and 39 deletions

View file

@ -24,7 +24,7 @@ use euclid::default::Size2D;
use ipc_channel::ipc;
use script_layout_interface::HTMLCanvasDataSource;
use std::cell::Cell;
use webgpu::{wgpu::id, wgt, WebGPU, WebGPURequest};
use webgpu::{wgpu::id, wgt, WebGPU, WebGPURequest, PRESENTATION_BUFFER_COUNT};
#[derive(Clone, Copy, Debug, Eq, Hash, MallocSizeOf, Ord, PartialEq, PartialOrd)]
pub struct WebGPUContextId(pub u64);
@ -130,8 +130,8 @@ impl GPUCanvasContextMethods for GPUCanvasContext {
}
*self.swap_chain.borrow_mut() = None;
let mut buffer_ids = ArrayVec::<[id::BufferId; 5]>::new();
for _ in 0..5 {
let mut buffer_ids = ArrayVec::<[id::BufferId; PRESENTATION_BUFFER_COUNT]>::new();
for _ in 0..PRESENTATION_BUFFER_COUNT {
buffer_ids.push(
self.global()
.wgpu_id_hub()

View file

@ -20,7 +20,8 @@ use servo_config::pref;
use smallvec::SmallVec;
use std::collections::HashMap;
use std::ffi::CString;
use std::ptr::{self, NonNull};
use std::ptr;
use std::rc::Rc;
use std::slice;
use std::sync::{Arc, Mutex};
use std::time::{Duration, Instant};
@ -38,6 +39,7 @@ use wgpu::{
};
const DEVICE_POLL_INTERVAL: u64 = 100;
pub const PRESENTATION_BUFFER_COUNT: usize = 10;
#[derive(Debug, Deserialize, Serialize)]
pub enum WebGPUResponse {
@ -145,7 +147,7 @@ pub enum WebGPURequest {
},
CreateSwapChain {
device_id: id::DeviceId,
buffer_ids: ArrayVec<[id::BufferId; 5]>,
buffer_ids: ArrayVec<[id::BufferId; PRESENTATION_BUFFER_COUNT]>,
external_id: u64,
sender: IpcSender<webrender_api::ImageKey>,
image_desc: webrender_api::ImageDescriptor,
@ -312,9 +314,9 @@ struct WGPU<'a> {
// Track invalid adapters https://gpuweb.github.io/gpuweb/#invalid
_invalid_adapters: Vec<WebGPUAdapter>,
// Buffers with pending mapping
buffer_maps: HashMap<id::BufferId, BufferMapInfo<'a, WebGPUResponseResult>>,
buffer_maps: HashMap<id::BufferId, Rc<BufferMapInfo<'a, WebGPUResponseResult>>>,
// Presentation Buffers with pending mapping
present_buffer_maps: HashMap<id::BufferId, BufferMapInfo<'a, WebGPURequest>>,
present_buffer_maps: HashMap<id::BufferId, Rc<BufferMapInfo<'a, WebGPURequest>>>,
webrender_api: webrender_api::RenderApi,
webrender_document: webrender_api::DocumentId,
external_images: Arc<Mutex<WebrenderExternalImageRegistry>>,
@ -374,16 +376,15 @@ impl<'a> WGPU<'a> {
size: (map_range.end - map_range.start) as usize,
external_id: None,
};
self.buffer_maps.insert(buffer_id, map_info);
self.buffer_maps.insert(buffer_id, Rc::new(map_info));
unsafe extern "C" fn callback(
status: BufferMapAsyncStatus,
userdata: *mut u8,
) {
let nonnull_info =
NonNull::new(userdata as *mut BufferMapInfo<WebGPUResponseResult>)
.unwrap();
let info = nonnull_info.as_ref();
let info = Rc::from_raw(
userdata as *const BufferMapInfo<WebGPUResponseResult>,
);
match status {
BufferMapAsyncStatus::Success => {
let global = &info.global;
@ -406,7 +407,7 @@ impl<'a> WGPU<'a> {
host: host_map,
callback,
user_data: convert_to_pointer(
self.buffer_maps.get(&buffer_id).unwrap(),
self.buffer_maps.get(&buffer_id).unwrap().clone(),
),
};
let global = &self.global;
@ -579,6 +580,19 @@ impl<'a> WGPU<'a> {
},
None => None,
};
let vert_buffers = vertex_state
.1
.iter()
.map(|&(array_stride, step_mode, ref attributes)| {
wgpu_core::pipeline::VertexBufferLayoutDescriptor {
array_stride,
step_mode,
attributes_length: attributes.len(),
attributes: attributes.as_ptr(),
}
})
.collect::<Vec<_>>();
let descriptor = wgpu_core::pipeline::RenderPipelineDescriptor {
layout: pipeline_layout_id,
vertex_stage: wgpu_core::pipeline::ProgrammableStageDescriptor {
@ -598,19 +612,7 @@ impl<'a> WGPU<'a> {
vertex_state: wgpu_core::pipeline::VertexStateDescriptor {
index_format: vertex_state.0,
vertex_buffers_length: vertex_state.1.len(),
vertex_buffers: vertex_state
.1
.iter()
.map(|buffer| {
wgpu_core::pipeline::VertexBufferLayoutDescriptor {
array_stride: buffer.0,
step_mode: buffer.1,
attributes_length: buffer.2.len(),
attributes: buffer.2.as_ptr(),
}
})
.collect::<Vec<_>>()
.as_ptr(),
vertex_buffers: vert_buffers.as_ptr(),
},
sample_count,
sample_mask,
@ -667,8 +669,12 @@ impl<'a> WGPU<'a> {
data: vec![255; (buffer_stride * height as u32) as usize],
size: Size2D::new(width, height),
unassigned_buffer_ids: buffer_ids,
available_buffer_ids: ArrayVec::<[id::BufferId; 5]>::new(),
queued_buffer_ids: ArrayVec::<[id::BufferId; 5]>::new(),
available_buffer_ids: ArrayVec::<
[id::BufferId; PRESENTATION_BUFFER_COUNT],
>::new(),
queued_buffer_ids: ArrayVec::<
[id::BufferId; PRESENTATION_BUFFER_COUNT],
>::new(),
buffer_stride,
image_key,
image_desc,
@ -952,15 +958,14 @@ impl<'a> WGPU<'a> {
size: buffer_size as usize,
external_id: Some(external_id),
};
self.present_buffer_maps.insert(buffer_id, map_info);
self.present_buffer_maps
.insert(buffer_id, Rc::new(map_info));
unsafe extern "C" fn callback(
status: BufferMapAsyncStatus,
userdata: *mut u8,
) {
let nonnull_info =
NonNull::new(userdata as *mut BufferMapInfo<WebGPURequest>)
.unwrap();
let info = nonnull_info.as_ref();
let info =
Rc::from_raw(userdata as *const BufferMapInfo<WebGPURequest>);
match status {
BufferMapAsyncStatus::Success => {
if let Err(e) =
@ -980,7 +985,7 @@ impl<'a> WGPU<'a> {
host: HostMap::Read,
callback,
user_data: convert_to_pointer(
self.present_buffer_maps.get(&buffer_id).unwrap(),
self.present_buffer_maps.get(&buffer_id).unwrap().clone(),
),
};
gfx_select!(buffer_id => global.buffer_map_async(buffer_id, 0..buffer_size, map_op));
@ -1072,8 +1077,8 @@ impl<'a> WGPU<'a> {
}
}
fn convert_to_pointer<T: Sized>(obj: &T) -> *mut u8 {
(obj as *const T) as *mut u8
fn convert_to_pointer<T: Sized>(obj: Rc<T>) -> *mut u8 {
Rc::into_raw(obj) as *mut u8
}
macro_rules! webgpu_resource {
@ -1150,9 +1155,9 @@ pub struct PresentationData {
queue_id: id::QueueId,
pub data: Vec<u8>,
pub size: Size2D<i32>,
unassigned_buffer_ids: ArrayVec<[id::BufferId; 5]>,
available_buffer_ids: ArrayVec<[id::BufferId; 5]>,
queued_buffer_ids: ArrayVec<[id::BufferId; 5]>,
unassigned_buffer_ids: ArrayVec<[id::BufferId; PRESENTATION_BUFFER_COUNT]>,
available_buffer_ids: ArrayVec<[id::BufferId; PRESENTATION_BUFFER_COUNT]>,
queued_buffer_ids: ArrayVec<[id::BufferId; PRESENTATION_BUFFER_COUNT]>,
buffer_stride: u32,
image_key: webrender_api::ImageKey,
image_desc: webrender_api::ImageDescriptor,