Skip to content

Commit

Permalink
Make draw image use SkM44
Browse files Browse the repository at this point in the history
SkMatrix is deprecated, we should be moving to SkM44s. Thought this
would maybe be a fix for crbug.com/1195276, but it wasn't. This is the
fix: crrev.com/c/2821714

Bug: 1155544
Change-Id: I5b79aba2c13872d4df4c883b77d4bc80b6e96d1e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2821850
Reviewed-by: vmpstr <vmpstr@chromium.org>
Reviewed-by: Aaron Krajeski <aaronhk@chromium.org>
Reviewed-by: Juanmi Huertas <juanmihd@chromium.org>
Commit-Queue: Aaron Krajeski <aaronhk@chromium.org>
Cr-Commit-Position: refs/heads/master@{#872913}
  • Loading branch information
mysteryDate authored and Chromium LUCI CQ committed Apr 15, 2021
1 parent 0c51020 commit 38ff085
Show file tree
Hide file tree
Showing 24 changed files with 100 additions and 104 deletions.
22 changes: 12 additions & 10 deletions cc/paint/discardable_image_map.cc
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ class DiscardableImageGenerator {
ScopedResult GetRasterContent(const DrawImage& draw_image) override {
generator_->AddImage(draw_image.paint_image(), false,
SkRect::Make(draw_image.src_rect()), op_rect_,
SkMatrix::I(), draw_image.filter_quality());
SkM44(), draw_image.filter_quality());
return ScopedResult();
}

Expand Down Expand Up @@ -124,7 +124,7 @@ class DiscardableImageGenerator {
op_rect = local_op_rect.value();
}

const SkMatrix& ctm = canvas->getTotalMatrix();
const SkM44& ctm = canvas->getLocalToDevice();
if (op->IsPaintOpWithFlags()) {
AddImageFromFlags(op_rect,
static_cast<const PaintOpWithFlags*>(op)->flags, ctm);
Expand All @@ -139,8 +139,11 @@ class DiscardableImageGenerator {
op_rect, ctm, image_op->flags.getFilterQuality());
} else if (op_type == PaintOpType::DrawImageRect) {
auto* image_rect_op = static_cast<DrawImageRectOp*>(op);
SkMatrix matrix =
SkMatrix::RectToRect(image_rect_op->src, image_rect_op->dst) * ctm;
// TODO(crbug.com/1155544): Make a RectToRect method that uses SkM44s
// in MathUtil.
SkM44 matrix = SkM44(SkMatrix::RectToRect(image_rect_op->src,
image_rect_op->dst)) *
ctm;
AddImage(image_rect_op->image,
image_rect_op->flags.useDarkModeForImage(), image_rect_op->src,
op_rect, matrix, image_rect_op->flags.getFilterQuality());
Expand All @@ -154,7 +157,7 @@ class DiscardableImageGenerator {

void AddImageFromFlags(const gfx::Rect& op_rect,
const PaintFlags& flags,
const SkMatrix& ctm) {
const SkM44& ctm) {
// TODO(prashant.n): Add dark mode support for images from shaders/filters.
AddImageFromShader(op_rect, flags.getShader(), ctm,
flags.getFilterQuality());
Expand All @@ -163,15 +166,14 @@ class DiscardableImageGenerator {

void AddImageFromShader(const gfx::Rect& op_rect,
const PaintShader* shader,
const SkMatrix& ctm,
const SkM44& ctm,
SkFilterQuality filter_quality) {
if (!shader || !shader->has_discardable_images())
return;

if (shader->shader_type() == PaintShader::Type::kImage) {
const PaintImage& paint_image = shader->paint_image();
SkMatrix matrix = ctm;
matrix.postConcat(shader->GetLocalMatrix());
SkM44 matrix = ctm * SkM44(shader->GetLocalMatrix());
// TODO(prashant.n): Add dark mode support for images from shader.
AddImage(paint_image, false,
SkRect::MakeWH(paint_image.width(), paint_image.height()),
Expand All @@ -188,7 +190,7 @@ class DiscardableImageGenerator {
}

SkRect scaled_tile_rect;
if (!shader->GetRasterizationTileRect(ctm, &scaled_tile_rect)) {
if (!shader->GetRasterizationTileRect(ctm.asM33(), &scaled_tile_rect)) {
return;
}

Expand Down Expand Up @@ -234,7 +236,7 @@ class DiscardableImageGenerator {
bool use_dark_mode,
const SkRect& src_rect,
const gfx::Rect& image_rect,
const SkMatrix& matrix,
const SkM44& matrix,
SkFilterQuality filter_quality) {
if (paint_image.IsTextureBacked())
return;
Expand Down
13 changes: 8 additions & 5 deletions cc/paint/draw_image.cc
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,13 @@ namespace {

// Helper funciton to extract a scale from the matrix. Returns true on success
// and false on failure.
bool ExtractScale(const SkMatrix& matrix, SkSize* scale) {
*scale = SkSize::Make(matrix.getScaleX(), matrix.getScaleY());
if (matrix.getType() & SkMatrix::kAffine_Mask) {
if (!matrix.decomposeScale(scale)) {
bool ExtractScale(const SkM44& matrix, SkSize* scale) {
*scale = SkSize::Make(matrix.rc(0, 0), matrix.rc(1, 1));
// TODO(crbug.com/1155544): Don't use SkMatrix here, add functionality to
// MathUtil.
SkMatrix mat33 = matrix.asM33();
if (mat33.getType() & SkMatrix::kAffine_Mask) {
if (!mat33.decomposeScale(scale)) {
scale->set(1, 1);
return false;
}
Expand Down Expand Up @@ -44,7 +47,7 @@ DrawImage::DrawImage(PaintImage image,
bool use_dark_mode,
const SkIRect& src_rect,
SkFilterQuality filter_quality,
const SkMatrix& matrix,
const SkM44& matrix,
base::Optional<size_t> frame_index,
const base::Optional<gfx::ColorSpace>& color_space,
float sdr_white_level)
Expand Down
4 changes: 2 additions & 2 deletions cc/paint/draw_image.h
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
#include "cc/paint/paint_image.h"
#include "third_party/skia/include/core/SkFilterQuality.h"
#include "third_party/skia/include/core/SkImage.h"
#include "third_party/skia/include/core/SkMatrix.h"
#include "third_party/skia/include/core/SkM44.h"
#include "third_party/skia/include/core/SkRect.h"
#include "third_party/skia/include/core/SkRefCnt.h"
#include "ui/gfx/color_space.h"
Expand All @@ -30,7 +30,7 @@ class CC_PAINT_EXPORT DrawImage {
bool use_dark_mode,
const SkIRect& src_rect,
SkFilterQuality filter_quality,
const SkMatrix& matrix,
const SkM44& matrix,
base::Optional<size_t> frame_index = base::nullopt,
const base::Optional<gfx::ColorSpace>& color_space = base::nullopt,
float sdr_white_level = gfx::ColorSpace::kDefaultSDRWhiteLevel);
Expand Down
2 changes: 1 addition & 1 deletion cc/paint/paint_filter.cc
Original file line number Diff line number Diff line change
Expand Up @@ -684,7 +684,7 @@ sk_sp<PaintFilter> ImagePaintFilter::SnapshotWithImagesInternal(
ImageProvider* image_provider) const {
DrawImage draw_image(image_, false,
SkIRect::MakeWH(image_.width(), image_.height()),
filter_quality_, SkMatrix::I());
filter_quality_, SkM44());
auto scoped_result = image_provider->GetRasterContent(draw_image);
if (!scoped_result)
return nullptr;
Expand Down
14 changes: 7 additions & 7 deletions cc/paint/paint_op_buffer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ SkFilterQuality sampling_to_quality(const SkSamplingOptions& sampling) {
DrawImage CreateDrawImage(const PaintImage& image,
const PaintFlags* flags,
const SkSamplingOptions& sampling,
const SkMatrix& matrix) {
const SkM44& matrix) {
if (!image)
return DrawImage();
return DrawImage(image, flags->useDarkModeForImage(),
Expand Down Expand Up @@ -493,7 +493,7 @@ size_t DrawImageOp::Serialize(const PaintOp* base_op,

SkSize scale_adjustment = SkSize::Make(1.f, 1.f);
helper.Write(CreateDrawImage(op->image, serialized_flags, op->sampling,
options.canvas->getTotalMatrix()),
options.canvas->getLocalToDevice()),
&scale_adjustment);
helper.AlignMemory(alignof(SkScalar));
helper.Write(scale_adjustment.width());
Expand All @@ -517,8 +517,8 @@ size_t DrawImageRectOp::Serialize(const PaintOp* base_op,
helper.Write(*serialized_flags);

// This adjustment mirrors DiscardableImageMap::GatherDiscardableImage logic.
SkMatrix matrix =
SkMatrix::RectToRect(op->src, op->dst) * options.canvas->getTotalMatrix();
SkM44 matrix = SkM44(SkMatrix::RectToRect(op->src, op->dst)) *
options.canvas->getLocalToDevice();
// Note that we don't request subsets here since the GpuImageCache has no
// optimizations for using subsets.
SkSize scale_adjustment = SkSize::Make(1.f, 1.f);
Expand Down Expand Up @@ -1461,7 +1461,7 @@ void DrawImageOp::RasterWithFlags(const DrawImageOp* op,
// Dark mode is applied only for OOP raster during serialization.
DrawImage draw_image(
op->image, false, SkIRect::MakeWH(op->image.width(), op->image.height()),
sampling_to_quality(op->sampling), canvas->getTotalMatrix());
sampling_to_quality(op->sampling), canvas->getLocalToDevice());
auto scoped_result = params.image_provider->GetRasterContent(draw_image);
if (!scoped_result)
return;
Expand Down Expand Up @@ -1539,8 +1539,8 @@ void DrawImageRectOp::RasterWithFlags(const DrawImageRectOp* op,
return;
}

SkMatrix matrix =
canvas->getTotalMatrix() * SkMatrix::RectToRect(op->src, op->dst);
SkM44 matrix = canvas->getLocalToDevice() *
SkM44(SkMatrix::RectToRect(op->src, op->dst));

SkIRect int_src_rect;
op->src.roundOut(&int_src_rect);
Expand Down
4 changes: 2 additions & 2 deletions cc/paint/paint_op_writer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -510,7 +510,7 @@ void PaintOpWriter::Write(const PaintShader* shader, SkFilterQuality quality) {

if (enable_security_constraints_) {
DrawImage draw_image(shader->image_, false, MakeSrcRect(shader->image_),
quality, SkMatrix::I());
quality, SkM44());
SkSize scale_adjustment = SkSize::Make(1.f, 1.f);
Write(draw_image, &scale_adjustment);
DCHECK_EQ(scale_adjustment.width(), 1.f);
Expand Down Expand Up @@ -761,7 +761,7 @@ void PaintOpWriter::Write(const ImagePaintFilter& filter) {
DrawImage draw_image(
filter.image(), false,
SkIRect::MakeWH(filter.image().width(), filter.image().height()),
filter.filter_quality(), SkMatrix::I());
filter.filter_quality(), SkM44());
SkSize scale_adjustment = SkSize::Make(1.f, 1.f);
Write(draw_image, &scale_adjustment);
DCHECK_EQ(scale_adjustment.width(), 1.f);
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 @@ -372,7 +372,7 @@ sk_sp<PaintShader> PaintShader::CreateDecodedImage(
SkIRect int_src_rect;
src_rect.roundOut(&int_src_rect);
DrawImage draw_image(image_, false, int_src_rect, quality,
total_image_matrix);
SkM44(total_image_matrix));
auto decoded_draw_image = image_provider->GetRasterContent(draw_image);
if (!decoded_draw_image)
return nullptr;
Expand Down
1 change: 1 addition & 0 deletions cc/paint/paint_shader.h
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ class CC_PAINT_EXPORT PaintShader : public SkRefCnt {

static sk_sp<PaintShader> MakeColor(SkColor color);

// TODO(crbug.com/1155544) SkMatrix is deprecated in favor of SkM44.
static sk_sp<PaintShader> MakeLinearGradient(
const SkPoint* points,
const SkColor* colors,
Expand Down
12 changes: 6 additions & 6 deletions cc/raster/playback_image_provider_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ TEST(PlaybackImageProviderTest, SkipsAllImages) {
PlaybackImageProvider provider(&cache, gfx::ColorSpace(), base::nullopt);

SkIRect rect = SkIRect::MakeWH(10, 10);
SkMatrix matrix = SkMatrix::I();
SkM44 matrix = SkM44();

EXPECT_FALSE(provider.GetRasterContent(DrawImage(
PaintImageBuilder::WithDefault()
Expand Down Expand Up @@ -100,7 +100,7 @@ TEST(PlaybackImageProviderTest, SkipsSomeImages) {
std::move(settings));

SkIRect rect = SkIRect::MakeWH(10, 10);
SkMatrix matrix = SkMatrix::I();
SkM44 matrix = SkM44();
EXPECT_FALSE(provider.GetRasterContent(
DrawImage(skip_image, false, rect, kMedium_SkFilterQuality, matrix)));
EXPECT_EQ(cache.images_decoded(), 0);
Expand All @@ -116,7 +116,7 @@ TEST(PlaybackImageProviderTest, RefAndUnrefDecode) {

{
SkRect rect = SkRect::MakeWH(10, 10);
SkMatrix matrix = SkMatrix::I();
SkM44 matrix = SkM44();
auto decode = provider.GetRasterContent(CreateDiscardableDrawImage(
gfx::Size(10, 10), nullptr, rect, kMedium_SkFilterQuality, matrix));
EXPECT_TRUE(decode);
Expand Down Expand Up @@ -144,7 +144,7 @@ TEST(PlaybackImageProviderTest, SwapsGivenFrames) {
std::move(settings));

SkIRect rect = SkIRect::MakeWH(10, 10);
SkMatrix matrix = SkMatrix::I();
SkM44 matrix = SkM44();
DrawImage draw_image(image, false, rect, kMedium_SkFilterQuality, matrix);
provider.GetRasterContent(draw_image);
ASSERT_TRUE(cache.last_image().paint_image());
Expand All @@ -162,7 +162,7 @@ TEST(PlaybackImageProviderTest, BitmapImages) {

{
SkIRect rect = SkIRect::MakeWH(10, 10);
SkMatrix matrix = SkMatrix::I();
SkM44 matrix = SkM44();
auto draw_image = DrawImage(CreateBitmapImage(gfx::Size(10, 10)), false,
rect, kMedium_SkFilterQuality, matrix);
auto decode = provider.GetRasterContent(draw_image);
Expand All @@ -183,7 +183,7 @@ TEST(PlaybackImageProviderTest, IgnoresImagesNotSupportedByCache) {
std::move(settings));
{
SkIRect rect = SkIRect::MakeWH(10, 10);
SkMatrix matrix = SkMatrix::I();
SkM44 matrix = SkM44();
auto draw_image = DrawImage(CreateBitmapImage(gfx::Size(10, 10)), false,
rect, kMedium_SkFilterQuality, matrix);
auto decode = provider.GetRasterContent(draw_image);
Expand Down
2 changes: 1 addition & 1 deletion cc/test/skia_common.cc
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,7 @@ DrawImage CreateDiscardableDrawImage(const gfx::Size& size,
sk_sp<SkColorSpace> color_space,
SkRect rect,
SkFilterQuality filter_quality,
const SkMatrix& matrix) {
const SkM44& matrix) {
SkIRect irect;
rect.roundOut(&irect);

Expand Down
2 changes: 1 addition & 1 deletion cc/test/skia_common.h
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ DrawImage CreateDiscardableDrawImage(const gfx::Size& size,
sk_sp<SkColorSpace> color_space,
SkRect rect,
SkFilterQuality filter_quality,
const SkMatrix& matrix);
const SkM44& matrix);

PaintImage CreateAnimatedImage(
const gfx::Size& size,
Expand Down
2 changes: 1 addition & 1 deletion cc/tiles/checker_image_tracker.cc
Original file line number Diff line number Diff line change
Expand Up @@ -411,7 +411,7 @@ void CheckerImageTracker::ScheduleNextImageDecode() {
candidate, it->second.use_dark_mode,
SkIRect::MakeWH(candidate.width(), candidate.height()),
it->second.filter_quality,
SkMatrix::Scale(it->second.scale.width(), it->second.scale.height()),
SkM44::Scale(it->second.scale.width(), it->second.scale.height()),
it->second.frame_index, it->second.color_space);
outstanding_image_decode_.emplace(candidate);
break;
Expand Down
8 changes: 4 additions & 4 deletions cc/tiles/checker_image_tracker_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ class CheckerImageTrackerTest : public testing::Test,
.set_decoding_mode(PaintImage::DecodingMode::kAsync)
.TakePaintImage(),
false, SkIRect::MakeWH(dimension, dimension),
kNone_SkFilterQuality, SkMatrix::I(),
kNone_SkFilterQuality, SkM44(),
PaintImage::kDefaultFrameIndex, gfx::ColorSpace());
}

Expand Down Expand Up @@ -438,7 +438,7 @@ TEST_F(CheckerImageTrackerTest, CheckersOnlyStaticCompletedImages) {
.set_paint_image_generator(CreatePaintImageGenerator(image_size))
.TakePaintImage(),
false, SkIRect::MakeWH(image_size.width(), image_size.height()),
kNone_SkFilterQuality, SkMatrix::I(), PaintImage::kDefaultFrameIndex,
kNone_SkFilterQuality, SkM44(), PaintImage::kDefaultFrameIndex,
gfx::ColorSpace());
EXPECT_FALSE(
ShouldCheckerImage(completed_paint_image, WhichTree::PENDING_TREE));
Expand Down Expand Up @@ -470,7 +470,7 @@ TEST_F(CheckerImageTrackerTest, ChoosesMaxScaleAndQuality) {
gfx::ColorSpace());
DrawImage scaled_image2 =
DrawImage(image.paint_image(), false, image.src_rect(),
kHigh_SkFilterQuality, SkMatrix::Scale(1.8f, 1.8f),
kHigh_SkFilterQuality, SkM44::Scale(1.8f, 1.8f),
PaintImage::kDefaultFrameIndex, gfx::ColorSpace());

std::vector<DrawImage> draw_images = {scaled_image1, scaled_image2};
Expand Down Expand Up @@ -545,7 +545,7 @@ TEST_F(CheckerImageTrackerTest, UseSrcRectForSize) {
// be checkered.
DrawImage image = CreateImage(ImageType::CHECKERABLE);
image = DrawImage(image.paint_image(), false, SkIRect::MakeWH(200, 200),
image.filter_quality(), SkMatrix::I(),
image.filter_quality(), SkM44(),
PaintImage::kDefaultFrameIndex, image.target_color_space());
EXPECT_FALSE(ShouldCheckerImage(image, WhichTree::PENDING_TREE));
}
Expand Down
2 changes: 1 addition & 1 deletion cc/tiles/decoded_image_tracker.cc
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ void DecodedImageTracker::QueueImageDecode(
// our own.
auto image_bounds = SkIRect::MakeWH(image.width(), image.height());
DrawImage draw_image(image, false, image_bounds, kNone_SkFilterQuality,
SkMatrix::I(), frame_index, target_color_space);
SkM44(), frame_index, target_color_space);
image_controller_->QueueImageDecode(
draw_image, base::BindOnce(&DecodedImageTracker::ImageDecodeFinished,
base::Unretained(this), std::move(callback),
Expand Down
12 changes: 5 additions & 7 deletions cc/tiles/decoded_image_tracker_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -110,11 +110,11 @@ TEST_F(DecodedImageTrackerTest, Colorspace) {
// filter quality here, since it shouldn't matter and the checks should
// succeed anyway.
DrawImage locked_draw_image(
paint_image, false, SkIRect::MakeWH(1, 1), kHigh_SkFilterQuality,
SkMatrix::I(), PaintImage::kDefaultFrameIndex, decoded_color_space);
paint_image, false, SkIRect::MakeWH(1, 1), kHigh_SkFilterQuality, SkM44(),
PaintImage::kDefaultFrameIndex, decoded_color_space);
EXPECT_TRUE(image_controller()->IsDrawImageLocked(locked_draw_image));
DrawImage srgb_draw_image(paint_image, false, SkIRect::MakeWH(1, 1),
kHigh_SkFilterQuality, SkMatrix::I(),
kHigh_SkFilterQuality, SkM44(),
PaintImage::kDefaultFrameIndex, srgb_color_space);
EXPECT_FALSE(image_controller()->IsDrawImageLocked(srgb_draw_image));
}
Expand Down Expand Up @@ -172,11 +172,9 @@ TEST_F(DecodedImageTrackerTest, ImageUsedInDraw) {

// Create dummy draw images for each:
DrawImage draw_image_1(paint_image_1, false, SkIRect::MakeWH(1, 1),
kHigh_SkFilterQuality, SkMatrix::I(), 0,
gfx::ColorSpace());
kHigh_SkFilterQuality, SkM44(), 0, gfx::ColorSpace());
DrawImage draw_image_2(paint_image_2, false, SkIRect::MakeWH(1, 1),
kHigh_SkFilterQuality, SkMatrix::I(), 0,
gfx::ColorSpace());
kHigh_SkFilterQuality, SkM44(), 0, gfx::ColorSpace());

// Both should be in the cache:
EXPECT_TRUE(image_controller()->IsDrawImageLocked(draw_image_1));
Expand Down
6 changes: 2 additions & 4 deletions cc/tiles/gpu_image_decode_cache_perftest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,8 @@ sk_sp<SkImage> CreateImage(int width, int height) {
return SkImage::MakeFromBitmap(bitmap);
}

SkMatrix CreateMatrix(const SkSize& scale) {
SkMatrix matrix;
matrix.setScale(scale.width(), scale.height());
return matrix;
SkM44 CreateMatrix(const SkSize& scale) {
return SkM44::Scale(scale.width(), scale.height());
}

enum class TestMode { kGpu, kTransferCache, kSw };
Expand Down
Loading

0 comments on commit 38ff085

Please sign in to comment.