0

Fix matrix multiply order in DrawImageRect serialization

The original code (over a series of API updates) multiplied
(src->dst matrix) * (local->device) matrix. The actual effective
behavior of the skia image rect API is (local->device)*(src->dst).

This switches the serialize function and the DiscardableImageMap to
use the correct order. The unit test is also updated to add a rotation
after the scale factor. This failed with the original code but passes
now.

Change-Id: Iacf0c977bd15f46c8ab8d2d186853e6e69e6050a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2851121
Reviewed-by: Aaron Krajeski <aaronhk@chromium.org>
Reviewed-by: Vasiliy Telezhnikov <vasilyt@chromium.org>
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
Cr-Commit-Position: refs/heads/master@{#876562}
This commit is contained in:
Michael Ludwig
2021-04-27 13:57:32 +00:00
committed by Chromium LUCI CQ
parent b9bca391a1
commit ec17e7d802
3 changed files with 12 additions and 7 deletions

@ -141,9 +141,8 @@ class DiscardableImageGenerator {
auto* image_rect_op = static_cast<DrawImageRectOp*>(op);
// 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;
SkM44 matrix = ctm * SkM44(SkMatrix::RectToRect(image_rect_op->src,
image_rect_op->dst));
AddImage(image_rect_op->image,
image_rect_op->flags.useDarkModeForImage(), image_rect_op->src,
op_rect, matrix, image_rect_op->flags.getFilterQuality());

@ -535,8 +535,8 @@ size_t DrawImageRectOp::Serialize(const PaintOp* base_op,
helper.Write(*flags_to_serialize);
// This adjustment mirrors DiscardableImageMap::GatherDiscardableImage logic.
SkM44 matrix = SkM44(SkMatrix::RectToRect(op->src, op->dst)) *
options.canvas->getLocalToDevice();
SkM44 matrix = options.canvas->getLocalToDevice() *
SkM44(SkMatrix::RectToRect(op->src, op->dst));
// 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);

@ -3540,15 +3540,21 @@ TEST(PaintOpBufferTest, SecurityConstrainedImageSerialization) {
TEST(PaintOpBufferTest, DrawImageRectSerializeScaledImages) {
auto buffer = sk_make_sp<PaintOpBuffer>();
buffer->push<ScaleOp>(0.5f, 2.0f);
// scales: x dimension = x0.25, y dimension = x5
// translations here are arbitrary
SkRect src = SkRect::MakeXYWH(3, 4, 20, 6);
SkRect dst = SkRect::MakeXYWH(20, 38, 5, 30);
// Adjust transform matrix so that order of operations for src->dst is
// confirmed to be applied before the canvas's transform.
buffer->push<TranslateOp>(.5f * dst.centerX(), 2.f * dst.centerY());
buffer->push<RotateOp>(90.f);
buffer->push<TranslateOp>(-.5f * dst.centerX(), -2.f * dst.centerY());
buffer->push<ScaleOp>(0.5f, 2.0f);
buffer->push<DrawImageRectOp>(CreateDiscardablePaintImage(gfx::Size(32, 16)),
src, dst, SkCanvas::kStrict_SrcRectConstraint);
std::unique_ptr<char, base::AlignedFreeDeleter> memory(
static_cast<char*>(base::AlignedAlloc(PaintOpBuffer::kInitialBufferSize,
PaintOpBuffer::PaintOpAlign)));