ImageBitmap: Change two enum uses.

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 "config.h"
 #include "core/frame/ImageBitmap.h"
 
 #include "core/html/HTMLCanvasElement.h"
 #include "core/html/HTMLVideoElement.h"
 #include "core/html/ImageData.h"
@@ skipping 80 matching lines @@
     ASSERT(canvas->isPaintable());
     m_bitmap = cropImage(canvas->copiedImage(BackBuffer, PreferAcceleration), cropRect);
 }
 
 ImageBitmap::ImageBitmap(ImageData* data, const IntRect& cropRect)
     : m_imageElement(nullptr)
     , m_cropRect(cropRect)
     , m_bitmapOffset(IntPoint())
 {
     IntRect srcRect = intersection(cropRect, IntRect(IntPoint(), data->size()));
-    OwnPtr<ImageBuffer> buffer = ImageBuffer::create(data->size(), Opaque, DoNotInitializeImagePixels);
+    OwnPtr<ImageBuffer> buffer = ImageBuffer::create(data->size(), NonOpaque, DoNotInitializeImagePixels);

[xidachen, 2015/10/18 14:08:48]

This part is tricky. It was set to be Opaque on purpose. Because in the
constructor of "UnacceleratedImageBufferSurface" class, the following line:

m_surface = adoptRef(SkSurface::NewRaster(info, Opaque == opacityMode ? 0 :
&disableLCDProps));

takes long time when it is NonOpaque, but takes almost no time when it is set to
Opaque. My opinion on this implementation is that the buffer will be filled in
by the source data later, so it doesn't matter whether it is opaque or
non-opaque at the buffer allocation step. However, this could be wrong.

[jbroman, 2015/10/18 15:44:14]

I'd like Justin's opinion, but it seems to me that this causes the image to have
kOpaque_AlphaType in its SkImageInfo, which shouldn't apply to images that
aren't opaque.

[xidachen, 2015/10/18 15:44:52]

Hmmm, I just found something very strange. With the change that you have in the
"UnacceleratedImageBufferSurface", it won't pass your layout test even if I
change this line to be:

OwnPtr<ImageBuffer> buffer = ImageBuffer::create(data->size(), NonOpaque,
InitializeImagePixels);

[jbroman, 2015/10/18 15:48:00]

Yeah, something fishy is going on (or I'm misunderstanding the ImageData API). I
spent about 30 min on Friday trying to figure out what was going on and why I
got very very wrong values, but I haven't dug any deeper.

[Justin Novosad, 2015/10/19 19:13:20]

I agree that the image buffer should be marked as non-opaque.  This will result
in the newImageSnapshot that is taken below to be marked as non-opaque, which is
conservatively accurate.

     if (!buffer)
         return;
 
     if (srcRect.width() > 0 && srcRect.height() > 0)
         buffer->putByteArray(Premultiplied, data->data()->data(), data->size(), srcRect, IntPoint(std::min(0, -cropRect.x()), std::min(0, -cropRect.y())));

[Justin Novosad, 2015/10/19 19:13:20]

This operation is an over-write. If it covers buffer entirely, we should not
need to intialize the pixels, and I think that is supposed to always be the
case.

 
     m_bitmap = buffer->newImageSnapshot();
     m_bitmapRect = IntRect(IntPoint(std::max(0, -cropRect.x()), std::max(0, -cropRect.y())),  srcRect.size());
 }
 
 ImageBitmap::ImageBitmap(ImageBitmap* bitmap, const IntRect& cropRect)
     : m_imageElement(bitmap->imageElement())
     , m_bitmap(nullptr)
     , m_cropRect(cropRect)
     , m_bitmapOffset(IntPoint())
@@ skipping 103 matching lines @@
     return FloatSize(width(), height());
 }
 
 DEFINE_TRACE(ImageBitmap)
 {
     visitor->trace(m_imageElement);
     ImageLoaderClient::trace(visitor);
 }
 
 } // namespace blink