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

Implement GPUBuffer.getMappedRange() #27126

Merged
merged 2 commits into from Jul 1, 2020
Merged
Changes from 1 commit
Commits
File filter...
Filter file types
Jump to…
Jump to file
Failed to load files.

Always

Just for now

Next

Implement GPUBuffer.getMappedRange()

  • Loading branch information
kunalmohan committed Jul 1, 2020
commit 575036bb88163807abbfb46773b7e124fd196fa4
@@ -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() {
@@ -2,32 +2,35 @@
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at https://mozilla.org/MPL/2.0/. */

use crate::dom::bindings::cell::DomRefCell;
use crate::dom::bindings::cell::{DomRefCell, RefCell};
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::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 {
@@ -49,8 +52,13 @@ pub struct GPUBuffer {
device: WebGPUDevice,
valid: Cell<bool>,
#[ignore_malloc_size_of = "defined in mozjs"]
mapping: RootedTraceableBox<Heap<*mut JSObject>>,
mapping_range: DomRefCell<Range<u64>>,
mapping: Rc<RefCell<Option<Vec<u8>>>>,
mapping_range: DomRefCell<Option<Range<u64>>>,
mapped_ranges: DomRefCell<Option<Vec<Range<u64>>>>,
#[ignore_malloc_size_of = "defined in mozjs"]
js_buffers: DomRefCell<Option<Vec<Box<Heap<*mut JSObject>>>>>,
#[ignore_malloc_size_of = "defined in mozjs"]
map_promise: DomRefCell<Option<Rc<Promise>>>,
size: GPUSize64,
map_mode: Cell<Option<u32>>,
}
@@ -63,7 +71,7 @@ impl GPUBuffer {
state: GPUBufferState,
size: GPUSize64,
valid: bool,
mapping: RootedTraceableBox<Heap<*mut JSObject>>,
mapping: Rc<RefCell<Option<Vec<u8>>>>,
mapping_range: Range<u64>,
) -> Self {
Self {
@@ -75,8 +83,11 @@ impl GPUBuffer {
device,
buffer,
mapping,
mapped_ranges: DomRefCell::new(None),
This conversation was marked as resolved by kunalmohan

This comment has been minimized.

Copy link
@kvark

kvark Jun 30, 2020

Member

do all of these need to be separate? Can we do DomRefCell<Option<SomeStructure>> with all the fields in the SomeStructure?

js_buffers: DomRefCell::new(None),
map_promise: DomRefCell::new(None),
size,
mapping_range: DomRefCell::new(mapping_range),
mapping_range: DomRefCell::new(Some(mapping_range)),
This conversation was marked as resolved by kunalmohan

This comment has been minimized.

Copy link
@kvark

kvark Jun 30, 2020

Member

Why do we initialize this with Some?

This comment has been minimized.

Copy link
@kunalmohan

kunalmohan Jun 30, 2020

Author Collaborator

Sorry, it's a mistake. I'll correct it.

map_mode: Cell::new(None),
}
}
@@ -90,7 +101,7 @@ impl GPUBuffer {
state: GPUBufferState,
size: GPUSize64,
valid: bool,
mapping: RootedTraceableBox<Heap<*mut JSObject>>,
mapping: Rc<RefCell<Option<Vec<u8>>>>,
mapping_range: Range<u64>,
) -> DomRoot<Self> {
reflect_dom_object(
@@ -142,51 +153,34 @@ 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
);
},
};
if let Err(e) = self.channel.0.send(WebGPURequest::UnmapBuffer {
buffer_id: self.id().0,
array_buffer: self.mapping.borrow().as_ref().unwrap().clone(),
is_map_read: self.map_mode.get() == Some(GPUMapModeConstants::READ),
offset: self.mapping_range.borrow().as_ref().unwrap().start,
This conversation was marked as resolved by kunalmohan

This comment has been minimized.

Copy link
@kvark

kvark Jun 30, 2020

Member

let's not repeat ourselves 3 times here :)

size: self.mapping_range.borrow().as_ref().unwrap().end -
self.mapping_range.borrow().as_ref().unwrap().start,
}) {
warn!("Failed to send Buffer unmap ({:?}) ({})", self.buffer.0, e);
}
// Step 3.3
let mut bufs = self.js_buffers.borrow_mut().take().unwrap();
bufs.drain(..).for_each(|obj| unsafe {
DetachArrayBuffer(*cx, obj.handle());
});
*self.mapped_ranges.borrow_mut() = None;
*self.mapping.borrow_mut() = None;
},
// 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.mapping_range.borrow_mut() = None;
}

/// https://gpuweb.github.io/gpuweb/#dom-gpubuffer-destroy
@@ -213,7 +207,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 +237,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 {
@@ -256,10 +255,72 @@ impl GPUBufferMethods for GPUBuffer {

self.state.set(GPUBufferState::MappingPending);
self.map_mode.set(Some(mode));
*self.mapping_range.borrow_mut() = map_range;
*self.mapping_range.borrow_mut() = Some(map_range);
*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>> {
if self.mapped_ranges.borrow().is_none() {
This conversation was marked as resolved by kunalmohan

This comment has been minimized.

Copy link
@kvark

kvark Jun 30, 2020

Member

It seems strange to have this lazy check here. We should know exactly when this function is valid to call (either after mapping, or in a buffer that's created with mapping), so those earlier stages need to prepare the state, and code here doesn't need to be lazy

*self.mapped_ranges.borrow_mut() = Some(Vec::new());
}
if self.js_buffers.borrow().is_none() {
*self.js_buffers.borrow_mut() = Some(Vec::new());
}
let act_size = if size == 0 { self.size - offset } else { size };
let mut valid = match self.state.get() {
GPUBufferState::Mapped | GPUBufferState::MappedAtCreation => true,
_ => false,
};
valid &= offset % RANGE_OFFSET_ALIGN_MASK == 0 &&
act_size % RANGE_SIZE_ALIGN_MASK == 0 &&
offset >= self.mapping_range.borrow().as_ref().unwrap().start &&
offset + act_size <= self.mapping_range.borrow().as_ref().unwrap().end;
valid &= self
.mapped_ranges
.borrow()
.as_ref()
.unwrap()
.iter()
.all(|range| range.start > offset + act_size || range.end < offset);
This conversation was marked as resolved by kunalmohan

This comment has been minimized.

Copy link
@kvark

kvark Jun 30, 2020

Member

these should be >= and <= correspondingly

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,
act_size as usize,
self.mapping.borrow_mut().as_mut().unwrap()
[offset as usize..(offset + act_size) as usize]
This conversation was marked as resolved by kunalmohan

This comment has been minimized.

Copy link
@kvark

kvark Jun 30, 2020

Member

we repeat offset + act_size quite a lot in this function

.as_mut_ptr() as _,
Some(free_func),
Rc::into_raw(self.mapping.clone()) as _,
)
};
self.mapped_ranges
.borrow_mut()
.as_mut()
.map(|v| v.push(offset..offset + act_size));
self.js_buffers
.borrow_mut()
.as_mut()
.map(|a| a.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 +337,16 @@ 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.mapping.borrow_mut() = Some(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
@@ -4,7 +4,7 @@

#![allow(unsafe_code)]

use crate::dom::bindings::cell::DomRefCell;
use crate::dom::bindings::cell::{DomRefCell, RefCell};
use crate::dom::bindings::codegen::Bindings::GPUBindGroupBinding::{
GPUBindGroupDescriptor, GPUBindingResource,
};
@@ -54,8 +54,8 @@ use crate::script_runtime::JSContext as SafeJSContext;
use arrayvec::ArrayVec;
use dom_struct::dom_struct;
use js::jsapi::{Heap, JSObject};
use js::typedarray::{ArrayBuffer, CreateWith};
use std::ptr::{self, NonNull};
use std::ptr::NonNull;
use std::rc::Rc;
use webgpu::wgpu::binding_model::BufferBinding;
use webgpu::{self, wgt, WebGPU, WebGPUBindings, WebGPURequest};

@@ -177,24 +177,16 @@ impl GPUDeviceMethods for GPUDevice {
.expect("Failed to create WebGPU buffer");

let buffer = webgpu::WebGPUBuffer(id);
let mapping = RootedTraceableBox::new(Heap::default());
let mapping;
let state;
let mapping_range;
if descriptor.mappedAtCreation {
let cx = self.global().get_cx();
rooted!(in(*cx) let mut array_buffer = ptr::null_mut::<JSObject>());
unsafe {
assert!(ArrayBuffer::create(
*cx,
CreateWith::Length(descriptor.size as u32),
array_buffer.handle_mut(),
)
.is_ok());
}
mapping.set(array_buffer.get());
let buf_data = vec![0u8; descriptor.size as usize];
mapping = Rc::new(RefCell::new(Some(buf_data)));
state = GPUBufferState::MappedAtCreation;
mapping_range = 0..descriptor.size;
} else {
mapping = Rc::new(RefCell::new(None));
state = GPUBufferState::Unmapped;
mapping_range = 0..0;
}
@@ -6,7 +6,7 @@
[Exposed=(Window, DedicatedWorker), Serializable, Pref="dom.webgpu.enabled"]
interface GPUBuffer {
Promise<void> mapAsync(GPUMapModeFlags mode, optional GPUSize64 offset = 0, optional GPUSize64 size = 0);
//ArrayBuffer getMappedRange(optional GPUSize64 offset = 0, optional GPUSize64 size = 0);
[Throws] ArrayBuffer getMappedRange(optional GPUSize64 offset = 0, optional GPUSize64 size = 0);
void unmap();

void destroy();
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.