Save a bitmap copy when advancing to dependent GIF and WebP animation frames

third_party/WebKit/Source/platform/image-decoders/ImageFrame.cpp

 /*
  * Copyright (C) 2006 Apple Computer, Inc.  All rights reserved.
  * Copyright (C) 2008, 2009 Google, Inc.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
  * are met:
  * 1. Redistributions of source code must retain the above copyright
  *    notice, this list of conditions and the following disclaimer.
  * 2. Redistributions in binary form must reproduce the above copyright
  *    notice, this list of conditions and the following disclaimer in the
  *    documentation and/or other materials provided with the distribution.
  *
  * THIS SOFTWARE IS PROVIDED BY APPLE COMPUTER, INC. ``AS IS'' AND ANY
  * EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
  * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
  * PURPOSE ARE DISCLAIMED.  IN NO EVENT SHALL APPLE COMPUTER, INC. OR
  * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL,
  * EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO,
  * PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR
  * PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY
  * OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
  * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
  * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
  */
 
 #include "platform/image-decoders/ImageFrame.h"
 
 #include "platform/RuntimeEnabledFeatures.h"
+#include "platform/graphics/skia/SkiaUtils.h"
 #include "platform/image-decoders/ImageDecoder.h"
 
 namespace blink {
 
 ImageFrame::ImageFrame()
     : m_allocator(0)
     , m_hasAlpha(true)
     , m_status(FrameEmpty)
     , m_duration(0)
     , m_disposalMethod(DisposeNotSpecified)
@@ skipping 41 matching lines @@
 }
 
 void ImageFrame::zeroFillPixelData()
 {
     m_bitmap.eraseARGB(0, 0, 0, 0);
     m_hasAlpha = true;
 }
 
 bool ImageFrame::copyBitmapData(const ImageFrame& other)
 {
-    if (this == &other)
-        return true;
-
+    DCHECK_NE(this, &other);
     m_hasAlpha = other.m_hasAlpha;
     m_bitmap.reset();
     return other.m_bitmap.copyTo(&m_bitmap, other.m_bitmap.colorType());
 }
 
+bool ImageFrame::takeBitmapDataIfWritable(ImageFrame* other)
+{
+    DCHECK(other);
+    DCHECK_NE(this, other);
+    if (other->m_bitmap.isImmutable())
+        return false;
+    m_hasAlpha = other->m_hasAlpha;
+    m_bitmap.reset();
+    m_bitmap.swap(other->m_bitmap);
+    other->m_status = FrameEmpty;
+    return true;
+}
+
 bool ImageFrame::setSizeAndColorProfile(int newWidth, int newHeight, const ICCProfile& newIccProfile)
 {
     // setSizeAndColorProfile() should only be called once, it leaks memory otherwise.
     ASSERT(!width() && !height());
 
     sk_sp<SkColorSpace> colorSpace;
     if (RuntimeEnabledFeatures::colorCorrectRenderingEnabled() && !newIccProfile.isEmpty())
         colorSpace = SkColorSpace::NewICC(newIccProfile.data(), newIccProfile.size());
 
     m_bitmap.setInfo(SkImageInfo::MakeN32(newWidth, newHeight,
         m_premultiplyAlpha ? kPremul_SkAlphaType : kUnpremul_SkAlphaType, colorSpace));
     if (!m_bitmap.tryAllocPixels(m_allocator, 0))
         return false;
 
     zeroFillPixelData();
     return true;
 }
 
 bool ImageFrame::hasAlpha() const
 {
     return m_hasAlpha;
 }
 
+sk_sp<SkImage> ImageFrame::finalizePixelsAndGetImage()
+{
+    DCHECK(m_status == FrameComplete);

[Peter Kasting, 2016/09/22 21:44:08]

Nit: DCHECK_EQ(FrameComplete, m_status)

[aleksandar.stojiljkovic, 2016/09/27 18:08:28]

Done.

+
+    // We don't set the status to immutable until there is a request to get
+    // SkImage and decoding is done.  We set the bitmap to immutable here to
+    // avoid copying in SkImage::MakeFromBitmap.

[Peter Kasting, 2016/09/22 21:44:08]

Nit: Second sentence is already in the header and can be eliminated.  Not really
sure what the first sentence is saying.  Are you justifying why setStatus() does
not mark the bitmap as immutable?  If so, move this comment there and say
something like this instead:

An immutable bitmap would mean callers could construct an SkImage from this
without a copy.  However, setting the bitmap immutable here would defeat
takeBitmapDataIfWritable().  Instead we let the bitmap stay mutable until
someone calls finalizePixelsAndGetImage() to actually get the SkImage.

[aleksandar.stojiljkovic, 2016/09/27 18:08:28]

Done. Rephrased it, removed the note about copy as it is in the header and
joined it with the comment in setStatus.

+    if (!m_bitmap.isImmutable())

[Peter Kasting, 2016/09/22 21:44:08]

Nit: Conditional check unnecessary

[aleksandar.stojiljkovic, 2016/09/27 18:08:28]

Done.

+        m_bitmap.setImmutable();
+    return SkImage::MakeFromBitmap(m_bitmap);
+}
+
 void ImageFrame::setHasAlpha(bool alpha)
 {
     m_hasAlpha = alpha;
 
     m_bitmap.setAlphaType(computeAlphaType());
 }
 
 void ImageFrame::setStatus(Status status)
 {
     m_status = status;
     if (m_status == FrameComplete) {
         m_bitmap.setAlphaType(computeAlphaType());
         // Send pending pixels changed notifications now, because we can't do this after
         // the bitmap has been marked immutable.
         notifyBitmapIfPixelsChanged();
-        m_bitmap.setImmutable(); // Tell the bitmap it's done.
     }
 }
 
 void ImageFrame::zeroFillFrameRect(const IntRect& rect)
 {
     if (rect.isEmpty())
         return;
 
     m_bitmap.eraseArea(rect, SkColorSetARGB(0, 0, 0, 0));
     setHasAlpha(true);
 }
 
 SkAlphaType ImageFrame::computeAlphaType() const
 {
     // If the frame is not fully loaded, there will be transparent pixels,
     // so we can't tell skia we're opaque, even for image types that logically
     // always are (e.g. jpeg).
     if (!m_hasAlpha && m_status == FrameComplete)
         return kOpaque_SkAlphaType;
 
     return m_premultiplyAlpha ? kPremul_SkAlphaType : kUnpremul_SkAlphaType;
 }
 
 } // namespace blink