Skip to content
Permalink
Browse files

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
  • Loading branch information
anholt committed Nov 5, 2016
1 parent 9a10666 commit 5e5eb18b0b48f008768487dd9dc0fd1c8b1adf18
@@ -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,23 +1807,92 @@ 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)))
.unwrap();

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

This file was deleted.

@@ -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

0 comments on commit 5e5eb18

Please sign in to comment.
You can’t perform that action at this time.