mirror of
https://github.com/servo/servo.git
synced 2025-07-22 23:03:42 +01:00
webgl: Fix out-of-bounds readpixels handling.
This fixes the crash in read-pixels-pack-alignment (which was trying to read out of bounds). Fixes #13901
This commit is contained in:
parent
9a10666941
commit
5e5eb18b0b
5 changed files with 122 additions and 21 deletions
|
@ -33,6 +33,7 @@ pub struct WebGLFramebuffer {
|
|||
/// target can only be gl::FRAMEBUFFER at the moment
|
||||
target: Cell<Option<u32>>,
|
||||
is_deleted: Cell<bool>,
|
||||
size: Cell<Option<(i32, i32)>>,
|
||||
status: Cell<u32>,
|
||||
#[ignore_heap_size_of = "Defined in ipc-channel"]
|
||||
renderer: IpcSender<CanvasMsg>,
|
||||
|
@ -55,6 +56,7 @@ impl WebGLFramebuffer {
|
|||
target: Cell::new(None),
|
||||
is_deleted: Cell::new(false),
|
||||
renderer: renderer,
|
||||
size: Cell::new(None),
|
||||
status: Cell::new(constants::FRAMEBUFFER_UNSUPPORTED),
|
||||
color: DOMRefCell::new(None),
|
||||
depth: DOMRefCell::new(None),
|
||||
|
@ -110,6 +112,10 @@ impl WebGLFramebuffer {
|
|||
self.is_deleted.get()
|
||||
}
|
||||
|
||||
pub fn size(&self) -> Option<(i32, i32)> {
|
||||
self.size.get()
|
||||
}
|
||||
|
||||
fn update_status(&self) {
|
||||
let c = self.color.borrow();
|
||||
let z = self.depth.borrow();
|
||||
|
@ -165,6 +171,7 @@ impl WebGLFramebuffer {
|
|||
}
|
||||
}
|
||||
}
|
||||
self.size.set(fb_size);
|
||||
|
||||
if has_c || has_z || has_zs || has_s {
|
||||
self.status.set(constants::FRAMEBUFFER_COMPLETE);
|
||||
|
|
|
@ -283,6 +283,16 @@ impl WebGLRenderingContext {
|
|||
.unwrap();
|
||||
}
|
||||
|
||||
fn get_current_framebuffer_size(&self) -> Option<(i32, i32)> {
|
||||
match self.bound_framebuffer.get() {
|
||||
Some(fb) => return fb.size(),
|
||||
|
||||
// The window system framebuffer is bound
|
||||
None => return Some((self.DrawingBufferWidth(),
|
||||
self.DrawingBufferHeight())),
|
||||
}
|
||||
}
|
||||
|
||||
fn validate_stencil_actions(&self, action: u32) -> bool {
|
||||
match action {
|
||||
0 | constants::KEEP | constants::REPLACE | constants::INCR | constants::DECR |
|
||||
|
@ -649,6 +659,26 @@ impl WebGLRenderingContextMethods for WebGLRenderingContext {
|
|||
return object_binding_to_js_or_null!(cx, &self.bound_texture_2d),
|
||||
constants::TEXTURE_BINDING_CUBE_MAP =>
|
||||
return object_binding_to_js_or_null!(cx, &self.bound_texture_cube_map),
|
||||
|
||||
// In readPixels we currently support RGBA/UBYTE only. If
|
||||
// we wanted to support other formats, we could ask the
|
||||
// driver, but we would need to check for
|
||||
// GL_OES_read_format support (assuming an underlying GLES
|
||||
// driver. Desktop is happy to format convert for us).
|
||||
constants::IMPLEMENTATION_COLOR_READ_FORMAT => {
|
||||
if !self.validate_framebuffer_complete() {
|
||||
return NullValue();
|
||||
} else {
|
||||
return Int32Value(constants::RGBA as i32);
|
||||
}
|
||||
}
|
||||
constants::IMPLEMENTATION_COLOR_READ_TYPE => {
|
||||
if !self.validate_framebuffer_complete() {
|
||||
return NullValue();
|
||||
} else {
|
||||
return Int32Value(constants::UNSIGNED_BYTE as i32);
|
||||
}
|
||||
}
|
||||
_ => {}
|
||||
}
|
||||
|
||||
|
@ -1777,10 +1807,80 @@ impl WebGLRenderingContextMethods for WebGLRenderingContext {
|
|||
_ => return Ok(self.webgl_error(InvalidOperation)),
|
||||
}
|
||||
|
||||
// From the WebGL specification, 5.14.12 Reading back pixels
|
||||
//
|
||||
// "Only two combinations of format and type are
|
||||
// accepted. The first is format RGBA and type
|
||||
// UNSIGNED_BYTE. The second is an implementation-chosen
|
||||
// format. The values of format and type for this format
|
||||
// may be determined by calling getParameter with the
|
||||
// symbolic constants IMPLEMENTATION_COLOR_READ_FORMAT
|
||||
// and IMPLEMENTATION_COLOR_READ_TYPE, respectively. The
|
||||
// implementation-chosen format may vary depending on the
|
||||
// format of the currently bound rendering
|
||||
// surface. Unsupported combinations of format and type
|
||||
// will generate an INVALID_OPERATION error."
|
||||
//
|
||||
// To avoid having to support general format packing math, we
|
||||
// always report RGBA/UNSIGNED_BYTE as our only supported
|
||||
// format.
|
||||
if format != constants::RGBA || pixel_type != constants::UNSIGNED_BYTE {
|
||||
return Ok(self.webgl_error(InvalidOperation));
|
||||
}
|
||||
let cpp = 4;
|
||||
|
||||
// "If pixels is non-null, but is not large enough to
|
||||
// retrieve all of the pixels in the specified rectangle
|
||||
// taking into account pixel store modes, an
|
||||
// INVALID_OPERATION error is generated."
|
||||
let stride = match width.checked_mul(cpp) {
|
||||
Some(stride) => stride,
|
||||
_ => return Ok(self.webgl_error(InvalidOperation)),
|
||||
};
|
||||
|
||||
match height.checked_mul(stride) {
|
||||
Some(size) if size <= data.len() as i32 => {}
|
||||
_ => return Ok(self.webgl_error(InvalidOperation)),
|
||||
}
|
||||
|
||||
// "For any pixel lying outside the frame buffer, the
|
||||
// corresponding destination buffer range remains
|
||||
// untouched; see Reading Pixels Outside the
|
||||
// Framebuffer."
|
||||
let mut x = x;
|
||||
let mut y = y;
|
||||
let mut width = width;
|
||||
let mut height = height;
|
||||
let mut dst_offset = 0;
|
||||
|
||||
if x < 0 {
|
||||
dst_offset += cpp * -x;
|
||||
width += x;
|
||||
x = 0;
|
||||
}
|
||||
|
||||
if y < 0 {
|
||||
dst_offset += stride * -y;
|
||||
height += y;
|
||||
y = 0;
|
||||
}
|
||||
|
||||
if width < 0 || height < 0 {
|
||||
return Ok(self.webgl_error(InvalidValue));
|
||||
}
|
||||
|
||||
match self.get_current_framebuffer_size() {
|
||||
Some((fb_width, fb_height)) => {
|
||||
if x + width > fb_width {
|
||||
width = fb_width - x;
|
||||
}
|
||||
if y + height > fb_height {
|
||||
height = fb_height - y;
|
||||
}
|
||||
}
|
||||
_ => return Ok(self.webgl_error(InvalidOperation)),
|
||||
};
|
||||
|
||||
let (sender, receiver) = ipc::channel().unwrap();
|
||||
self.ipc_renderer
|
||||
.send(CanvasMsg::WebGL(WebGLCommand::ReadPixels(x, y, width, height, format, pixel_type, sender)))
|
||||
|
@ -1788,12 +1888,11 @@ impl WebGLRenderingContextMethods for WebGLRenderingContext {
|
|||
|
||||
let result = receiver.recv().unwrap();
|
||||
|
||||
if result.len() > data.len() {
|
||||
return Ok(self.webgl_error(InvalidOperation));
|
||||
}
|
||||
|
||||
for i in 0..result.len() {
|
||||
data[i] = result[i]
|
||||
for i in 0..height {
|
||||
for j in 0..(width * cpp) {
|
||||
data[(dst_offset + i * stride + j) as usize] =
|
||||
result[(i * width * cpp + j) as usize];
|
||||
}
|
||||
}
|
||||
|
||||
Ok(())
|
||||
|
|
|
@ -174,9 +174,6 @@
|
|||
[WebGL test #171: gl.getFramebufferAttachmentParameter(gl.FRAMEBUFFER, gl.COLOR_ATTACHMENT0, gl.FRAMEBUFFER_ATTACHMENT_OBJECT_TYPE) threw exception TypeError: gl.getFramebufferAttachmentParameter is not a function]
|
||||
expected: FAIL
|
||||
|
||||
[WebGL test #187: gl.checkFramebufferStatus(gl.FRAMEBUFFER) should not be 36053.]
|
||||
expected: FAIL
|
||||
|
||||
[WebGL test #196: gl.getFramebufferAttachmentParameter(gl.FRAMEBUFFER, gl.COLOR_ATTACHMENT0, gl.FRAMEBUFFER_ATTACHMENT_OBJECT_NAME) should be [object WebGLTexture\]. Threw exception TypeError: gl.getFramebufferAttachmentParameter is not a function]
|
||||
expected: FAIL
|
||||
|
||||
|
|
|
@ -1,8 +0,0 @@
|
|||
[readPixelsBadArgs.html]
|
||||
type: testharness
|
||||
expected:
|
||||
if os == "linux": CRASH
|
||||
if os == "mac": TIMEOUT
|
||||
[WebGL test #0: testReadPixels]
|
||||
expected: FAIL
|
||||
|
|
@ -1,8 +1,14 @@
|
|||
[read-pixels-pack-alignment.html]
|
||||
type: testharness
|
||||
expected:
|
||||
if os == "linux": CRASH
|
||||
if os == "mac": TIMEOUT
|
||||
[WebGL test #3: successfullyParsed should be true (of type boolean). Was undefined (of type undefined).]
|
||||
[WebGL test #17: pixel should be 255,102,0,255. Was 0,0,0,0.]
|
||||
expected: FAIL
|
||||
|
||||
[WebGL test #33: pixel should be 255,102,0,255. Was 0,0,0,0.]
|
||||
expected: FAIL
|
||||
|
||||
[WebGL test #53: pixel should be 255,102,0,255. Was 0,0,0,0.]
|
||||
expected: FAIL
|
||||
|
||||
[WebGL test #61: pixel should be 255,102,0,255. Was 0,0,0,0.]
|
||||
expected: FAIL
|
||||
|
||||
|
|
Loading…
Add table
Add a link
Reference in a new issue