Commit 445fcc90 authored by Reilly Grant's avatar Reilly Grant Committed by Commit Bot

Revert "Stop building SkCodec"

This reverts commit fd493b4b.

Reason for revert: Broken build: https://build.chromium.org/p/chromium.webkit/builders/WebKit%20Win%20x64%20Builder%20%28dbg%29/builds/114259

Original change's description:
> Stop building SkCodec
> 
> Bug: 768878
> 
> Building SkCodec seems to have caused a paint regression on a webpage
> without any images. This leads us to suspect "some minor compiler
> optimization tickling". Stop building it to confirm. Two CLs rely on
> SkCodec:
> 
> "Use SkCodec internally in GIFImageDecoder"
> 4fed3346. This introduced building
> SkCodec.
> 
> "Enable Skia's SkImageGenerator implementation"
> f5eb27c2. This used SkCodec to fix
> crbug.com/758459, but that seems to have been fixed in another way.
> 
> In addition, this corrects some formatting in the old code (as
> commanded by presubmit), and makes some other minor changes (no more
> PassRefPtr, FrameDurationAtIndex now returns a TimeDelta).
> 
> Change-Id: Ic2bdd87740da0232c9c07e27eed6049efc26d76c
> Reviewed-on: https://chromium-review.googlesource.com/718918
> Commit-Queue: Leon Scroggins <scroggo@chromium.org>
> Reviewed-by: Chris Blume <cblume@chromium.org>
> Reviewed-by: Fredrik Söderquist <fs@opera.com>
> Reviewed-by: Leon Scroggins <scroggo@chromium.org>
> Reviewed-by: Philip Rogers <pdr@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#509570}

TBR=scroggo@chromium.org,pdr@chromium.org,fs@opera.com,cblume@chromium.org

Change-Id: Ib4c5be2e885f483d881ca65689cb5e9f3dc755df
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 768878
Reviewed-on: https://chromium-review.googlesource.com/724359Reviewed-by: default avatarReilly Grant <reillyg@chromium.org>
Commit-Queue: Reilly Grant <reillyg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#509577}
parent d36ebac4
......@@ -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",
......@@ -282,16 +286,39 @@ component("skia") {
"//third_party/skia/src/images/SkPngEncoder.cpp",
"//third_party/skia/src/images/SkWebpEncoder.cpp",
"//third_party/skia/src/ports/SkGlobalInitialization_default.cpp",
"//third_party/skia/src/ports/SkImageGenerator_none.cpp",
"//third_party/skia/src/ports/SkOSFile_stdio.cpp",
"//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/SkCodecImageGenerator.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/src/images/SkJPEGWriteUtility.cpp",
"//third_party/skia/src/images/SkJpegEncoder.cpp",
"//third_party/skia/src/ports/SkImageGenerator_skia.cpp",
"//third_party/skia/third_party/gif/SkGifImageReader.cpp",
]
} else {
sources += [ "//third_party/skia/src/ports/SkImageGenerator_none.cpp" ]
}
# This and skia_opts are really the same conceptual target so share headers.
......
......@@ -1178,14 +1178,14 @@ jumbo_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",
......@@ -1859,6 +1859,7 @@ jumbo_source_set("blink_platform_unittests_sources") {
"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",
......
......@@ -253,15 +253,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
......@@ -401,7 +392,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:
......
......@@ -82,8 +82,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);
}
......@@ -98,6 +102,7 @@ bool ImageFrame::TakeBitmapDataIfWritable(ImageFrame* other) {
bitmap_.reset();
bitmap_.swap(other->bitmap_);
other->status_ = kFrameEmpty;
status_ = kFrameAllocated;
return true;
}
......@@ -111,7 +116,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 {
......
......@@ -46,7 +46,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,13 +29,13 @@
#include <memory>
#include "platform/image-decoders/ImageDecoder.h"
#include "platform/wtf/Noncopyable.h"
#include "platform/wtf/RefPtr.h"
#include "platform/wtf/Time.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 {
......@@ -45,56 +45,43 @@ 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;
TimeDelta 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;
int prior_frame_;
};
} // namespace blink
......
......@@ -58,16 +58,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
......@@ -78,7 +70,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();
......@@ -103,10 +94,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());
......@@ -320,10 +307,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());
}
......@@ -390,11 +380,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);
......@@ -404,11 +394,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