Avoid GPU readback in tex(Sub)Image2D(ImageBitmap)

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

Index: third_party/WebKit/Source/modules/webgl/WebGLRenderingContextBase.cpp
diff --git a/third_party/WebKit/Source/modules/webgl/WebGLRenderingContextBase.cpp b/third_party/WebKit/Source/modules/webgl/WebGLRenderingContextBase.cpp
index 70d454fe26280c8b1fb0f5a6742bb196a21fb48d..9de4966b6305c42c8dbfd86522d8aeecda00c21e 100644
--- a/third_party/WebKit/Source/modules/webgl/WebGLRenderingContextBase.cpp
+++ b/third_party/WebKit/Source/modules/webgl/WebGLRenderingContextBase.cpp
@@ -4141,15 +4141,33 @@ void WebGLRenderingContextBase::texImage2D(GLenum target, GLint level, GLint int
     texImage2DImpl(target, level, internalformat, format, type, imageForRender.get(), WebGLImageConversion::HtmlDomImage, m_unpackFlipY, m_unpackPremultiplyAlpha);
 }
 
-bool WebGLRenderingContextBase::canUseTexImageCanvasByGPU(GLint internalformat, GLenum type)
+bool WebGLRenderingContextBase::canUseTexImageByGPU(GLint internalformat, GLenum type)
 {
     if (isFloatType(type) || isIntegerFormat(internalformat) || isSRGBFormat(internalformat))
         return false;
     return true;
 }
 
-void WebGLRenderingContextBase::texImageCanvasByGPU(TexImageByGPUType functionType, WebGLTexture* texture, GLenum target,
-    GLint level, GLint internalformat, GLenum type, GLint xoffset, GLint yoffset, GLint zoffset, HTMLCanvasElement* canvas)
+void WebGLRenderingContextBase::texImageCanvasByGPU(HTMLCanvasElement* canvas, GLuint targetTexture, GLenum targetInternalformat, GLenum targetType, GLint targetLevel)
+{
+    if (!canvas->is3D()) {
+        ImageBuffer* buffer = canvas->buffer();
+        if (!buffer->copyToPlatformTexture(contextGL(), targetTexture, targetInternalformat, targetType,
+            targetLevel, m_unpackPremultiplyAlpha, m_unpackFlipY)) {
+            NOTREACHED();
+        }
+    } else {
+        WebGLRenderingContextBase* gl = toWebGLRenderingContextBase(canvas->renderingContext());
+        ScopedTexture2DRestorer restorer(gl);
+        if (!gl->drawingBuffer()->copyToPlatformTexture(contextGL(), targetTexture, targetInternalformat, targetType,
+            targetLevel, m_unpackPremultiplyAlpha, !m_unpackFlipY, BackBuffer)) {
+            NOTREACHED();
+        }
+    }
+}
+
+void WebGLRenderingContextBase::texImageByGPU(TexImageByGPUType functionType, WebGLTexture* texture, GLenum target,
+    GLint level, GLint internalformat, GLenum type, GLint xoffset, GLint yoffset, GLint zoffset, HTMLCanvasElement* canvas, ImageBitmap* bitmap)

[Ken Russell (switch to Gerrit), 2016/06/01 22:55:09]

Please add DCHECK(!canvas || !bitmap) and DCHECK(canvas || bitmap).

[xidachen, 2016/06/02 13:52:52]

Done.

 {
     ScopedTexture2DRestorer restorer(this);
 
@@ -4177,20 +4195,10 @@ void WebGLRenderingContextBase::texImageCanvasByGPU(TexImageByGPUType functionTy
             canvas->height(), 0, GL_RGBA, targetType, 0);
     }
 
-    if (!canvas->is3D()) {
-        ImageBuffer* buffer = canvas->buffer();
-        if (!buffer->copyToPlatformTexture(contextGL(), targetTexture, targetInternalformat, targetType,
-            targetLevel, m_unpackPremultiplyAlpha, m_unpackFlipY)) {
-            ASSERT_NOT_REACHED();
-        }
-    } else {
-        WebGLRenderingContextBase* gl = toWebGLRenderingContextBase(canvas->renderingContext());
-        ScopedTexture2DRestorer restorer(gl);
-        if (!gl->drawingBuffer()->copyToPlatformTexture(contextGL(), targetTexture, targetInternalformat, targetType,
-            targetLevel, m_unpackPremultiplyAlpha, !m_unpackFlipY, BackBuffer)) {
-            ASSERT_NOT_REACHED();
-        }
-    }
+    if (canvas)
+        texImageCanvasByGPU(canvas, targetTexture, targetInternalformat, targetType, targetLevel);
+    else
+        texImageBitmapByGPU(bitmap, targetTexture, targetInternalformat, targetType, targetLevel);
 
     if (!possibleDirectCopy) {
         GLuint tmpFBO;
@@ -4225,9 +4233,9 @@ void WebGLRenderingContextBase::texImage2D(GLenum target, GLint level, GLint int
     if (!validateTexFunc("texImage2D", TexImage, SourceHTMLCanvasElement, target, level, internalformat, canvas->width(), canvas->height(), 1, 0, format, type, 0, 0, 0))
         return;
 
-    // texImageCanvasByGPU relies on copyTextureCHROMIUM which doesn't support float/integer/sRGB internal format.
+    // texImageByGPU relies on copyTextureCHROMIUM which doesn't support float/integer/sRGB internal format.
     // FIXME: relax the constrains if copyTextureCHROMIUM is upgraded to handle more formats.
-    if (!canvas->renderingContext() || !canvas->renderingContext()->isAccelerated() || !canUseTexImageCanvasByGPU(internalformat, type)) {
+    if (!canvas->renderingContext() || !canvas->renderingContext()->isAccelerated() || !canUseTexImageByGPU(internalformat, type)) {
         // 2D canvas has only FrontBuffer.
         texImage2DImpl(target, level, internalformat, format, type, canvas->copiedImage(FrontBuffer, PreferAcceleration).get(),
             WebGLImageConversion::HtmlDomCanvas, m_unpackFlipY, m_unpackPremultiplyAlpha);
@@ -4235,7 +4243,7 @@ void WebGLRenderingContextBase::texImage2D(GLenum target, GLint level, GLint int
     }
 
     texImage2DBase(target, level, internalformat, canvas->width(), canvas->height(), 0, format, type, 0);
-    texImageCanvasByGPU(TexImage2DByGPU, texture, target, level, internalformat, type, 0, 0, 0, canvas);
+    texImageByGPU(TexImage2DByGPU, texture, target, level, internalformat, type, 0, 0, 0, canvas, nullptr);
 }
 
 PassRefPtr<Image> WebGLRenderingContextBase::videoFrameToImage(HTMLVideoElement* video)
@@ -4298,6 +4306,12 @@ void WebGLRenderingContextBase::texImage2D(GLenum target, GLint level, GLint int
     texImage2DImpl(target, level, internalformat, format, type, image.get(), WebGLImageConversion::HtmlDomVideo, m_unpackFlipY, m_unpackPremultiplyAlpha);
 }
 
+void WebGLRenderingContextBase::texImageBitmapByGPU(ImageBitmap* bitmap, GLuint targetTexture, GLenum targetInternalformat, GLenum targetType, GLint targetLevel)
+{
+    RefPtr<SkImage> skImage = bitmap->bitmapImage()->imageForCurrentFrame(drawingBuffer()->contextProvider());
+    bitmap->bitmapImage()->copyTexture(drawingBuffer()->contextProvider(), targetTexture, targetInternalformat, targetType);
+}
+
 void WebGLRenderingContextBase::texImage2D(GLenum target, GLint level, GLint internalformat,
     GLenum format, GLenum type, ImageBitmap* bitmap, ExceptionState& exceptionState)
 {
@@ -4305,29 +4319,34 @@ void WebGLRenderingContextBase::texImage2D(GLenum target, GLint level, GLint int
         return;
     if (!validateImageBitmap("texImage2D", bitmap, exceptionState))
         return;
-    if (!validateTexture2DBinding("texImage2D", target))
+    WebGLTexture* texture = validateTexture2DBinding("texImage2D", target);
+    if (!texture)
         return;
     if (!validateTexFunc("texImage2D", TexImage, SourceImageBitmap, target, level, internalformat, bitmap->width(), bitmap->height(), 1, 0, format, type, 0, 0, 0))
         return;
     ASSERT(bitmap->bitmapImage());
+    if (bitmap->isTextureBacked() && canUseTexImageByGPU(internalformat, type)) {
+        texImage2DBase(target, level, internalformat, bitmap->width(), bitmap->height(), 0, format, type, 0);
+        texImageByGPU(TexImage2DByGPU, texture, target, level, internalformat, type, 0, 0, 0, nullptr, bitmap);
+        return;
+    }
     RefPtr<SkImage> skImage = bitmap->bitmapImage()->imageForCurrentFrame();
     SkPixmap pixmap;
     OwnPtr<uint8_t[]> pixelData;
     uint8_t* pixelDataPtr = nullptr;
-    // TODO(crbug.com/613411): peekPixels fails if the SkImage is texture-backed
-    // Use texture mailbox in that case.
+    // In the case where we cannot use texImageByGPU, peekPixels() fails.

[Ken Russell (switch to Gerrit), 2016/06/01 22:55:09]

This comment seems to indicate that peekPixels() will always fail here. Is that
true? If so, why does the code covered by "peekSucceed" still exist? If not, the
comment should be adjusted.

[xidachen, 2016/06/02 13:52:52]

The comment is bad, it doesn't convey what I want it to say. I have changed it.

     bool peekSucceed = skImage->peekPixels(&pixmap);
     if (peekSucceed) {
         pixelDataPtr = static_cast<uint8_t*>(pixmap.writable_addr());
-    } else if (skImage->isTextureBacked()) {
+    } else {
         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) {
+    bool isPixelDataRGBA = (havePeekableRGBA || !peekSucceed);
+    if (isPixelDataRGBA && format == GL_RGBA && type == GL_UNSIGNED_BYTE) {
         needConversion = false;
     } else {
         if (type == GL_UNSIGNED_INT_10F_11F_11F_REV) {
@@ -4335,9 +4354,9 @@ void WebGLRenderingContextBase::texImage2D(GLenum target, GLint level, GLint int
             type = GL_FLOAT;
         }
         // In the case of ImageBitmap, we do not need to apply flipY or premultiplyAlpha.
-        bool isPixelDataBGRA = (peekSucceed && pixmap.colorType() == SkColorType::kBGRA_8888_SkColorType);
+        bool isPixelDataBGRA = 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))) {
+            || (isPixelDataRGBA && !WebGLImageConversion::extractImageData(pixelDataPtr, WebGLImageConversion::DataFormat::DataFormatRGBA8, bitmap->size(), format, type, false, false, data))) {
             synthesizeGLError(GL_INVALID_VALUE, "texImage2D", "bad image data");
             return;
         }
@@ -4550,7 +4569,7 @@ void WebGLRenderingContextBase::texSubImage2D(GLenum target, GLint level, GLint
         || extensionEnabled(OESTextureFloatName)
         || extensionEnabled(OESTextureHalfFloatName)
         || extensionEnabled(EXTsRGBName);
-    // texImageCanvasByGPU relies on copyTextureCHROMIUM which doesn't support float/integer/sRGB internal format.
+    // texImageByGPU relies on copyTextureCHROMIUM which doesn't support float/integer/sRGB internal format.
     // FIXME: relax the constrains if copyTextureCHROMIUM is upgraded to handle more formats.
     if (!canvas->renderingContext() || !canvas->renderingContext()->isAccelerated() || useReadBackPath) {
         // 2D canvas has only FrontBuffer.
@@ -4559,7 +4578,7 @@ void WebGLRenderingContextBase::texSubImage2D(GLenum target, GLint level, GLint
         return;
     }
 
-    texImageCanvasByGPU(TexSubImage2DByGPU, texture, target, level, GL_RGBA, type, xoffset, yoffset, 0, canvas);
+    texImageByGPU(TexSubImage2DByGPU, texture, target, level, GL_RGBA, type, xoffset, yoffset, 0, canvas, nullptr);
 }
 
 void WebGLRenderingContextBase::texSubImage2D(GLenum target, GLint level, GLint xoffset, GLint yoffset,
@@ -4587,37 +4606,46 @@ void WebGLRenderingContextBase::texSubImage2D(GLenum target, GLint level, GLint
         return;
     if (!validateImageBitmap("texSubImage2D", bitmap, exceptionState))
         return;
-    if (!validateTexture2DBinding("texSubImage2D", target))
+    WebGLTexture* texture = validateTexture2DBinding("texSubImage2D", target);
+    if (!texture)
         return;
-    if (!validateTexFunc("texSubImage2D", TexSubImage, SourceImageBitmap, target, level, 0, bitmap->width(), bitmap->height(), 1, 0, format, type, 0, 0, 0))
+    if (!validateTexFunc("texSubImage2D", TexSubImage, SourceImageBitmap, target, level, 0, bitmap->width(), bitmap->height(), 1, 0, format, type, xoffset, yoffset, 0))
+        return;
+    bool useReadBackPath = isWebGL2OrHigher()
+        || extensionEnabled(OESTextureFloatName)
+        || extensionEnabled(OESTextureHalfFloatName)
+        || extensionEnabled(EXTsRGBName);

[Ken Russell (switch to Gerrit), 2016/06/01 22:55:09]

This looks wrong. Whether the readback path is used should depend on the format
of the texture, not on whether these extensions are enabled. If these extensions
are enabled at all then this will destroy performance for texSubImage2D calls
taking ImageBitmaps. Why is this not using canUseTexImageByGPU like the
texImage2D case? Will generalizing copyTextureChromium (http://crbug.com/612542)
avoid the need for these special cases?

[xidachen, 2016/06/02 13:52:52]

This part of code is actually copied from line 4549--4553, which is from
texSubImage2D(canvas). My understanding of that code is that when a canvas is
texture backed, useReadBackPath indicates that it cannot do a GPU-GPU texture
copy? Is that correct?

+    if (bitmap->isTextureBacked() && !useReadBackPath) {
+        texImageByGPU(TexSubImage2DByGPU, texture, target, level, GL_RGBA, type, xoffset, yoffset, 0, nullptr, bitmap);
         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;
+    // In the case where we cannot use texImageByGPU, peekPixels() fails.

[Ken Russell (switch to Gerrit), 2016/06/01 22:55:09]

Same comment as above here.

[xidachen, 2016/06/02 13:52:52]

Done.

     bool peekSucceed = skImage->peekPixels(&pixmap);
     if (peekSucceed) {
         pixelDataPtr = static_cast<uint8_t*>(pixmap.writable_addr());
-    } else if (skImage->isTextureBacked()) {
+    } else {
         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) {
+    bool isPixelDataRGBA = (havePeekableRGBA || !peekSucceed);
+    if (isPixelDataRGBA && format == GL_RGBA && type == GL_UNSIGNED_BYTE) {
         needConversion = false;
     } else {
+        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;
+        }
         // In the case of ImageBitmap, we do not need to apply flipY or premultiplyAlpha.
-        bool isPixelDataBGRA = (peekSucceed && pixmap.colorType() == SkColorType::kBGRA_8888_SkColorType);
+        bool isPixelDataBGRA = 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))) {
+            || (isPixelDataRGBA && !WebGLImageConversion::extractImageData(pixelDataPtr, WebGLImageConversion::DataFormat::DataFormatRGBA8, bitmap->size(), format, type, false, false, data))) {
             synthesizeGLError(GL_INVALID_VALUE, "texSubImage2D", "bad image data");
             return;
         }