Remove duplicate validateTexFuncLevel

Source/modules/webgl/WebGLRenderingContextBase.cpp

 /*
  * Copyright (C) 2009 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
  * are met:
  * 1. Redistributions of source code must retain the above copyright
  *    notice, this list of conditions and the following disclaimer.
  * 2. Redistributions in binary form must reproduce the above copyright
  *    notice, this list of conditions and the following disclaimer in the
@@ skipping 1812 matching lines @@
         synthesizeGLError(GL_INVALID_OPERATION, functionName, "format can not be set, only rendered to");
         return false;
     }
     return true;
 }
 
 void WebGLRenderingContextBase::copyTexImage2D(GLenum target, GLint level, GLenum internalformat, GLint x, GLint y, GLsizei width, GLsizei height, GLint border)
 {
     if (isContextLost())
         return;
+    if (!validateTexFuncLevel("copyTexImage2D", target, level))
+        return;
     if (!validateTexFuncParameters("copyTexImage2D", NotTexSubImage2D, target, level, internalformat, width, height, border, internalformat, GL_UNSIGNED_BYTE))
         return;
     if (!validateSettableTexFormat("copyTexImage2D", internalformat))
         return;
     WebGLTexture* tex = validateTextureBinding("copyTexImage2D", target, true);
     if (!tex)
         return;
     if (tex->isImmutable()) {
         synthesizeGLError(GL_INVALID_OPERATION, "copyTexImage2D", "attempted to modify immutable texture");
         return;
@@ skipping 2132 matching lines @@
         && extensionsUtil()->isExtensionEnabled("GL_CHROMIUM_color_buffer_float_rgb"))
         return GL_RGB32F_EXT;
     return internalformat;
 }
 
 void WebGLRenderingContextBase::texImage2DBase(GLenum target, GLint level, GLenum internalformat, GLsizei width, GLsizei height, GLint border, GLenum format, GLenum type, const void* pixels)
 {
     // All calling functions check isContextLost, so a duplicate check is not needed here.
     // FIXME: Handle errors.
     WebGLTexture* tex = validateTextureBinding("texImage2D", target, true);
+    ASSERT(validateTexFuncLevel("texImage2D", target, level));
     ASSERT(validateTexFuncParameters("texImage2D", NotTexSubImage2D, target, level, internalformat, width, height, border, format, type));
     ASSERT(tex);
     ASSERT(!isNPOTStrict() || !level || !WebGLTexture::isNPOT(width, height));
     ASSERT(!pixels || validateSettableTexFormat("texImage2D", internalformat));
     webContext()->texImage2D(target, level, convertTexInternalFormat(internalformat, type), width, height, border, format, type, pixels);
     tex->setLevelInfo(target, level, internalformat, width, height, 1, type);
 }
 
 void WebGLRenderingContextBase::texImage2DImpl(GLenum target, GLint level, GLenum internalformat, GLenum format, GLenum type, Image* image, WebGLImageConversion::ImageHtmlDomSource domSource, bool flipY, bool premultiplyAlpha)
 {
@@ skipping 20 matching lines @@
 
     if (m_unpackAlignment != 1)
         webContext()->pixelStorei(GL_UNPACK_ALIGNMENT, 1);
     texImage2DBase(target, level, internalformat, imageExtractor.imageWidth(), imageExtractor.imageHeight(), 0, format, type, needConversion ? data.data() : imagePixelData);
     if (m_unpackAlignment != 1)
         webContext()->pixelStorei(GL_UNPACK_ALIGNMENT, m_unpackAlignment);
 }
 
 bool WebGLRenderingContextBase::validateTexFunc(const char* functionName, TexImageFunctionType functionType, TexFuncValidationSourceType sourceType, GLenum target, GLint level, GLenum internalformat, GLsizei width, GLsizei height, GLint border, GLenum format, GLenum type, GLint xoffset, GLint yoffset)
 {
+    if (!validateTexFuncLevel(functionName, target, level))
+        return false;
     WebGLTexture* texture = validateTextureBinding(functionName, target, true);
     if (!texture)
         return false;
 
     if (functionType == TexSubImage2D) {
-        if (!validateTexFuncLevel(functionName, target, level))
-            return false;
         if (!texture->isValid(target, level)) {
             synthesizeGLError(GL_INVALID_OPERATION, "texSubImage2D", "no previously defined texture image");
             return false;
         }
     }
 
     if (internalformat == 0)
         internalformat = texture->getInternalFormat(target, level);
     if (!validateTexFuncParameters(functionName, functionType, target, level, internalformat, width, height, border, format, type))
         return false;
@@ skipping 67 matching lines @@
     SkPaint paint;
     image->draw(buf->canvas(), paint, destRect, srcRect, DoNotRespectImageOrientation, Image::DoNotClampImageToSourceRect);
     return buf->newImageSnapshot();
 }
 
 void WebGLRenderingContextBase::texImage2D(GLenum target, GLint level, GLenum internalformat,
     GLsizei width, GLsizei height, GLint border,
     GLenum format, GLenum type, DOMArrayBufferView* pixels)
 {
     if (isContextLost() || !validateTexFunc("texImage2D", NotTexSubImage2D, SourceArrayBufferView, target, level, internalformat, width, height, border, format, type, 0, 0)
-        || !validateTexFuncData("texImage2D", level, width, height, internalformat, format, type, pixels, NullAllowed))
+        || !validateTexFuncData("texImage2D", level, width, height, format, type, pixels, NullAllowed))
         return;
     void* data = pixels ? pixels->baseAddress() : 0;
     Vector<uint8_t> tempData;
     bool changeUnpackAlignment = false;
     if (data && (m_unpackFlipY || m_unpackPremultiplyAlpha)) {
         if (!WebGLImageConversion::extractTextureData(width, height, format, type, m_unpackAlignment, m_unpackFlipY, m_unpackPremultiplyAlpha, data, tempData))
             return;
         data = tempData.data();
         changeUnpackAlignment = true;
     }
@@ skipping 254 matching lines @@
 
 void WebGLRenderingContextBase::texSubImage2DBase(GLenum target, GLint level, GLint xoffset, GLint yoffset, GLsizei width, GLsizei height, GLenum format, GLenum type, const void* pixels)
 {
     // FIXME: Handle errors.
     ASSERT(!isContextLost());
     WebGLTexture* tex = validateTextureBinding("texSubImage2D", target, true);
     if (!tex) {
         ASSERT_NOT_REACHED();
         return;
     }
+    ASSERT(validateTexFuncLevel("texSubImage2D", target, level));
     ASSERT(validateTexFuncParameters("texSubImage2D", TexSubImage2D, target, level, tex->getInternalFormat(target, level), width, height, 0, format, type));
     ASSERT(validateSize("texSubImage2D", xoffset, yoffset));
     ASSERT(validateSettableTexFormat("texSubImage2D", format));
     ASSERT((xoffset + width) >= 0);
     ASSERT((yoffset + height) >= 0);
     ASSERT(tex->getWidth(target, level) >= (xoffset + width));
     ASSERT(tex->getHeight(target, level) >= (yoffset + height));
     ASSERT(tex->getType(target, level) == type);
     webContext()->texSubImage2D(target, level, xoffset, yoffset, width, height, format, type, pixels);
 }
@@ skipping 29 matching lines @@
 }
 
 void WebGLRenderingContextBase::texSubImage2D(GLenum target, GLint level, GLint xoffset, GLint yoffset,
     GLsizei width, GLsizei height,
     GLenum format, GLenum type, DOMArrayBufferView* pixels)
 {
     WebGLTexture* texture = validateTextureBinding("texSubImage2D", target, true);
     if (!texture)
         return;
 
-    GLenum internalformat = texture->getInternalFormat(target, level);
     if (isContextLost() || !validateTexFunc("texSubImage2D", TexSubImage2D, SourceArrayBufferView, target, level, 0, width, height, 0, format, type, xoffset, yoffset)
-        || !validateTexFuncData("texSubImage2D", level, width, height, internalformat, format, type, pixels, NullNotAllowed))
+        || !validateTexFuncData("texSubImage2D", level, width, height, format, type, pixels, NullNotAllowed))
         return;
     void* data = pixels->baseAddress();
     Vector<uint8_t> tempData;
     bool changeUnpackAlignment = false;
     if (data && (m_unpackFlipY || m_unpackPremultiplyAlpha)) {
         if (!WebGLImageConversion::extractTextureData(width, height, format, type,
                                            m_unpackAlignment,
                                            m_unpackFlipY, m_unpackPremultiplyAlpha,
                                            data,
                                            tempData))
@@ skipping 1031 matching lines @@
     default:
         synthesizeGLError(GL_INVALID_ENUM, functionName, "invalid target");
         return false;
     }
     return true;
 }
 
 bool WebGLRenderingContextBase::validateTexFuncParameters(const char* functionName, TexImageFunctionType functionType, GLenum target,
     GLint level, GLenum internalformat, GLsizei width, GLsizei height, GLint border, GLenum format, GLenum type)
 {
-    if (!validateTexFuncLevel(functionName, target, level))
-        return false;
-
     // We absolutely have to validate the format and type combination.
     // The texImage2D entry points taking HTMLImage, etc. will produce
     // temporary data based on this combination, so it must be legal.
     if (!validateTexFuncFormatAndType(functionName, internalformat, format, type, level))
         return false;
 
     if (!validateTexFuncDimensions(functionName, functionType, target, level, width, height))
         return false;
 
     if (border) {
         synthesizeGLError(GL_INVALID_VALUE, functionName, "border != 0");
         return false;
     }
 
     return true;
 }
 
-bool WebGLRenderingContextBase::validateTexFuncData(const char* functionName, GLint level, GLsizei width, GLsizei height, GLenum internalformat, GLenum format, GLenum type, DOMArrayBufferView* pixels, NullDisposition disposition)

[Ken Russell (switch to Gerrit), 2015/09/01 22:52:49]

It's surprising that this argument was unused. I wonder whether the validation
was changed recently and some check was removed. The command buffer code has
back-to-back checks involving the internal_format in
TextureManager::ValidateTextureParameters. Not sure what the second one checks
that the first one doesn't.

[qiankun, 2015/09/02 09:49:07]

I move formats and types validation out of validateTexFuncData in
https://codereview.chromium.org/1311413003/. So internalformat isn't used any
more. internal_format/format/type is checked in
WebGLRenderingContextBase::validateTexFuncParameters now.

+bool WebGLRenderingContextBase::validateTexFuncData(const char* functionName, GLint level, GLsizei width, GLsizei height, GLenum format, GLenum type, DOMArrayBufferView* pixels, NullDisposition disposition)
 {
     // All calling functions check isContextLost, so a duplicate check is not needed here.
     if (!pixels) {
         if (disposition == NullAllowed)
             return true;
         synthesizeGLError(GL_INVALID_VALUE, functionName, "no pixels");
         return false;
     }
 
     if (!validateSettableTexFormat(functionName, format))
@@ skipping 1159 matching lines @@
 
     return totalBytesPerPixel;
 }
 
 DrawingBuffer* WebGLRenderingContextBase::drawingBuffer() const
 {
     return m_drawingBuffer.get();
 }
 
 } // namespace blink