From b1765c68821d12a21cd304f7dffaa3bdc8f101e4 Mon Sep 17 00:00:00 2001 From: ecoal95 Date: Sun, 14 Jun 2015 22:55:50 +0200 Subject: [PATCH 1/4] webgl: Refactor implementation to move logic inside the DOM interfaces This improves the encapsulation and consistency in our WebGL implementation. Also allows to implement new methods such as `getShaderSource()`. It will also allow us to use `delete()` in the destructors of them (note that we will want to keep track of them from the context). --- components/canvas/webgl_paint_task.rs | 28 ++- components/canvas_traits/lib.rs | 24 ++- components/script/dom/webglbuffer.rs | 40 ++++- components/script/dom/webglframebuffer.rs | 40 ++++- components/script/dom/webglprogram.rs | 99 +++++++++- components/script/dom/webglrenderbuffer.rs | 38 +++- .../script/dom/webglrenderingcontext.rs | 169 +++++++++--------- components/script/dom/webglshader.rs | 100 ++++++++++- components/script/dom/webgltexture.rs | 38 +++- components/script/dom/webgluniformlocation.rs | 4 +- .../dom/webidls/WebGLRenderingContext.webidl | 2 +- 11 files changed, 452 insertions(+), 130 deletions(-) diff --git a/components/canvas/webgl_paint_task.rs b/components/canvas/webgl_paint_task.rs index 24258954c1e..4f6e0605dd8 100644 --- a/components/canvas/webgl_paint_task.rs +++ b/components/canvas/webgl_paint_task.rs @@ -361,19 +361,35 @@ impl WebGLPaintTask { gl::enable_vertex_attrib_array(attrib_id); } - fn get_attrib_location(&self, program_id: u32, name: String, chan: Sender ) { + fn get_attrib_location(&self, program_id: u32, name: String, chan: Sender> ) { let attrib_location = gl::get_attrib_location(program_id, &name); + + let attrib_location = if attrib_location == -1 { + None + } else { + Some(attrib_location) + }; + chan.send(attrib_location).unwrap(); } - fn get_shader_info_log(&self, shader_id: u32, chan: Sender) { + fn get_shader_info_log(&self, shader_id: u32, chan: Sender>) { + // TODO(ecoal95): Right now we always return a value, we should + // check for gl errors and return None there let info = gl::get_shader_info_log(shader_id); - chan.send(info).unwrap(); + chan.send(Some(info)).unwrap(); } - fn get_shader_parameter(&self, shader_id: u32, param_id: u32, chan: Sender) { - let parameter = gl::get_shader_iv(shader_id, param_id); - chan.send(parameter as i32).unwrap(); + fn get_shader_parameter(&self, shader_id: u32, param_id: u32, chan: Sender) { + let result = match param_id { + gl::SHADER_TYPE => + WebGLShaderParameter::Int(gl::get_shader_iv(shader_id, param_id)), + gl::DELETE_STATUS | gl::COMPILE_STATUS => + WebGLShaderParameter::Bool(gl::get_shader_iv(shader_id, param_id) != 0), + _ => panic!("Unexpected shader parameter type"), + }; + + chan.send(result).unwrap(); } fn get_uniform_location(&self, program_id: u32, name: String, chan: Sender>) { diff --git a/components/canvas_traits/lib.rs b/components/canvas_traits/lib.rs index 0fee050d78d..51c9a1dafb2 100644 --- a/components/canvas_traits/lib.rs +++ b/components/canvas_traits/lib.rs @@ -107,9 +107,9 @@ pub enum CanvasWebGLMsg { BindTexture(u32, u32), DrawArrays(u32, i32, i32), EnableVertexAttribArray(u32), - GetAttribLocation(u32, String, Sender), - GetShaderInfoLog(u32, Sender), - GetShaderParameter(u32, u32, Sender), + GetShaderInfoLog(u32, Sender>), + GetShaderParameter(u32, u32, Sender), + GetAttribLocation(u32, String, Sender>), GetUniformLocation(u32, String, Sender>), LinkProgram(u32), ShaderSource(u32, String), @@ -121,6 +121,24 @@ pub enum CanvasWebGLMsg { DrawingBufferHeight(Sender), } +#[derive(Clone)] +pub enum WebGLError { + InvalidEnum, + InvalidOperation, + InvalidValue, + OutOfMemory, + ContextLost, +} + +pub type WebGLResult = Result; + +#[derive(Clone)] +pub enum WebGLShaderParameter { + Int(i32), + Bool(bool), + Invalid, +} + #[derive(Clone)] pub enum CanvasCommonMsg { Close, diff --git a/components/script/dom/webglbuffer.rs b/components/script/dom/webglbuffer.rs index f7c17cb8f04..b9941026353 100644 --- a/components/script/dom/webglbuffer.rs +++ b/components/script/dom/webglbuffer.rs @@ -5,35 +5,63 @@ // https://www.khronos.org/registry/webgl/specs/latest/1.0/webgl.idl use dom::bindings::codegen::Bindings::WebGLBufferBinding; use dom::bindings::global::GlobalRef; -use dom::bindings::js::Root; +use dom::bindings::js::{Temporary, JSRef}; use dom::bindings::utils::reflect_dom_object; use dom::webglobject::WebGLObject; +use canvas_traits::{CanvasMsg, CanvasWebGLMsg}; +use std::sync::mpsc::{channel, Sender}; +use std::cell::Cell; + #[dom_struct] pub struct WebGLBuffer { webgl_object: WebGLObject, id: u32, + is_deleted: Cell, + renderer: Sender, } impl WebGLBuffer { - fn new_inherited(id: u32) -> WebGLBuffer { + fn new_inherited(renderer: Sender, id: u32) -> WebGLBuffer { WebGLBuffer { webgl_object: WebGLObject::new_inherited(), id: id, + is_deleted: Cell::new(false), + renderer: renderer, } } - pub fn new(global: GlobalRef, id: u32) -> Root { - reflect_dom_object(box WebGLBuffer::new_inherited(id), global, WebGLBufferBinding::Wrap) + pub fn maybe_new(global: GlobalRef, renderer: Sender) -> Option> { + let (sender, receiver) = channel(); + renderer.send(CanvasMsg::WebGL(CanvasWebGLMsg::CreateBuffer(sender))).unwrap(); + receiver.recv().unwrap() + .map(|buffer_id| WebGLBuffer::new(global, renderer, *buffer_id)) + } + + pub fn new(global: GlobalRef, renderer: Sender, id: u32) -> Root { + reflect_dom_object(box WebGLBuffer::new_inherited(renderer, id), global, WebGLBufferBinding::Wrap) } } pub trait WebGLBufferHelpers { - fn get_id(self) -> u32; + fn id(&self) -> u32; + fn bind(&self, target: u32); + fn delete(&self); } impl<'a> WebGLBufferHelpers for &'a WebGLBuffer { - fn get_id(self) -> u32 { + fn id(self) -> u32 { self.id } + + fn bind(self, target: u32) { + self.renderer.send(CanvasMsg::WebGL(CanvasWebGLMsg::BindBuffer(target, self.id))).unwrap(); + } + + fn delete(self) { + if !self.is_deleted.get() { + self.is_deleted.set(true); + self.renderer.send(CanvasMsg::WebGL(CanvasWebGLMsg::DeleteBuffer(self.id))).unwrap(); + } + } } diff --git a/components/script/dom/webglframebuffer.rs b/components/script/dom/webglframebuffer.rs index f4bb4b2b043..cd23765dfff 100644 --- a/components/script/dom/webglframebuffer.rs +++ b/components/script/dom/webglframebuffer.rs @@ -9,31 +9,59 @@ use dom::bindings::js::Root; use dom::bindings::utils::reflect_dom_object; use dom::webglobject::WebGLObject; +use canvas_traits::{CanvasMsg, CanvasWebGLMsg}; +use std::sync::mpsc::{channel, Sender}; +use std::cell::Cell; + #[dom_struct] pub struct WebGLFramebuffer { webgl_object: WebGLObject, id: u32, + is_deleted: Cell, + renderer: Sender, } impl WebGLFramebuffer { - fn new_inherited(id: u32) -> WebGLFramebuffer { + fn new_inherited(renderer: Sender, id: u32) -> WebGLFramebuffer { WebGLFramebuffer { webgl_object: WebGLObject::new_inherited(), id: id, + is_deleted: Cell::new(false), + renderer: renderer, } } - pub fn new(global: GlobalRef, id: u32) -> Root { - reflect_dom_object(box WebGLFramebuffer::new_inherited(id), global, WebGLFramebufferBinding::Wrap) + pub fn maybe_new(global: GlobalRef, renderer: Sender) -> Option> { + let (sender, receiver) = channel(); + renderer.send(CanvasMsg::WebGL(CanvasWebGLMsg::CreateFramebuffer(sender))).unwrap(); + receiver.recv().unwrap() + .map(|fb_id| WebGLFramebuffer::new(global, renderer, *fb_id)) + } + + pub fn new(global: GlobalRef, renderer: Sender, id: u32) -> Root { + reflect_dom_object(box WebGLFramebuffer::new_inherited(renderer, id), global, WebGLFramebufferBinding::Wrap) } } pub trait WebGLFramebufferHelpers { - fn get_id(self) -> u32; + fn id(self) -> u32; + fn bind(self, target: u32); + fn delete(self); } -impl<'a> WebGLFramebufferHelpers for &'a WebGLFramebuffer { - fn get_id(self) -> u32 { +impl<'a> WebGLFramebufferHelpers for JSRef<'a, WebGLFramebuffer> { + fn id(self) -> u32 { self.id } + + fn bind(self, target: u32) { + self.renderer.send(CanvasMsg::WebGL(CanvasWebGLMsg::BindFramebuffer(target, self.id))).unwrap(); + } + + fn delete(self) { + if !self.is_deleted.get() { + self.is_deleted.set(true); + self.renderer.send(CanvasMsg::WebGL(CanvasWebGLMsg::DeleteFramebuffer(self.id))).unwrap(); + } + } } diff --git a/components/script/dom/webglprogram.rs b/components/script/dom/webglprogram.rs index 3e6191d764c..63496f9bff6 100644 --- a/components/script/dom/webglprogram.rs +++ b/components/script/dom/webglprogram.rs @@ -13,27 +13,114 @@ use dom::webglobject::WebGLObject; pub struct WebGLProgram { webgl_object: WebGLObject, id: u32, + is_deleted: Cell, + fragment_shader: MutNullableHeap>, + vertex_shader: MutNullableHeap>, + renderer: Sender, } impl WebGLProgram { - fn new_inherited(id: u32) -> WebGLProgram { + fn new_inherited(renderer: Sender, id: u32) -> WebGLProgram { WebGLProgram { webgl_object: WebGLObject::new_inherited(), id: id, + is_deleted: Cell::new(false), + fragment_shader: Default::default(), + vertex_shader: Default::default(), + renderer: renderer, } } - pub fn new(global: GlobalRef, id: u32) -> Root { - reflect_dom_object(box WebGLProgram::new_inherited(id), global, WebGLProgramBinding::Wrap) + pub fn maybe_new(global: GlobalRef, renderer: Sender) -> Option> { + let (sender, receiver) = channel(); + renderer.send(CanvasMsg::WebGL(CanvasWebGLMsg::CreateProgram(sender))).unwrap(); + receiver.recv().unwrap() + .map(|program_id| WebGLProgram::new(global, renderer, *program_id)) + } + + pub fn new(global: GlobalRef, renderer: Sender, id: u32) -> Root { + reflect_dom_object(box WebGLProgram::new_inherited(renderer, id), global, WebGLProgramBinding::Wrap) } } pub trait WebGLProgramHelpers { - fn get_id(self) -> u32; + fn delete(self); + fn link(self); + fn use_program(self); + fn attach_shader(self, shader: JSRef) -> WebGLResult<()>; + fn get_attrib_location(self, name: String) -> WebGLResult>; + fn get_uniform_location(self, name: String) -> WebGLResult>; } impl<'a> WebGLProgramHelpers for &'a WebGLProgram { - fn get_id(self) -> u32 { - self.id + /// glDeleteProgram + fn delete(self) { + if !self.is_deleted.get() { + self.is_deleted.set(true); + self.renderer.send(CanvasMsg::WebGL(CanvasWebGLMsg::DeleteProgram(self.id))).unwrap(); + } + } + + /// glLinkProgram + fn link(self) { + self.renderer.send(CanvasMsg::WebGL(CanvasWebGLMsg::LinkProgram(self.id))).unwrap(); + } + + /// glUseProgram + fn use_program(self) { + self.renderer.send(CanvasMsg::WebGL(CanvasWebGLMsg::UseProgram(self.id))).unwrap(); + } + + /// glAttachShader + fn attach_shader(self, shader: &'a WebGLShader) -> WebGLResult<()> { + let shader_slot = match shader.gl_type() { + constants::FRAGMENT_SHADER => &self.fragment_shader, + constants::VERTEX_SHADER => &self.vertex_shader, + _ => return Err(WebGLError::InvalidOperation), + }; + + // TODO(ecoal95): Differenciate between same shader already assigned and other previous + // shader. + if shader_slot.get().is_some() { + return Err(WebGLError::InvalidOperation); + } + + shader_slot.set(Some(JS::from_rooted(shader))); + + self.renderer.send(CanvasMsg::WebGL(CanvasWebGLMsg::AttachShader(self.id, shader.id()))).unwrap(); + + Ok(()) + } + + /// glGetAttribLocation + fn get_attrib_location(self, name: String) -> WebGLResult> { + if name.len() > MAX_UNIFORM_AND_ATTRIBUTE_LEN { + return Err(WebGLError::InvalidValue); + } + + // Check if the name is reserved + if name.starts_with("webgl") || name.starts_with("_webgl_") { + return Ok(None); + } + + let (sender, receiver) = channel(); + self.renderer.send(CanvasMsg::WebGL(CanvasWebGLMsg::GetAttribLocation(self.id, name, sender))).unwrap(); + Ok(receiver.recv().unwrap()) + } + + /// glGetUniformLocation + fn get_uniform_location(self, name: String) -> WebGLResult> { + if name.len() > MAX_UNIFORM_AND_ATTRIBUTE_LEN { + return Err(WebGLError::InvalidValue); + } + + // Check if the name is reserved + if name.starts_with("webgl") || name.starts_with("_webgl_") { + return Ok(None); + } + + let (sender, receiver) = channel(); + self.renderer.send(CanvasMsg::WebGL(CanvasWebGLMsg::GetUniformLocation(self.id, name, sender))).unwrap(); + Ok(receiver.recv().unwrap()) } } diff --git a/components/script/dom/webglrenderbuffer.rs b/components/script/dom/webglrenderbuffer.rs index 5165235acfb..71369d55661 100644 --- a/components/script/dom/webglrenderbuffer.rs +++ b/components/script/dom/webglrenderbuffer.rs @@ -9,31 +9,59 @@ use dom::bindings::js::Root; use dom::bindings::utils::reflect_dom_object; use dom::webglobject::WebGLObject; +use canvas_traits::{CanvasMsg, CanvasWebGLMsg}; +use std::sync::mpsc::{channel, Sender}; +use std::cell::Cell; + #[dom_struct] pub struct WebGLRenderbuffer { webgl_object: WebGLObject, id: u32, + is_deleted: Cell, + renderer: Sender, } impl WebGLRenderbuffer { - fn new_inherited(id: u32) -> WebGLRenderbuffer { + fn new_inherited(renderer: Sender, id: u32) -> WebGLRenderbuffer { WebGLRenderbuffer { webgl_object: WebGLObject::new_inherited(), id: id, + is_deleted: Cell::new(false), + renderer: renderer, } } - pub fn new(global: GlobalRef, id: u32) -> Root { - reflect_dom_object(box WebGLRenderbuffer::new_inherited(id), global, WebGLRenderbufferBinding::Wrap) + pub fn maybe_new(global: GlobalRef, renderer: Sender) -> Option> { + let (sender, receiver) = channel(); + renderer.send(CanvasMsg::WebGL(CanvasWebGLMsg::CreateRenderbuffer(sender))).unwrap(); + receiver.recv().unwrap() + .map(|renderbuffer_id| WebGLRenderbuffer::new(global, renderer, *renderbuffer_id)) + } + + pub fn new(global: GlobalRef, renderer: Sender, id: u32) -> Root { + reflect_dom_object(box WebGLRenderbuffer::new_inherited(renderer, id), global, WebGLRenderbufferBinding::Wrap) } } pub trait WebGLRenderbufferHelpers { - fn get_id(self) -> u32; + fn id(self) -> u32; + fn bind(self, target: u32); + fn delete(self); } impl<'a> WebGLRenderbufferHelpers for &'a WebGLRenderbuffer { - fn get_id(self) -> u32 { + fn id(self) -> u32 { self.id } + + fn bind(self, target: u32) { + self.renderer.send(CanvasMsg::WebGL(CanvasWebGLMsg::BindRenderbuffer(target, self.id))).unwrap(); + } + + fn delete(self) { + if !self.is_deleted.get() { + self.is_deleted.set(true); + self.renderer.send(CanvasMsg::WebGL(CanvasWebGLMsg::DeleteRenderbuffer(self.id))).unwrap(); + } + } } diff --git a/components/script/dom/webglrenderingcontext.rs b/components/script/dom/webglrenderingcontext.rs index 766c61fa57c..42e748d51d6 100644 --- a/components/script/dom/webglrenderingcontext.rs +++ b/components/script/dom/webglrenderingcontext.rs @@ -30,6 +30,20 @@ use std::sync::mpsc::{channel, Sender}; use util::str::DOMString; use offscreen_gl_context::GLContextAttributes; +pub const MAX_UNIFORM_AND_ATTRIBUTE_LEN : usize = 256; + +macro_rules! handle_potential_webgl_error { + ($context:ident, $call:expr, $return_on_error:expr) => { + match $call { + Ok(ret) => ret, + Err(error) => { + $context.handle_webgl_error(error); + $return_on_error + } + } + } +} + #[dom_struct] pub struct WebGLRenderingContext { reflector_: Reflector, @@ -174,48 +188,44 @@ impl<'a> WebGLRenderingContextMethods for &'a WebGLRenderingContext { // https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.9 fn AttachShader(self, program: Option<&WebGLProgram>, shader: Option<&WebGLShader>) { - let program_id = match program { - Some(program) => program.get_id(), - None => return, - }; - let shader_id = match shader { - Some(shader) => shader.get_id(), - None => return, - }; - self.renderer.send(CanvasMsg::WebGL(CanvasWebGLMsg::AttachShader(program_id, shader_id))).unwrap() + if let Some(program) = program { + if let Some(shader) = shader { + handle_potential_webgl_error!(self, program.attach_shader(shader), ()); + } + } } // https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.5 fn BindBuffer(self, target: u32, buffer: Option<&WebGLBuffer>) { - let id = buffer.map(|buf| buf.get_id()).unwrap_or(0); - self.renderer.send( - CanvasMsg::WebGL(CanvasWebGLMsg::BindBuffer(target, id))).unwrap() + if let Some(buffer) = buffer { + buffer.bind(target) + } } // https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.6 fn BindFramebuffer(self, target: u32, framebuffer: Option<&WebGLFramebuffer>) { - let id = framebuffer.map(|fb| fb.get_id()).unwrap_or(0); - self.renderer.send( - CanvasMsg::WebGL(CanvasWebGLMsg::BindFramebuffer(target, id))).unwrap() + if let Some(framebuffer) = framebuffer { + framebuffer.bind(target) + } } // https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.7 fn BindRenderbuffer(self, target: u32, renderbuffer: Option<&WebGLRenderbuffer>) { - let id = renderbuffer.map(|rb| rb.get_id()).unwrap_or(0); - self.renderer.send( - CanvasMsg::WebGL(CanvasWebGLMsg::BindRenderbuffer(target, id))).unwrap() + if let Some(renderbuffer) = renderbuffer { + renderbuffer.bind(target) + } } // https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.8 fn BindTexture(self, target: u32, texture: Option<&WebGLTexture>) { - let id = texture.map(|tex| tex.get_id()).unwrap_or(0); - self.renderer.send( - CanvasMsg::WebGL(CanvasWebGLMsg::BindTexture(target, id))).unwrap() + if let Some(texture) = texture { + texture.bind(target) + } } // https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.5 #[allow(unsafe_code)] - fn BufferData(self, _cx: *mut JSContext, target: u32, data: Option<*mut JSObject>, usage: u32) { + fn BufferData(self, cx: *mut JSContext, target: u32, data: Option<*mut JSObject>, usage: u32) { let data = match data { Some(data) => data, None => return, @@ -246,104 +256,84 @@ impl<'a> WebGLRenderingContextMethods for &'a WebGLRenderingContext { // https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.9 fn CompileShader(self, shader: Option<&WebGLShader>) { - let shader_id = match shader { - Some(shader) => shader.get_id(), - None => return, - }; - self.renderer.send(CanvasMsg::WebGL(CanvasWebGLMsg::CompileShader(shader_id))).unwrap() + if let Some(shader) = shader { + shader.compile() + } } // TODO(ecoal95): Probably in the future we should keep track of the // generated objects, either here or in the webgl task // https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.5 fn CreateBuffer(self) -> Option> { - let (sender, receiver) = channel(); - self.renderer.send(CanvasMsg::WebGL(CanvasWebGLMsg::CreateBuffer(sender))).unwrap(); - receiver.recv().unwrap() - .map(|buffer_id| WebGLBuffer::new(self.global.root().r(), *buffer_id)) + WebGLBuffer::maybe_new(self.global.root().r(), self.renderer.clone()) } // https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.6 fn CreateFramebuffer(self) -> Option> { - let (sender, receiver) = channel(); - self.renderer.send(CanvasMsg::WebGL(CanvasWebGLMsg::CreateFramebuffer(sender))).unwrap(); - receiver.recv().unwrap() - .map(|fb_id| WebGLFramebuffer::new(self.global.root().r(), *fb_id)) + WebGLFramebuffer::maybe_new(self.global.root().r(), self.renderer.clone()) } // https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.7 fn CreateRenderbuffer(self) -> Option> { - let (sender, receiver) = channel(); - self.renderer.send(CanvasMsg::WebGL(CanvasWebGLMsg::CreateRenderbuffer(sender))).unwrap(); - receiver.recv().unwrap() - .map(|program_id| WebGLRenderbuffer::new(self.global.root().r(), *program_id)) + WebGLRenderbuffer::maybe_new(self.global.root().r(), self.renderer.clone()) } // https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.8 fn CreateTexture(self) -> Option> { - let (sender, receiver) = channel(); - self.renderer.send(CanvasMsg::WebGL(CanvasWebGLMsg::CreateTexture(sender))).unwrap(); - receiver.recv().unwrap() - .map(|texture_id| WebGLTexture::new(self.global.root().r(), *texture_id)) + WebGLTexture::maybe_new(self.global.root().r(), self.renderer.clone()) } // https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.9 fn CreateProgram(self) -> Option> { - let (sender, receiver) = channel(); - self.renderer.send(CanvasMsg::WebGL(CanvasWebGLMsg::CreateProgram(sender))).unwrap(); - receiver.recv().unwrap() - .map(|program_id| WebGLProgram::new(self.global.root().r(), *program_id)) + WebGLProgram::maybe_new(self.global.root().r(), self.renderer.clone()) } // https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.9 // TODO(ecoal95): Check if constants are cross-platform or if we must make a translation // between WebGL constants and native ones. fn CreateShader(self, shader_type: u32) -> Option> { - let (sender, receiver) = channel(); - self.renderer.send(CanvasMsg::WebGL(CanvasWebGLMsg::CreateShader(shader_type, sender))).unwrap(); - receiver.recv().unwrap() - .map(|shader_id| WebGLShader::new(self.global.root().r(), *shader_id)) + WebGLShader::maybe_new(self.global.root().r(), self.renderer.clone(), shader_type) } // https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.5 fn DeleteBuffer(self, buffer: Option<&WebGLBuffer>) { if let Some(buffer) = buffer { - self.renderer.send(CanvasMsg::WebGL(CanvasWebGLMsg::DeleteBuffer(buffer.get_id()))).unwrap(); + buffer.delete() } } // https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.6 fn DeleteFramebuffer(self, framebuffer: Option<&WebGLFramebuffer>) { if let Some(framebuffer) = framebuffer { - self.renderer.send(CanvasMsg::WebGL(CanvasWebGLMsg::DeleteFramebuffer(framebuffer.get_id()))).unwrap(); + framebuffer.delete() } } // https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.7 fn DeleteRenderbuffer(self, renderbuffer: Option<&WebGLRenderbuffer>) { if let Some(renderbuffer) = renderbuffer { - self.renderer.send(CanvasMsg::WebGL(CanvasWebGLMsg::DeleteRenderbuffer(renderbuffer.get_id()))).unwrap(); + renderbuffer.delete() } } // https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.8 fn DeleteTexture(self, texture: Option<&WebGLTexture>) { if let Some(texture) = texture { - self.renderer.send(CanvasMsg::WebGL(CanvasWebGLMsg::DeleteTexture(texture.get_id()))).unwrap(); + texture.delete() } } // https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.9 fn DeleteProgram(self, program: Option<&WebGLProgram>) { if let Some(program) = program { - self.renderer.send(CanvasMsg::WebGL(CanvasWebGLMsg::DeleteProgram(program.get_id()))).unwrap(); + program.delete() } } // https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.9 fn DeleteShader(self, shader: Option<&WebGLShader>) { if let Some(shader) = shader { - self.renderer.send(CanvasMsg::WebGL(CanvasWebGLMsg::DeleteShader(shader.get_id()))).unwrap(); + shader.delete() } } @@ -359,21 +349,17 @@ impl<'a> WebGLRenderingContextMethods for &'a WebGLRenderingContext { // https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.10 fn GetAttribLocation(self, program: Option<&WebGLProgram>, name: DOMString) -> i32 { - let program_id = match program { - Some(program) => program.get_id(), - None => return -1, - }; - let (sender, receiver) = channel(); - self.renderer.send(CanvasMsg::WebGL(CanvasWebGLMsg::GetAttribLocation(program_id, name, sender))).unwrap(); - receiver.recv().unwrap() + if let Some(program) = program { + handle_potential_webgl_error!(self, program.get_attrib_location(name), None).unwrap_or(-1) + } else { + -1 + } } // https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.9 fn GetShaderInfoLog(self, shader: Option<&WebGLShader>) -> Option { if let Some(shader) = shader { - let (sender, receiver) = channel(); - self.renderer.send(CanvasMsg::WebGL(CanvasWebGLMsg::GetShaderInfoLog(shader.get_id(), sender))).unwrap(); - Some(receiver.recv().unwrap()) + shader.info_log() } else { None } @@ -382,10 +368,11 @@ impl<'a> WebGLRenderingContextMethods for &'a WebGLRenderingContext { // https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.9 fn GetShaderParameter(self, _: *mut JSContext, shader: Option<&WebGLShader>, param_id: u32) -> JSVal { if let Some(shader) = shader { - let (sender, receiver) = channel(); - self.renderer.send( - CanvasMsg::WebGL(CanvasWebGLMsg::GetShaderParameter(shader.get_id(), param_id, sender))).unwrap(); - Int32Value(receiver.recv().unwrap()) + match handle_potential_webgl_error!(self, shader.parameter(param_id), WebGLShaderParameter::Invalid) { + WebGLShaderParameter::Int(val) => Int32Value(val), + WebGLShaderParameter::Bool(val) => BooleanValue(val), + WebGLShaderParameter::Invalid => NullValue(), + } } else { NullValue() } @@ -396,10 +383,7 @@ impl<'a> WebGLRenderingContextMethods for &'a WebGLRenderingContext { program: Option<&WebGLProgram>, name: DOMString) -> Option> { if let Some(program) = program { - let (sender, receiver) = channel(); - self.renderer.send( - CanvasMsg::WebGL(CanvasWebGLMsg::GetUniformLocation(program.get_id(), name, sender))).unwrap(); - receiver.recv().unwrap() + handle_potential_webgl_error!(self, program.get_uniform_location(name), None) .map(|location| WebGLUniformLocation::new(self.global.root().r(), location)) } else { None @@ -409,15 +393,23 @@ impl<'a> WebGLRenderingContextMethods for &'a WebGLRenderingContext { // https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.9 fn LinkProgram(self, program: Option<&WebGLProgram>) { if let Some(program) = program { - self.renderer.send(CanvasMsg::WebGL(CanvasWebGLMsg::LinkProgram(program.get_id()))).unwrap() + program.link() } } // https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.9 fn ShaderSource(self, shader: Option<&WebGLShader>, source: DOMString) { if let Some(shader) = shader { - self.renderer.send( - CanvasMsg::WebGL(CanvasWebGLMsg::ShaderSource(shader.get_id(), source))).unwrap(); + shader.set_source(source) + } + } + + // https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.9 + fn GetShaderSource(self, shader: Option<&WebGLShader>) -> Option { + if let Some(shader) = shader { + shader.source() + } else { + None } } @@ -428,7 +420,7 @@ impl<'a> WebGLRenderingContextMethods for &'a WebGLRenderingContext { uniform: Option<&WebGLUniformLocation>, data: Option<*mut JSObject>) { let uniform_id = match uniform { - Some(uniform) => uniform.get_id(), + Some(uniform) => uniform.id(), None => return, }; @@ -446,11 +438,9 @@ impl<'a> WebGLRenderingContextMethods for &'a WebGLRenderingContext { // https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.9 fn UseProgram(self, program: Option<&WebGLProgram>) { - let program_id = match program { - Some(program) => program.get_id(), - None => return, - }; - self.renderer.send(CanvasMsg::WebGL(CanvasWebGLMsg::UseProgram(program_id as u32))).unwrap() + if let Some(program) = program { + program.use_program() + } } // https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.10 @@ -473,6 +463,17 @@ impl<'a> WebGLRenderingContextMethods for &'a WebGLRenderingContext { } } +pub trait WebGLRenderingContextHelpers { + fn handle_webgl_error(&self, err: WebGLError); +} + +impl<'a> WebGLRenderingContextHelpers for &'a WebGLRenderingContext { + fn handle_webgl_error(&self, _: WebGLError) { + debug!("WebGL error received"); + // ignore for now + } +} + pub trait LayoutCanvasWebGLRenderingContextHelpers { #[allow(unsafe_code)] unsafe fn get_renderer(&self) -> Sender; diff --git a/components/script/dom/webglshader.rs b/components/script/dom/webglshader.rs index 7030b1fda99..5750046e8fc 100644 --- a/components/script/dom/webglshader.rs +++ b/components/script/dom/webglshader.rs @@ -9,32 +9,120 @@ use dom::bindings::js::Root; use dom::bindings::utils::reflect_dom_object; use dom::webglobject::WebGLObject; +use dom::bindings::codegen::Bindings::WebGLRenderingContextBinding::WebGLRenderingContextConstants as constants; + +use canvas_traits::{CanvasMsg, CanvasWebGLMsg, WebGLResult, WebGLError, WebGLShaderParameter}; +use std::sync::mpsc::{channel, Sender}; +use std::cell::Cell; +use std::cell::RefCell; + #[dom_struct] pub struct WebGLShader { webgl_object: WebGLObject, id: u32, + gl_type: u32, + // TODO(ecoal95): is RefCell ok? + source: RefCell>, + is_deleted: Cell, + // TODO(ecoal95): Valorate moving this to `WebGLObject` + renderer: Sender, } impl WebGLShader { - fn new_inherited(id: u32) -> WebGLShader { + fn new_inherited(renderer: Sender, id: u32, shader_type: u32) -> WebGLShader { WebGLShader { webgl_object: WebGLObject::new_inherited(), id: id, + gl_type: shader_type, + source: RefCell::new(None), + is_deleted: Cell::new(false), + renderer: renderer, } } - pub fn new(global: GlobalRef, id: u32) -> Root { - reflect_dom_object(box WebGLShader::new_inherited(id), global, WebGLShaderBinding::Wrap) + pub fn maybe_new(global: GlobalRef, + renderer: Sender, + shader_type: u32) -> Option> { + let (sender, receiver) = channel(); + renderer.send(CanvasMsg::WebGL(CanvasWebGLMsg::CreateShader(shader_type, sender))).unwrap(); + receiver.recv().unwrap() + .map(|shader_id| WebGLShader::new(global, renderer, *shader_id, shader_type)) + } + + pub fn new(global: GlobalRef, + renderer: Sender, + id: u32, + shader_type: u32) -> Root { + reflect_dom_object( + box WebGLShader::new_inherited(renderer, id, shader_type), global, WebGLShaderBinding::Wrap) } } pub trait WebGLShaderHelpers { - fn get_id(self) -> u32; + fn id(self) -> u32; + fn gl_type(self) -> u32; + fn compile(self); + fn delete(self); + fn info_log(self) -> Option; + fn parameter(self, param_id: u32) -> WebGLResult; + fn source(self) -> Option; + fn set_source(self, src: String); } -impl<'a> WebGLShaderHelpers for &'a WebGLShader { - fn get_id(self) -> u32 { +impl<'a> WebGLShaderHelpers for JSRef<'a, WebGLShader> { + fn id(self) -> u32 { self.id } + + fn gl_type(self) -> u32 { + self.gl_type + } + + // TODO(ecoal95): Validate shaders to be conforming to the WebGL spec + /// glCompileShader + fn compile(self) { + // NB(ecoal95): We intentionally don't check for source, we don't wan't + // to change gl error behavior + self.renderer.send(CanvasMsg::WebGL(CanvasWebGLMsg::CompileShader(self.id))).unwrap() + } + + /// Mark this shader as deleted (if it wasn't previously) + /// and delete it as if calling tr glDeleteShader. + fn delete(self) { + if !self.is_deleted.get() { + self.is_deleted.set(true); + self.renderer.send(CanvasMsg::WebGL(CanvasWebGLMsg::DeleteShader(self.id))).unwrap() + } + } + + /// glGetShaderInfoLog + fn info_log(self) -> Option { + let (sender, receiver) = channel(); + self.renderer.send(CanvasMsg::WebGL(CanvasWebGLMsg::GetShaderInfoLog(self.id, sender))).unwrap(); + receiver.recv().unwrap() + } + + /// glGetShaderParameter + fn parameter(self, param_id: u32) -> WebGLResult { + match param_id { + constants::SHADER_TYPE | constants::DELETE_STATUS | constants::COMPILE_STATUS => {}, + _ => return Err(WebGLError::InvalidEnum), + } + + let (sender, receiver) = channel(); + self.renderer.send(CanvasMsg::WebGL(CanvasWebGLMsg::GetShaderParameter(self.id, param_id, sender))).unwrap(); + Ok(receiver.recv().unwrap()) + } + + /// Get the shader source + fn source(self) -> Option { + (*self.source.borrow()).clone() + } + + /// glShaderSource + fn set_source(self, source: String) { + *self.source.borrow_mut() = Some(source.clone()); + self.renderer.send(CanvasMsg::WebGL(CanvasWebGLMsg::ShaderSource(self.id, source))).unwrap() + } } diff --git a/components/script/dom/webgltexture.rs b/components/script/dom/webgltexture.rs index d871bc598da..6848e3a7f71 100644 --- a/components/script/dom/webgltexture.rs +++ b/components/script/dom/webgltexture.rs @@ -9,31 +9,59 @@ use dom::bindings::js::Root; use dom::bindings::utils::reflect_dom_object; use dom::webglobject::WebGLObject; +use canvas_traits::{CanvasMsg, CanvasWebGLMsg}; +use std::sync::mpsc::{channel, Sender}; +use std::cell::Cell; + #[dom_struct] pub struct WebGLTexture { webgl_object: WebGLObject, id: u32, + is_deleted: Cell, + renderer: Sender, } impl WebGLTexture { - fn new_inherited(id: u32) -> WebGLTexture { + fn new_inherited(renderer: Sender, id: u32) -> WebGLTexture { WebGLTexture { webgl_object: WebGLObject::new_inherited(), id: id, + is_deleted: Cell::new(false), + renderer: renderer, } } - pub fn new(global: GlobalRef, id: u32) -> Root { - reflect_dom_object(box WebGLTexture::new_inherited(id), global, WebGLTextureBinding::Wrap) + pub fn maybe_new(global: GlobalRef, renderer: Sender) -> Option> { + let (sender, receiver) = channel(); + renderer.send(CanvasMsg::WebGL(CanvasWebGLMsg::CreateTexture(sender))).unwrap(); + receiver.recv().unwrap() + .map(|texture_id| WebGLTexture::new(global, renderer, *texture_id)) + } + + pub fn new(global: GlobalRef, renderer: Sender, id: u32) -> Root { + reflect_dom_object(box WebGLTexture::new_inherited(renderer, id), global, WebGLTextureBinding::Wrap) } } pub trait WebGLTextureHelpers { - fn get_id(self) -> u32; + fn id(self) -> u32; + fn bind(self, target: u32); + fn delete(self); } impl<'a> WebGLTextureHelpers for &'a WebGLTexture { - fn get_id(self) -> u32 { + fn id(&self) -> u32 { self.id } + + fn bind(self, target: u32) { + self.renderer.send(CanvasMsg::WebGL(CanvasWebGLMsg::BindTexture(self.id, target))).unwrap(); + } + + fn delete(self) { + if !self.is_deleted.get() { + self.is_deleted.set(true); + self.renderer.send(CanvasMsg::WebGL(CanvasWebGLMsg::DeleteTexture(self.id))).unwrap(); + } + } } diff --git a/components/script/dom/webgluniformlocation.rs b/components/script/dom/webgluniformlocation.rs index 675ddf42ca0..56a97555ce6 100644 --- a/components/script/dom/webgluniformlocation.rs +++ b/components/script/dom/webgluniformlocation.rs @@ -28,11 +28,11 @@ impl WebGLUniformLocation { } pub trait WebGLUniformLocationHelpers { - fn get_id(self) -> i32; + fn id(self) -> i32; } impl<'a> WebGLUniformLocationHelpers for &'a WebGLUniformLocation { - fn get_id(self) -> i32 { + fn id(self) -> i32 { self.id } } diff --git a/components/script/dom/webidls/WebGLRenderingContext.webidl b/components/script/dom/webidls/WebGLRenderingContext.webidl index d3865c91c4a..5177dd22cb1 100644 --- a/components/script/dom/webidls/WebGLRenderingContext.webidl +++ b/components/script/dom/webidls/WebGLRenderingContext.webidl @@ -569,7 +569,7 @@ interface WebGLRenderingContextBase //WebGLShaderPrecisionFormat? getShaderPrecisionFormat(GLenum shadertype, GLenum precisiontype); DOMString? getShaderInfoLog(WebGLShader? shader); - //DOMString? getShaderSource(WebGLShader? shader); + DOMString? getShaderSource(WebGLShader? shader); //any getTexParameter(GLenum target, GLenum pname); From 42bd43a696939c3259284a01b8ef64aa13a9939c Mon Sep 17 00:00:00 2001 From: ecoal95 Date: Mon, 15 Jun 2015 00:22:15 +0200 Subject: [PATCH 2/4] webgl: Make bind* calls more spec-compliant --- components/canvas/webgl_paint_task.rs | 14 +++++--- components/canvas_traits/lib.rs | 25 ++++++++------ components/script/dom/webglbuffer.rs | 8 ++--- components/script/dom/webglframebuffer.rs | 8 +++-- components/script/dom/webglprogram.rs | 16 ++++++--- .../script/dom/webglrenderingcontext.rs | 34 +++++++++++++------ components/script/dom/webglshader.rs | 2 +- components/script/dom/webgltexture.rs | 2 +- 8 files changed, 72 insertions(+), 37 deletions(-) diff --git a/components/canvas/webgl_paint_task.rs b/components/canvas/webgl_paint_task.rs index 4f6e0605dd8..6aecf0eba33 100644 --- a/components/canvas/webgl_paint_task.rs +++ b/components/canvas/webgl_paint_task.rs @@ -2,7 +2,7 @@ * License, v. 2.0. If a copy of the MPL was not distributed with this * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ -use canvas_traits::{CanvasMsg, CanvasWebGLMsg, CanvasCommonMsg}; +use canvas_traits::{CanvasMsg, CanvasWebGLMsg, CanvasCommonMsg, WebGLShaderParameter, WebGLFramebufferBindingRequest}; use euclid::size::Size2D; use core::nonzero::NonZero; use gleam::gl; @@ -109,8 +109,8 @@ impl WebGLPaintTask { self.delete_shader(id), CanvasWebGLMsg::BindBuffer(target, id) => self.bind_buffer(target, id), - CanvasWebGLMsg::BindFramebuffer(target, id) => - self.bind_framebuffer(target, id), + CanvasWebGLMsg::BindFramebuffer(target, request) => + self.bind_framebuffer(target, request), CanvasWebGLMsg::BindRenderbuffer(target, id) => self.bind_renderbuffer(target, id), CanvasWebGLMsg::BindTexture(target, id) => @@ -329,7 +329,13 @@ impl WebGLPaintTask { } #[inline] - fn bind_framebuffer(&self, target: u32, id: u32) { + fn bind_framebuffer(&self, target: u32, request: WebGLFramebufferBindingRequest) { + let id = match request { + WebGLFramebufferBindingRequest::Explicit(id) => id, + WebGLFramebufferBindingRequest::Default => + self.gl_context.borrow_draw_buffer().unwrap().get_framebuffer(), + }; + gl::bind_framebuffer(target, id); } diff --git a/components/canvas_traits/lib.rs b/components/canvas_traits/lib.rs index 51c9a1dafb2..01925408b5e 100644 --- a/components/canvas_traits/lib.rs +++ b/components/canvas_traits/lib.rs @@ -37,6 +37,14 @@ pub enum CanvasMsg { WebGL(CanvasWebGLMsg), } +#[derive(Clone)] +pub enum CanvasCommonMsg { + Close, + Recreate(Size2D), + SendPixelContents(Sender>), + SendNativeSurface(Sender), +} + #[derive(Clone)] pub enum Canvas2dMsg { Arc(Point2D, f32, f32, f32, bool), @@ -102,7 +110,7 @@ pub enum CanvasWebGLMsg { DeleteProgram(u32), DeleteShader(u32), BindBuffer(u32, u32), - BindFramebuffer(u32, u32), + BindFramebuffer(u32, WebGLFramebufferBindingRequest), BindRenderbuffer(u32, u32), BindTexture(u32, u32), DrawArrays(u32, i32, i32), @@ -132,6 +140,12 @@ pub enum WebGLError { pub type WebGLResult = Result; +#[derive(Clone)] +pub enum WebGLFramebufferBindingRequest { + Explicit(u32), + Default, +} + #[derive(Clone)] pub enum WebGLShaderParameter { Int(i32), @@ -139,15 +153,6 @@ pub enum WebGLShaderParameter { Invalid, } -#[derive(Clone)] -pub enum CanvasCommonMsg { - Close, - Recreate(Size2D), - SendPixelContents(Sender>), - SendNativeSurface(Sender), -} - - #[derive(Clone)] pub struct CanvasGradientStop { pub offset: f64, diff --git a/components/script/dom/webglbuffer.rs b/components/script/dom/webglbuffer.rs index b9941026353..bf6d8618118 100644 --- a/components/script/dom/webglbuffer.rs +++ b/components/script/dom/webglbuffer.rs @@ -5,7 +5,7 @@ // https://www.khronos.org/registry/webgl/specs/latest/1.0/webgl.idl use dom::bindings::codegen::Bindings::WebGLBufferBinding; use dom::bindings::global::GlobalRef; -use dom::bindings::js::{Temporary, JSRef}; +use dom::bindings::js::Root; use dom::bindings::utils::reflect_dom_object; use dom::webglobject::WebGLObject; @@ -44,9 +44,9 @@ impl WebGLBuffer { } pub trait WebGLBufferHelpers { - fn id(&self) -> u32; - fn bind(&self, target: u32); - fn delete(&self); + fn id(self) -> u32; + fn bind(self, target: u32); + fn delete(self); } impl<'a> WebGLBufferHelpers for &'a WebGLBuffer { diff --git a/components/script/dom/webglframebuffer.rs b/components/script/dom/webglframebuffer.rs index cd23765dfff..bbf59529241 100644 --- a/components/script/dom/webglframebuffer.rs +++ b/components/script/dom/webglframebuffer.rs @@ -9,7 +9,7 @@ use dom::bindings::js::Root; use dom::bindings::utils::reflect_dom_object; use dom::webglobject::WebGLObject; -use canvas_traits::{CanvasMsg, CanvasWebGLMsg}; +use canvas_traits::{CanvasMsg, CanvasWebGLMsg, WebGLFramebufferBindingRequest}; use std::sync::mpsc::{channel, Sender}; use std::cell::Cell; @@ -49,13 +49,15 @@ pub trait WebGLFramebufferHelpers { fn delete(self); } -impl<'a> WebGLFramebufferHelpers for JSRef<'a, WebGLFramebuffer> { +impl<'a> WebGLFramebufferHelpers for &'a WebGLFramebuffer { fn id(self) -> u32 { self.id } fn bind(self, target: u32) { - self.renderer.send(CanvasMsg::WebGL(CanvasWebGLMsg::BindFramebuffer(target, self.id))).unwrap(); + self.renderer.send( + CanvasMsg::WebGL( + CanvasWebGLMsg::BindFramebuffer(target, WebGLFramebufferBindingRequest::Explicit(self.id)))).unwrap(); } fn delete(self) { diff --git a/components/script/dom/webglprogram.rs b/components/script/dom/webglprogram.rs index 63496f9bff6..35ac00689a8 100644 --- a/components/script/dom/webglprogram.rs +++ b/components/script/dom/webglprogram.rs @@ -5,9 +5,17 @@ // https://www.khronos.org/registry/webgl/specs/latest/1.0/webgl.idl use dom::bindings::codegen::Bindings::WebGLProgramBinding; use dom::bindings::global::GlobalRef; -use dom::bindings::js::Root; +use dom::bindings::js::{JS, MutNullableHeap, Root}; use dom::bindings::utils::reflect_dom_object; use dom::webglobject::WebGLObject; +use dom::webglshader::{WebGLShader, WebGLShaderHelpers}; +use dom::webglrenderingcontext::MAX_UNIFORM_AND_ATTRIBUTE_LEN; + +use dom::bindings::codegen::Bindings::WebGLRenderingContextBinding::WebGLRenderingContextConstants as constants; + +use canvas_traits::{CanvasMsg, CanvasWebGLMsg, WebGLResult, WebGLError}; +use std::sync::mpsc::{channel, Sender}; +use std::cell::Cell; #[dom_struct] pub struct WebGLProgram { @@ -47,7 +55,7 @@ pub trait WebGLProgramHelpers { fn delete(self); fn link(self); fn use_program(self); - fn attach_shader(self, shader: JSRef) -> WebGLResult<()>; + fn attach_shader(self, shader: &WebGLShader) -> WebGLResult<()>; fn get_attrib_location(self, name: String) -> WebGLResult>; fn get_uniform_location(self, name: String) -> WebGLResult>; } @@ -72,7 +80,7 @@ impl<'a> WebGLProgramHelpers for &'a WebGLProgram { } /// glAttachShader - fn attach_shader(self, shader: &'a WebGLShader) -> WebGLResult<()> { + fn attach_shader(self, shader: &WebGLShader) -> WebGLResult<()> { let shader_slot = match shader.gl_type() { constants::FRAGMENT_SHADER => &self.fragment_shader, constants::VERTEX_SHADER => &self.vertex_shader, @@ -85,7 +93,7 @@ impl<'a> WebGLProgramHelpers for &'a WebGLProgram { return Err(WebGLError::InvalidOperation); } - shader_slot.set(Some(JS::from_rooted(shader))); + shader_slot.set(Some(JS::from_ref(shader))); self.renderer.send(CanvasMsg::WebGL(CanvasWebGLMsg::AttachShader(self.id, shader.id()))).unwrap(); diff --git a/components/script/dom/webglrenderingcontext.rs b/components/script/dom/webglrenderingcontext.rs index 42e748d51d6..23090a73cfb 100644 --- a/components/script/dom/webglrenderingcontext.rs +++ b/components/script/dom/webglrenderingcontext.rs @@ -3,10 +3,13 @@ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ use canvas::webgl_paint_task::WebGLPaintTask; -use canvas_traits::{CanvasMsg, CanvasWebGLMsg, CanvasCommonMsg}; -use dom::bindings::codegen::Bindings::WebGLRenderingContextBinding; -use dom::bindings::codegen::Bindings::WebGLRenderingContextBinding::{ - WebGLContextAttributes, WebGLRenderingContextMethods, WebGLRenderingContextConstants}; +use canvas_traits:: + {CanvasMsg, CanvasWebGLMsg, CanvasCommonMsg, WebGLError, + WebGLShaderParameter, WebGLFramebufferBindingRequest}; +use dom::bindings::codegen::Bindings::WebGLRenderingContextBinding:: + {self, WebGLContextAttributes, WebGLRenderingContextMethods}; +use dom::bindings::codegen::Bindings::WebGLRenderingContextBinding::WebGLRenderingContextConstants as constants; + use dom::bindings::global::{GlobalRef, GlobalField}; use dom::bindings::js::{JS, LayoutJS, Root}; use dom::bindings::utils::{Reflector, reflect_dom_object}; @@ -22,7 +25,7 @@ use dom::webgluniformlocation::{WebGLUniformLocation, WebGLUniformLocationHelper use euclid::size::Size2D; use js::jsapi::{JSContext, JSObject, RootedValue}; use js::jsapi::{JS_GetFloat32ArrayData, JS_GetObjectAsArrayBufferView}; -use js::jsval::{JSVal, UndefinedValue, NullValue, Int32Value}; +use js::jsval::{JSVal, UndefinedValue, NullValue, Int32Value, BooleanValue}; use std::mem; use std::ptr; use std::slice; @@ -116,10 +119,10 @@ impl<'a> WebGLRenderingContextMethods for &'a WebGLRenderingContext { // TODO(ecoal95): Implement the missing parameters from the spec let mut rval = RootedValue::new(cx, UndefinedValue()); match parameter { - WebGLRenderingContextConstants::VERSION => + constants::VERSION => "WebGL 1.0".to_jsval(cx, rval.handle_mut()), - WebGLRenderingContextConstants::RENDERER | - WebGLRenderingContextConstants::VENDOR => + constants::RENDERER | + constants::VENDOR => "Mozilla/Servo".to_jsval(cx, rval.handle_mut()), _ => rval.ptr = NullValue(), } @@ -199,6 +202,9 @@ impl<'a> WebGLRenderingContextMethods for &'a WebGLRenderingContext { fn BindBuffer(self, target: u32, buffer: Option<&WebGLBuffer>) { if let Some(buffer) = buffer { buffer.bind(target) + } else { + // Unbind the current buffer + self.renderer.send(CanvasMsg::WebGL(CanvasWebGLMsg::BindBuffer(target, 0))).unwrap() } } @@ -206,6 +212,11 @@ impl<'a> WebGLRenderingContextMethods for &'a WebGLRenderingContext { fn BindFramebuffer(self, target: u32, framebuffer: Option<&WebGLFramebuffer>) { if let Some(framebuffer) = framebuffer { framebuffer.bind(target) + } else { + // Bind the default framebuffer + self.renderer.send( + CanvasMsg::WebGL( + CanvasWebGLMsg::BindFramebuffer(target, WebGLFramebufferBindingRequest::Default))).unwrap() } } @@ -213,6 +224,9 @@ impl<'a> WebGLRenderingContextMethods for &'a WebGLRenderingContext { fn BindRenderbuffer(self, target: u32, renderbuffer: Option<&WebGLRenderbuffer>) { if let Some(renderbuffer) = renderbuffer { renderbuffer.bind(target) + } else { + // Unbind the currently bound renderbuffer + self.renderer.send(CanvasMsg::WebGL(CanvasWebGLMsg::BindRenderbuffer(target, 0))).unwrap() } } @@ -225,7 +239,7 @@ impl<'a> WebGLRenderingContextMethods for &'a WebGLRenderingContext { // https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.5 #[allow(unsafe_code)] - fn BufferData(self, cx: *mut JSContext, target: u32, data: Option<*mut JSObject>, usage: u32) { + fn BufferData(self, _cx: *mut JSContext, target: u32, data: Option<*mut JSObject>, usage: u32) { let data = match data { Some(data) => data, None => return, @@ -447,7 +461,7 @@ impl<'a> WebGLRenderingContextMethods for &'a WebGLRenderingContext { fn VertexAttribPointer(self, attrib_id: u32, size: i32, data_type: u32, normalized: bool, stride: i32, offset: i64) { match data_type { - WebGLRenderingContextConstants::FLOAT => { + constants::FLOAT => { let msg = CanvasMsg::WebGL( CanvasWebGLMsg::VertexAttribPointer2f(attrib_id, size, normalized, stride, offset)); self.renderer.send(msg).unwrap() diff --git a/components/script/dom/webglshader.rs b/components/script/dom/webglshader.rs index 5750046e8fc..e3c4389365b 100644 --- a/components/script/dom/webglshader.rs +++ b/components/script/dom/webglshader.rs @@ -69,7 +69,7 @@ pub trait WebGLShaderHelpers { fn set_source(self, src: String); } -impl<'a> WebGLShaderHelpers for JSRef<'a, WebGLShader> { +impl<'a> WebGLShaderHelpers for &'a WebGLShader { fn id(self) -> u32 { self.id } diff --git a/components/script/dom/webgltexture.rs b/components/script/dom/webgltexture.rs index 6848e3a7f71..111c0ab07c3 100644 --- a/components/script/dom/webgltexture.rs +++ b/components/script/dom/webgltexture.rs @@ -50,7 +50,7 @@ pub trait WebGLTextureHelpers { } impl<'a> WebGLTextureHelpers for &'a WebGLTexture { - fn id(&self) -> u32 { + fn id(self) -> u32 { self.id } From 9b306aced6f8649e606a5c9377df9219b914adcd Mon Sep 17 00:00:00 2001 From: ecoal95 Date: Sat, 20 Jun 2015 20:37:44 +0200 Subject: [PATCH 3/4] webgl: implement getError --- components/canvas_traits/lib.rs | 3 ++- components/script/dom/bindings/trace.rs | 2 ++ .../script/dom/webglrenderingcontext.rs | 26 ++++++++++++++++--- .../dom/webidls/WebGLRenderingContext.webidl | 2 +- 4 files changed, 28 insertions(+), 5 deletions(-) diff --git a/components/canvas_traits/lib.rs b/components/canvas_traits/lib.rs index 01925408b5e..ff68685ca60 100644 --- a/components/canvas_traits/lib.rs +++ b/components/canvas_traits/lib.rs @@ -129,8 +129,9 @@ pub enum CanvasWebGLMsg { DrawingBufferHeight(Sender), } -#[derive(Clone)] +#[derive(Clone, Copy, PartialEq)] pub enum WebGLError { + NoError, InvalidEnum, InvalidOperation, InvalidValue, diff --git a/components/script/dom/bindings/trace.rs b/components/script/dom/bindings/trace.rs index 5c854953f8f..b64d46a212e 100644 --- a/components/script/dom/bindings/trace.rs +++ b/components/script/dom/bindings/trace.rs @@ -36,6 +36,7 @@ use script_task::ScriptChan; use canvas_traits::{CanvasGradientStop, LinearGradientStyle, RadialGradientStyle}; use canvas_traits::{LineCapStyle, LineJoinStyle, CompositionOrBlending, RepetitionStyle}; +use canvas_traits::WebGLError; use cssparser::RGBA; use encoding::types::EncodingRef; use euclid::matrix2d::Matrix2D; @@ -297,6 +298,7 @@ no_jsmanaged_fields!(StorageType); no_jsmanaged_fields!(CanvasGradientStop, LinearGradientStyle, RadialGradientStyle); no_jsmanaged_fields!(LineCapStyle, LineJoinStyle, CompositionOrBlending); no_jsmanaged_fields!(RepetitionStyle); +no_jsmanaged_fields!(WebGLError); impl JSTraceable for Box { #[inline] diff --git a/components/script/dom/webglrenderingcontext.rs b/components/script/dom/webglrenderingcontext.rs index 23090a73cfb..c87b901d391 100644 --- a/components/script/dom/webglrenderingcontext.rs +++ b/components/script/dom/webglrenderingcontext.rs @@ -26,6 +26,7 @@ use euclid::size::Size2D; use js::jsapi::{JSContext, JSObject, RootedValue}; use js::jsapi::{JS_GetFloat32ArrayData, JS_GetObjectAsArrayBufferView}; use js::jsval::{JSVal, UndefinedValue, NullValue, Int32Value, BooleanValue}; +use std::cell::Cell; use std::mem; use std::ptr; use std::slice; @@ -53,6 +54,7 @@ pub struct WebGLRenderingContext { global: GlobalField, renderer: Sender, canvas: JS, + last_error: Cell, } impl WebGLRenderingContext { @@ -67,6 +69,7 @@ impl WebGLRenderingContext { reflector_: Reflector::new(), global: GlobalField::from_rooted(&global), renderer: chan, + last_error: Cell::new(WebGLError::NoError), canvas: JS::from_ref(canvas), }) } @@ -129,6 +132,20 @@ impl<'a> WebGLRenderingContextMethods for &'a WebGLRenderingContext { rval.ptr } + // https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.3 + fn GetError(self) -> u32 { + let error_code = match self.last_error.get() { + WebGLError::NoError => constants::NO_ERROR, + WebGLError::InvalidEnum => constants::INVALID_ENUM, + WebGLError::InvalidValue => constants::INVALID_VALUE, + WebGLError::InvalidOperation => constants::INVALID_OPERATION, + WebGLError::OutOfMemory => constants::OUT_OF_MEMORY, + WebGLError::ContextLost => constants::CONTEXT_LOST_WEBGL, + }; + self.last_error.set(WebGLError::NoError); + error_code + } + // https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.2 fn GetContextAttributes(self) -> Option { let (sender, receiver) = channel(); @@ -482,9 +499,12 @@ pub trait WebGLRenderingContextHelpers { } impl<'a> WebGLRenderingContextHelpers for &'a WebGLRenderingContext { - fn handle_webgl_error(&self, _: WebGLError) { - debug!("WebGL error received"); - // ignore for now + fn handle_webgl_error(&self, err: WebGLError) { + // If an error has been detected no further errors must be + // recorded until `getError` has been called + if self.last_error.get() == WebGLError::NoError { + self.last_error.set(err); + } } } diff --git a/components/script/dom/webidls/WebGLRenderingContext.webidl b/components/script/dom/webidls/WebGLRenderingContext.webidl index 5177dd22cb1..8f4b6f95f02 100644 --- a/components/script/dom/webidls/WebGLRenderingContext.webidl +++ b/components/script/dom/webidls/WebGLRenderingContext.webidl @@ -558,7 +558,7 @@ interface WebGLRenderingContextBase //any getBufferParameter(GLenum target, GLenum pname); any getParameter(GLenum pname); - //[WebGLHandlesContextLoss] GLenum getError(); + [WebGLHandlesContextLoss] GLenum getError(); //any getFramebufferAttachmentParameter(GLenum target, GLenum attachment, // GLenum pname); From 8438db89e151ed07b32a5b37fd4c4f903605c126 Mon Sep 17 00:00:00 2001 From: ecoal95 Date: Mon, 6 Jul 2015 22:56:42 +0200 Subject: [PATCH 4/4] address review comments --- components/canvas_traits/lib.rs | 1 - components/script/dom/webglbuffer.rs | 5 +-- components/script/dom/webglframebuffer.rs | 10 +++--- components/script/dom/webglprogram.rs | 7 ++-- components/script/dom/webglrenderbuffer.rs | 5 +-- .../script/dom/webglrenderingcontext.rs | 34 ++++++++++--------- components/script/dom/webglshader.rs | 14 ++++---- components/script/dom/webgltexture.rs | 5 +-- 8 files changed, 42 insertions(+), 39 deletions(-) diff --git a/components/canvas_traits/lib.rs b/components/canvas_traits/lib.rs index ff68685ca60..6f5e73bb2c1 100644 --- a/components/canvas_traits/lib.rs +++ b/components/canvas_traits/lib.rs @@ -131,7 +131,6 @@ pub enum CanvasWebGLMsg { #[derive(Clone, Copy, PartialEq)] pub enum WebGLError { - NoError, InvalidEnum, InvalidOperation, InvalidValue, diff --git a/components/script/dom/webglbuffer.rs b/components/script/dom/webglbuffer.rs index bf6d8618118..4970b577bb7 100644 --- a/components/script/dom/webglbuffer.rs +++ b/components/script/dom/webglbuffer.rs @@ -34,8 +34,9 @@ impl WebGLBuffer { pub fn maybe_new(global: GlobalRef, renderer: Sender) -> Option> { let (sender, receiver) = channel(); renderer.send(CanvasMsg::WebGL(CanvasWebGLMsg::CreateBuffer(sender))).unwrap(); - receiver.recv().unwrap() - .map(|buffer_id| WebGLBuffer::new(global, renderer, *buffer_id)) + + let result = receiver.recv().unwrap(); + result.map(|buffer_id| WebGLBuffer::new(global, renderer, *buffer_id)) } pub fn new(global: GlobalRef, renderer: Sender, id: u32) -> Root { diff --git a/components/script/dom/webglframebuffer.rs b/components/script/dom/webglframebuffer.rs index bbf59529241..26773ca4b17 100644 --- a/components/script/dom/webglframebuffer.rs +++ b/components/script/dom/webglframebuffer.rs @@ -34,8 +34,9 @@ impl WebGLFramebuffer { pub fn maybe_new(global: GlobalRef, renderer: Sender) -> Option> { let (sender, receiver) = channel(); renderer.send(CanvasMsg::WebGL(CanvasWebGLMsg::CreateFramebuffer(sender))).unwrap(); - receiver.recv().unwrap() - .map(|fb_id| WebGLFramebuffer::new(global, renderer, *fb_id)) + + let result = receiver.recv().unwrap(); + result.map(|fb_id| WebGLFramebuffer::new(global, renderer, *fb_id)) } pub fn new(global: GlobalRef, renderer: Sender, id: u32) -> Root { @@ -55,9 +56,8 @@ impl<'a> WebGLFramebufferHelpers for &'a WebGLFramebuffer { } fn bind(self, target: u32) { - self.renderer.send( - CanvasMsg::WebGL( - CanvasWebGLMsg::BindFramebuffer(target, WebGLFramebufferBindingRequest::Explicit(self.id)))).unwrap(); + let cmd = CanvasWebGLMsg::BindFramebuffer(target, WebGLFramebufferBindingRequest::Explicit(self.id)); + self.renderer.send(CanvasMsg::WebGL(cmd)).unwrap(); } fn delete(self) { diff --git a/components/script/dom/webglprogram.rs b/components/script/dom/webglprogram.rs index 35ac00689a8..71e0d2e6616 100644 --- a/components/script/dom/webglprogram.rs +++ b/components/script/dom/webglprogram.rs @@ -42,8 +42,9 @@ impl WebGLProgram { pub fn maybe_new(global: GlobalRef, renderer: Sender) -> Option> { let (sender, receiver) = channel(); renderer.send(CanvasMsg::WebGL(CanvasWebGLMsg::CreateProgram(sender))).unwrap(); - receiver.recv().unwrap() - .map(|program_id| WebGLProgram::new(global, renderer, *program_id)) + + let result = receiver.recv().unwrap(); + result.map(|program_id| WebGLProgram::new(global, renderer, *program_id)) } pub fn new(global: GlobalRef, renderer: Sender, id: u32) -> Root { @@ -87,7 +88,7 @@ impl<'a> WebGLProgramHelpers for &'a WebGLProgram { _ => return Err(WebGLError::InvalidOperation), }; - // TODO(ecoal95): Differenciate between same shader already assigned and other previous + // TODO(ecoal95): Differentiate between same shader already assigned and other previous // shader. if shader_slot.get().is_some() { return Err(WebGLError::InvalidOperation); diff --git a/components/script/dom/webglrenderbuffer.rs b/components/script/dom/webglrenderbuffer.rs index 71369d55661..d7084564ec3 100644 --- a/components/script/dom/webglrenderbuffer.rs +++ b/components/script/dom/webglrenderbuffer.rs @@ -34,8 +34,9 @@ impl WebGLRenderbuffer { pub fn maybe_new(global: GlobalRef, renderer: Sender) -> Option> { let (sender, receiver) = channel(); renderer.send(CanvasMsg::WebGL(CanvasWebGLMsg::CreateRenderbuffer(sender))).unwrap(); - receiver.recv().unwrap() - .map(|renderbuffer_id| WebGLRenderbuffer::new(global, renderer, *renderbuffer_id)) + + let result = receiver.recv().unwrap(); + result.map(|renderbuffer_id| WebGLRenderbuffer::new(global, renderer, *renderbuffer_id)) } pub fn new(global: GlobalRef, renderer: Sender, id: u32) -> Root { diff --git a/components/script/dom/webglrenderingcontext.rs b/components/script/dom/webglrenderingcontext.rs index c87b901d391..123c1e200c3 100644 --- a/components/script/dom/webglrenderingcontext.rs +++ b/components/script/dom/webglrenderingcontext.rs @@ -34,7 +34,7 @@ use std::sync::mpsc::{channel, Sender}; use util::str::DOMString; use offscreen_gl_context::GLContextAttributes; -pub const MAX_UNIFORM_AND_ATTRIBUTE_LEN : usize = 256; +pub const MAX_UNIFORM_AND_ATTRIBUTE_LEN: usize = 256; macro_rules! handle_potential_webgl_error { ($context:ident, $call:expr, $return_on_error:expr) => { @@ -54,7 +54,7 @@ pub struct WebGLRenderingContext { global: GlobalField, renderer: Sender, canvas: JS, - last_error: Cell, + last_error: Cell>, } impl WebGLRenderingContext { @@ -69,7 +69,7 @@ impl WebGLRenderingContext { reflector_: Reflector::new(), global: GlobalField::from_rooted(&global), renderer: chan, - last_error: Cell::new(WebGLError::NoError), + last_error: Cell::new(None), canvas: JS::from_ref(canvas), }) } @@ -134,15 +134,18 @@ impl<'a> WebGLRenderingContextMethods for &'a WebGLRenderingContext { // https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.3 fn GetError(self) -> u32 { - let error_code = match self.last_error.get() { - WebGLError::NoError => constants::NO_ERROR, - WebGLError::InvalidEnum => constants::INVALID_ENUM, - WebGLError::InvalidValue => constants::INVALID_VALUE, - WebGLError::InvalidOperation => constants::INVALID_OPERATION, - WebGLError::OutOfMemory => constants::OUT_OF_MEMORY, - WebGLError::ContextLost => constants::CONTEXT_LOST_WEBGL, + let error_code = if let Some(error) = self.last_error.get() { + match error { + WebGLError::InvalidEnum => constants::INVALID_ENUM, + WebGLError::InvalidValue => constants::INVALID_VALUE, + WebGLError::InvalidOperation => constants::INVALID_OPERATION, + WebGLError::OutOfMemory => constants::OUT_OF_MEMORY, + WebGLError::ContextLost => constants::CONTEXT_LOST_WEBGL, + } + } else { + constants::NO_ERROR }; - self.last_error.set(WebGLError::NoError); + self.last_error.set(None); error_code } @@ -231,9 +234,8 @@ impl<'a> WebGLRenderingContextMethods for &'a WebGLRenderingContext { framebuffer.bind(target) } else { // Bind the default framebuffer - self.renderer.send( - CanvasMsg::WebGL( - CanvasWebGLMsg::BindFramebuffer(target, WebGLFramebufferBindingRequest::Default))).unwrap() + let cmd = CanvasWebGLMsg::BindFramebuffer(target, WebGLFramebufferBindingRequest::Default); + self.renderer.send(CanvasMsg::WebGL(cmd)).unwrap(); } } @@ -502,8 +504,8 @@ impl<'a> WebGLRenderingContextHelpers for &'a WebGLRenderingContext { fn handle_webgl_error(&self, err: WebGLError) { // If an error has been detected no further errors must be // recorded until `getError` has been called - if self.last_error.get() == WebGLError::NoError { - self.last_error.set(err); + if self.last_error.get().is_none() { + self.last_error.set(Some(err)); } } } diff --git a/components/script/dom/webglshader.rs b/components/script/dom/webglshader.rs index e3c4389365b..f5d40b16699 100644 --- a/components/script/dom/webglshader.rs +++ b/components/script/dom/webglshader.rs @@ -21,10 +21,9 @@ pub struct WebGLShader { webgl_object: WebGLObject, id: u32, gl_type: u32, - // TODO(ecoal95): is RefCell ok? source: RefCell>, is_deleted: Cell, - // TODO(ecoal95): Valorate moving this to `WebGLObject` + // TODO(ecoal95): Evaluate moving this to `WebGLObject` renderer: Sender, } @@ -45,8 +44,9 @@ impl WebGLShader { shader_type: u32) -> Option> { let (sender, receiver) = channel(); renderer.send(CanvasMsg::WebGL(CanvasWebGLMsg::CreateShader(shader_type, sender))).unwrap(); - receiver.recv().unwrap() - .map(|shader_id| WebGLShader::new(global, renderer, *shader_id, shader_type)) + + let result = receiver.recv().unwrap(); + result.map(|shader_id| WebGLShader::new(global, renderer, *shader_id, shader_type)) } pub fn new(global: GlobalRef, @@ -81,13 +81,11 @@ impl<'a> WebGLShaderHelpers for &'a WebGLShader { // TODO(ecoal95): Validate shaders to be conforming to the WebGL spec /// glCompileShader fn compile(self) { - // NB(ecoal95): We intentionally don't check for source, we don't wan't - // to change gl error behavior self.renderer.send(CanvasMsg::WebGL(CanvasWebGLMsg::CompileShader(self.id))).unwrap() } /// Mark this shader as deleted (if it wasn't previously) - /// and delete it as if calling tr glDeleteShader. + /// and delete it as if calling glDeleteShader. fn delete(self) { if !self.is_deleted.get() { self.is_deleted.set(true); @@ -116,7 +114,7 @@ impl<'a> WebGLShaderHelpers for &'a WebGLShader { /// Get the shader source fn source(self) -> Option { - (*self.source.borrow()).clone() + self.source.borrow().clone() } /// glShaderSource diff --git a/components/script/dom/webgltexture.rs b/components/script/dom/webgltexture.rs index 111c0ab07c3..ab934a5f47d 100644 --- a/components/script/dom/webgltexture.rs +++ b/components/script/dom/webgltexture.rs @@ -34,8 +34,9 @@ impl WebGLTexture { pub fn maybe_new(global: GlobalRef, renderer: Sender) -> Option> { let (sender, receiver) = channel(); renderer.send(CanvasMsg::WebGL(CanvasWebGLMsg::CreateTexture(sender))).unwrap(); - receiver.recv().unwrap() - .map(|texture_id| WebGLTexture::new(global, renderer, *texture_id)) + + let result = receiver.recv().unwrap(); + result.map(|texture_id| WebGLTexture::new(global, renderer, *texture_id)) } pub fn new(global: GlobalRef, renderer: Sender, id: u32) -> Root {