From 2b561084d640681e39ad41b94aee62ffafe5c248 Mon Sep 17 00:00:00 2001 From: Dan Glastonbury Date: Fri, 24 Jan 2014 13:53:53 +1000 Subject: [PATCH] Bug 948002 - Fix WebGL framebuffer completeness checks. r=bjacob When changing WebGLTexture::ImageInfo to consistently store GL internal format instead of format, code that checked for depth textures broke because general depth component format type was being checked instead of the sized formats. With :bjacob, we audited the locations of the checks and updated the code to accept the internal formats by utilizing helper functions that check the GLenum. --- content/canvas/src/WebGLContextGL.cpp | 96 ++++++++++------- content/canvas/src/WebGLContextUtils.cpp | 35 ++++--- content/canvas/src/WebGLContextUtils.h | 3 + content/canvas/src/WebGLFramebuffer.cpp | 113 ++++++++++++++------- content/canvas/src/WebGLTexelConversions.h | 27 +++++ content/canvas/src/WebGLTexture.cpp | 3 +- 6 files changed, 191 insertions(+), 86 deletions(-) diff --git a/content/canvas/src/WebGLContextGL.cpp b/content/canvas/src/WebGLContextGL.cpp index 0a5b4108c5b..457902904f5 100644 --- a/content/canvas/src/WebGLContextGL.cpp +++ b/content/canvas/src/WebGLContextGL.cpp @@ -638,10 +638,10 @@ WebGLContext::CopyTexSubImage2D(GLenum target, if (yoffset + height > texHeight || yoffset + height < 0) return ErrorInvalidValue("copyTexSubImage2D: yoffset+height is too large"); - GLenum format = imageInfo.InternalFormat(); - bool texFormatRequiresAlpha = format == LOCAL_GL_RGBA || - format == LOCAL_GL_ALPHA || - format == LOCAL_GL_LUMINANCE_ALPHA; + GLenum internalFormat = imageInfo.InternalFormat(); + bool texFormatRequiresAlpha = (internalFormat == LOCAL_GL_RGBA || + internalFormat == LOCAL_GL_ALPHA || + internalFormat == LOCAL_GL_LUMINANCE_ALPHA); bool fboFormatHasAlpha = mBoundFramebuffer ? mBoundFramebuffer->ColorAttachment(0).HasAlpha() : bool(gl->GetPixelFormat().alpha > 0); @@ -649,9 +649,11 @@ WebGLContext::CopyTexSubImage2D(GLenum target, return ErrorInvalidOperation("copyTexSubImage2D: texture format requires an alpha channel " "but the framebuffer doesn't have one"); - if (format == LOCAL_GL_DEPTH_COMPONENT || - format == LOCAL_GL_DEPTH_STENCIL) + if (IsGLDepthFormat(internalFormat) || + IsGLDepthStencilFormat(internalFormat)) + { return ErrorInvalidOperation("copyTexSubImage2D: a base internal format of DEPTH_COMPONENT or DEPTH_STENCIL isn't supported"); + } if (mBoundFramebuffer) if (!mBoundFramebuffer->CheckAndInitializeAttachments()) @@ -661,7 +663,7 @@ WebGLContext::CopyTexSubImage2D(GLenum target, tex->DoDeferredImageInitialization(target, level); } - return CopyTexSubImage2D_base(target, level, format, xoffset, yoffset, x, y, width, height, true); + return CopyTexSubImage2D_base(target, level, internalFormat, xoffset, yoffset, x, y, width, height, true); } @@ -1182,15 +1184,17 @@ WebGLContext::GenerateMipmap(GLenum target) if (!tex->IsFirstImagePowerOfTwo()) return ErrorInvalidOperation("generateMipmap: Level zero of texture does not have power-of-two width and height."); - GLenum format = tex->ImageInfoAt(imageTarget, 0).InternalFormat(); - if (IsTextureFormatCompressed(format)) + GLenum internalFormat = tex->ImageInfoAt(imageTarget, 0).InternalFormat(); + if (IsTextureFormatCompressed(internalFormat)) return ErrorInvalidOperation("generateMipmap: Texture data at level zero is compressed."); if (IsExtensionEnabled(WEBGL_depth_texture) && - (format == LOCAL_GL_DEPTH_COMPONENT || format == LOCAL_GL_DEPTH_STENCIL)) + (IsGLDepthFormat(internalFormat) || IsGLDepthStencilFormat(internalFormat))) + { return ErrorInvalidOperation("generateMipmap: " "A texture that has a base internal format of " "DEPTH_COMPONENT or DEPTH_STENCIL isn't supported"); + } if (!tex->AreAllLevel0ImageInfosEqual()) return ErrorInvalidOperation("generateMipmap: The six faces of this cube map have different dimensions, format, or type."); @@ -4125,31 +4129,44 @@ BaseTypeAndSizeFromUniformType(GLenum uType, GLenum *baseType, GLint *unitSize) } -WebGLTexelFormat mozilla::GetWebGLTexelFormat(GLenum format, GLenum type) +WebGLTexelFormat mozilla::GetWebGLTexelFormat(GLenum internalformat, GLenum type) { // // WEBGL_depth_texture - if (format == LOCAL_GL_DEPTH_COMPONENT) { + if (internalformat == LOCAL_GL_DEPTH_COMPONENT) { switch (type) { case LOCAL_GL_UNSIGNED_SHORT: return WebGLTexelFormat::D16; case LOCAL_GL_UNSIGNED_INT: return WebGLTexelFormat::D32; - default: - MOZ_CRASH("Invalid WebGL texture format/type?"); } - } else if (format == LOCAL_GL_DEPTH_STENCIL) { + + MOZ_CRASH("Invalid WebGL texture format/type?"); + } + + if (internalformat == LOCAL_GL_DEPTH_STENCIL) { switch (type) { case LOCAL_GL_UNSIGNED_INT_24_8_EXT: return WebGLTexelFormat::D24S8; - default: - MOZ_CRASH("Invalid WebGL texture format/type?"); } + + MOZ_CRASH("Invalid WebGL texture format/type?"); } + if (internalformat == LOCAL_GL_DEPTH_COMPONENT16) { + return WebGLTexelFormat::D16; + } + + if (internalformat == LOCAL_GL_DEPTH_COMPONENT32) { + return WebGLTexelFormat::D32; + } + + if (internalformat == LOCAL_GL_DEPTH24_STENCIL8) { + return WebGLTexelFormat::D24S8; + } if (type == LOCAL_GL_UNSIGNED_BYTE) { - switch (format) { + switch (internalformat) { case LOCAL_GL_RGBA: case LOCAL_GL_SRGB_ALPHA_EXT: return WebGLTexelFormat::RGBA8; @@ -4162,14 +4179,16 @@ WebGLTexelFormat mozilla::GetWebGLTexelFormat(GLenum format, GLenum type) return WebGLTexelFormat::R8; case LOCAL_GL_LUMINANCE_ALPHA: return WebGLTexelFormat::RA8; - default: - MOZ_ASSERT(false, "Coding mistake?! Should never reach this point."); - return WebGLTexelFormat::BadFormat; } - } else if (type == LOCAL_GL_FLOAT) { + + MOZ_CRASH("Invalid WebGL texture format/type?"); + } + + if (type == LOCAL_GL_FLOAT) { // OES_texture_float - switch (format) { + switch (internalformat) { case LOCAL_GL_RGBA: + case LOCAL_GL_RGBA32F: return WebGLTexelFormat::RGBA32F; case LOCAL_GL_RGB: return WebGLTexelFormat::RGB32F; @@ -4179,23 +4198,24 @@ WebGLTexelFormat mozilla::GetWebGLTexelFormat(GLenum format, GLenum type) return WebGLTexelFormat::R32F; case LOCAL_GL_LUMINANCE_ALPHA: return WebGLTexelFormat::RA32F; - default: - MOZ_ASSERT(false, "Coding mistake?! Should never reach this point."); - return WebGLTexelFormat::BadFormat; - } - } else { - switch (type) { - case LOCAL_GL_UNSIGNED_SHORT_4_4_4_4: - return WebGLTexelFormat::RGBA4444; - case LOCAL_GL_UNSIGNED_SHORT_5_5_5_1: - return WebGLTexelFormat::RGBA5551; - case LOCAL_GL_UNSIGNED_SHORT_5_6_5: - return WebGLTexelFormat::RGB565; - default: - MOZ_ASSERT(false, "Coding mistake?! Should never reach this point."); - return WebGLTexelFormat::BadFormat; } + + MOZ_CRASH("Invalid WebGL texture format/type?"); } + + switch (type) { + case LOCAL_GL_UNSIGNED_SHORT_4_4_4_4: + return WebGLTexelFormat::RGBA4444; + case LOCAL_GL_UNSIGNED_SHORT_5_5_5_1: + return WebGLTexelFormat::RGBA5551; + case LOCAL_GL_UNSIGNED_SHORT_5_6_5: + return WebGLTexelFormat::RGB565; + default: + MOZ_ASSERT(false, "Coding mistake?! Should never reach this point."); + return WebGLTexelFormat::BadFormat; + } + + MOZ_CRASH("Invalid WebGL texture format/type?"); } GLenum diff --git a/content/canvas/src/WebGLContextUtils.cpp b/content/canvas/src/WebGLContextUtils.cpp index 32dcc8abfd3..43ad3541fdc 100644 --- a/content/canvas/src/WebGLContextUtils.cpp +++ b/content/canvas/src/WebGLContextUtils.cpp @@ -23,6 +23,25 @@ using namespace mozilla; +namespace mozilla { + +bool +IsGLDepthFormat(GLenum internalFormat) +{ + return (internalFormat == LOCAL_GL_DEPTH_COMPONENT || + internalFormat == LOCAL_GL_DEPTH_COMPONENT16 || + internalFormat == LOCAL_GL_DEPTH_COMPONENT32); +} + +bool +IsGLDepthStencilFormat(GLenum internalFormat) +{ + return (internalFormat == LOCAL_GL_DEPTH_STENCIL || + internalFormat == LOCAL_GL_DEPTH24_STENCIL8); +} + +} // namespace mozilla + void WebGLContext::GenerateWarning(const char *fmt, ...) { @@ -197,16 +216,7 @@ WebGLContext::ErrorName(GLenum error) bool WebGLContext::IsTextureFormatCompressed(GLenum format) { - switch(format) { - case LOCAL_GL_RGB: - case LOCAL_GL_RGBA: - case LOCAL_GL_ALPHA: - case LOCAL_GL_LUMINANCE: - case LOCAL_GL_LUMINANCE_ALPHA: - case LOCAL_GL_DEPTH_COMPONENT: - case LOCAL_GL_DEPTH_STENCIL: - return false; - + switch (format) { case LOCAL_GL_COMPRESSED_RGB_S3TC_DXT1_EXT: case LOCAL_GL_COMPRESSED_RGBA_S3TC_DXT1_EXT: case LOCAL_GL_COMPRESSED_RGBA_S3TC_DXT3_EXT: @@ -219,10 +229,9 @@ WebGLContext::IsTextureFormatCompressed(GLenum format) case LOCAL_GL_COMPRESSED_RGBA_PVRTC_4BPPV1: case LOCAL_GL_COMPRESSED_RGBA_PVRTC_2BPPV1: return true; + default: + return false; } - - MOZ_ASSERT(false, "Invalid WebGL texture format?"); - return false; } void diff --git a/content/canvas/src/WebGLContextUtils.h b/content/canvas/src/WebGLContextUtils.h index 26c96a93bd1..0bdf6d65fed 100644 --- a/content/canvas/src/WebGLContextUtils.h +++ b/content/canvas/src/WebGLContextUtils.h @@ -12,6 +12,9 @@ namespace mozilla { +bool IsGLDepthFormat(GLenum internalFormat); +bool IsGLDepthStencilFormat(GLenum internalFormat); + template JS::Value WebGLContext::WebGLObjectAsJSValue(JSContext *cx, const WebGLObjectType *object, ErrorResult& rv) const diff --git a/content/canvas/src/WebGLFramebuffer.cpp b/content/canvas/src/WebGLFramebuffer.cpp index 6f3f9bea257..22343c9ed31 100644 --- a/content/canvas/src/WebGLFramebuffer.cpp +++ b/content/canvas/src/WebGLFramebuffer.cpp @@ -128,24 +128,63 @@ WebGLFramebuffer::Attachment::RectangleObject() const } static inline bool -IsValidAttachedTextureColorFormat(GLenum format) +IsValidFBOTextureColorFormat(GLenum internalFormat) { return ( /* linear 8-bit formats */ - format == LOCAL_GL_ALPHA || - format == LOCAL_GL_LUMINANCE || - format == LOCAL_GL_LUMINANCE_ALPHA || - format == LOCAL_GL_RGB || - format == LOCAL_GL_RGBA || + internalFormat == LOCAL_GL_ALPHA || + internalFormat == LOCAL_GL_LUMINANCE || + internalFormat == LOCAL_GL_LUMINANCE_ALPHA || + internalFormat == LOCAL_GL_RGB || + internalFormat == LOCAL_GL_RGBA || /* sRGB 8-bit formats */ - format == LOCAL_GL_SRGB_EXT || - format == LOCAL_GL_SRGB_ALPHA_EXT || + internalFormat == LOCAL_GL_SRGB_EXT || + internalFormat == LOCAL_GL_SRGB_ALPHA_EXT || /* linear float32 formats */ - format == LOCAL_GL_ALPHA32F_ARB || - format == LOCAL_GL_LUMINANCE32F_ARB || - format == LOCAL_GL_LUMINANCE_ALPHA32F_ARB || - format == LOCAL_GL_RGB32F_ARB || - format == LOCAL_GL_RGBA32F_ARB); + internalFormat == LOCAL_GL_ALPHA32F_ARB || + internalFormat == LOCAL_GL_LUMINANCE32F_ARB || + internalFormat == LOCAL_GL_LUMINANCE_ALPHA32F_ARB || + internalFormat == LOCAL_GL_RGB32F_ARB || + internalFormat == LOCAL_GL_RGBA32F_ARB); +} + +static inline bool +IsValidFBOTextureDepthFormat(GLenum internalFormat) { + return ( + internalFormat == LOCAL_GL_DEPTH_COMPONENT || + internalFormat == LOCAL_GL_DEPTH_COMPONENT16 || + internalFormat == LOCAL_GL_DEPTH_COMPONENT32); +} + +static inline bool +IsValidFBOTextureDepthStencilFormat(GLenum internalFormat) { + return ( + internalFormat == LOCAL_GL_DEPTH_STENCIL || + internalFormat == LOCAL_GL_DEPTH24_STENCIL8); +} + +static inline bool +IsValidFBORenderbufferColorFormat(GLenum internalFormat) { + return ( + internalFormat == LOCAL_GL_RGB565 || + internalFormat == LOCAL_GL_RGB5_A1 || + internalFormat == LOCAL_GL_RGBA4 || + internalFormat == LOCAL_GL_SRGB8_ALPHA8_EXT); +} + +static inline bool +IsValidFBORenderbufferDepthFormat(GLenum internalFormat) { + return internalFormat == LOCAL_GL_DEPTH_COMPONENT16; +} + +static inline bool +IsValidFBORenderbufferDepthStencilFormat(GLenum internalFormat) { + return internalFormat == LOCAL_GL_DEPTH24_STENCIL8; +} + +static inline bool +IsValidFBORenderbufferStencilFormat(GLenum internalFormat) { + return internalFormat == LOCAL_GL_STENCIL_INDEX8; } bool @@ -164,37 +203,43 @@ WebGLFramebuffer::Attachment::IsComplete() const if (mTexturePtr) { MOZ_ASSERT(mTexturePtr->HasImageInfoAt(mTexImageTarget, mTexImageLevel)); - GLenum format = mTexturePtr->ImageInfoAt(mTexImageTarget, mTexImageLevel).InternalFormat(); + const WebGLTexture::ImageInfo& imageInfo = + mTexturePtr->ImageInfoAt(mTexImageTarget, mTexImageLevel); + GLenum internalFormat = imageInfo.InternalFormat(); - if (mAttachmentPoint == LOCAL_GL_DEPTH_ATTACHMENT) { - return format == LOCAL_GL_DEPTH_COMPONENT; - } else if (mAttachmentPoint == LOCAL_GL_DEPTH_STENCIL_ATTACHMENT) { - return format == LOCAL_GL_DEPTH_STENCIL; - } else if (mAttachmentPoint >= LOCAL_GL_COLOR_ATTACHMENT0 && - mAttachmentPoint < GLenum(LOCAL_GL_COLOR_ATTACHMENT0 + WebGLContext::sMaxColorAttachments)) + if (mAttachmentPoint == LOCAL_GL_DEPTH_ATTACHMENT) + return IsValidFBOTextureDepthFormat(internalFormat); + + if (mAttachmentPoint == LOCAL_GL_DEPTH_STENCIL_ATTACHMENT) + return IsValidFBOTextureDepthStencilFormat(internalFormat); + + if (mAttachmentPoint >= LOCAL_GL_COLOR_ATTACHMENT0 && + mAttachmentPoint < GLenum(LOCAL_GL_COLOR_ATTACHMENT0 + + WebGLContext::sMaxColorAttachments)) { - return IsValidAttachedTextureColorFormat(format); + return IsValidFBOTextureColorFormat(internalFormat); } MOZ_ASSERT(false, "Invalid WebGL attachment point?"); return false; } if (mRenderbufferPtr) { - GLenum format = mRenderbufferPtr->InternalFormat(); + GLenum internalFormat = mRenderbufferPtr->InternalFormat(); - if (mAttachmentPoint == LOCAL_GL_DEPTH_ATTACHMENT) { - return format == LOCAL_GL_DEPTH_COMPONENT16; - } else if (mAttachmentPoint == LOCAL_GL_STENCIL_ATTACHMENT) { - return format == LOCAL_GL_STENCIL_INDEX8; - } else if (mAttachmentPoint == LOCAL_GL_DEPTH_STENCIL_ATTACHMENT) { - return format == LOCAL_GL_DEPTH_STENCIL; - } else if (mAttachmentPoint >= LOCAL_GL_COLOR_ATTACHMENT0 && - mAttachmentPoint < GLenum(LOCAL_GL_COLOR_ATTACHMENT0 + WebGLContext::sMaxColorAttachments)) + if (mAttachmentPoint == LOCAL_GL_DEPTH_ATTACHMENT) + return IsValidFBORenderbufferDepthFormat(internalFormat); + + if (mAttachmentPoint == LOCAL_GL_STENCIL_ATTACHMENT) + return IsValidFBORenderbufferStencilFormat(internalFormat); + + if (mAttachmentPoint == LOCAL_GL_DEPTH_STENCIL_ATTACHMENT) + return IsValidFBORenderbufferDepthStencilFormat(internalFormat); + + if (mAttachmentPoint >= LOCAL_GL_COLOR_ATTACHMENT0 && + mAttachmentPoint < GLenum(LOCAL_GL_COLOR_ATTACHMENT0 + + WebGLContext::sMaxColorAttachments)) { - return format == LOCAL_GL_RGB565 || - format == LOCAL_GL_RGB5_A1 || - format == LOCAL_GL_RGBA4 || - format == LOCAL_GL_SRGB8_ALPHA8_EXT; + return IsValidFBORenderbufferColorFormat(internalFormat); } MOZ_ASSERT(false, "Invalid WebGL attachment point?"); return false; diff --git a/content/canvas/src/WebGLTexelConversions.h b/content/canvas/src/WebGLTexelConversions.h index 85e49624d06..df8f808bd6d 100644 --- a/content/canvas/src/WebGLTexelConversions.h +++ b/content/canvas/src/WebGLTexelConversions.h @@ -95,6 +95,33 @@ struct IntermediateFormat : WebGLTexelFormat::RGBA8; }; +inline GLenum +GLFormatForTexelFormat(WebGLTexelFormat format) { + switch (format) { + case WebGLTexelFormat::R8: return LOCAL_GL_LUMINANCE; + case WebGLTexelFormat::A8: return LOCAL_GL_ALPHA; + case WebGLTexelFormat::RA8: return LOCAL_GL_LUMINANCE_ALPHA; + case WebGLTexelFormat::RGBA5551: return LOCAL_GL_RGBA; + case WebGLTexelFormat::RGBA4444: return LOCAL_GL_RGBA; + case WebGLTexelFormat::RGB565: return LOCAL_GL_RGB; + case WebGLTexelFormat::D16: return LOCAL_GL_DEPTH_COMPONENT; + case WebGLTexelFormat::RGB8: return LOCAL_GL_RGB; + case WebGLTexelFormat::RGBA8: return LOCAL_GL_RGBA; + case WebGLTexelFormat::BGRA8: return LOCAL_GL_BGRA; + case WebGLTexelFormat::BGRX8: return LOCAL_GL_BGR; + case WebGLTexelFormat::R32F: return LOCAL_GL_LUMINANCE; + case WebGLTexelFormat::A32F: return LOCAL_GL_ALPHA; + case WebGLTexelFormat::D32: return LOCAL_GL_DEPTH_COMPONENT; + case WebGLTexelFormat::D24S8: return LOCAL_GL_DEPTH_STENCIL; + case WebGLTexelFormat::RA32F: return LOCAL_GL_LUMINANCE_ALPHA; + case WebGLTexelFormat::RGB32F: return LOCAL_GL_RGB; + case WebGLTexelFormat::RGBA32F: return LOCAL_GL_RGBA; + default: + MOZ_CRASH("Unknown texel format. Coding mistake?"); + return LOCAL_GL_INVALID_ENUM; + } +} + inline size_t TexelBytesForFormat(WebGLTexelFormat format) { switch (format) { case WebGLTexelFormat::R8: diff --git a/content/canvas/src/WebGLTexture.cpp b/content/canvas/src/WebGLTexture.cpp index 35a592c4ebc..76a784e7674 100644 --- a/content/canvas/src/WebGLTexture.cpp +++ b/content/canvas/src/WebGLTexture.cpp @@ -430,10 +430,11 @@ WebGLTexture::DoDeferredImageInitialization(GLenum imageTarget, GLint level) MOZ_ASSERT(checked_byteLength.isValid()); // should have been checked earlier void *zeros = calloc(1, checked_byteLength.value()); + GLenum format = WebGLTexelConversions::GLFormatForTexelFormat(texelformat); mContext->UpdateWebGLErrorAndClearGLError(); mContext->gl->fTexImage2D(imageTarget, level, imageInfo.mInternalFormat, imageInfo.mWidth, imageInfo.mHeight, - 0, imageInfo.mInternalFormat, imageInfo.mType, + 0, format, imageInfo.mType, zeros); GLenum error = LOCAL_GL_NO_ERROR; mContext->UpdateWebGLErrorAndClearGLError(&error);