Commit 4fed3346 authored by cblume's avatar cblume Committed by Commit Bot

Use SkCodec internally in GIFImageDecoder

Previously, GIFImageDecoder used GIFImageReader internally. SkCodec uses a
modified version of that class (SkGifImageReader; adapted in
crrev.com/2045293002). SkCodec provides the following benefits:
- SIMD optimized code for writing pixels
- an API that allows the client to handle caching
- flexibility regarding the required frame to use
- the ability to decode scaled versions of images
- subset decoding (i.e. issue 468914)
  (not fully implemented for GIF)
- the ability to decode to half-width float format

In addition, this patch enables sharing code between Android, Skia, and
Chromium. This means that new features/bug fixes in Android benefit Chromium
and vice versa.

For larger images (above ~60x60), the SIMD optimizations show a much bigger
benefit (up to 24% in one case). For most images, decoding speed is about the
same. Images with many frames that contain tiny update regions are a hair
slower. The mean decode time across all tested images showed an improvement. Raw
performance numbers can be found here:
https://docs.google.com/spreadsheets/d/1JqCPdYmbasOwKRdvuG6ZI4gwq9dPvnJA9oDquh7SxOQ/edit?usp=sharing

GIFImageDecoder still handles the cached frames currently, but this change will
allow future changes in Blink to make wiser caching Decisions (such as keeping
all frames of a 3-frame animation if those frames are small). Full road map:
https://docs.google.com/a/chromium.org/document/d/1T06pxiff3oy8KDqWGqWL-_nZqUJ8AMppouPQ_DLrvpk/edit?usp=sharing

This results in some behavior changes:
- SkCodec does not check the alpha of each pixel during decode (for speed and
  simplicity).
  As a result, GIFImageDecoder no longer corrects opacity or the required frame
  after decoding a frame. No performance penalty has been observed for
  incorrectly leaving a frame marked as having transparency.
- SkCodec guesses transparency based on the presence of a
  transparent index (in addition to being subset) and uses this to
  potentially determine an earlier required frame.

BUG=715812

Review-Url: https://codereview.chromium.org/2565323003
Cr-Commit-Position: refs/heads/master@{#495230}
parent a89466ce
......@@ -45,6 +45,10 @@ config("skia_config") {
"//third_party/skia/third_party/vulkan",
]
if (!is_ios) {
include_dirs += [ "//third_party/skia/include/codec" ]
}
defines = skia_for_chromium_defines
defines += [
"SK_HAS_PNG_LIBRARY",
......@@ -111,6 +115,7 @@ config("skia_library_config") {
"//third_party/skia/src/sfnt",
"//third_party/skia/src/utils",
"//third_party/skia/src/lazy",
"//third_party/skia/third_party/gif",
]
if (is_mac || is_ios) {
include_dirs += [ "//third_party/skia/include/utils/mac" ]
......@@ -295,6 +300,30 @@ component("skia") {
"//third_party/skia/src/sfnt/SkOTTable_name.cpp",
"//third_party/skia/src/sfnt/SkOTUtils.cpp",
]
if (!is_ios) {
sources += [
"//third_party/skia/src/codec/SkBmpBaseCodec.cpp",
"//third_party/skia/src/codec/SkBmpCodec.cpp",
"//third_party/skia/src/codec/SkBmpMaskCodec.cpp",
"//third_party/skia/src/codec/SkBmpRLECodec.cpp",
"//third_party/skia/src/codec/SkBmpStandardCodec.cpp",
"//third_party/skia/src/codec/SkCodec.cpp",
"//third_party/skia/src/codec/SkGifCodec.cpp",
"//third_party/skia/src/codec/SkIcoCodec.cpp",
"//third_party/skia/src/codec/SkJpegCodec.cpp",
"//third_party/skia/src/codec/SkJpegDecoderMgr.cpp",
"//third_party/skia/src/codec/SkJpegUtility.cpp",
"//third_party/skia/src/codec/SkMaskSwizzler.cpp",
"//third_party/skia/src/codec/SkMasks.cpp",
"//third_party/skia/src/codec/SkPngCodec.cpp",
"//third_party/skia/src/codec/SkSampler.cpp",
"//third_party/skia/src/codec/SkStreamBuffer.cpp",
"//third_party/skia/src/codec/SkSwizzler.cpp",
"//third_party/skia/src/codec/SkWbmpCodec.cpp",
"//third_party/skia/src/codec/SkWebpCodec.cpp",
"//third_party/skia/third_party/gif/SkGifImageReader.cpp",
]
}
# This and skia_opts are really the same conceptual target so share headers.
allow_circular_includes_from = [ ":skia_opts" ]
......
......@@ -1180,14 +1180,14 @@ component("platform") {
"image-decoders/ImageFrame.h",
"image-decoders/SegmentReader.cpp",
"image-decoders/SegmentReader.h",
"image-decoders/SegmentStream.cpp",
"image-decoders/SegmentStream.h",
"image-decoders/bmp/BMPImageDecoder.cpp",
"image-decoders/bmp/BMPImageDecoder.h",
"image-decoders/bmp/BMPImageReader.cpp",
"image-decoders/bmp/BMPImageReader.h",
"image-decoders/gif/GIFImageDecoder.cpp",
"image-decoders/gif/GIFImageDecoder.h",
"image-decoders/gif/GIFImageReader.cpp",
"image-decoders/gif/GIFImageReader.h",
"image-decoders/ico/ICOImageDecoder.cpp",
"image-decoders/ico/ICOImageDecoder.h",
"image-decoders/jpeg/JPEGImageDecoder.cpp",
......@@ -1826,6 +1826,7 @@ test("blink_platform_unittests") {
"image-decoders/ImageDecoderTest.cpp",
"image-decoders/ImageDecoderTestHelpers.cpp",
"image-decoders/ImageDecoderTestHelpers.h",
"image-decoders/SegmentStreamTest.cpp",
"image-decoders/bmp/BMPImageDecoderTest.cpp",
"image-decoders/gif/GIFImageDecoderTest.cpp",
"image-decoders/ico/ICOImageDecoderTest.cpp",
......
......@@ -248,15 +248,6 @@ class PLATFORM_EXPORT ImageDecoder {
// Callers may pass WTF::kNotFound to clear all frames.
// Note: If |frame_buffer_cache_| contains only one frame, it won't be
// cleared. Returns the number of bytes of frame data actually cleared.
//
// This is a virtual method because MockImageDecoder needs to override it in
// order to run the test ImageFrameGeneratorTest::ClearMultiFrameDecode.
//
// @TODO Let MockImageDecoder override ImageFrame::ClearFrameBuffer instead,
// so this method can be made non-virtual. It is used in the test
// ImageFrameGeneratorTest::ClearMultiFrameDecode. The test needs to
// be modified since two frames may be kept in cache, instead of
// always just one, with this ClearCacheExceptFrame implementation.
virtual size_t ClearCacheExceptFrame(size_t);
// If the image has a cursor hot-spot, stores it in the argument
......@@ -396,7 +387,9 @@ class PLATFORM_EXPORT ImageDecoder {
// |index| is smaller than |frame_buffer_cache_|.size().
virtual bool FrameStatusSufficientForSuccessors(size_t index) {
DCHECK(index < frame_buffer_cache_.size());
return frame_buffer_cache_[index].GetStatus() != ImageFrame::kFrameEmpty;
ImageFrame::Status frame_status = frame_buffer_cache_[index].GetStatus();
return frame_status == ImageFrame::kFramePartial ||
frame_status == ImageFrame::kFrameComplete;
}
private:
......
......@@ -84,8 +84,12 @@ bool ImageFrame::CopyBitmapData(const ImageFrame& other) {
has_alpha_ = other.has_alpha_;
bitmap_.reset();
SkImageInfo info = other.bitmap_.info();
return bitmap_.tryAllocPixels(info) &&
other.bitmap_.readPixels(info, bitmap_.getPixels(), bitmap_.rowBytes(),
if (!bitmap_.tryAllocPixels(info)) {
return false;
}
status_ = kFrameAllocated;
return other.bitmap_.readPixels(info, bitmap_.getPixels(), bitmap_.rowBytes(),
0, 0);
}
......@@ -100,6 +104,7 @@ bool ImageFrame::TakeBitmapDataIfWritable(ImageFrame* other) {
bitmap_.reset();
bitmap_.swap(other->bitmap_);
other->status_ = kFrameEmpty;
status_ = kFrameAllocated;
return true;
}
......@@ -113,7 +118,11 @@ bool ImageFrame::AllocatePixelData(int new_width,
new_width, new_height,
premultiply_alpha_ ? kPremul_SkAlphaType : kUnpremul_SkAlphaType,
std::move(color_space)));
return bitmap_.tryAllocPixels(allocator_);
bool allocated = bitmap_.tryAllocPixels(allocator_);
if (allocated)
status_ = kFrameAllocated;
return allocated;
}
bool ImageFrame::HasAlpha() const {
......
......@@ -45,7 +45,7 @@ class PLATFORM_EXPORT ImageFrame final {
DISALLOW_NEW_EXCEPT_PLACEMENT_NEW();
public:
enum Status { kFrameEmpty, kFramePartial, kFrameComplete };
enum Status { kFrameEmpty, kFrameAllocated, kFramePartial, kFrameComplete };
enum DisposalMethod {
// If you change the numeric values of these, make sure you audit
// all users, as some users may cast raw values to/from these
......
// Copyright (c) 2017 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 "platform/image-decoders/SegmentStream.h"
#include <utility>
#include "platform/image-decoders/SegmentReader.h"
namespace blink {
SegmentStream::SegmentStream() = default;
SegmentStream::SegmentStream(SegmentStream&& rhs)
: reader_(std::move(rhs.reader_)), position_(rhs.position_) {}
SegmentStream& SegmentStream::operator=(SegmentStream&& rhs) {
reader_ = std::move(rhs.reader_);
position_ = rhs.position_;
return *this;
}
SegmentStream::~SegmentStream() = default;
void SegmentStream::SetReader(WTF::RefPtr<SegmentReader> reader) {
reader_ = std::move(reader);
}
bool SegmentStream::IsCleared() const {
return !reader_ || position_ > reader_->size();
}
size_t SegmentStream::read(void* buffer, size_t size) {
DCHECK(!IsCleared());
size = std::min(size, reader_->size() - position_);
size_t bytes_advanced = 0;
if (!buffer) { // skipping, not reading
bytes_advanced = size;
} else {
bytes_advanced = peek(buffer, size);
}
position_ += bytes_advanced;
return bytes_advanced;
}
size_t SegmentStream::peek(void* buffer, size_t size) const {
DCHECK(!IsCleared());
size = std::min(size, reader_->size() - position_);
size_t total_bytes_peeked = 0;
char* buffer_as_char_ptr = reinterpret_cast<char*>(buffer);
while (size) {
const char* segment = nullptr;
size_t bytes_peeked =
reader_->GetSomeData(segment, position_ + total_bytes_peeked);
if (!bytes_peeked)
break;
if (bytes_peeked > size)
bytes_peeked = size;
memcpy(buffer_as_char_ptr, segment, bytes_peeked);
buffer_as_char_ptr += bytes_peeked;
size -= bytes_peeked;
total_bytes_peeked += bytes_peeked;
}
return total_bytes_peeked;
}
bool SegmentStream::isAtEnd() const {
return !reader_ || position_ >= reader_->size();
}
bool SegmentStream::rewind() {
position_ = 0;
return true;
}
bool SegmentStream::seek(size_t position) {
position_ = position;
return true;
}
bool SegmentStream::move(long offset) {
DCHECK_GT(offset, 0);
position_ += offset;
return true;
}
size_t SegmentStream::getLength() const {
if (reader_)
return reader_->size();
return 0;
}
} // namespace blink
// Copyright (c) 2017 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.
#ifndef SegmentStream_h
#define SegmentStream_h
#include <algorithm>
#include "platform/PlatformExport.h"
#include "platform/wtf/RefPtr.h"
#include "third_party/skia/include/core/SkStream.h"
namespace blink {
class SegmentReader;
class PLATFORM_EXPORT SegmentStream : public SkStream {
public:
SegmentStream();
SegmentStream(const SegmentStream&) = delete;
SegmentStream& operator=(const SegmentStream&) = delete;
SegmentStream(SegmentStream&&);
SegmentStream& operator=(SegmentStream&&);
~SegmentStream() override;
void SetReader(WTF::RefPtr<SegmentReader>);
// If a buffer has shrunk beyond the point we have read, it has been cleared.
// This allows clients to be aware of when data suddenly disappears.
bool IsCleared() const;
// From SkStream:
size_t read(void* buffer, size_t) override;
size_t peek(void* buffer, size_t) const override;
bool isAtEnd() const override;
bool rewind() override;
bool hasPosition() const override { return true; }
size_t getPosition() const override { return position_; }
bool seek(size_t position) override;
bool move(long offset) override;
bool hasLength() const override { return true; }
size_t getLength() const override;
private:
WTF::RefPtr<SegmentReader> reader_;
size_t position_ = 0;
};
} // namespace blink
#endif
......@@ -29,12 +29,12 @@
#include <memory>
#include "platform/image-decoders/ImageDecoder.h"
#include "platform/wtf/Noncopyable.h"
#include "platform/wtf/RefPtr.h"
#include "third_party/skia/include/codec/SkCodec.h"
namespace blink {
class GIFImageReader;
using GIFRow = Vector<unsigned char>;
class SegmentStream;
// This class decodes the GIF image format.
class PLATFORM_EXPORT GIFImageDecoder final : public ImageDecoder {
......@@ -44,56 +44,42 @@ class PLATFORM_EXPORT GIFImageDecoder final : public ImageDecoder {
GIFImageDecoder(AlphaOption, const ColorBehavior&, size_t max_decoded_bytes);
~GIFImageDecoder() override;
enum GIFParseQuery { kGIFSizeQuery, kGIFFrameCountQuery };
// ImageDecoder:
String FilenameExtension() const override { return "gif"; }
void OnSetData(SegmentReader* data) override;
int RepetitionCount() const override;
bool FrameIsReceivedAtIndex(size_t) const override;
float FrameDurationAtIndex(size_t) const override;
// CAUTION: SetFailed() deletes |reader_|. Be careful to avoid
// accessing deleted memory, especially when calling this from inside
// GIFImageReader!
// CAUTION: SetFailed() deletes |codec_|. Be careful to avoid
// accessing deleted memory.
bool SetFailed() override;
// Callbacks from the GIF reader.
bool HaveDecodedRow(size_t frame_index,
GIFRow::const_iterator row_begin,
size_t width,
size_t row_number,
unsigned repeat_count,
bool write_transparent_pixels);
bool FrameComplete(size_t frame_index);
// For testing.
bool ParseCompleted() const;
size_t ClearCacheExceptFrame(size_t) override;
private:
// ImageDecoder:
void ClearFrameBuffer(size_t frame_index) override;
virtual void DecodeSize() { Parse(kGIFSizeQuery); }
void DecodeSize() override {}
size_t DecodeFrameCount() override;
void InitializeNewFrame(size_t) override;
void Decode(size_t) override;
// Parses as much as is needed to answer the query, ignoring bitmap
// data. If parsing fails, sets the "decode failure" flag.
void Parse(GIFParseQuery);
// Reset the alpha tracker for this frame. Before calling this method, the
// caller must verify that the frame exists.
void OnInitFrameBuffer(size_t) override;
// When the disposal method of the frame is DisposeOverWritePrevious, the
// next frame will use the previous frame's buffer as its starting state, so
// next frame will use a previous frame's buffer as its starting state, so
// we can't take over the data in that case. Before calling this method, the
// caller must verify that the frame exists.
bool CanReusePreviousFrameBuffer(size_t) const override;
bool current_buffer_saw_alpha_;
mutable int repetition_count_;
std::unique_ptr<GIFImageReader> reader_;
// When a frame depends on a previous frame's content, there is a list of
// candidate reference frames. This function will find a previous frame from
// that list which satisfies the requirements of being a reference frame
// (kFrameComplete, not kDisposeOverwritePrevious).
// If no frame is found, it returns kNotFound.
size_t GetViableReferenceFrameIndex(size_t) const;
std::unique_ptr<SkCodec> codec_;
// |codec_| owns the SegmentStream, but we need access to it to append more
// data as it arrives.
SegmentStream* segment_stream_;
mutable int repetition_count_ = kAnimationLoopOnce;
};
} // namespace blink
......
......@@ -59,16 +59,8 @@ void TestRepetitionCount(const char* dir,
RefPtr<SharedBuffer> data = ReadFile(dir, file);
ASSERT_TRUE(data.Get());
decoder->SetData(data.Get(), true);
EXPECT_EQ(kAnimationLoopOnce,
decoder->RepetitionCount()); // Default value before decode.
for (size_t i = 0; i < decoder->FrameCount(); ++i) {
ImageFrame* frame = decoder->DecodeFrameBufferAtIndex(i);
EXPECT_EQ(ImageFrame::kFrameComplete, frame->GetStatus());
}
EXPECT_EQ(expected_repetition_count,
decoder->RepetitionCount()); // Expected value after decode.
EXPECT_EQ(expected_repetition_count, decoder->RepetitionCount());
}
} // anonymous namespace
......@@ -79,7 +71,6 @@ TEST(GIFImageDecoderTest, decodeTwoFrames) {
RefPtr<SharedBuffer> data = ReadFile(kLayoutTestResourcesDir, "animated.gif");
ASSERT_TRUE(data.Get());
decoder->SetData(data.Get(), true);
EXPECT_EQ(kAnimationLoopOnce, decoder->RepetitionCount());
ImageFrame* frame = decoder->DecodeFrameBufferAtIndex(0);
uint32_t generation_id0 = frame->Bitmap().getGenerationID();
......@@ -104,10 +95,6 @@ TEST(GIFImageDecoderTest, parseAndDecode) {
RefPtr<SharedBuffer> data = ReadFile(kLayoutTestResourcesDir, "animated.gif");
ASSERT_TRUE(data.Get());
decoder->SetData(data.Get(), true);
EXPECT_EQ(kAnimationLoopOnce, decoder->RepetitionCount());
// This call will parse the entire file.
EXPECT_EQ(2u, decoder->FrameCount());
ImageFrame* frame = decoder->DecodeFrameBufferAtIndex(0);
EXPECT_EQ(ImageFrame::kFrameComplete, frame->GetStatus());
......@@ -321,10 +308,13 @@ TEST(GIFImageDecoderTest, invalidDisposalMethod) {
EXPECT_EQ(2u, decoder->FrameCount());
// Disposal method 4 is converted to ImageFrame::DisposeOverwritePrevious.
// This is because some specs say method 3 is "overwrite previous", while
// others say setting the third bit (i.e. method 4) is.
EXPECT_EQ(ImageFrame::kDisposeOverwritePrevious,
decoder->DecodeFrameBufferAtIndex(0)->GetDisposalMethod());
// Disposal method 5 is ignored.
EXPECT_EQ(ImageFrame::kDisposeNotSpecified,
// Unknown disposal methods (5 in this case) are converted to
// ImageFrame::DisposeKeep.
EXPECT_EQ(ImageFrame::kDisposeKeep,
decoder->DecodeFrameBufferAtIndex(1)->GetDisposalMethod());
}
......@@ -391,11 +381,11 @@ TEST(GIFImageDecoderTest, bitmapAlphaType) {
ImageFrame* premul_frame = premul_decoder->DecodeFrameBufferAtIndex(0);
EXPECT_TRUE(premul_frame &&
premul_frame->GetStatus() != ImageFrame::kFrameComplete);
EXPECT_EQ(premul_frame->Bitmap().alphaType(), kPremul_SkAlphaType);
EXPECT_EQ(kPremul_SkAlphaType, premul_frame->Bitmap().alphaType());
ImageFrame* unpremul_frame = unpremul_decoder->DecodeFrameBufferAtIndex(0);
EXPECT_TRUE(unpremul_frame &&
unpremul_frame->GetStatus() != ImageFrame::kFrameComplete);
EXPECT_EQ(unpremul_frame->Bitmap().alphaType(), kUnpremul_SkAlphaType);
EXPECT_EQ(kUnpremul_SkAlphaType, unpremul_frame->Bitmap().alphaType());
// Fully decoded frame => the frame alpha type is known (opaque).
premul_decoder->SetData(full_data_buffer.Get(), true);
......@@ -405,11 +395,11 @@ TEST(GIFImageDecoderTest, bitmapAlphaType) {
premul_frame = premul_decoder->DecodeFrameBufferAtIndex(0);
EXPECT_TRUE(premul_frame &&
premul_frame->GetStatus() == ImageFrame::kFrameComplete);
EXPECT_EQ(premul_frame->Bitmap().alphaType(), kOpaque_SkAlphaType);
EXPECT_EQ(kOpaque_SkAlphaType, premul_frame->Bitmap().alphaType());
unpremul_frame = unpremul_decoder->DecodeFrameBufferAtIndex(0);
EXPECT_TRUE(unpremul_frame &&
unpremul_frame->GetStatus() == ImageFrame::kFrameComplete);
EXPECT_EQ(unpremul_frame->Bitmap().alphaType(), kOpaque_SkAlphaType);
EXPECT_EQ(kOpaque_SkAlphaType, unpremul_frame->Bitmap().alphaType());
}
namespace {
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment