Pack repeated code in tex(Sub)Image2D and texSubImage3D into helper func

third_party/WebKit/Source/modules/webgl/WebGL2RenderingContextBase.cpp

 // Copyright 2015 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "modules/webgl/WebGL2RenderingContextBase.h"
 
 #include "bindings/modules/v8/WebGLAny.h"
 #include "core/frame/ImageBitmap.h"
 #include "core/html/HTMLCanvasElement.h"
 #include "core/html/HTMLImageElement.h"
@@ skipping 912 matching lines @@
 void WebGL2RenderingContextBase::texStorage3D(GLenum target, GLsizei levels, GLenum internalformat, GLsizei width, GLsizei height, GLsizei depth)
 {
     if (isContextLost() || !validateTexStorage("texStorage3D", target, levels, internalformat, width, height, depth, TexStorageType3D))
         return;
 
     contextGL()->TexStorage3D(target, levels, internalformat, width, height, depth);
 }
 
 void WebGL2RenderingContextBase::texImage3D(GLenum target, GLint level, GLint internalformat, GLsizei width, GLsizei height, GLsizei depth, GLint border, GLenum format, GLenum type, DOMArrayBufferView* pixels)
 {
-    if (isContextLost())
-        return;
-    if (!validateTexture3DBinding("texImage3D", target))
-        return;
-    if (!validateTexFunc("texImage3D", TexImage, SourceArrayBufferView, target, level, internalformat, width, height, depth, border, format, type, 0, 0, 0))
-        return;
-    if (!validateTexFuncData("texImage3D", Tex3D, level, width, height, depth, format, type, pixels, NullAllowed))
-        return;
-
-    void* data = pixels ? pixels->baseAddress() : 0;
-    Vector<uint8_t> tempData;
-    if (data && (m_unpackFlipY || m_unpackPremultiplyAlpha)) {
-        // FIXME: WebGLImageConversion needs to be updated to accept image depth.
-        NOTIMPLEMENTED();
-        return;
-    }
-
-    contextGL()->TexImage3D(target, level, convertTexInternalFormat(internalformat, type), width, height, depth, border, format, type, data);
+    texImageHelperDOMArrayBufferView(TexImage3D, target, level, internalformat, width, height, border, format, type, depth, 0, 0, 0, pixels);
 }
 
 void WebGL2RenderingContextBase::texImage3D(GLenum target, GLint level, GLint internalformat, GLsizei width, GLsizei height, GLsizei depth, GLint border, GLenum format, GLenum type, GLintptr offset)
 {
     if (isContextLost())
         return;
     if (!validateTexture3DBinding("texImage3D", target))
         return;
     if (!m_boundPixelUnpackBuffer) {
         synthesizeGLError(GL_INVALID_OPERATION, "texImage3D", "no bound PIXEL_UNPACK_BUFFER");
@@ skipping 34 matching lines @@
         }
     }
 
     resetUnpackParameters();
     contextGL()->TexSubImage3D(target, level, xoffset, yoffset, zoffset, imageExtractor.imageWidth(), imageExtractor.imageHeight(), 1, format, type, needConversion ? data.data() : imagePixelData);
     restoreUnpackParameters();
 }
 
 void WebGL2RenderingContextBase::texSubImage3D(GLenum target, GLint level, GLint xoffset, GLint yoffset, GLint zoffset, GLsizei width, GLsizei height, GLsizei depth, GLenum format, GLenum type, DOMArrayBufferView* pixels)
 {
-    if (isContextLost())
-        return;
-    if (!validateTexture3DBinding("texSubImage3D", target))
-        return;
-    if (!validateTexFunc("texSubImage3D", TexSubImage, SourceArrayBufferView, target, level, 0, width, height, depth, 0, format, type, xoffset, yoffset, zoffset))
-        return;
-    if (!validateTexFuncData("texSubImage3D", Tex3D, level, width, height, depth, format, type, pixels, NullNotAllowed))
-        return;
-
-    void* data = pixels->baseAddress();
-    Vector<uint8_t> tempData;
-    bool changeUnpackParameters = false;
-    if (data && (m_unpackFlipY || m_unpackPremultiplyAlpha)) {
-        // FIXME: WebGLImageConversion needs to be updated to accept image depth.
-        NOTIMPLEMENTED();
-        changeUnpackParameters = true;
-    }
-    if (changeUnpackParameters)
-        resetUnpackParameters();
-    contextGL()->TexSubImage3D(target, level, xoffset, yoffset, zoffset, width, height, depth, format, type, data);
-    if (changeUnpackParameters)
-        restoreUnpackParameters();
+    texImageHelperDOMArrayBufferView(TexSubImage3D, target, level, 0, width, height, 0, format, type, depth, xoffset, yoffset, zoffset, pixels);
 }
 
 void WebGL2RenderingContextBase::texSubImage3D(GLenum target, GLint level, GLint xoffset, GLint yoffset, GLint zoffset, GLsizei width, GLsizei height, GLsizei depth, GLenum format, GLenum type, GLintptr offset)
 {
     if (isContextLost())
         return;
     if (!validateTexture3DBinding("texSubImage3D", target))
         return;
     if (!m_boundPixelUnpackBuffer) {
         synthesizeGLError(GL_INVALID_OPERATION, "texSubImage3D", "no bound PIXEL_UNPACK_BUFFER");
         return;
     }
     if (!validateTexFunc("texSubImage3D", TexSubImage, SourceUnpackBuffer, target, level, 0, width, height, depth, 0, format, type, xoffset, yoffset, zoffset))
         return;
     if (!validateValueFitNonNegInt32("texSubImage3D", "offset", offset))
         return;
 
     contextGL()->TexSubImage3D(target, level, xoffset, yoffset, zoffset, width, height, depth, format, type, reinterpret_cast<const void*>(offset));
 }
 
 void WebGL2RenderingContextBase::texSubImage3D(GLenum target, GLint level, GLint xoffset, GLint yoffset, GLint zoffset, GLenum format, GLenum type, ImageData* pixels)
 {
-    if (isContextLost())
-        return;
-    if (!pixels) {
-        synthesizeGLError(GL_INVALID_VALUE, "texSubImage3D", "no image data");
-        return;
-    }
-    if (pixels->data()->bufferBase()->isNeutered()) {
-        synthesizeGLError(GL_INVALID_VALUE, "texSubImage3D", "The source data has been neutered.");
-        return;
-    }
-    if (!validateTexture3DBinding("texSubImage3D", target))
-        return;
-    if (!validateTexFunc("texSubImage3D", TexSubImage, SourceImageData, target, level, 0, pixels->width(), pixels->height(), 1, 0, format, type, xoffset, yoffset, zoffset))
-        return;
-
-    if (type == GL_UNSIGNED_INT_10F_11F_11F_REV) {
-        // The UNSIGNED_INT_10F_11F_11F_REV type pack/unpack isn't implemented.
-        type = GL_FLOAT;
-    }
-    Vector<uint8_t> data;
-    bool needConversion = true;
-    // The data from ImageData is always of format RGBA8.
-    // No conversion is needed if destination format is RGBA and type is USIGNED_BYTE and no Flip or Premultiply operation is required.
-    if (format == GL_RGBA && type == GL_UNSIGNED_BYTE && !m_unpackFlipY && !m_unpackPremultiplyAlpha) {
-        needConversion = false;
-    } else {
-        if (!WebGLImageConversion::extractImageData(pixels->data()->data(), WebGLImageConversion::DataFormat::DataFormatRGBA8, pixels->size(), format, type, m_unpackFlipY, m_unpackPremultiplyAlpha, data)) {
-            synthesizeGLError(GL_INVALID_VALUE, "texSubImage3D", "bad image data");
-            return;
-        }
-    }
-    resetUnpackParameters();
-    contextGL()->TexSubImage3D(target, level, xoffset, yoffset, zoffset, pixels->width(), pixels->height(), 1, format, type, needConversion ? data.data() : pixels->data()->data());
-    restoreUnpackParameters();
+    texImageHelperImageData(TexSubImage3D, target, level, 0, 0, format, type, 1, xoffset, yoffset, zoffset, pixels);
 }
 
 void WebGL2RenderingContextBase::texSubImage3D(GLenum target, GLint level, GLint xoffset, GLint yoffset, GLint zoffset, GLenum format, GLenum type, HTMLImageElement* image, ExceptionState& exceptionState)
 {
-    if (isContextLost())
-        return;
-    if (!validateHTMLImageElement("texSubImage3D", image, exceptionState))
-        return;
-    if (!validateTexture3DBinding("texSubImage3D", target))
+    bool succeed = texImageHelperHTMLImageElement(TexSubImage3D, target, level, 0, format, type, xoffset, yoffset, zoffset, image, exceptionState);

[Ken Russell (switch to Gerrit), 2016/06/01 00:34:03]

It's very confusing that this is expected to fall through here and execute the
code after the helper function in order to do the 3D texture upload.

[xidachen, 2016/06/01 19:59:15]

I agree. It seems that the only reason that there are still some code after this
Helper function is that it calls texSubImage3DImpl, while texSubImage3DImpl is
not accessible from WebGL 1.

I looked deeper into it, and I found that texSubImage3DImpl is almost identical
with texSubImage2DImpl, which is also very similar to texImage2DImpl. It feels
like a lot of code can be shared here. But if I do everything in one patch, it
will be too big and hard to manage. I will probably break things into small CLs.

Also, I found that this if statement is duplicated in several places. I will try
to clean it up.
if (type == GL_UNSIGNED_INT_10F_11F_11F_REV) {
        // The UNSIGNED_INT_10F_11F_11F_REV type pack/unpack isn't implemented.
        type = GL_FLOAT;
}

Does the above makes sense?

[Ken Russell (switch to Gerrit), 2016/06/02 00:58:49]

Yes, but as we discussed offline, the very different behavior of the various
helpers is too confusing in this CL. It looks like there are three categories of
behaviors of the new helper functions; those which return void, those which
return bool, and those which return an object (WebGLTexture*) for later
consumption. Per our discussion, I'll wait for a revised CL that unifies these
behaviors somewhat.

+    if (!succeed)
         return;
 
     RefPtr<Image> imageForRender = image->cachedImage()->getImage();
     if (imageForRender->isSVGImage())
         imageForRender = drawImageIntoBuffer(imageForRender.get(), image->width(), image->height(), "texSubImage3D");
 
     if (!imageForRender || !validateTexFunc("texSubImage3D", TexSubImage, SourceHTMLImageElement, target, level, 0, imageForRender->width(), imageForRender->height(), 1, 0, format, type, xoffset, yoffset, zoffset))
         return;
 
     texSubImage3DImpl(target, level, xoffset, yoffset, zoffset, format, type, imageForRender.get(), WebGLImageConversion::HtmlDomImage, m_unpackFlipY, m_unpackPremultiplyAlpha);
 }
 
 void WebGL2RenderingContextBase::texSubImage3D(GLenum target, GLint level, GLint xoffset, GLint yoffset, GLint zoffset, GLenum format, GLenum type, HTMLCanvasElement* canvas, ExceptionState& exceptionState)
 {
-    if (isContextLost())
-        return;
-    if (!validateHTMLCanvasElement("texSubImage3D", canvas, exceptionState))
-        return;
-    if (!validateTexture3DBinding("texSubImage3D", target))
-        return;
-    if (!validateTexFunc("texSubImage3D", TexSubImage, SourceHTMLCanvasElement, target, level, 0, canvas->width(), canvas->height(), 1, 0, format, type, xoffset, yoffset, zoffset))
+    WebGLTexture* texture = texImageHelperHTMLCanvasElement(TexSubImage3D, target, level, 0, format, type, xoffset, yoffset, zoffset, canvas, exceptionState);
+    if (!texture)
         return;
 
     // FIXME: Implement GPU-to-GPU copy path (crbug.com/586269).
     texSubImage3DImpl(target, level, xoffset, yoffset, zoffset, format, type, canvas->copiedImage(FrontBuffer, PreferAcceleration).get(),
         WebGLImageConversion::HtmlDomCanvas, m_unpackFlipY, m_unpackPremultiplyAlpha);
 }
 
 void WebGL2RenderingContextBase::texSubImage3D(GLenum target, GLint level, GLint xoffset, GLint yoffset, GLint zoffset, GLenum format, GLenum type, HTMLVideoElement* video, ExceptionState& exceptionState)
 {
-    if (isContextLost())
-        return;
-    if (!validateHTMLVideoElement("texSubImage3D", video, exceptionState))
-        return;
-    if (!validateTexture3DBinding("texSubImage3D", target))
-        return;
-    if (!validateTexFunc("texSubImage3D", TexSubImage, SourceHTMLVideoElement, target, level, 0, video->videoWidth(), video->videoHeight(), 1, 0, format, type, xoffset, yoffset, zoffset))
+    WebGLTexture* texture = texImageHelperHTMLVideoElement(TexSubImage3D, target, level, 0, format, type, xoffset, yoffset, zoffset, video, exceptionState);
+    if (!texture)
         return;
 
     RefPtr<Image> image = videoFrameToImage(video);
     if (!image)
         return;
     texSubImage3DImpl(target, level, xoffset, yoffset, zoffset, format, type, image.get(), WebGLImageConversion::HtmlDomVideo, m_unpackFlipY, m_unpackPremultiplyAlpha);
 }
 
 void WebGL2RenderingContextBase::texSubImage3D(GLenum target, GLint level, GLint xoffset, GLint yoffset, GLint zoffset, GLenum format, GLenum type, ImageBitmap* bitmap, ExceptionState& exceptionState)
 {
-    if (isContextLost())
-        return;
-    if (!validateImageBitmap("texSubImage3D", bitmap, exceptionState))
-        return;
-    if (!validateTexture3DBinding("texSubImage3D", target))
-        return;
-    if (!validateTexFunc("texSubImage3D", TexSubImage, SourceImageBitmap, target, level, 0, bitmap->width(), bitmap->height(), 1, 0, format, type, xoffset, yoffset, zoffset))
-        return;
-    if (type == GL_UNSIGNED_INT_10F_11F_11F_REV) {
-        // The UNSIGNED_INT_10F_11F_11F_REV type pack/unpack isn't implemented.
-        type = GL_FLOAT;
-    }
-    ASSERT(bitmap->bitmapImage());
-    RefPtr<SkImage> skImage = bitmap->bitmapImage()->imageForCurrentFrame();
-    SkPixmap pixmap;
-    OwnPtr<uint8_t[]> pixelData;
-    uint8_t* pixelDataPtr = nullptr;
-    bool peekSucceed = skImage->peekPixels(&pixmap);
-    if (peekSucceed) {
-        pixelDataPtr = static_cast<uint8_t*>(pixmap.writable_addr());
-    } else if (skImage->isTextureBacked()) {
-        pixelData = bitmap->copyBitmapData(bitmap->isPremultiplied() ? PremultiplyAlpha : DontPremultiplyAlpha);
-        pixelDataPtr = pixelData.get();
-    }
-    Vector<uint8_t> data;
-    bool needConversion = true;
-    bool havePeekableRGBA = (peekSucceed && pixmap.colorType() == SkColorType::kRGBA_8888_SkColorType);
-    bool isPixelDataRBGA = (havePeekableRGBA || !peekSucceed);
-    if (isPixelDataRBGA && format == GL_RGBA && type == GL_UNSIGNED_BYTE) {
-        needConversion = false;
-    } else {
-        // In the case of ImageBitmap, we do not need to apply flipY or premultiplyAlpha.
-        bool isPixelDataBGRA = (peekSucceed && pixmap.colorType() == SkColorType::kBGRA_8888_SkColorType);
-        if ((isPixelDataBGRA && !WebGLImageConversion::extractImageData(pixelDataPtr, WebGLImageConversion::DataFormat::DataFormatBGRA8, bitmap->size(), format, type, false, false, data))
-            || (isPixelDataRBGA && !WebGLImageConversion::extractImageData(pixelDataPtr, WebGLImageConversion::DataFormat::DataFormatRGBA8, bitmap->size(), format, type, false, false, data))) {
-            synthesizeGLError(GL_INVALID_VALUE, "texSubImage3D", "bad image data");
-            return;
-        }
-    }
-    resetUnpackParameters();
-    contextGL()->TexSubImage3D(target, level, xoffset, yoffset, zoffset, bitmap->width(), bitmap->height(), 1, format, type, needConversion ? data.data() : pixelDataPtr);
-    restoreUnpackParameters();
+    texImageHelperImageBitmap(TexSubImage3D, target, level, 0, format, type, xoffset, yoffset, zoffset, bitmap, exceptionState);
 }
 
 void WebGL2RenderingContextBase::copyTexSubImage3D(GLenum target, GLint level, GLint xoffset, GLint yoffset, GLint zoffset, GLint x, GLint y, GLsizei width, GLsizei height)
 {
     if (isContextLost())
         return;
     if (!validateTexture3DBinding("copyTexSubImage3D", target))
         return;
     WebGLFramebuffer* readFramebufferBinding = nullptr;
     if (!validateReadBufferAndGetInfo("copyTexSubImage3D", readFramebufferBinding))
@@ skipping 440 matching lines @@
             return false;
         }
         break;
     default:
         synthesizeGLError(GL_INVALID_ENUM, functionName, "invalid buffer");
         return false;
     }
     return true;
 }
 
+WebGLTexture* WebGL2RenderingContextBase::validateTexImageBinding(const char* funcName, TexImageFunctionName functionName, GLenum target)
+{
+    if (functionName == TexImage3D || functionName == TexSubImage3D)
+        return validateTexture3DBinding(funcName, target);
+    return validateTexture2DBinding(funcName, target);
+}
+
 void WebGL2RenderingContextBase::clearBufferiv(GLenum buffer, GLint drawbuffer, DOMInt32Array* value)
 {
     if (isContextLost() || !validateClearBuffer("clearBufferiv", buffer, value->length()))
         return;
 
     contextGL()->ClearBufferiv(buffer, drawbuffer, value->data());
 }
 
 void WebGL2RenderingContextBase::clearBufferiv(GLenum buffer, GLint drawbuffer, const Vector<GLint>& value)
 {
@@ skipping 1952 matching lines @@
     params.skipPixels = m_unpackSkipPixels;
     params.skipRows = m_unpackSkipRows;
     if (dimension == Tex3D) {
         params.imageHeight = m_unpackImageHeight;
         params.skipImages = m_unpackSkipImages;
     }
     return params;
 }
 
 } // namespace blink