Auto merge of #21461 - jdm:webgltmp2, r=nox

Various webgl fixes for framebuffer attachment test

These changes resolve all panics on macOS when running framebuffer-object-attachment.html in headless and headful testing.

---
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] There are tests for these changes OR
- [x] Fixes #13710. Fixes #20570.

<!-- 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/21461)
<!-- Reviewable:end -->
This commit is contained in:
bors-servo 2018-09-12 16:36:08 -04:00 committed by GitHub
commit 26745b2741
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
12 changed files with 655 additions and 177 deletions

View file

@ -18,6 +18,12 @@ use dom::webgltexture::WebGLTexture;
use dom_struct::dom_struct;
use std::cell::Cell;
pub enum CompleteForRendering {
Complete,
Incomplete,
MissingColorAttachment,
}
#[must_root]
#[derive(Clone, JSTraceable, MallocSizeOf)]
enum WebGLFramebufferAttachment {
@ -25,6 +31,31 @@ enum WebGLFramebufferAttachment {
Texture { texture: Dom<WebGLTexture>, level: i32 },
}
impl WebGLFramebufferAttachment {
fn needs_initialization(&self) -> bool {
match *self {
WebGLFramebufferAttachment::Renderbuffer(ref r) => !r.is_initialized(),
WebGLFramebufferAttachment::Texture { .. } => false,
}
}
fn mark_initialized(&self) {
match *self {
WebGLFramebufferAttachment::Renderbuffer(ref r) => r.mark_initialized(),
WebGLFramebufferAttachment::Texture { .. } => ()
}
}
fn root(&self) -> WebGLFramebufferAttachmentRoot {
match *self {
WebGLFramebufferAttachment::Renderbuffer(ref rb) =>
WebGLFramebufferAttachmentRoot::Renderbuffer(DomRoot::from_ref(&rb)),
WebGLFramebufferAttachment::Texture { ref texture, .. } =>
WebGLFramebufferAttachmentRoot::Texture(DomRoot::from_ref(&texture)),
}
}
}
#[derive(Clone, JSTraceable, MallocSizeOf)]
pub enum WebGLFramebufferAttachmentRoot {
Renderbuffer(DomRoot<WebGLRenderbuffer>),
@ -46,6 +77,7 @@ pub struct WebGLFramebuffer {
depth: DomRefCell<Option<WebGLFramebufferAttachment>>,
stencil: DomRefCell<Option<WebGLFramebufferAttachment>>,
depthstencil: DomRefCell<Option<WebGLFramebufferAttachment>>,
is_initialized: Cell<bool>,
}
impl WebGLFramebuffer {
@ -56,11 +88,12 @@ impl WebGLFramebuffer {
target: Cell::new(None),
is_deleted: Cell::new(false),
size: Cell::new(None),
status: Cell::new(constants::FRAMEBUFFER_UNSUPPORTED),
status: Cell::new(constants::FRAMEBUFFER_INCOMPLETE_MISSING_ATTACHMENT),
color: DomRefCell::new(None),
depth: DomRefCell::new(None),
stencil: DomRefCell::new(None),
depthstencil: DomRefCell::new(None),
is_initialized: Cell::new(false),
}
}
@ -127,6 +160,12 @@ impl WebGLFramebuffer {
let has_s = s.is_some();
let has_zs = zs.is_some();
let attachments = [&*c, &*z, &*s, &*zs];
let attachment_constraints = [
&[constants::RGBA4, constants::RGB5_A1, constants::RGB565, constants::RGBA][..],
&[constants::DEPTH_COMPONENT16][..],
&[constants::STENCIL_INDEX8][..],
&[constants::DEPTH_STENCIL][..],
];
// From the WebGL spec, 6.6 ("Framebuffer Object Attachments"):
//
@ -148,17 +187,18 @@ impl WebGLFramebuffer {
}
let mut fb_size = None;
for attachment in &attachments {
for (attachment, constraints) in attachments.iter().zip(&attachment_constraints) {
// Get the size of this attachment.
let size = match **attachment {
let (format, size) = match **attachment {
Some(WebGLFramebufferAttachment::Renderbuffer(ref att_rb)) => {
att_rb.size()
(Some(att_rb.internal_format()), att_rb.size())
}
Some(WebGLFramebufferAttachment::Texture { texture: ref att_tex, level } ) => {
let info = att_tex.image_info_at_face(0, level as u32);
Some((info.width() as i32, info.height() as i32))
(info.internal_format().map(|t| t.as_gl_constant()),
Some((info.width() as i32, info.height() as i32)))
}
None => None,
None => (None, None),
};
// Make sure that, if we've found any other attachment,
@ -171,6 +211,13 @@ impl WebGLFramebuffer {
fb_size = size;
}
}
if let Some(format) = format {
if constraints.iter().all(|c| *c != format) {
self.status.set(constants::FRAMEBUFFER_INCOMPLETE_ATTACHMENT);
return;
}
}
}
self.size.set(fb_size);
@ -181,7 +228,7 @@ impl WebGLFramebuffer {
self.status.set(constants::FRAMEBUFFER_INCOMPLETE_ATTACHMENT);
}
} else {
self.status.set(constants::FRAMEBUFFER_UNSUPPORTED);
self.status.set(constants::FRAMEBUFFER_INCOMPLETE_MISSING_ATTACHMENT);
}
}
@ -189,25 +236,52 @@ impl WebGLFramebuffer {
return self.status.get();
}
pub fn check_status_for_rendering(&self) -> CompleteForRendering {
let result = self.check_status();
if result != constants::FRAMEBUFFER_COMPLETE {
return CompleteForRendering::Incomplete;
}
if self.color.borrow().is_none() {
return CompleteForRendering::MissingColorAttachment;
}
if !self.is_initialized.get() {
let attachments = [
(&self.color, constants::COLOR_BUFFER_BIT),
(&self.depth, constants::DEPTH_BUFFER_BIT),
(&self.stencil, constants::STENCIL_BUFFER_BIT),
(&self.depthstencil, constants::DEPTH_BUFFER_BIT | constants::STENCIL_BUFFER_BIT)
];
let mut clear_bits = 0;
for &(attachment, bits) in &attachments {
if let Some(ref att) = *attachment.borrow() {
if att.needs_initialization() {
att.mark_initialized();
clear_bits |= bits;
}
}
}
self.upcast::<WebGLObject>().context().initialize_framebuffer(clear_bits);
self.is_initialized.set(true);
}
CompleteForRendering::Complete
}
pub fn renderbuffer(&self, attachment: u32, rb: Option<&WebGLRenderbuffer>) -> WebGLResult<()> {
let binding = match attachment {
constants::COLOR_ATTACHMENT0 => &self.color,
constants::DEPTH_ATTACHMENT => &self.depth,
constants::STENCIL_ATTACHMENT => &self.stencil,
constants::DEPTH_STENCIL_ATTACHMENT => &self.depthstencil,
_ => return Err(WebGLError::InvalidEnum),
};
let binding = self.attachment_binding(attachment).ok_or(WebGLError::InvalidEnum)?;
let rb_id = match rb {
Some(rb) => {
if !rb.ever_bound() {
return Err(WebGLError::InvalidOperation);
}
*binding.borrow_mut() = Some(WebGLFramebufferAttachment::Renderbuffer(Dom::from_ref(rb)));
Some(rb.id())
}
_ => {
*binding.borrow_mut() = None;
None
}
_ => None
};
self.upcast::<WebGLObject>().context().send_command(
@ -219,38 +293,91 @@ impl WebGLFramebuffer {
),
);
if rb.is_none() {
self.detach_binding(binding, attachment);
}
self.update_status();
self.is_initialized.set(false);
Ok(())
}
pub fn attachment(&self, attachment: u32) -> Option<WebGLFramebufferAttachmentRoot> {
let binding = match attachment {
constants::COLOR_ATTACHMENT0 => &self.color,
constants::DEPTH_ATTACHMENT => &self.depth,
constants::STENCIL_ATTACHMENT => &self.stencil,
constants::DEPTH_STENCIL_ATTACHMENT => &self.depthstencil,
_ => return None,
fn detach_binding(
&self,
binding: &DomRefCell<Option<WebGLFramebufferAttachment>>,
attachment: u32,
) {
*binding.borrow_mut() = None;
if INTERESTING_ATTACHMENT_POINTS.contains(&attachment) {
self.reattach_depth_stencil();
}
}
fn attachment_binding(
&self,
attachment: u32
) -> Option<&DomRefCell<Option<WebGLFramebufferAttachment>>>
{
match attachment {
constants::COLOR_ATTACHMENT0 => Some(&self.color),
constants::DEPTH_ATTACHMENT => Some(&self.depth),
constants::STENCIL_ATTACHMENT => Some(&self.stencil),
constants::DEPTH_STENCIL_ATTACHMENT => Some(&self.depthstencil),
_ => None
}
}
fn reattach_depth_stencil(&self) {
let reattach = |attachment: &WebGLFramebufferAttachment, attachment_point| {
let context = self.upcast::<WebGLObject>().context();
match *attachment {
WebGLFramebufferAttachment::Renderbuffer(ref rb) => {
context.send_command(
WebGLCommand::FramebufferRenderbuffer(
constants::FRAMEBUFFER,
attachment_point,
constants::RENDERBUFFER,
Some(rb.id())
)
);
}
WebGLFramebufferAttachment::Texture { ref texture, level } => {
context.send_command(
WebGLCommand::FramebufferTexture2D(
constants::FRAMEBUFFER,
attachment_point,
texture.target().expect("missing texture target"),
Some(texture.id()),
level
)
);
}
}
};
binding.borrow().as_ref().map(|bin| {
match bin {
&WebGLFramebufferAttachment::Renderbuffer(ref rb) =>
WebGLFramebufferAttachmentRoot::Renderbuffer(DomRoot::from_ref(&rb)),
&WebGLFramebufferAttachment::Texture { ref texture, .. } =>
WebGLFramebufferAttachmentRoot::Texture(DomRoot::from_ref(&texture)),
}
})
// Since the DEPTH_STENCIL attachment causes both the DEPTH and STENCIL
// attachments to be overwritten, we need to ensure that we reattach
// the DEPTH and STENCIL attachments when any of those attachments
// is cleared.
if let Some(ref depth) = *self.depth.borrow() {
reattach(depth, constants::DEPTH_ATTACHMENT);
}
if let Some(ref stencil) = *self.stencil.borrow() {
reattach(stencil, constants::STENCIL_ATTACHMENT);
}
if let Some(ref depth_stencil) = *self.depthstencil.borrow() {
reattach(depth_stencil, constants::DEPTH_STENCIL_ATTACHMENT);
}
}
pub fn attachment(&self, attachment: u32) -> Option<WebGLFramebufferAttachmentRoot> {
let binding = self.attachment_binding(attachment)?;
binding.borrow().as_ref().map(WebGLFramebufferAttachment::root)
}
pub fn texture2d(&self, attachment: u32, textarget: u32, texture: Option<&WebGLTexture>,
level: i32) -> WebGLResult<()> {
let binding = match attachment {
constants::COLOR_ATTACHMENT0 => &self.color,
constants::DEPTH_ATTACHMENT => &self.depth,
constants::STENCIL_ATTACHMENT => &self.stencil,
constants::DEPTH_STENCIL_ATTACHMENT => &self.depthstencil,
_ => return Err(WebGLError::InvalidEnum),
};
let binding = self.attachment_binding(attachment).ok_or(WebGLError::InvalidEnum)?;
let tex_id = match texture {
// Note, from the GLES 2.0.25 spec, page 113:
@ -304,7 +431,6 @@ impl WebGLFramebuffer {
}
_ => {
*binding.borrow_mut() = None;
None
}
};
@ -319,19 +445,26 @@ impl WebGLFramebuffer {
),
);
if texture.is_none() {
self.detach_binding(binding, attachment);
}
self.update_status();
self.is_initialized.set(false);
Ok(())
}
fn with_matching_renderbuffers<F>(&self, rb: &WebGLRenderbuffer, mut closure: F)
where F: FnMut(&DomRefCell<Option<WebGLFramebufferAttachment>>)
where F: FnMut(&DomRefCell<Option<WebGLFramebufferAttachment>>, u32)
{
let attachments = [&self.color,
&self.depth,
&self.stencil,
&self.depthstencil];
let attachments = [
(&self.color, constants::COLOR_ATTACHMENT0),
(&self.depth, constants::DEPTH_ATTACHMENT),
(&self.stencil, constants::STENCIL_ATTACHMENT),
(&self.depthstencil, constants::DEPTH_STENCIL_ATTACHMENT)
];
for attachment in &attachments {
for (attachment, name) in &attachments {
let matched = {
match *attachment.borrow() {
Some(WebGLFramebufferAttachment::Renderbuffer(ref att_rb))
@ -341,20 +474,22 @@ impl WebGLFramebuffer {
};
if matched {
closure(attachment);
closure(attachment, *name);
}
}
}
fn with_matching_textures<F>(&self, texture: &WebGLTexture, mut closure: F)
where F: FnMut(&DomRefCell<Option<WebGLFramebufferAttachment>>)
where F: FnMut(&DomRefCell<Option<WebGLFramebufferAttachment>>, u32)
{
let attachments = [&self.color,
&self.depth,
&self.stencil,
&self.depthstencil];
let attachments = [
(&self.color, constants::COLOR_ATTACHMENT0),
(&self.depth, constants::DEPTH_ATTACHMENT),
(&self.stencil, constants::STENCIL_ATTACHMENT),
(&self.depthstencil, constants::DEPTH_STENCIL_ATTACHMENT)
];
for attachment in &attachments {
for (attachment, name) in &attachments {
let matched = {
match *attachment.borrow() {
Some(WebGLFramebufferAttachment::Texture { texture: ref att_texture, .. })
@ -364,33 +499,45 @@ impl WebGLFramebuffer {
};
if matched {
closure(attachment);
closure(attachment, *name);
}
}
}
pub fn detach_renderbuffer(&self, rb: &WebGLRenderbuffer) {
self.with_matching_renderbuffers(rb, |att| {
let mut depth_or_stencil_updated = false;
self.with_matching_renderbuffers(rb, |att, name| {
depth_or_stencil_updated |= INTERESTING_ATTACHMENT_POINTS.contains(&name);
*att.borrow_mut() = None;
self.update_status();
});
if depth_or_stencil_updated {
self.reattach_depth_stencil();
}
}
pub fn detach_texture(&self, texture: &WebGLTexture) {
self.with_matching_textures(texture, |att| {
let mut depth_or_stencil_updated = false;
self.with_matching_textures(texture, |att, name| {
depth_or_stencil_updated |= INTERESTING_ATTACHMENT_POINTS.contains(&name);
*att.borrow_mut() = None;
self.update_status();
});
if depth_or_stencil_updated {
self.reattach_depth_stencil();
}
}
pub fn invalidate_renderbuffer(&self, rb: &WebGLRenderbuffer) {
self.with_matching_renderbuffers(rb, |_att| {
self.with_matching_renderbuffers(rb, |_att, _| {
self.update_status();
});
}
pub fn invalidate_texture(&self, texture: &WebGLTexture) {
self.with_matching_textures(texture, |_att| {
self.with_matching_textures(texture, |_att, _name| {
self.update_status();
});
}
@ -405,3 +552,9 @@ impl Drop for WebGLFramebuffer {
self.delete();
}
}
static INTERESTING_ATTACHMENT_POINTS: &[u32] = &[
constants::DEPTH_ATTACHMENT,
constants::STENCIL_ATTACHMENT,
constants::DEPTH_STENCIL_ATTACHMENT,
];

View file

@ -23,6 +23,7 @@ pub struct WebGLRenderbuffer {
is_deleted: Cell<bool>,
size: Cell<Option<(i32, i32)>>,
internal_format: Cell<Option<u32>>,
is_initialized: Cell<bool>,
}
impl WebGLRenderbuffer {
@ -34,6 +35,7 @@ impl WebGLRenderbuffer {
is_deleted: Cell::new(false),
internal_format: Cell::new(None),
size: Cell::new(None),
is_initialized: Cell::new(false),
}
}
@ -66,6 +68,14 @@ impl WebGLRenderbuffer {
self.internal_format.get().unwrap_or(constants::RGBA4)
}
pub fn mark_initialized(&self) {
self.is_initialized.set(true);
}
pub fn is_initialized(&self) -> bool {
self.is_initialized.get()
}
pub fn bind(&self, target: u32) {
self.ever_bound.set(true);
self.upcast::<WebGLObject>()
@ -76,6 +86,22 @@ impl WebGLRenderbuffer {
pub fn delete(&self) {
if !self.is_deleted.get() {
self.is_deleted.set(true);
/*
If a renderbuffer object is deleted while its image is attached to the currently
bound framebuffer, then it is as if FramebufferRenderbuffer had been called, with
a renderbuffer of 0, for each attachment point to which this image was attached
in the currently bound framebuffer.
- GLES 2.0, 4.4.3, "Attaching Renderbuffer Images to a Framebuffer"
*/
let currently_bound_framebuffer =
self.upcast::<WebGLObject>()
.context()
.bound_framebuffer();
if let Some(fb) = currently_bound_framebuffer {
fb.detach_renderbuffer(self);
}
self.upcast::<WebGLObject>()
.context()
.send_command(WebGLCommand::DeleteRenderbuffer(self.id));

View file

@ -39,7 +39,7 @@ use dom::webgl_validations::types::{TexDataType, TexFormat, TexImageTarget};
use dom::webglactiveinfo::WebGLActiveInfo;
use dom::webglbuffer::WebGLBuffer;
use dom::webglcontextevent::WebGLContextEvent;
use dom::webglframebuffer::{WebGLFramebuffer, WebGLFramebufferAttachmentRoot};
use dom::webglframebuffer::{WebGLFramebuffer, WebGLFramebufferAttachmentRoot, CompleteForRendering};
use dom::webglobject::WebGLObject;
use dom::webglprogram::WebGLProgram;
use dom::webglrenderbuffer::WebGLRenderbuffer;
@ -339,10 +339,14 @@ impl WebGLRenderingContext {
// this: clear() and getParameter(IMPLEMENTATION_COLOR_READ_*).
fn validate_framebuffer(&self) -> WebGLResult<()> {
match self.bound_framebuffer.get() {
Some(ref fb) if fb.check_status() != constants::FRAMEBUFFER_COMPLETE => {
Err(InvalidFramebufferOperation)
Some(fb) => match fb.check_status_for_rendering() {
CompleteForRendering::Complete => Ok(()),
CompleteForRendering::Incomplete =>
Err(InvalidFramebufferOperation),
CompleteForRendering::MissingColorAttachment =>
Err(InvalidOperation),
},
_ => Ok(()),
None => Ok(()),
}
}
@ -1057,6 +1061,21 @@ impl WebGLRenderingContext {
_ => Err(InvalidEnum),
}
}
pub fn initialize_framebuffer(&self, clear_bits: u32) {
if clear_bits == 0 {
return;
}
self.send_command(WebGLCommand::InitializeFramebuffer {
color: clear_bits & constants::COLOR_BUFFER_BIT != 0,
depth: clear_bits & constants::DEPTH_BUFFER_BIT != 0,
stencil: clear_bits & constants::STENCIL_BUFFER_BIT != 0,
});
}
pub fn bound_framebuffer(&self) -> Option<DomRoot<WebGLFramebuffer>> {
self.bound_framebuffer.get()
}
}
impl Drop for WebGLRenderingContext {
@ -1572,6 +1591,10 @@ impl WebGLRenderingContextMethods for WebGLRenderingContext {
renderbuffer.bind(target);
}
_ => {
if renderbuffer.is_some() {
self.webgl_error(InvalidOperation);
}
self.bound_renderbuffer.set(None);
// Unbind the currently bound renderbuffer
self.send_command(WebGLCommand::BindRenderbuffer(target, None));
@ -1971,20 +1994,6 @@ impl WebGLRenderingContextMethods for WebGLRenderingContext {
handle_potential_webgl_error!(self, self.validate_ownership(renderbuffer), return);
handle_object_deletion!(self, self.bound_renderbuffer, renderbuffer,
Some(WebGLCommand::BindRenderbuffer(constants::RENDERBUFFER, None)));
// From the GLES 2.0.25 spec, page 113:
//
// "If a renderbuffer object is deleted while its
// image is attached to the currently bound
// framebuffer, then it is as if
// FramebufferRenderbuffer had been called, with a
// renderbuffer of 0, for each attachment point to
// which this image was attached in the currently
// bound framebuffer."
//
if let Some(fb) = self.bound_framebuffer.get() {
fb.detach_renderbuffer(renderbuffer);
}
renderbuffer.delete()
}
}
@ -2020,18 +2029,6 @@ impl WebGLRenderingContextMethods for WebGLRenderingContext {
));
}
// From the GLES 2.0.25 spec, page 113:
//
// "If a texture object is deleted while its image is
// attached to the currently bound framebuffer, then
// it is as if FramebufferTexture2D had been called,
// with a texture of 0, for each attachment point to
// which this image was attached in the currently
// bound framebuffer."
if let Some(fb) = self.bound_framebuffer.get() {
fb.detach_texture(texture);
}
texture.delete()
}
}
@ -2723,10 +2720,12 @@ impl WebGLRenderingContextMethods for WebGLRenderingContext {
// https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.3
fn StencilMaskSeparate(&self, face: u32, mask: u32) {
match face {
constants::FRONT | constants::BACK | constants::FRONT_AND_BACK =>
constants::FRONT |
constants::BACK |
constants::FRONT_AND_BACK =>
self.send_command(WebGLCommand::StencilMaskSeparate(face, mask)),
_ => return self.webgl_error(InvalidEnum),
}
};
}
// https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.3

View file

@ -146,7 +146,7 @@ impl WebGLTexture {
}
};
let base_image_info = self.base_image_info().unwrap();
let base_image_info = self.base_image_info();
if !base_image_info.is_initialized() {
return Err(WebGLError::InvalidOperation);
}
@ -186,6 +186,22 @@ impl WebGLTexture {
DOMToTextureCommand::Detach(self.id),
);
}
/*
If a texture object is deleted while its image is attached to the currently
bound framebuffer, then it is as if FramebufferTexture2D had been called, with
a texture of 0, for each attachment point to which this image was attached
in the currently bound framebuffer.
- GLES 2.0, 4.4.3, "Attaching Texture Images to a Framebuffer"
*/
let currently_bound_framebuffer =
self.upcast::<WebGLObject>()
.context()
.bound_framebuffer();
if let Some(fb) = currently_bound_framebuffer {
fb.detach_texture(self);
}
context.send_command(WebGLCommand::DeleteTexture(self.id));
}
}
@ -327,7 +343,7 @@ impl WebGLTexture {
fn is_cube_complete(&self) -> bool {
debug_assert_eq!(self.face_count.get(), 6);
let image_info = self.base_image_info().unwrap();
let image_info = self.base_image_info();
if !image_info.is_defined() {
return false;
}
@ -389,10 +405,10 @@ impl WebGLTexture {
self.image_info_array.borrow_mut()[pos as usize] = image_info;
}
fn base_image_info(&self) -> Option<ImageInfo> {
fn base_image_info(&self) -> ImageInfo {
assert!((self.base_mipmap_level as usize) < MAX_LEVEL_COUNT);
Some(self.image_info_at_face(0, self.base_mipmap_level))
self.image_info_at_face(0, self.base_mipmap_level)
}
pub fn set_attached_to_dom(&self) {