Skip to content

Commit

Permalink
Plumb the ability to store non-SkImage in PaintImage.
Browse files Browse the repository at this point in the history
This patch adds the ability for PaintImage to be backed by
structure other than SkImage. In this patch, the following
changes are made:
 - Add a PaintImageBuilder for partially setting some of
   the flags for use on the final PaintImage
 - Removes ImageForCurrentFrame from blink and replaces it
   with PopulateImageForCurrentFrame which takes a paint
   image builder instead.
 - Replaces SkImages constructed from paint records to
   store paint records directly on the PaintImage
 - Renames sk_image() to GetSkImage(), which generates an
   underlying SkImage and caches it on PaintImage
 - Exposes width/height and IsLazyGenerated from PaintImage
   so that access to the underlying SkImage is limited.

Follow-up work:
 - Add ability to store a decoder on PaintImage
 - Stop using GetSkImage internally for things like
   width/height.

R=chrishtr@chromium.org, khushalsagar@chromium.org

Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
Change-Id: I0b95e13d437d7a99ad80e2286c2bb0c58330a9b3
Reviewed-on: https://chromium-review.googlesource.com/583930
Commit-Queue: Vladimir Levin <vmpstr@chromium.org>
Reviewed-by: Khushal <khushalsagar@chromium.org>
Reviewed-by: Chris Harrelson <chrishtr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#490227}
  • Loading branch information
vmpstr authored and Commit Bot committed Jul 28, 2017
1 parent d2dfc3e commit 4f2c08c
Show file tree
Hide file tree
Showing 59 changed files with 405 additions and 225 deletions.
3 changes: 1 addition & 2 deletions cc/blink/web_image_layer_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,7 @@ blink::WebLayer* WebImageLayerImpl::Layer() {

void WebImageLayerImpl::SetImage(cc::PaintImage image) {
static_cast<WebLayerImplFixedBounds*>(layer_.get())
->SetFixedBounds(
gfx::Size(image.sk_image()->width(), image.sk_image()->height()));
->SetFixedBounds(gfx::Size(image.width(), image.height()));
static_cast<cc::PictureImageLayer*>(layer_->layer())
->SetImage(std::move(image));
}
Expand Down
12 changes: 6 additions & 6 deletions cc/layers/picture_image_layer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ std::unique_ptr<LayerImpl> PictureImageLayer::CreateLayerImpl(
}

bool PictureImageLayer::HasDrawableContent() const {
return image_.sk_image() && PictureLayer::HasDrawableContent();
return image_ && PictureLayer::HasDrawableContent();
}

void PictureImageLayer::SetImage(PaintImage image) {
Expand All @@ -54,15 +54,15 @@ gfx::Rect PictureImageLayer::PaintableRegion() {

scoped_refptr<DisplayItemList> PictureImageLayer::PaintContentsToDisplayList(
ContentLayerClient::PaintingControlSetting painting_control) {
DCHECK(image_.sk_image());
DCHECK_GT(image_.sk_image()->width(), 0);
DCHECK_GT(image_.sk_image()->height(), 0);
DCHECK(image_);
DCHECK_GT(image_.width(), 0);
DCHECK_GT(image_.height(), 0);
DCHECK(layer_tree_host());

float content_to_layer_scale_x =
static_cast<float>(bounds().width()) / image_.sk_image()->width();
static_cast<float>(bounds().width()) / image_.width();
float content_to_layer_scale_y =
static_cast<float>(bounds().height()) / image_.sk_image()->height();
static_cast<float>(bounds().height()) / image_.height();
bool has_scale = !MathUtil::IsWithinEpsilon(content_to_layer_scale_x, 1.f) ||
!MathUtil::IsWithinEpsilon(content_to_layer_scale_y, 1.f);

Expand Down
2 changes: 2 additions & 0 deletions cc/paint/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ cc_component("paint") {
"paint_flags.h",
"paint_image.cc",
"paint_image.h",
"paint_image_builder.cc",
"paint_image_builder.h",
"paint_op_buffer.cc",
"paint_op_buffer.h",
"paint_op_reader.cc",
Expand Down
8 changes: 4 additions & 4 deletions cc/paint/discardable_image_map.cc
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ class DiscardableImageGenerator {
PaintOpType op_type = static_cast<PaintOpType>(op->type);
if (op_type == PaintOpType::DrawImage) {
auto* image_op = static_cast<DrawImageOp*>(op);
auto* sk_image = image_op->image.sk_image().get();
auto* sk_image = image_op->image.GetSkImage().get();
AddImage(image_op->image,
SkRect::MakeIWH(sk_image->width(), sk_image->height()),
SkRect::MakeXYWH(image_op->left, image_op->top,
Expand Down Expand Up @@ -156,7 +156,7 @@ class DiscardableImageGenerator {
const PaintImage& paint_image = flags.getShader()->paint_image();
SkMatrix local_matrix = flags.getShader()->GetLocalMatrix();
AddImage(paint_image,
SkRect::MakeFromIRect(paint_image.sk_image()->bounds()), rect,
SkRect::MakeWH(paint_image.width(), paint_image.height()), rect,
&local_matrix, flags);
}

Expand All @@ -165,7 +165,7 @@ class DiscardableImageGenerator {
const SkRect& rect,
const SkMatrix* local_matrix,
const PaintFlags& flags) {
if (!paint_image.sk_image()->isLazyGenerated())
if (!paint_image.IsLazyGenerated())
return;

const SkRect& clip_rect = SkRect::Make(canvas_.getDeviceClipBounds());
Expand Down Expand Up @@ -204,7 +204,7 @@ class DiscardableImageGenerator {

// Make a note if any image was originally specified in a non-sRGB color
// space.
SkColorSpace* source_color_space = paint_image.sk_image()->colorSpace();
SkColorSpace* source_color_space = paint_image.GetSkImage()->colorSpace();
color_stats_total_pixel_count_ += image_rect.size().GetCheckedArea();
color_stats_total_image_count_++;
if (!source_color_space || source_color_space->isSRGB()) {
Expand Down
2 changes: 1 addition & 1 deletion cc/paint/draw_image.h
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ class CC_PAINT_EXPORT DrawImage {
bool operator==(const DrawImage& other) const;

const PaintImage& paint_image() const { return paint_image_; }
const sk_sp<SkImage>& image() const { return paint_image_.sk_image(); }
const sk_sp<SkImage>& image() const { return paint_image_.GetSkImage(); }
const SkSize& scale() const { return scale_; }
const SkIRect& src_rect() const { return src_rect_; }
SkFilterQuality filter_quality() const { return filter_quality_; }
Expand Down
24 changes: 22 additions & 2 deletions cc/paint/paint_image.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@

#include "cc/paint/paint_image.h"
#include "base/atomic_sequence_num.h"
#include "cc/paint/paint_record.h"
#include "ui/gfx/skia_util.h"

namespace cc {
namespace {
Expand All @@ -17,8 +19,8 @@ PaintImage::PaintImage(Id id,
CompletionState completion_state,
size_t frame_count,
bool is_multipart)
: id_(id),
sk_image_(std::move(sk_image)),
: sk_image_(std::move(sk_image)),
id_(id),
animation_type_(animation_type),
completion_state_(completion_state),
frame_count_(frame_count),
Expand All @@ -45,7 +47,25 @@ PaintImage::Id PaintImage::GetNextId() {
PaintImage PaintImage::CloneWithSkImage(sk_sp<SkImage> new_image) const {
PaintImage result(*this);
result.sk_image_ = std::move(new_image);
result.cached_sk_image_ = nullptr;
result.paint_record_ = nullptr;
result.paint_record_rect_ = gfx::Rect();
return result;
}

const sk_sp<SkImage>& PaintImage::GetSkImage() const {
if (cached_sk_image_)
return cached_sk_image_;

if (sk_image_) {
cached_sk_image_ = sk_image_;
} else if (paint_record_) {
cached_sk_image_ = SkImage::MakeFromPicture(
ToSkPicture(paint_record_, gfx::RectToSkRect(paint_record_rect_)),
SkISize::Make(paint_record_rect_.width(), paint_record_rect_.height()),
nullptr, nullptr, SkImage::BitDepth::kU8, SkColorSpace::MakeSRGB());
}
return cached_sk_image_;
}

} // namespace cc
26 changes: 22 additions & 4 deletions cc/paint/paint_image.h
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,13 @@
#include "base/logging.h"
#include "cc/paint/paint_export.h"
#include "third_party/skia/include/core/SkImage.h"
#include "ui/gfx/geometry/rect.h"

namespace cc {

class PaintOpBuffer;
using PaintRecord = PaintOpBuffer;

// TODO(vmpstr): Add a persistent id to the paint image.
class CC_PAINT_EXPORT PaintImage {
public:
Expand Down Expand Up @@ -56,32 +60,46 @@ class CC_PAINT_EXPORT PaintImage {
PaintImage& operator=(PaintImage&& other);

bool operator==(const PaintImage& other) const;
explicit operator bool() const { return static_cast<bool>(sk_image_); }

Id stable_id() const { return id_; }
const sk_sp<SkImage>& sk_image() const { return sk_image_; }
const sk_sp<SkImage>& GetSkImage() const;
AnimationType animation_type() const { return animation_type_; }
CompletionState completion_state() const { return completion_state_; }
size_t frame_count() const { return frame_count_; }
bool is_multipart() const { return is_multipart_; }

// TODO(vmpstr): Don't get the SkImage here if you don't need to.
explicit operator bool() const { return !!GetSkImage(); }
bool IsLazyGenerated() const { return GetSkImage()->isLazyGenerated(); }
int width() const { return GetSkImage()->width(); }
int height() const { return GetSkImage()->height(); }

// Returns a PaintImage that has the same fields as this PaintImage, except
// with a replaced sk_image_. This can be used to swap out a specific SkImage
// in an otherwise unchanged PaintImage.
// in an otherwise unchanged PaintImage. This is also used to swap out a
// paint record representation with an SkImage instead.
PaintImage CloneWithSkImage(sk_sp<SkImage> new_image) const;

private:
Id id_ = kUnknownStableId;
friend class PaintImageBuilder;

sk_sp<SkImage> sk_image_;
sk_sp<PaintRecord> paint_record_;
gfx::Rect paint_record_rect_;

Id id_ = kUnknownStableId;
AnimationType animation_type_ = AnimationType::UNKNOWN;
CompletionState completion_state_ = CompletionState::UNKNOWN;

// The number of frames known to exist in this image (eg number of GIF frames
// loaded). 0 indicates either unknown or only a single frame, both of which
// should be treated similarly.
size_t frame_count_ = 0;

// Whether the data fetched for this image is a part of a multpart response.
bool is_multipart_ = false;

mutable sk_sp<SkImage> cached_sk_image_;
};

} // namespace cc
Expand Down
17 changes: 17 additions & 0 deletions cc/paint/paint_image_builder.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
// Copyright 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 "cc/paint/paint_image_builder.h"

namespace cc {

PaintImageBuilder::PaintImageBuilder() = default;
PaintImageBuilder::~PaintImageBuilder() = default;

PaintImage PaintImageBuilder::TakePaintImage() const {
DCHECK(!paint_image_.sk_image_ || !paint_image_.paint_record_);
return std::move(paint_image_);
}

} // namespace cc
49 changes: 49 additions & 0 deletions cc/paint/paint_image_builder.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
// Copyright 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 CC_PAINT_PAINT_IMAGE_BUILDER_H_
#define CC_PAINT_PAINT_IMAGE_BUILDER_H_

#include "cc/paint/paint_export.h"
#include "cc/paint/paint_image.h"
#include "cc/paint/paint_op_buffer.h"

namespace cc {

class CC_PAINT_EXPORT PaintImageBuilder {
public:
PaintImageBuilder();
~PaintImageBuilder();

void set_id(PaintImage::Id id) { paint_image_.id_ = id; }
void set_image(sk_sp<SkImage> sk_image) {
paint_image_.sk_image_ = std::move(sk_image);
}
void set_paint_record(sk_sp<PaintRecord> paint_record,
const gfx::Rect& rect) {
paint_image_.paint_record_ = std::move(paint_record);
paint_image_.paint_record_rect_ = rect;
}
void set_animation_type(PaintImage::AnimationType type) {
paint_image_.animation_type_ = type;
}
void set_completion_state(PaintImage::CompletionState state) {
paint_image_.completion_state_ = state;
}
void set_frame_count(size_t frame_count) {
paint_image_.frame_count_ = frame_count;
}
void set_is_multipart(bool is_multipart) {
paint_image_.is_multipart_ = is_multipart;
}

PaintImage TakePaintImage() const;

private:
PaintImage paint_image_;
};

} // namespace cc

#endif // CC_PAINT_PAINT_IMAGE_BUILDER_H_
23 changes: 11 additions & 12 deletions cc/paint/paint_op_buffer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -75,8 +75,8 @@ class ScopedImageFlags {

SkMatrix total_image_matrix = matrix;
total_image_matrix.preConcat(ctm);
SkRect src_rect = SkRect::MakeIWH(paint_image.sk_image()->width(),
paint_image.sk_image()->height());
SkRect src_rect =
SkRect::MakeIWH(paint_image.width(), paint_image.height());
holder_ = image_provider->GetDecodedImage(
paint_image, src_rect, flags.getFilterQuality(), total_image_matrix);

Expand Down Expand Up @@ -1252,8 +1252,7 @@ void DrawImageOp::RasterWithFlags(const DrawImageOp* op,
SkPaint paint = flags->ToSkPaint();

if (params.image_provider) {
SkRect image_rect = SkRect::MakeIWH(op->image.sk_image()->width(),
op->image.sk_image()->height());
SkRect image_rect = SkRect::MakeIWH(op->image.width(), op->image.height());
auto decoded_image_holder = params.image_provider->GetDecodedImage(
op->image, image_rect,
flags ? flags->getFilterQuality() : kNone_SkFilterQuality,
Expand All @@ -1278,7 +1277,7 @@ void DrawImageOp::RasterWithFlags(const DrawImageOp* op,
if (need_scale)
canvas->restore();
} else {
canvas->drawImage(op->image.sk_image().get(), op->left, op->top, &paint);
canvas->drawImage(op->image.GetSkImage().get(), op->left, op->top, &paint);
}
}

Expand Down Expand Up @@ -1321,8 +1320,8 @@ void DrawImageRectOp::RasterWithFlags(const DrawImageRectOp* op,
canvas->drawImageRect(decoded_image.image().get(), adjusted_src, op->dst,
&paint, skconstraint);
} else {
canvas->drawImageRect(op->image.sk_image().get(), op->src, op->dst, &paint,
skconstraint);
canvas->drawImageRect(op->image.GetSkImage().get(), op->src, op->dst,
&paint, skconstraint);
}
}

Expand Down Expand Up @@ -1546,9 +1545,9 @@ bool PaintOp::GetBounds(const PaintOp* op, SkRect* rect) {
}
case PaintOpType::DrawImage: {
auto* image_op = static_cast<const DrawImageOp*>(op);
*rect = SkRect::MakeXYWH(image_op->left, image_op->top,
image_op->image.sk_image()->width(),
image_op->image.sk_image()->height());
*rect =
SkRect::MakeXYWH(image_op->left, image_op->top,
image_op->image.width(), image_op->image.height());
rect->sort();
return true;
}
Expand Down Expand Up @@ -1701,7 +1700,7 @@ DrawImageOp::DrawImageOp(const PaintImage& image,
bool DrawImageOp::HasDiscardableImages() const {
// TODO(khushalsagar): Callers should not be able to change the lazy generated
// state for a PaintImage.
return image.sk_image()->isLazyGenerated();
return image.IsLazyGenerated();
}

DrawImageOp::~DrawImageOp() = default;
Expand All @@ -1720,7 +1719,7 @@ DrawImageRectOp::DrawImageRectOp(const PaintImage& image,
constraint(constraint) {}

bool DrawImageRectOp::HasDiscardableImages() const {
return image.sk_image()->isLazyGenerated();
return image.IsLazyGenerated();
}

DrawImageRectOp::~DrawImageRectOp() = default;
Expand Down
7 changes: 6 additions & 1 deletion cc/paint/paint_op_buffer.h
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
#include <stdint.h>

#include <string>
#include <type_traits>

#include "base/debug/alias.h"
#include "base/logging.h"
Expand Down Expand Up @@ -891,7 +892,11 @@ class CC_PAINT_EXPORT TranslateOp final : public PaintOp {

#undef HAS_SERIALIZATION_FUNCTIONS

using LargestPaintOp = DrawDRRectOp;
// TODO(vmpstr): Revisit this when sizes of DrawImageRectOp change.
using LargestPaintOp =
typename std::conditional<(sizeof(DrawImageRectOp) > sizeof(DrawDRRectOp)),
DrawImageRectOp,
DrawDRRectOp>::type;

class CC_PAINT_EXPORT PaintOpBuffer : public SkRefCnt {
public:
Expand Down
4 changes: 2 additions & 2 deletions cc/paint/paint_op_buffer_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2379,8 +2379,8 @@ TEST(PaintOpBufferTest, BoundingRect_DrawImageOp) {
auto* op = static_cast<DrawImageOp*>(base_op);

SkRect image_rect =
SkRect::MakeXYWH(op->left, op->top, op->image.sk_image()->width(),
op->image.sk_image()->height());
SkRect::MakeXYWH(op->left, op->top, op->image.GetSkImage()->width(),
op->image.GetSkImage()->height());
ASSERT_TRUE(PaintOp::GetBounds(op, &rect));
EXPECT_EQ(rect, image_rect.makeSorted());
}
Expand Down
2 changes: 1 addition & 1 deletion cc/paint/paint_shader.cc
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ sk_sp<SkShader> PaintShader::GetSkShader() const {
local_matrix_ ? &*local_matrix_ : nullptr);
break;
case Type::kImage:
cached_shader_ = image_.sk_image()->makeShader(
cached_shader_ = image_.GetSkImage()->makeShader(
tx_, ty_, local_matrix_ ? &*local_matrix_ : nullptr);
break;
case Type::kPaintRecord: {
Expand Down
Loading

0 comments on commit 4f2c08c

Please sign in to comment.