Avoid copy pixel data in texImage2D(ImageBitmap)

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

 // Copyright 2013 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 "core/frame/ImageBitmap.h"
 
 #include "core/html/HTMLCanvasElement.h"
 #include "core/html/HTMLVideoElement.h"
 #include "core/html/ImageData.h"
 #include "platform/graphics/skia/SkiaUtils.h"
@@ skipping 73 matching lines @@
     return newSkImageFromRaster(info, std::move(imagePixels), imageRowBytes);
 }
 
 static PassRefPtr<SkImage> premulSkImageToUnPremul(SkImage* input)
 {
     SkImageInfo info = SkImageInfo::Make(input->width(), input->height(), kN32_SkColorType, kUnpremul_SkAlphaType);
     OwnPtr<uint8_t[]> dstPixels = copySkImageData(input, info);
     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())
         return nullptr;
     ImageFrame* frame = decoder->frameBufferAtIndex(0);
     if (!frame || frame->getStatus() != ImageFrame::FrameComplete)
         return nullptr;
     SkBitmap bitmap = frame->bitmap();
     if (!frameIsValid(bitmap))
         return nullptr;
     return adoptRef(SkImage::NewFromBitmap(bitmap));
 }
 
 // The parameter imageFormat indicates whether the first parameter "image" is unpremultiplied or not.
+// imageFormat = PremultiplyAlpha means the image is in premuliplied format
 // For example, if the image is already in unpremultiplied format and we want the created ImageBitmap
 // in the same format, then we don't need to use the ImageDecoder to decode the image.
-static PassRefPtr<StaticBitmapImage> cropImage(Image* image, const IntRect& cropRect, bool flipY, bool premultiplyAlpha, AlphaDisposition imageFormat = DontPremultiplyAlpha, ImageDecoder::GammaAndColorProfileOption colorSpaceOp = ImageDecoder::GammaAndColorProfileApplied)
+static PassRefPtr<StaticBitmapImage> cropImage(Image* image, const IntRect& cropRect, bool flipY, bool premultiplyAlpha, AlphaDisposition imageFormat = PremultiplyAlpha, ImageDecoder::GammaAndColorProfileOption colorSpaceOp = ImageDecoder::GammaAndColorProfileApplied)
 {
     ASSERT(image);
 
     IntRect imgRect(IntPoint(), IntSize(image->width(), image->height()));
     const IntRect srcRect = intersection(imgRect, cropRect);
 
     // In the case when cropRect doesn't intersect the source image and it requires a umpremul image
     // We immediately return a transparent black image with cropRect.size()
     if (srcRect.isEmpty() && !premultiplyAlpha) {
         SkImageInfo info = SkImageInfo::Make(cropRect.width(), cropRect.height(), kN32_SkColorType, kUnpremul_SkAlphaType);
         OwnPtr<uint8_t[]> dstPixels = adoptArrayPtr(new uint8_t[cropRect.width() * cropRect.height() * info.bytesPerPixel()]());
         return StaticBitmapImage::create(newSkImageFromRaster(info, std::move(dstPixels), cropRect.width() * info.bytesPerPixel()));
     }
 
     RefPtr<SkImage> skiaImage = image->imageForCurrentFrame();
     // Attempt to get raw unpremultiplied image data, executed only when skiaImage is premultiplied.
-    if ((((!premultiplyAlpha && !skiaImage->isOpaque()) || !skiaImage) && image->data() && imageFormat == DontPremultiplyAlpha) || colorSpaceOp == ImageDecoder::GammaAndColorProfileIgnored) {
+    if ((((!premultiplyAlpha && !skiaImage->isOpaque()) || !skiaImage) && image->data() && imageFormat == PremultiplyAlpha) || colorSpaceOp == ImageDecoder::GammaAndColorProfileIgnored) {
         OwnPtr<ImageDecoder> decoder(ImageDecoder::create(*(image->data()),
             premultiplyAlpha ? ImageDecoder::AlphaPremultiplied : ImageDecoder::AlphaNotPremultiplied,
             colorSpaceOp));
         if (!decoder)
             return nullptr;
         decoder->setData(image->data(), true);
         skiaImage = ImageBitmap::getSkImageFromDecoder(std::move(decoder));
         if (!skiaImage)
             return nullptr;
     }
 
     if (cropRect == srcRect) {
         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 == DontPremultiplyAlpha)
+            return StaticBitmapImage::create(unPremulSkImageToPremul(croppedSkImage));
         // Call preroll to trigger image decoding.
         croppedSkImage->preroll();
         return StaticBitmapImage::create(adoptRef(croppedSkImage));
     }
 
     sk_sp<SkSurface> surface = SkSurface::MakeRasterN32Premul(cropRect.width(), cropRect.height());
     if (!surface)
         return nullptr;
     if (srcRect.isEmpty())
         return StaticBitmapImage::create(adoptRef(surface->newImageSnapshot()));
 
     SkScalar dstLeft = std::min(0, -cropRect.x());
     SkScalar dstTop = std::min(0, -cropRect.y());
     if (cropRect.x() < 0)
         dstLeft = -cropRect.x();
     if (cropRect.y() < 0)
         dstTop = -cropRect.y();
     surface->getCanvas()->drawImage(skiaImage.get(), dstLeft, dstTop);
     if (flipY)
         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()));
 }
 
 ImageBitmap::ImageBitmap(HTMLImageElement* image, const IntRect& cropRect, Document* document, const ImageBitmapOptions& options)
 {
     bool flipY;
     bool premultiplyAlpha;
     parseOptions(options, flipY, premultiplyAlpha);
 
     if (options.colorSpaceConversion() == "none")
-        m_image = cropImage(image->cachedImage()->getImage(), cropRect, flipY, premultiplyAlpha, DontPremultiplyAlpha, ImageDecoder::GammaAndColorProfileIgnored);
+        m_image = cropImage(image->cachedImage()->getImage(), cropRect, flipY, premultiplyAlpha, PremultiplyAlpha, ImageDecoder::GammaAndColorProfileIgnored);
     else
-        m_image = cropImage(image->cachedImage()->getImage(), cropRect, flipY, premultiplyAlpha, DontPremultiplyAlpha, ImageDecoder::GammaAndColorProfileApplied);
+        m_image = cropImage(image->cachedImage()->getImage(), cropRect, flipY, premultiplyAlpha, PremultiplyAlpha, ImageDecoder::GammaAndColorProfileApplied);

[Ken Russell (switch to Gerrit), 2016/05/27 21:17:08]

This change is concerning. It's important that PNGs with an alpha channel can be
used to construct an ImageBitmap without multiplying the alpha channel into the
color channels. Perhaps the new code supports this semantic, but it's difficult
to tell just by reading the code. Is this well tested?

[xidachen, 2016/05/28 00:51:11]

Actually there is no change in here in terms of code execution. Please refer to
the change in cropImage() function, I changed all the code that uses this
parameter from "DontPremultiplyAlpha" to "PremultiplyAlpha". The only reason for
this change is to make the code more readable. After this change, the 
"PremultiplyAlpha" indicates that the first parameter of cropImage() is already
in premultiplied format, which is more readable.

     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->isTextureBacked() && !skImage->peekPixels(&pixmap)) {
+        sk_sp<SkSurface> surface = SkSurface::MakeRasterN32Premul(skImage->width(), skImage->height());
+        surface->getCanvas()->drawImage(skImage.get(), 0, 0);

[Ken Russell (switch to Gerrit), 2016/05/27 21:17:08]

This looks to me like it will always force the alpha channel to be multiplied in
to the color channels, which is supposed to be a configurable option. How is
this code path tested?

[xidachen, 2016/05/28 00:51:11]

I think the concern here is that this piece of code might be executed when
caller request the premultiplyAlpha=none. However, the code in cropImage() makes
sure that this won't happen. The reason is that the SkImage extracted from an
HTMLImageElement is always in premultiplied format, and if caller request
premultiplyAlpha=none, it will go into the part of code in cropImage() where a
decoder is generated to decode the SkImage. After that, the return value of
cropImage() is always in decoded state and peekPixels() always succeed.

As a matter of fact, I looked into the layout test:
tex-image-and-sub-image-2d-image-bitmap-from-image-bitmap-from-image.html, 
and this drawImage only gets called when premultiplyAlpha is set to
"premultiply" or "default". In the case when premultiplyAlpha="none", this piece
of code is never reached.

+        m_image = StaticBitmapImage::create(adoptRef(surface->newImageSnapshot()));
+    }
     m_image->setOriginClean(!image->wouldTaintOrigin(document->getSecurityOrigin()));
     m_image->setPremultiplied(premultiplyAlpha);
 }
 
 ImageBitmap::ImageBitmap(HTMLVideoElement* video, const IntRect& cropRect, Document* document, const ImageBitmapOptions& options)
 {
     IntSize playerSize;
     if (video->webMediaPlayer())
         playerSize = video->webMediaPlayer()->naturalSize();
 
@@ skipping 121 matching lines @@
         m_image = StaticBitmapImage::create(flipSkImageVertically(buffer->newSkImageSnapshot(PreferNoAcceleration, SnapshotReasonUnknown).get(), PremultiplyAlpha));
     else
         m_image = StaticBitmapImage::create(buffer->newSkImageSnapshot(PreferNoAcceleration, SnapshotReasonUnknown));
 }
 
 ImageBitmap::ImageBitmap(ImageBitmap* bitmap, const IntRect& cropRect, const ImageBitmapOptions& options)
 {
     bool flipY;
     bool premultiplyAlpha;
     parseOptions(options, flipY, premultiplyAlpha);
-    m_image = cropImage(bitmap->bitmapImage(), cropRect, flipY, premultiplyAlpha, bitmap->isPremultiplied() ? DontPremultiplyAlpha : PremultiplyAlpha);
+    m_image = cropImage(bitmap->bitmapImage(), cropRect, flipY, premultiplyAlpha, bitmap->isPremultiplied() ? PremultiplyAlpha : DontPremultiplyAlpha);
     if (!m_image)
         return;
     m_image->setOriginClean(bitmap->originClean());
     m_image->setPremultiplied(premultiplyAlpha);
 }
 
 ImageBitmap::ImageBitmap(PassRefPtr<StaticBitmapImage> image, const IntRect& cropRect, const ImageBitmapOptions& options)
 {
     bool flipY;
     bool premultiplyAlpha;
     parseOptions(options, flipY, premultiplyAlpha);
-    m_image = cropImage(image.get(), cropRect, flipY, premultiplyAlpha, PremultiplyAlpha);
+    m_image = cropImage(image.get(), cropRect, flipY, premultiplyAlpha, DontPremultiplyAlpha);
     if (!m_image)
         return;
     m_image->setOriginClean(image->originClean());
     m_image->setPremultiplied(premultiplyAlpha);
 }
 
 ImageBitmap::ImageBitmap(PassRefPtr<StaticBitmapImage> image)
 {
     m_image = image;
 }
@@ skipping 143 matching lines @@
 FloatSize ImageBitmap::elementSize(const FloatSize&) const
 {
     return FloatSize(width(), height());
 }
 
 DEFINE_TRACE(ImageBitmap)
 {
 }
 
 } // namespace blink