Avoid copy pixel data in texImage2D(ImageBitmap)

third_party/WebKit/Source/core/frame/ImageBitmap.cpp

Index: third_party/WebKit/Source/core/frame/ImageBitmap.cpp
diff --git a/third_party/WebKit/Source/core/frame/ImageBitmap.cpp b/third_party/WebKit/Source/core/frame/ImageBitmap.cpp
index 5948ce5b57be0f025f050f34ee7b3af099b128b6..5e0a92a56132185f54c78438a83a28d84867fbd4 100644
--- a/third_party/WebKit/Source/core/frame/ImageBitmap.cpp
+++ b/third_party/WebKit/Source/core/frame/ImageBitmap.cpp
@@ -91,6 +91,13 @@ static PassRefPtr<SkImage> premulSkImageToUnPremul(SkImage* input)
     return newSkImageFromRaster(info, std::move(dstPixels), input->width() * info.bytesPerPixel());
 }
 
+static PassRefPtr<SkImage> unPremulSkImageToPremul(SkImage* input)
+{
+    SkImageInfo info = SkImageInfo::Make(input->width(), input->height(), kN32_SkColorType, kPremul_SkAlphaType);
+    OwnPtr<uint8_t[]> dstPixels = copySkImageData(input, info);
+    return newSkImageFromRaster(info, std::move(dstPixels), input->width() * info.bytesPerPixel());
+}
+
 PassRefPtr<SkImage> ImageBitmap::getSkImageFromDecoder(PassOwnPtr<ImageDecoder> decoder)
 {
     if (!decoder->frameCount())
@@ -140,6 +147,9 @@ static PassRefPtr<StaticBitmapImage> cropImage(Image* image, const IntRect& crop
         if (flipY)
             return StaticBitmapImage::create(flipSkImageVertically(skiaImage->newSubset(srcRect), premultiplyAlpha ? PremultiplyAlpha : DontPremultiplyAlpha));
         SkImage* croppedSkImage = skiaImage->newSubset(srcRect);
+        // Special case: The first parameter image is unpremul but we need to turn it into premul.
+        if (premultiplyAlpha && imageFormat == PremultiplyAlpha)

[Justin Novosad, 2016/05/27 14:16:18]

Unless I am misreading this, I think you mean "premultiplyAlpha && imageFormat
== DontPremultiplyAlpha"

[xidachen, 2016/05/27 15:10:13]

Made changes so that imageFormat == DontPremultiplyAlpha means the image is in
unpremul format.

+            return StaticBitmapImage::create(unPremulSkImageToPremul(croppedSkImage));
         // Call preroll to trigger image decoding.
         croppedSkImage->preroll();
         return StaticBitmapImage::create(adoptRef(croppedSkImage));
@@ -162,8 +172,11 @@ static PassRefPtr<StaticBitmapImage> cropImage(Image* image, const IntRect& crop
         skiaImage = flipSkImageVertically(surface->newImageSnapshot(), PremultiplyAlpha);
     else
         skiaImage = adoptRef(surface->newImageSnapshot());
-    if (premultiplyAlpha)
+    if (premultiplyAlpha) {
+        if (imageFormat == PremultiplyAlpha)
+            return StaticBitmapImage::create(unPremulSkImageToPremul(skiaImage.get()));
         return StaticBitmapImage::create(skiaImage.release());
+    }
     return StaticBitmapImage::create(premulSkImageToUnPremul(skiaImage.get()));
 }
 
@@ -179,6 +192,15 @@ ImageBitmap::ImageBitmap(HTMLImageElement* image, const IntRect& cropRect, Docum
         m_image = cropImage(image->cachedImage()->getImage(), cropRect, flipY, premultiplyAlpha, DontPremultiplyAlpha, ImageDecoder::GammaAndColorProfileApplied);
     if (!m_image)
         return;
+    // In the case where the source image is lazy-decoded, m_image may not be in
+    // a decoded state, we trigger it here.
+    RefPtr<SkImage> skImage = m_image->imageForCurrentFrame();
+    SkPixmap pixmap;
+    if (!skImage->peekPixels(&pixmap)) {

[Justin Novosad, 2016/05/27 14:16:18]

This condition will force an immediate readback of texture backed images, which
I don't think is what you intend here.

[xidachen, 2016/05/27 15:10:13]

Double checked with fmalita@, peekPixels() will not trigger a readback. It
returns false and ignore the pixmap completely, no readback is involved.

[Justin Novosad, 2016/05/27 16:13:47]

That is not what I meant.
I was not referring to the peekPixels call. I was referring to the condition of
if statement being wrong. If skImage is texture backed this code will enter the
if statement.  Inside the "if" the call to drawImage will trigger a readback.
What I am suggesting is that the condition in the if statement should be
"!skImage->isTextureBacked() && !skImage->peekPixels(&pixmap)" in order to
prevent the readback in drawImage().

[xidachen, 2016/05/27 17:14:56]

Yes, that makes sense, changed.

+        sk_sp<SkSurface> surface = SkSurface::MakeRasterN32Premul(skImage->width(), skImage->height());
+        surface->getCanvas()->drawImage(skImage.get(), 0, 0);
+        m_image = StaticBitmapImage::create(adoptRef(surface->newImageSnapshot()));
+    }
     m_image->setOriginClean(!image->wouldTaintOrigin(document->getSecurityOrigin()));
     m_image->setPremultiplied(premultiplyAlpha);
 }