Implement SVGImage::applyShader()

Implement SVGImage::applyShader()

Currently, when SVGImages need to be painted via shaders (e.g. rounded
borders) we build an SkImage backed by an SkPictureImageGenerator.  The
problem is SkPictureImageGenerator has an intrinsic resolution, so it
locks the rasterization scale prematurely.  If we later scale this
SkImage (pinch zoom, or device scale factor on certain platforms), we're
scaling a bitmap - not a vector recording.

To avoid this, implement an applyShader() override and use a
SkPictureShader (which does not lock the resolution) instead of
SkImage/SkPictureImageGenerator.

BUG=696575
R=fs@opera.com,schenney@chromium.org

Review-Url: https://codereview.chromium.org/2753633003
Cr-Commit-Position: refs/heads/master@{#457429}
Committed: https://chromium.googlesource.com/chromium/src/+/f3470d2b864f14e076b1c891d39f7a65b5ab78b0

[f(malita), 2017-03-15 18:48:11]

The CQ bit was checked by fmalita@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-15 18:48:54]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[f(malita), 2017-03-15 18:52:54]

The CQ bit was checked by fmalita@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-15 18:53:42]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[f(malita), 2017-03-15 19:46:18]

The CQ bit was checked by fmalita@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-15 19:46:26]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-15 19:54:02]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-15 19:54:02]

Dry run: Try jobs failed on following builders:
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_tsan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[f(malita), 2017-03-15 19:57:27]

The CQ bit was checked by fmalita@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-15 19:57:54]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[f(malita), 2017-03-15 20:41:39]

Still waiting for rebaseline for the modified test
(https://storage.googleapis.com/chromium-layout-test-archives/mac10_9_blink_re...),
but other than that I think this is good to go.

[Stephen Chennney, 2017-03-15 20:52:35]

I think it's good but I'd like fs@ to also take a look, given my general
exhausted state right now.

[fs, 2017-03-15 21:08:59]

On 2017/03/15 at 20:52:35, schenney wrote:
> I think it's good but I'd like fs@ to also take a look, given my general
exhausted state right now.

LGTM

https://codereview.chromium.org/2753633003/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/svg/graphics/SVGImage.cpp (right):

https://codereview.chromium.org/2753633003/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/svg/graphics/SVGImage.cpp:262: scaledSrc.scale(1
/ zoom);
Maybe we could move this into the lambda scope as well? (To make it more obvious
there's no dependency outside of it.)

[commit-bot: I haz the power, 2017-03-15 21:44:08]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-15 21:44:11]

Dry run: Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[f(malita), 2017-03-16 12:59:04]

https://codereview.chromium.org/2753633003/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/svg/graphics/SVGImage.cpp (right):

https://codereview.chromium.org/2753633003/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/svg/graphics/SVGImage.cpp:262: scaledSrc.scale(1
/ zoom);
On 2017/03/15 21:08:58, fs wrote:
> Maybe we could move this into the lambda scope as well? (To make it more
obvious
> there's no dependency outside of it.)

Done.

[f(malita), 2017-03-16 12:59:08]

The CQ bit was checked by fmalita@chromium.org

[f(malita), 2017-03-16 12:59:08]

The patchset sent to the CQ was uploaded after l-g-t-m from fs@opera.com
Link to the patchset: https://codereview.chromium.org/2753633003/#ps80001
(title: "rebaseline")

[commit-bot: I haz the power, 2017-03-16 12:59:18]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-16 14:25:40]

CQ is committing da patch.

Bot data: {"patchset_id": 80001, "attempt_start_ts": 1489669148344220,
"parent_rev": "7dd150883791d4d7aecf7bb93edb3f733fa11237", "commit_rev":
"f3470d2b864f14e076b1c891d39f7a65b5ab78b0"}

[commit-bot: I haz the power, 2017-03-16 14:26:25]

Description was changed from

==========
Implement SVGImage::applyShader()

Currently, when SVGImages need to be painted via shaders (e.g. rounded
borders) we build an SkImage backed by an SkPictureImageGenerator.  The
problem is SkPictureImageGenerator has an intrinsic resolution, so it
locks the rasterization scale prematurely.  If we later scale this
SkImage (pinch zoom, or device scale factor on certain platforms), we're
scaling a bitmap - not a vector recording.

To avoid this, implement an applyShader() override and use a
SkPictureShader (which does not lock the resolution) instead of
SkImage/SkPictureImageGenerator.

BUG=696575
R=fs@opera.com,schenney@chromium.org
==========

to

==========
Implement SVGImage::applyShader()

Currently, when SVGImages need to be painted via shaders (e.g. rounded
borders) we build an SkImage backed by an SkPictureImageGenerator.  The
problem is SkPictureImageGenerator has an intrinsic resolution, so it
locks the rasterization scale prematurely.  If we later scale this
SkImage (pinch zoom, or device scale factor on certain platforms), we're
scaling a bitmap - not a vector recording.

To avoid this, implement an applyShader() override and use a
SkPictureShader (which does not lock the resolution) instead of
SkImage/SkPictureImageGenerator.

BUG=696575
R=fs@opera.com,schenney@chromium.org

Review-Url: https://codereview.chromium.org/2753633003
Cr-Commit-Position: refs/heads/master@{#457429}
Committed:
https://chromium.googlesource.com/chromium/src/+/f3470d2b864f14e076b1c891d39f...
==========

[commit-bot: I haz the power, 2017-03-16 14:26:27]

Committed patchset #5 (id:80001) as
https://chromium.googlesource.com/chromium/src/+/f3470d2b864f14e076b1c891d39f...