Skip to content

Commit

Permalink
Auto merge of #20369 - gootorov:move-validation, r=emilio
Browse files Browse the repository at this point in the history 
Move WebGL validation

This moves validation from `canvas/webgl_thread` to `dom/webglrenderingcontext` for consistency and speed and simplifies it where it is possible.

---
<!-- 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
- [x] These changes do not require tests because: refactoring

<!-- 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. -->

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/20369)
<!-- Reviewable:end -->
  • Loading branch information
bors-servo committed Mar 20, 2018
2 parents 730bd5e + bdd53f3 commit f92f080
Show file tree
Hide file tree
Showing 3 changed files with 78 additions and 90 deletions.
51 changes: 8 additions & 43 deletions components/canvas/webgl_thread.rs
View file
Expand Up @@ -1066,21 +1066,8 @@ impl WebGLImpl {
fn get_tex_parameter(gl: &gl::Gl,
target: u32,
pname: u32,
chan: WebGLSender<WebGLResult<WebGLParameter>> ) {
let result = match pname {
gl::TEXTURE_MAG_FILTER |
gl::TEXTURE_MIN_FILTER |
gl::TEXTURE_WRAP_S |
gl::TEXTURE_WRAP_T => {
let parameter = gl.get_tex_parameter_iv(target, pname);
if parameter == 0 {
Ok(WebGLParameter::Invalid)
} else {
Ok(WebGLParameter::Int(parameter))
}
}
_ => Err(WebGLError::InvalidEnum)
};
chan: WebGLSender<i32> ) {
let result = gl.get_tex_parameter_iv(target, pname);
chan.send(result).unwrap();
}

Expand Down Expand Up @@ -1117,25 +1104,16 @@ impl WebGLImpl {
fn vertex_attrib_offset(gl: &gl::Gl,
index: u32,
pname: u32,
chan: WebGLSender<WebGLResult<isize>>) {
let result = match pname {
gl::VERTEX_ATTRIB_ARRAY_POINTER => Ok(gl.get_vertex_attrib_pointer_v(index, pname)),
_ => Err(WebGLError::InvalidEnum),
};

chan: WebGLSender<isize>) {
let result = gl.get_vertex_attrib_pointer_v(index, pname);
chan.send(result).unwrap();
}

fn buffer_parameter(gl: &gl::Gl,
target: u32,
param_id: u32,
chan: WebGLSender<WebGLResult<WebGLParameter>>) {
let result = match param_id {
gl::BUFFER_SIZE |
gl::BUFFER_USAGE =>
Ok(WebGLParameter::Int(gl.get_buffer_parameter_iv(target, param_id))),
_ => Err(WebGLError::InvalidEnum),
};
chan: WebGLSender<i32>) {
let result = gl.get_buffer_parameter_iv(target, param_id);

chan.send(result).unwrap();
}
Expand Down Expand Up @@ -1178,21 +1156,8 @@ impl WebGLImpl {
fn shader_precision_format(gl: &gl::Gl,
shader_type: u32,
precision_type: u32,
chan: WebGLSender<WebGLResult<(i32, i32, i32)>>) {
let result = match precision_type {
gl::LOW_FLOAT |
gl::MEDIUM_FLOAT |
gl::HIGH_FLOAT |
gl::LOW_INT |
gl::MEDIUM_INT |
gl::HIGH_INT => {
Ok(gl.get_shader_precision_format(shader_type, precision_type))
},
_=> {
Err(WebGLError::InvalidEnum)
}
};

chan: WebGLSender<(i32, i32, i32)>) {
let result = gl.get_shader_precision_format(shader_type, precision_type);
chan.send(result).unwrap();
}

Expand Down
8 changes: 4 additions & 4 deletions components/canvas_traits/webgl.rs
View file
Expand Up @@ -204,19 +204,19 @@ pub enum WebGLCommand {
EnableVertexAttribArray(u32),
FramebufferRenderbuffer(u32, u32, u32, Option<WebGLRenderbufferId>),
FramebufferTexture2D(u32, u32, u32, Option<WebGLTextureId>, i32),
GetBufferParameter(u32, u32, WebGLSender<WebGLResult<WebGLParameter>>),
GetBufferParameter(u32, u32, WebGLSender<i32>),
GetExtensions(WebGLSender<String>),
GetParameter(u32, WebGLSender<WebGLResult<WebGLParameter>>),
GetTexParameter(u32, u32, WebGLSender<WebGLResult<WebGLParameter>>),
GetTexParameter(u32, u32, WebGLSender<i32>),
GetProgramParameter(WebGLProgramId, u32, WebGLSender<WebGLResult<WebGLParameter>>),
GetShaderParameter(WebGLShaderId, u32, WebGLSender<WebGLResult<WebGLParameter>>),
GetShaderPrecisionFormat(u32, u32, WebGLSender<WebGLResult<(i32, i32, i32)>>),
GetShaderPrecisionFormat(u32, u32, WebGLSender<(i32, i32, i32)>),
GetActiveAttrib(WebGLProgramId, u32, WebGLSender<WebGLResult<(i32, u32, String)>>),
GetActiveUniform(WebGLProgramId, u32, WebGLSender<WebGLResult<(i32, u32, String)>>),
GetAttribLocation(WebGLProgramId, String, WebGLSender<Option<i32>>),
GetUniformLocation(WebGLProgramId, String, WebGLSender<Option<i32>>),
GetVertexAttrib(u32, u32, WebGLSender<WebGLResult<WebGLParameter>>),
GetVertexAttribOffset(u32, u32, WebGLSender<WebGLResult<isize>>),
GetVertexAttribOffset(u32, u32, WebGLSender<isize>),
GetShaderInfoLog(WebGLShaderId, WebGLSender<String>),
GetProgramInfoLog(WebGLProgramId, WebGLSender<String>),
PolygonOffset(f32, f32),
Expand Down
109 changes: 66 additions & 43 deletions components/script/dom/webglrenderingcontext.rs
View file
Expand Up @@ -1246,17 +1246,21 @@ impl WebGLRenderingContextMethods for WebGLRenderingContext {
#[allow(unsafe_code)]
// https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.5
unsafe fn GetBufferParameter(&self, _cx: *mut JSContext, target: u32, parameter: u32) -> JSVal {
let parameter_matches = match parameter {
constants::BUFFER_SIZE |
constants::BUFFER_USAGE => true,
_ => false,
};

if !parameter_matches {
self.webgl_error(InvalidEnum);
return NullValue();
}

let (sender, receiver) = webgl_channel().unwrap();
self.send_command(WebGLCommand::GetBufferParameter(target, parameter, sender));

match handle_potential_webgl_error!(self, receiver.recv().unwrap(), WebGLParameter::Invalid) {
WebGLParameter::Int(val) => Int32Value(val),
WebGLParameter::Bool(_) => panic!("Buffer parameter should not be bool"),
WebGLParameter::Float(_) => panic!("Buffer parameter should not be float"),
WebGLParameter::FloatArray(_) => panic!("Buffer parameter should not be float array"),
WebGLParameter::String(_) => panic!("Buffer parameter should not be string"),
WebGLParameter::Invalid => NullValue(),
}
Int32Value(receiver.recv().unwrap())
}

#[allow(unsafe_code)]
Expand Down Expand Up @@ -1342,31 +1346,39 @@ impl WebGLRenderingContextMethods for WebGLRenderingContext {
#[allow(unsafe_code)]
// https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.8
unsafe fn GetTexParameter(&self, _cx: *mut JSContext, target: u32, pname: u32) -> JSVal {
let texture = match target {
let target_matches = match target {
constants::TEXTURE_2D |
constants::TEXTURE_CUBE_MAP => self.bound_texture(target),
constants::TEXTURE_CUBE_MAP => true,
_ => false,
};

let pname_matches = match pname {
constants::TEXTURE_MAG_FILTER |
constants::TEXTURE_MIN_FILTER |
constants::TEXTURE_WRAP_S |
constants::TEXTURE_WRAP_T => true,
_ => false,
};

if !target_matches || !pname_matches {
self.webgl_error(InvalidEnum);
return NullValue();
}

if self.bound_texture(target).is_none() {
self.webgl_error(InvalidOperation);
return NullValue();
}

let (sender, receiver) = webgl_channel().unwrap();
self.send_command(WebGLCommand::GetTexParameter(target, pname, sender));

match receiver.recv().unwrap() {
value if value != 0 => Int32Value(value),
_ => {
self.webgl_error(InvalidEnum);
return NullValue();
}
};
if texture.is_some() {
let (sender, receiver) = webgl_channel().unwrap();
self.send_command(WebGLCommand::GetTexParameter(target, pname, sender));
match handle_potential_webgl_error!(self, receiver.recv().unwrap(), WebGLParameter::Invalid) {
WebGLParameter::Int(val) => Int32Value(val),
WebGLParameter::Bool(_) => panic!("Texture parameter should not be bool"),
WebGLParameter::Float(_) => panic!("Texture parameter should not be float"),
WebGLParameter::FloatArray(_) => panic!("Texture parameter should not be float array"),
WebGLParameter::String(_) => panic!("Texture parameter should not be string"),
WebGLParameter::Invalid => {
self.webgl_error(InvalidEnum);
NullValue()
}
NullValue()
}
} else {
self.webgl_error(InvalidOperation);
NullValue()
}
}

Expand Down Expand Up @@ -2345,24 +2357,31 @@ impl WebGLRenderingContextMethods for WebGLRenderingContext {
}

// https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.9
fn GetShaderPrecisionFormat(&self,
shader_type: u32,
precision_type: u32)
-> Option<DomRoot<WebGLShaderPrecisionFormat>> {
fn GetShaderPrecisionFormat(
&self,
shader_type: u32,
precision_type: u32
) -> Option<DomRoot<WebGLShaderPrecisionFormat>> {
match precision_type {
constants::LOW_FLOAT |
constants::MEDIUM_FLOAT |
constants::HIGH_FLOAT |
constants::LOW_INT |
constants::MEDIUM_INT |
constants::HIGH_INT => (),
_ => {
self.webgl_error(InvalidEnum);
return None;
},
}

let (sender, receiver) = webgl_channel().unwrap();
self.send_command(WebGLCommand::GetShaderPrecisionFormat(shader_type,
precision_type,
sender));

match receiver.recv().unwrap() {
Ok((range_min, range_max, precision)) => {
Some(WebGLShaderPrecisionFormat::new(self.global().as_window(), range_min, range_max, precision))
},
Err(error) => {
self.webgl_error(error);
None
}
}
let (range_min, range_max, precision) = receiver.recv().unwrap();
Some(WebGLShaderPrecisionFormat::new(self.global().as_window(), range_min, range_max, precision))
}

// https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.10
Expand Down Expand Up @@ -2413,10 +2432,14 @@ impl WebGLRenderingContextMethods for WebGLRenderingContext {

// https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.10
fn GetVertexAttribOffset(&self, index: u32, pname: u32) -> i64 {
if pname != constants::VERTEX_ATTRIB_ARRAY_POINTER {
self.webgl_error(InvalidEnum);
return 0;
}
let (sender, receiver) = webgl_channel().unwrap();
self.send_command(WebGLCommand::GetVertexAttribOffset(index, pname, sender));

handle_potential_webgl_error!(self, receiver.recv().unwrap(), 0) as i64
receiver.recv().unwrap() as i64
}

// https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.3
Expand Down

0 comments on commit f92f080

Please sign in to comment.