Skip to content

/ servo

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

Implement GPUBuffer.getMappedRange() #27126

Merged
merged 2 commits into from Jul 1, 2020
+144 −112
Merged

Implement GPUBuffer.getMappedRange() #27126

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
575036b
Implement GPUBuffer.getMappedRange()
kunalmohan Jun 29, 2020
891a3bd
Encapsulate buffer map fields in a separate struct
kunalmohan Jul 1, 2020
File filter...
Filter file types
Jump to…
Jump to file
Failed to load files.

Always

Just for now

14 components/constellation/constellation.rs
View file
Open in desktop
@@ -2769,13 +2769,6 @@ where
warn!("Exit Canvas Paint thread failed ({})", e);
}

if let Some(webgl_threads) = self.webgl_threads.as_ref() {
debug!("Exiting WebGL thread.");
if let Err(e) = webgl_threads.exit() {
warn!("Exit WebGL Thread failed ({})", e);
}
}

debug!("Exiting WebGPU threads.");
let receivers = self
.browsing_context_group_set
@@ -2800,6 +2793,13 @@ where
}
}

if let Some(webgl_threads) = self.webgl_threads.as_ref() {
debug!("Exiting WebGL thread.");
if let Err(e) = webgl_threads.exit() {
warn!("Exit WebGL Thread failed ({})", e);
}
}

debug!("Exiting GLPlayer thread.");
if let Some(glplayer_threads) = self.glplayer_threads.as_ref() {
if let Err(e) = glplayer_threads.exit() {
205 components/script/dom/gpubuffer.rs
View file
Open in desktop
@@ -5,29 +5,32 @@
use crate::dom::bindings::cell::DomRefCell;
use crate::dom::bindings::codegen::Bindings::GPUBufferBinding::{GPUBufferMethods, GPUSize64};
use crate::dom::bindings::codegen::Bindings::GPUMapModeBinding::GPUMapModeConstants;
use crate::dom::bindings::error::Error;
use crate::dom::bindings::error::{Error, Fallible};
use crate::dom::bindings::reflector::DomObject;
use crate::dom::bindings::reflector::{reflect_dom_object, Reflector};
use crate::dom::bindings::root::DomRoot;
use crate::dom::bindings::str::DOMString;
use crate::dom::bindings::trace::RootedTraceableBox;
use crate::dom::globalscope::GlobalScope;
use crate::dom::gpu::{response_async, AsyncWGPUListener};
use crate::dom::promise::Promise;
use crate::realms::InRealm;
use crate::script_runtime::JSContext;
use dom_struct::dom_struct;
use js::jsapi::DetachArrayBuffer;
use js::jsapi::NewExternalArrayBuffer;
use js::jsapi::{Heap, JSObject};
use js::jsval::UndefinedValue;
use js::rust::jsapi_wrapped::{DetachArrayBuffer, IsPromiseObject, RejectPromise};
use js::typedarray::{ArrayBuffer, CreateWith};
use std::cell::Cell;
use std::cell::{Cell, RefCell};
use std::ffi::c_void;
use std::ops::Range;
use std::ptr;
use std::ptr::NonNull;
use std::rc::Rc;
use webgpu::{
wgpu::device::HostMap, WebGPU, WebGPUBuffer, WebGPUDevice, WebGPURequest, WebGPUResponse,
};

const RANGE_OFFSET_ALIGN_MASK: u64 = 8;
const RANGE_SIZE_ALIGN_MASK: u64 = 4;

// https://gpuweb.github.io/gpuweb/#buffer-state
#[derive(Clone, Copy, MallocSizeOf, PartialEq)]
pub enum GPUBufferState {
@@ -38,6 +41,17 @@ pub enum GPUBufferState {
Destroyed,
}

#[derive(JSTraceable, MallocSizeOf)]
pub struct GPUBufferMapInfo {
#[ignore_malloc_size_of = "Rc"]
pub mapping: Rc<RefCell<Vec<u8>>>,
pub mapping_range: Range<u64>,
pub mapped_ranges: Vec<Range<u64>>,
#[ignore_malloc_size_of = "defined in mozjs"]
pub js_buffers: Vec<Box<Heap<*mut JSObject>>>,
pub map_mode: Option<u32>,
}

#[dom_struct]
pub struct GPUBuffer {
reflector_: Reflector,
@@ -48,11 +62,10 @@ pub struct GPUBuffer {
buffer: WebGPUBuffer,
device: WebGPUDevice,
valid: Cell<bool>,
#[ignore_malloc_size_of = "defined in mozjs"]
mapping: RootedTraceableBox<Heap<*mut JSObject>>,
mapping_range: DomRefCell<Range<u64>>,
size: GPUSize64,
map_mode: Cell<Option<u32>>,
#[ignore_malloc_size_of = "promises are hard"]
map_promise: DomRefCell<Option<Rc<Promise>>>,
map_info: DomRefCell<Option<GPUBufferMapInfo>>,
}

impl GPUBuffer {
@@ -63,8 +76,7 @@ impl GPUBuffer {
state: GPUBufferState,
size: GPUSize64,
valid: bool,
mapping: RootedTraceableBox<Heap<*mut JSObject>>,
mapping_range: Range<u64>,
map_info: DomRefCell<Option<GPUBufferMapInfo>>,
) -> Self {
Self {
reflector_: Reflector::new(),
@@ -74,10 +86,9 @@ impl GPUBuffer {
valid: Cell::new(valid),
device,
buffer,
mapping,
map_promise: DomRefCell::new(None),
size,
mapping_range: DomRefCell::new(mapping_range),
map_mode: Cell::new(None),
map_info,
}
}

@@ -90,19 +101,11 @@ impl GPUBuffer {
state: GPUBufferState,
size: GPUSize64,
valid: bool,
mapping: RootedTraceableBox<Heap<*mut JSObject>>,
mapping_range: Range<u64>,
map_info: DomRefCell<Option<GPUBufferMapInfo>>,
) -> DomRoot<Self> {
reflect_dom_object(
Box::new(GPUBuffer::new_inherited(
channel,
buffer,
device,
state,
size,
valid,
mapping,
mapping_range,
channel, buffer, device, state, size, valid, map_info,
)),
global,
)
@@ -142,51 +145,32 @@ impl GPUBufferMethods for GPUBuffer {
},
// Step 3
GPUBufferState::Mapped | GPUBufferState::MappedAtCreation => {
match ArrayBuffer::from(self.mapping.get()) {
Ok(array_buffer) => {
// Step 3.2
self.channel
.0
.send(WebGPURequest::UnmapBuffer {
buffer_id: self.id().0,
array_buffer: array_buffer.to_vec(),
is_map_read: self.map_mode.get() == Some(GPUMapModeConstants::READ),
offset: self.mapping_range.borrow().start,
size: self.mapping_range.borrow().end -
self.mapping_range.borrow().start,
})
.unwrap();
// Step 3.3
unsafe {
DetachArrayBuffer(*cx, self.mapping.handle());
}
},
Err(_) => {
warn!(
"Could not find ArrayBuffer of Mapped buffer ({:?})",
self.buffer.0
);
},
};
let mut info = self.map_info.borrow_mut();
let m_info = info.as_mut().unwrap();
let m_range = m_info.mapping_range.clone();
if let Err(e) = self.channel.0.send(WebGPURequest::UnmapBuffer {
buffer_id: self.id().0,
array_buffer: m_info.mapping.borrow().clone(),
is_map_read: m_info.map_mode == Some(GPUMapModeConstants::READ),
offset: m_range.start,
size: m_range.end - m_range.start,
}) {
warn!("Failed to send Buffer unmap ({:?}) ({})", self.buffer.0, e);
}
// Step 3.3
m_info.js_buffers.drain(..).for_each(|obj| unsafe {
DetachArrayBuffer(*cx, obj.handle());
});
},
// Step 2
GPUBufferState::MappingPending => unsafe {
if IsPromiseObject(self.mapping.handle()) {
let err = Error::Operation;
rooted!(in(*cx) let mut undef = UndefinedValue());
err.to_jsval(*cx, &self.global(), undef.handle_mut());
RejectPromise(*cx, self.mapping.handle(), undef.handle());
} else {
warn!("No promise object for pending mapping found");
}
GPUBufferState::MappingPending => {
let promise = self.map_promise.borrow_mut().take().unwrap();
promise.reject_error(Error::Operation);
},
};
// Step 3.3
self.mapping.set(ptr::null_mut());
// Step 4
self.state.set(GPUBufferState::Unmapped);
self.map_mode.set(None);
*self.mapping_range.borrow_mut() = 0..0;
*self.map_info.borrow_mut() = None;
}

/// https://gpuweb.github.io/gpuweb/#dom-gpubuffer-destroy
@@ -213,7 +197,13 @@ impl GPUBufferMethods for GPUBuffer {

#[allow(unsafe_code)]
/// https://gpuweb.github.io/gpuweb/#dom-gpubuffer-mapasync-offset-size
fn MapAsync(&self, mode: u32, offset: u64, size: u64, comp: InRealm) -> Rc<Promise> {
fn MapAsync(
&self,
mode: u32,
offset: GPUSize64,
size: GPUSize64,
comp: InRealm,
) -> Rc<Promise> {
let promise = Promise::new_in_current_realm(&self.global(), comp);
let map_range = if size == 0 {
offset..self.size
@@ -237,7 +227,6 @@ impl GPUBufferMethods for GPUBuffer {
promise.reject_error(Error::Abort);
return promise;
}
self.mapping.set(*promise.promise_obj());

let sender = response_async(&promise, self);
if let Err(e) = self.channel.0.send(WebGPURequest::BufferMapAsync {
@@ -255,11 +244,65 @@ impl GPUBufferMethods for GPUBuffer {
}

self.state.set(GPUBufferState::MappingPending);
self.map_mode.set(Some(mode));
*self.mapping_range.borrow_mut() = map_range;
*self.map_info.borrow_mut() = Some(GPUBufferMapInfo {
mapping: Rc::new(RefCell::new(Vec::with_capacity(0))),
mapping_range: map_range,
mapped_ranges: Vec::new(),
js_buffers: Vec::new(),
map_mode: Some(mode),
});
*self.map_promise.borrow_mut() = Some(promise.clone());
promise
}

/// https://gpuweb.github.io/gpuweb/#dom-gpubuffer-getmappedrange
#[allow(unsafe_code)]
fn GetMappedRange(
&self,
cx: JSContext,
offset: GPUSize64,
size: GPUSize64,
) -> Fallible<NonNull<JSObject>> {
let m_end = if size == 0 { self.size } else { offset + size };
let mut info = self.map_info.borrow_mut();
let m_info = info.as_mut().unwrap();

let mut valid = match self.state.get() {
GPUBufferState::Mapped | GPUBufferState::MappedAtCreation => true,
_ => false,
};
valid &= offset % RANGE_OFFSET_ALIGN_MASK == 0 &&
(m_end - offset) % RANGE_SIZE_ALIGN_MASK == 0 &&
offset >= m_info.mapping_range.start &&
m_end <= m_info.mapping_range.end;
valid &= m_info
.mapped_ranges
.iter()
.all(|range| range.start >= m_end || range.end <= offset);
if !valid {
return Err(Error::Operation);
}

unsafe extern "C" fn free_func(_contents: *mut c_void, free_user_data: *mut c_void) {
let _ = Rc::from_raw(free_user_data as _);
}

let array_buffer = unsafe {
NewExternalArrayBuffer(
*cx,
(m_end - offset) as usize,
m_info.mapping.borrow_mut()[offset as usize..m_end as usize].as_mut_ptr() as _,
Some(free_func),
Rc::into_raw(m_info.mapping.clone()) as _,

This comment has been minimized.

Sign in to view
Copy link
@kvark

kvark Jul 1, 2020

Member

I was looking at this piece a bit. So this would keep the memory for the ArrayBuffer around until it's GC-ed.
I think it's totally fine to land in this stage. Just wanted to note that for the end game, we probably want to "detach" the array buffer on unmap(), so that any access to its contents would be invalid and trigger an exception. Perhaps, we could leave a comment somewhere about this TODO

This comment has been minimized.

Sign in to view
Copy link
@kunalmohan

kunalmohan Jul 1, 2020 •
edited

Author Collaborator

We already detach the actual array buffers(which are the only ones exposed to the user) on unmap. We also drop the Rc pointer for this array buffer in free_func and set the map_info to None (so we drop all Rc pointers on unmap). What else are we missing here? Should the contents be set to ptr::null() in free_func?
)
};

m_info.mapped_ranges.push(offset..m_end);
m_info.js_buffers.push(Heap::boxed(array_buffer));

Ok(NonNull::new(array_buffer).unwrap())
}

/// https://gpuweb.github.io/gpuweb/#dom-gpuobjectbase-label
fn GetLabel(&self) -> Option<DOMString> {
self.label.borrow().clone()
@@ -276,28 +319,22 @@ impl AsyncWGPUListener for GPUBuffer {
fn handle_response(&self, response: WebGPUResponse, promise: &Rc<Promise>) {
match response {
WebGPUResponse::BufferMapAsync(bytes) => {
let cx = self.global().get_cx();
rooted!(in(*cx) let mut array_buffer = ptr::null_mut::<JSObject>());
match unsafe {
ArrayBuffer::create(*cx, CreateWith::Slice(&bytes), array_buffer.handle_mut())
} {
Ok(_) => promise.resolve_native(&()),
Err(()) => {
warn!(
"Failed to create ArrayBuffer for buffer({:?})",
self.buffer.0
);
promise.reject_error(Error::Operation);
},
}
self.mapping.set(array_buffer.get());
*self
.map_info
.borrow_mut()
.as_mut()
.unwrap()
.mapping
.borrow_mut() = bytes;
promise.resolve_native(&());
self.state.set(GPUBufferState::Mapped);
},
_ => {
warn!("Wrong WebGPUResponse received");
promise.reject_error(Error::Operation);
},
}
*self.map_promise.borrow_mut() = None;
if let Err(e) = self
.channel
.0
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.