Reland "fix crbug 1073670"
This is a reland of 553deb66e4
Original change's description:
> fix crbug 1073670
>
> When drawing a path with effects, the deviceMatrix must not be modified.
>
> * added GM for regression checking
>
> Bug: chromium:1073670
>
> Change-Id: Id75d6f00aa50d891ec807f10be72c0068ec80356
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/285387
> Commit-Queue: Herb Derby <herb@google.com>
> Reviewed-by: Brian Osman <brianosman@google.com>
Bug: chromium:1073670
Change-Id: I518497997f09e37d13fc05499b68135ebd4e0a96
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/285497
Reviewed-by: Mike Klein <mtklein@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Herb Derby <herb@google.com>
This commit is contained in:
parent
6d54e84dc5
commit
438775b19e
27
gm/crbug_1073670.cpp
Normal file
27
gm/crbug_1073670.cpp
Normal file
@ -0,0 +1,27 @@
|
||||
/*
|
||||
* Copyright 2020 Google Inc.
|
||||
*
|
||||
* Use of this source code is governed by a BSD-style license that can be
|
||||
* found in the LICENSE file.
|
||||
*/
|
||||
|
||||
#include "gm/gm.h"
|
||||
#include "include/core/SkCanvas.h"
|
||||
#include "include/core/SkFont.h"
|
||||
#include "include/core/SkPaint.h"
|
||||
#include "include/core/SkPath.h"
|
||||
#include "include/effects/SkGradientShader.h"
|
||||
|
||||
DEF_SIMPLE_GM(crbug_1073670, canvas, 250, 250) {
|
||||
SkPoint pts[] = {{0, 0}, {0, 250}};
|
||||
SkColor colors[] = {0xFFFF0000, 0xFF0000FF};
|
||||
auto sh = SkGradientShader::MakeLinear(pts, colors, nullptr, 2, SkTileMode::kClamp);
|
||||
SkPaint p;
|
||||
p.setShader(sh);
|
||||
|
||||
SkFont f;
|
||||
f.setSize(325);
|
||||
f.setEdging(SkFont::Edging::kAntiAlias);
|
||||
|
||||
canvas->drawString("Gradient", 10, 250, f, p);
|
||||
}
|
@ -106,6 +106,7 @@ gm_sources = [
|
||||
"$_gm/convexpolyeffect.cpp",
|
||||
"$_gm/copy_to_4444.cpp",
|
||||
"$_gm/crbug_1041204.cpp",
|
||||
"$_gm/crbug_1073670.cpp",
|
||||
"$_gm/crbug_224618.cpp",
|
||||
"$_gm/crbug_691386.cpp",
|
||||
"$_gm/crbug_788500.cpp",
|
||||
|
@ -489,7 +489,7 @@ void GrTextBlob::flush(GrTextTarget* target,
|
||||
const SkPaint& paint,
|
||||
const SkPMColor4f& filteredColor,
|
||||
const GrClip& clip,
|
||||
const SkMatrixProvider& matrixProvider,
|
||||
const SkMatrixProvider& deviceMatrix,
|
||||
SkPoint drawOrigin) {
|
||||
for (SubRun* subRun = fFirstSubRun; subRun != nullptr; subRun = subRun->fNextSubRun) {
|
||||
if (subRun->drawAsPaths()) {
|
||||
@ -500,38 +500,42 @@ void GrTextBlob::flush(GrTextTarget* target,
|
||||
// different effects.
|
||||
GrStyle style(runPaint);
|
||||
|
||||
bool scalePath = runPaint.getShader()
|
||||
|| style.applies()
|
||||
|| runPaint.getMaskFilter();
|
||||
bool needsExactCTM = runPaint.getShader()
|
||||
|| style.applies()
|
||||
|| runPaint.getMaskFilter();
|
||||
|
||||
// Calculate the matrix that maps the path glyphs from their size in the strike to
|
||||
// the graphics source space.
|
||||
SkMatrix strikeToSource = SkMatrix::MakeScale(
|
||||
subRun->fStrikeSpec.strikeToSourceRatio());
|
||||
strikeToSource.postTranslate(drawOrigin.x(), drawOrigin.y());
|
||||
if (!needsExactCTM) {
|
||||
for (const auto& pathPos : subRun->fPaths) {
|
||||
const SkPath& path = pathPos.fPath;
|
||||
const SkPoint pos = pathPos.fOrigin; // Transform the glyph to source space.
|
||||
SkMatrix pathMatrix = strikeToSource;
|
||||
pathMatrix.postTranslate(pos.x(), pos.y());
|
||||
SkPreConcatMatrixProvider strikeToDevice(deviceMatrix, pathMatrix);
|
||||
|
||||
for (const auto& pathGlyph : subRun->fPaths) {
|
||||
SkMatrix preMatrix = SkMatrix::MakeTrans(drawOrigin);
|
||||
SkMatrix pathMatrix = SkMatrix::MakeScale(
|
||||
subRun->fStrikeSpec.strikeToSourceRatio());
|
||||
pathMatrix.postTranslate(pathGlyph.fOrigin.x(), pathGlyph.fOrigin.y());
|
||||
|
||||
// TmpPath must be in the same scope as GrStyledShape shape below.
|
||||
SkTLazy<SkPath> tmpPath;
|
||||
const SkPath* path = &pathGlyph.fPath;
|
||||
if (!scalePath) {
|
||||
// Scale can be applied to CTM -- no effects.
|
||||
preMatrix.preConcat(pathMatrix);
|
||||
} else {
|
||||
// Scale the outline into source space.
|
||||
|
||||
// Transform the path form the normalized outline to source space. This
|
||||
// way the CTM will remain the same so it can be used by the effects.
|
||||
SkPath* sourceOutline = tmpPath.init();
|
||||
path->transform(pathMatrix, sourceOutline);
|
||||
sourceOutline->setIsVolatile(true);
|
||||
path = sourceOutline;
|
||||
GrStyledShape shape(path, paint);
|
||||
target->drawShape(clip, runPaint, strikeToDevice, shape);
|
||||
}
|
||||
} else {
|
||||
// Transform the path to device because the deviceMatrix must be unchanged to
|
||||
// draw effect, filter or shader paths.
|
||||
for (const auto& pathPos : subRun->fPaths) {
|
||||
const SkPath& path = pathPos.fPath;
|
||||
const SkPoint pos = pathPos.fOrigin;
|
||||
// Transform the glyph to source space.
|
||||
SkMatrix pathMatrix = strikeToSource;
|
||||
pathMatrix.postTranslate(pos.x(), pos.y());
|
||||
|
||||
// TODO: we are losing the mutability of the path here
|
||||
GrStyledShape shape(*path, paint);
|
||||
SkPreConcatMatrixProvider preConcatMatrixProvider(matrixProvider, preMatrix);
|
||||
target->drawShape(clip, runPaint, preConcatMatrixProvider, shape);
|
||||
SkPath deviceOutline;
|
||||
path.transform(pathMatrix, &deviceOutline);
|
||||
deviceOutline.setIsVolatile(true);
|
||||
GrStyledShape shape(deviceOutline, paint);
|
||||
target->drawShape(clip, runPaint, deviceMatrix, shape);
|
||||
}
|
||||
}
|
||||
} else {
|
||||
int glyphCount = subRun->fGlyphs.count();
|
||||
@ -553,7 +557,7 @@ void GrTextBlob::flush(GrTextTarget* target,
|
||||
skipClip = true;
|
||||
// We only need to do clipping work if the subrun isn't contained by the clip
|
||||
SkRect subRunBounds;
|
||||
this->computeSubRunBounds(&subRunBounds, *subRun, matrixProvider.localToDevice(),
|
||||
this->computeSubRunBounds(&subRunBounds, *subRun, deviceMatrix.localToDevice(),
|
||||
drawOrigin, false);
|
||||
if (!clipRRect.getBounds().contains(subRunBounds)) {
|
||||
// If the subrun is completely outside, don't add an op for it
|
||||
@ -567,7 +571,7 @@ void GrTextBlob::flush(GrTextTarget* target,
|
||||
}
|
||||
|
||||
if (submitOp) {
|
||||
auto op = this->makeOp(*subRun, glyphCount, matrixProvider, drawOrigin, clipRect,
|
||||
auto op = this->makeOp(*subRun, glyphCount, deviceMatrix, drawOrigin, clipRect,
|
||||
paint, filteredColor, props, target);
|
||||
if (op) {
|
||||
if (skipClip) {
|
||||
|
@ -134,7 +134,7 @@ public:
|
||||
const SkPaint& paint,
|
||||
const SkPMColor4f& filteredColor,
|
||||
const GrClip& clip,
|
||||
const SkMatrixProvider& matrixProvider,
|
||||
const SkMatrixProvider& deviceMatrix,
|
||||
SkPoint drawOrigin);
|
||||
|
||||
void computeSubRunBounds(SkRect* outBounds, const SubRun& subRun,
|
||||
|
Loading…
Reference in New Issue
Block a user