Bug 1325516 - Simplify IMPL_COLOR_READ_FORMAT/TYPE and ensure that we only return valid ones. - r=daoshengmu
MozReview-Commit-ID: KPicqcBy8l1
--- a/dom/canvas/WebGLContext.h
+++ b/dom/canvas/WebGLContext.h
@@ -654,16 +654,22 @@ public:
bool IsVertexArray(const WebGLVertexArray* vao);
void LineWidth(GLfloat width);
void LinkProgram(WebGLProgram& prog);
void PixelStorei(GLenum pname, GLint param);
void PolygonOffset(GLfloat factor, GLfloat units);
already_AddRefed<layers::SharedSurfaceTextureClient> GetVRFrame();
bool StartVRPresentation();
+
+ ////
+
+ webgl::PackingInfo
+ ValidImplementationColorReadPI(const webgl::FormatUsageInfo* usage) const;
+
protected:
bool ReadPixels_SharedPrecheck(ErrorResult* const out_error);
void ReadPixelsImpl(GLint x, GLint y, GLsizei width, GLsizei height, GLenum format,
GLenum type, void* data, uint32_t dataLen);
bool DoReadPixelsAndConvert(const webgl::FormatInfo* srcFormat, GLint x, GLint y,
GLsizei width, GLsizei height, GLenum format,
GLenum destType, void* dest, uint32_t dataLen,
uint32_t rowStride);
@@ -682,16 +688,18 @@ public:
void ReadPixels(GLint x, GLint y, GLsizei width, GLsizei height, GLenum format,
GLenum type, WebGLsizeiptr offset, ErrorResult& out_error);
void ReadPixels(GLint x, GLint y, GLsizei width, GLsizei height, GLenum format,
GLenum type, const dom::ArrayBufferView& dstData, GLuint dstOffset,
ErrorResult& out_error);
+ ////
+
void RenderbufferStorage(GLenum target, GLenum internalFormat,
GLsizei width, GLsizei height);
protected:
void RenderbufferStorage_base(const char* funcName, GLenum target,
GLsizei samples, GLenum internalformat,
GLsizei width, GLsizei height);
public:
void SampleCoverage(GLclampf value, WebGLboolean invert);
--- a/dom/canvas/WebGLContextGL.cpp
+++ b/dom/canvas/WebGLContextGL.cpp
@@ -1374,116 +1374,128 @@ WebGLContext::ReadPixels(GLint x, GLint
}
gl->MakeCurrent();
const ScopedLazyBind lazyBind(gl, LOCAL_GL_PIXEL_PACK_BUFFER, buffer);
ReadPixelsImpl(x, y, width, height, format, type, (void*)offset, bytesAfterOffset);
}
-static bool
-ValidateReadPixelsFormatAndType(const webgl::FormatInfo* srcFormat,
- const webgl::PackingInfo& pi, gl::GLContext* gl,
- WebGLContext* webgl)
+static webgl::PackingInfo
+DefaultReadPixelPI(const webgl::FormatUsageInfo* usage)
{
- // Check the format and type params to assure they are an acceptable pair (as per spec)
- GLenum mainFormat;
- GLenum mainType;
-
- switch (srcFormat->componentType) {
- case webgl::ComponentType::Float:
- mainFormat = LOCAL_GL_RGBA;
- mainType = LOCAL_GL_FLOAT;
- break;
-
- case webgl::ComponentType::UInt:
- mainFormat = LOCAL_GL_RGBA_INTEGER;
- mainType = LOCAL_GL_UNSIGNED_INT;
- break;
+ MOZ_ASSERT(usage->IsRenderable());
+
+ switch (usage->format->componentType) {
+ case webgl::ComponentType::NormUInt:
+ return { LOCAL_GL_RGBA, LOCAL_GL_UNSIGNED_BYTE };
case webgl::ComponentType::Int:
- mainFormat = LOCAL_GL_RGBA_INTEGER;
- mainType = LOCAL_GL_INT;
- break;
-
- case webgl::ComponentType::NormInt:
- case webgl::ComponentType::NormUInt:
- mainFormat = LOCAL_GL_RGBA;
- mainType = LOCAL_GL_UNSIGNED_BYTE;
- break;
+ return { LOCAL_GL_RGBA_INTEGER, LOCAL_GL_INT };
+
+ case webgl::ComponentType::UInt:
+ return { LOCAL_GL_RGBA_INTEGER, LOCAL_GL_UNSIGNED_INT };
+
+ case webgl::ComponentType::Float:
+ return { LOCAL_GL_RGBA, LOCAL_GL_FLOAT };
default:
- gfxCriticalNote << "Unhandled srcFormat->componentType: "
- << (uint32_t)srcFormat->componentType;
- webgl->ErrorInvalidOperation("readPixels: Unhandled srcFormat->componentType."
- " Please file a bug!");
- return false;
+ MOZ_CRASH();
}
-
- if (pi.format == mainFormat && pi.type == mainType)
- return true;
-
- //////
- // OpenGL ES 3.0.4 p194 - When the internal format of the rendering surface is
- // RGB10_A2, a third combination of format RGBA and type UNSIGNED_INT_2_10_10_10_REV
- // is accepted.
-
- if (webgl->IsWebGL2() &&
- srcFormat->effectiveFormat == webgl::EffectiveFormat::RGB10_A2 &&
- pi.format == LOCAL_GL_RGBA &&
- pi.type == LOCAL_GL_UNSIGNED_INT_2_10_10_10_REV)
- {
- return true;
- }
-
- //////
+}
+
+static bool
+ArePossiblePackEnums(const WebGLContext* webgl, const webgl::PackingInfo& pi)
+{
// OpenGL ES 2.0 $4.3.1 - IMPLEMENTATION_COLOR_READ_{TYPE/FORMAT} is a valid
// combination for glReadPixels()...
// So yeah, we are actually checking that these are valid as /unpack/ formats, instead
// of /pack/ formats here, but it should cover the INVALID_ENUM cases.
- if (!webgl->mFormatUsage->AreUnpackEnumsValid(pi.format, pi.type)) {
- webgl->ErrorInvalidEnum("readPixels: Bad format and/or type.");
+ if (!webgl->mFormatUsage->AreUnpackEnumsValid(pi.format, pi.type))
return false;
- }
// Only valid when pulled from:
// * GLES 2.0.25 p105:
// "table 3.4, excluding formats LUMINANCE and LUMINANCE_ALPHA."
// * GLES 3.0.4 p193:
// "table 3.2, excluding formats DEPTH_COMPONENT and DEPTH_STENCIL."
switch (pi.format) {
case LOCAL_GL_LUMINANCE:
case LOCAL_GL_LUMINANCE_ALPHA:
case LOCAL_GL_DEPTH_COMPONENT:
case LOCAL_GL_DEPTH_STENCIL:
- webgl->ErrorInvalidEnum("readPixels: Invalid format: 0x%04x", pi.format);
return false;
}
- if (pi.type == LOCAL_GL_UNSIGNED_INT_24_8) {
- webgl->ErrorInvalidEnum("readPixels: Invalid type: 0x%04x", pi.type);
+ if (pi.type == LOCAL_GL_UNSIGNED_INT_24_8)
+ return false;
+
+ return true;
+}
+
+webgl::PackingInfo
+WebGLContext::ValidImplementationColorReadPI(const webgl::FormatUsageInfo* usage) const
+{
+ const auto defaultPI = DefaultReadPixelPI(usage);
+ if (!gl->IsSupported(gl::GLFeature::ES2_compatibility))
+ return defaultPI;
+
+ webgl::PackingInfo implPI;
+ gl->fGetIntegerv(LOCAL_GL_IMPLEMENTATION_COLOR_READ_FORMAT, (GLint*)&implPI.format);
+ gl->fGetIntegerv(LOCAL_GL_IMPLEMENTATION_COLOR_READ_FORMAT, (GLint*)&implPI.type);
+
+ if (!ArePossiblePackEnums(this, implPI))
+ return defaultPI;
+
+ return implPI;
+}
+
+static bool
+ValidateReadPixelsFormatAndType(const webgl::FormatUsageInfo* srcUsage,
+ const webgl::PackingInfo& pi, gl::GLContext* gl,
+ WebGLContext* webgl)
+{
+ const char funcName[] = "readPixels";
+
+ if (!ArePossiblePackEnums(webgl, pi)) {
+ webgl->ErrorInvalidEnum("%s: Unexpected format or type.", funcName);
return false;
}
+ ////
+
+ const auto defaultPI = DefaultReadPixelPI(srcUsage);
+ if (pi == defaultPI)
+ return true;
+
+ ////
+
+ // OpenGL ES 3.0.4 p194 - When the internal format of the rendering surface is
+ // RGB10_A2, a third combination of format RGBA and type UNSIGNED_INT_2_10_10_10_REV
+ // is accepted.
+
+ if (webgl->IsWebGL2() &&
+ srcUsage->format->effectiveFormat == webgl::EffectiveFormat::RGB10_A2 &&
+ pi.format == LOCAL_GL_RGBA &&
+ pi.type == LOCAL_GL_UNSIGNED_INT_2_10_10_10_REV)
+ {
+ return true;
+ }
+
+ ////
+
MOZ_ASSERT(gl->IsCurrent());
- if (gl->IsSupported(gl::GLFeature::ES2_compatibility)) {
- const auto auxFormat = gl->GetIntAs<GLenum>(LOCAL_GL_IMPLEMENTATION_COLOR_READ_FORMAT);
- const auto auxType = gl->GetIntAs<GLenum>(LOCAL_GL_IMPLEMENTATION_COLOR_READ_TYPE);
-
- if (auxFormat && auxType &&
- pi.format == auxFormat && pi.type == auxType)
- {
- return true;
- }
- }
-
- //////
-
- webgl->ErrorInvalidOperation("readPixels: Invalid format or type.");
+ const auto implPI = webgl->ValidImplementationColorReadPI(srcUsage);
+ if (pi == implPI)
+ return true;
+
+ ////
+
+ webgl->ErrorInvalidOperation("%s: Incompatible format or type.", funcName);
return false;
}
void
WebGLContext::ReadPixelsImpl(GLint x, GLint y, GLsizei rawWidth, GLsizei rawHeight,
GLenum packFormat, GLenum packType, void* dest,
uint32_t dataLen)
{
@@ -1503,17 +1515,17 @@ WebGLContext::ReadPixelsImpl(GLint x, GL
uint32_t srcWidth;
uint32_t srcHeight;
if (!ValidateCurFBForRead("readPixels", &srcFormat, &srcWidth, &srcHeight))
return;
//////
const webgl::PackingInfo pi = {packFormat, packType};
- if (!ValidateReadPixelsFormatAndType(srcFormat->format, pi, gl, this))
+ if (!ValidateReadPixelsFormatAndType(srcFormat, pi, gl, this))
return;
uint8_t bytesPerPixel;
if (!webgl::GetBytesPerPixel(pi, &bytesPerPixel)) {
ErrorInvalidOperation("readPixels: Unsupported format and type.");
return;
}
--- a/dom/canvas/WebGLContextState.cpp
+++ b/dom/canvas/WebGLContextState.cpp
@@ -385,52 +385,32 @@ WebGLContext::GetParameter(JSContext* cx
case LOCAL_GL_BLEND_DST_ALPHA:
case LOCAL_GL_BLEND_EQUATION_RGB:
case LOCAL_GL_BLEND_EQUATION_ALPHA:
case LOCAL_GL_GENERATE_MIPMAP_HINT: {
GLint i = 0;
gl->fGetIntegerv(pname, &i);
return JS::NumberValue(uint32_t(i));
}
+ case LOCAL_GL_IMPLEMENTATION_COLOR_READ_FORMAT:
case LOCAL_GL_IMPLEMENTATION_COLOR_READ_TYPE: {
const webgl::FormatUsageInfo* usage;
uint32_t width, height;
if (!ValidateCurFBForRead(funcName, &usage, &width, &height))
return JS::NullValue();
- GLint i = 0;
- if (gl->IsSupported(gl::GLFeature::ES2_compatibility)) {
- gl->fGetIntegerv(pname, &i);
- } else {
- i = LOCAL_GL_UNSIGNED_BYTE;
- }
-
- return JS::NumberValue(uint32_t(i));
- }
- case LOCAL_GL_IMPLEMENTATION_COLOR_READ_FORMAT: {
- const webgl::FormatUsageInfo* usage;
- uint32_t width, height;
- if (!ValidateCurFBForRead(funcName, &usage, &width, &height))
- return JS::NullValue();
+ const auto implPI = ValidImplementationColorReadPI(usage);
- GLint i = 0;
- if (gl->IsSupported(gl::GLFeature::ES2_compatibility)) {
- gl->fGetIntegerv(pname, &i);
+ GLenum ret;
+ if (pname == LOCAL_GL_IMPLEMENTATION_COLOR_READ_FORMAT) {
+ ret = implPI.format;
} else {
- i = LOCAL_GL_RGBA;
+ ret = implPI.type;
}
-
- // OpenGL ES 3.0.4 p112 Table 3.2 shows that read format SRGB_ALPHA is
- // not supported. And if internal format of fbo is SRGB8_ALPHA8, then
- // IMPLEMENTATION_COLOR_READ_FORMAT is SRGB_ALPHA which is not supported
- // by ReadPixels. So, just return RGBA here.
- if (i == LOCAL_GL_SRGB_ALPHA)
- i = LOCAL_GL_RGBA;
-
- return JS::NumberValue(uint32_t(i));
+ return JS::NumberValue(uint32_t(ret));
}
// int
case LOCAL_GL_STENCIL_REF:
case LOCAL_GL_STENCIL_BACK_REF: {
GLint stencilBits = 0;
if (!GetStencilBits(&stencilBits))
return JS::NullValue();
--- a/dom/canvas/WebGLFormats.h
+++ b/dom/canvas/WebGLFormats.h
@@ -264,16 +264,21 @@ struct PackingInfo
bool operator <(const PackingInfo& x) const
{
if (format != x.format)
return format < x.format;
return type < x.type;
}
+
+ bool operator ==(const PackingInfo& x) const {
+ return (format == x.format &&
+ type == x.type);
+ }
};
struct DriverUnpackInfo
{
GLenum internalFormat;
GLenum unpackFormat;
GLenum unpackType;