Remove intermediate SkPicture recording from GPU tile rasterization.

Remove intermediate SkPicture recording from GPU tile rasterization.

R=ericrk@chromium.org
R=vmpstr@chromium.org
BUG=669214
BUG=628394
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel

Committed: https://crrev.com/f7c765c5d213236ea8e76b6d6cee1f81c1982fc6
Cr-Commit-Position: refs/heads/master@{#436171}

[vmiura, 2016-11-29 01:32:57]

Description was changed from

==========
Remove intermediate SkPicture recording from GPU tile rasterization.

R=ericrk@chromium.org
R=vmpstr@chromium.org
BUG=669214
==========

to

==========
Remove intermediate SkPicture recording from GPU tile rasterization.

R=ericrk@chromium.org
R=vmpstr@chromium.org
BUG=669214
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel
==========

[vmiura, 2016-11-29 01:33:02]

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

[vmiura, 2016-11-29 01:33:32]

PTAL

[commit-bot: I haz the power, 2016-11-29 01:34:12]

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

[chrishtr, 2016-11-29 02:04:10]

chrishtr@chromium.org changed reviewers:
+ chrishtr@chromium.org

[chrishtr, 2016-11-29 02:04:11]

Curious: why did it paint into an intermediate SkPicture in the first place?

[ericrk, 2016-11-29 02:16:20]

LGTM

Now that the code is simplified like this, can you remove
RasterSource::image_decode_controller() and mark crbug.com/628394 as obsolete?
That was only added to support the logic this CL removes.

Thanks!

[vmiura, 2016-11-29 02:24:00]

On 2016/11/29 02:04:11, chrishtr wrote:
> Curious: why did it paint into an intermediate SkPicture in the first place?

It's partly legacy from picture piles, and before threaded GPU raster, we used
the SkMultiPictureDraw API to give Ganesh all tile pictures for a frame in one
monolithic draw.  This enabled SaveLayer optimizations.  If a SaveLayer
overlapped multiple tiles, the layer was rendered only once.

With threaded raster, to enable individual tile raster scheduling we stopped
doing a single monolithic draw.  We issue only 1 picture to MultiPictureDraw.  I
think this is not beneficial, but it would be good to check with bsalomon.

In the long term we should implement the save layer optimization in cc.

[vmiura, 2016-11-29 02:24:59]

Description was changed from

==========
Remove intermediate SkPicture recording from GPU tile rasterization.

R=ericrk@chromium.org
R=vmpstr@chromium.org
BUG=669214
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel
==========

to

==========
Remove intermediate SkPicture recording from GPU tile rasterization.

R=ericrk@chromium.org
R=vmpstr@chromium.org
BUG=669214
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel
==========

[vmiura, 2016-11-29 02:24:59]

vmiura@chromium.org changed reviewers:
+ bsalomon@google.com

[vmiura, 2016-11-29 02:26:32]

On 2016/11/29 02:24:00, vmiura wrote:
> On 2016/11/29 02:04:11, chrishtr wrote:
> > Curious: why did it paint into an intermediate SkPicture in the first place?
> 
> It's partly legacy from picture piles, and before threaded GPU raster, we used
> the SkMultiPictureDraw API to give Ganesh all tile pictures for a frame in one
> monolithic draw.  This enabled SaveLayer optimizations.  If a SaveLayer
> overlapped multiple tiles, the layer was rendered only once.
> 
> With threaded raster, to enable individual tile raster scheduling we stopped
> doing a single monolithic draw.  We issue only 1 picture to MultiPictureDraw. 
I
> think this is not beneficial, but it would be good to check with bsalomon.
> 
> In the long term we should implement the save layer optimization in cc.

Bsalomon@, does MultiPictureDraw do layer hoisting for 1 picture?

[commit-bot: I haz the power, 2016-11-29 03:17:48]

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

[commit-bot: I haz the power, 2016-11-29 03:17:49]

Dry run: This issue passed the CQ dry run.

[bsalomon, 2016-11-29 14:21:04]

bsalomon@google.com changed reviewers:
+ robertphillips@google.com

[bsalomon, 2016-11-29 14:21:25]

On 2016/11/29 02:26:32, vmiura wrote:
> On 2016/11/29 02:24:00, vmiura wrote:
> > On 2016/11/29 02:04:11, chrishtr wrote:
> > > Curious: why did it paint into an intermediate SkPicture in the first
place?
> > 
> > It's partly legacy from picture piles, and before threaded GPU raster, we
used
> > the SkMultiPictureDraw API to give Ganesh all tile pictures for a frame in
one
> > monolithic draw.  This enabled SaveLayer optimizations.  If a SaveLayer
> > overlapped multiple tiles, the layer was rendered only once.
> > 
> > With threaded raster, to enable individual tile raster scheduling we stopped
> > doing a single monolithic draw.  We issue only 1 picture to
MultiPictureDraw. 
> I
> > think this is not beneficial, but it would be good to check with bsalomon.
> > 
> > In the long term we should implement the save layer optimization in cc.
> 
> Bsalomon@, does MultiPictureDraw do layer hoisting for 1 picture?

I believe +robertphillips@ decided that Chrome wasn't seeing much benefit from
MPD hoisting and disabled it so that we wouldn't have to maintain it while
working on the replacement.

[vmiura, 2016-11-30 05:27:21]

On 2016/11/29 14:21:25, bsalomon wrote:
> On 2016/11/29 02:26:32, vmiura wrote:
> > On 2016/11/29 02:24:00, vmiura wrote:
> > > On 2016/11/29 02:04:11, chrishtr wrote:
> > > > Curious: why did it paint into an intermediate SkPicture in the first
> place?
> > > 
> > > It's partly legacy from picture piles, and before threaded GPU raster, we
> used
> > > the SkMultiPictureDraw API to give Ganesh all tile pictures for a frame in
> one
> > > monolithic draw.  This enabled SaveLayer optimizations.  If a SaveLayer
> > > overlapped multiple tiles, the layer was rendered only once.
> > > 
> > > With threaded raster, to enable individual tile raster scheduling we
stopped
> > > doing a single monolithic draw.  We issue only 1 picture to
> MultiPictureDraw. 
> > I
> > > think this is not beneficial, but it would be good to check with bsalomon.
> > > 
> > > In the long term we should implement the save layer optimization in cc.
> > 
> > Bsalomon@, does MultiPictureDraw do layer hoisting for 1 picture?
> 
> I believe +robertphillips@ decided that Chrome wasn't seeing much benefit from
> MPD hoisting and disabled it so that we wouldn't have to maintain it while
> working on the replacement.

It sounds like there's no down side to removing this then.

[vmiura, 2016-11-30 05:28:07]

Description was changed from

==========
Remove intermediate SkPicture recording from GPU tile rasterization.

R=ericrk@chromium.org
R=vmpstr@chromium.org
BUG=669214
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel
==========

to

==========
Remove intermediate SkPicture recording from GPU tile rasterization.

R=ericrk@chromium.org
R=vmpstr@chromium.org
BUG=669214
BUG=628394
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel
==========

[vmiura, 2016-11-30 05:41:18]

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

[commit-bot: I haz the power, 2016-11-30 05:41:44]

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, 2016-11-30 08:31:36]

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

[commit-bot: I haz the power, 2016-11-30 08:31:37]

Dry run: This issue passed the CQ dry run.

[vmiura, 2016-12-02 21:05:49]

The CQ bit was checked by vmiura@chromium.org

[vmiura, 2016-12-02 21:05:49]

The patchset sent to the CQ was uploaded after l-g-t-m from ericrk@chromium.org
Link to the patchset: https://codereview.chromium.org/2535153002/#ps20001
(title: "Remove RasterSource::image_decode_controller()")

[commit-bot: I haz the power, 2016-12-02 21:06:58]

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, 2016-12-02 21:11:13]

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

[commit-bot: I haz the power, 2016-12-02 21:11:14]

Try jobs failed on following builders:
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[vmiura, 2016-12-02 23:48:27]

The CQ bit was checked by vmiura@chromium.org

[vmiura, 2016-12-02 23:48:27]

The patchset sent to the CQ was uploaded after l-g-t-m from ericrk@chromium.org
Link to the patchset: https://codereview.chromium.org/2535153002/#ps40001
(title: "Rebase.")

[commit-bot: I haz the power, 2016-12-02 23:49:26]

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, 2016-12-03 01:45:09]

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

[commit-bot: I haz the power, 2016-12-03 01:45:10]

Try jobs failed on following builders:
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)

[vmiura, 2016-12-03 05:05:32]

The CQ bit was checked by vmiura@chromium.org

[commit-bot: I haz the power, 2016-12-03 05:05:45]

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, 2016-12-03 21:02:34]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1480741532405870,
"parent_rev": "9f1095a588152fcdd92b0a5b7a4d0f4b83d624be", "commit_rev":
"da6ac858072cf7b7be97cb42afedb0c8085a1b1c"}

[commit-bot: I haz the power, 2016-12-03 21:02:57]

Description was changed from

==========
Remove intermediate SkPicture recording from GPU tile rasterization.

R=ericrk@chromium.org
R=vmpstr@chromium.org
BUG=669214
BUG=628394
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel
==========

to

==========
Remove intermediate SkPicture recording from GPU tile rasterization.

R=ericrk@chromium.org
R=vmpstr@chromium.org
BUG=669214
BUG=628394
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel
==========

[commit-bot: I haz the power, 2016-12-03 21:02:58]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-12-03 21:05:48]

Description was changed from

==========
Remove intermediate SkPicture recording from GPU tile rasterization.

R=ericrk@chromium.org
R=vmpstr@chromium.org
BUG=669214
BUG=628394
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel
==========

to

==========
Remove intermediate SkPicture recording from GPU tile rasterization.

R=ericrk@chromium.org
R=vmpstr@chromium.org
BUG=669214
BUG=628394
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel

Committed: https://crrev.com/f7c765c5d213236ea8e76b6d6cee1f81c1982fc6
Cr-Commit-Position: refs/heads/master@{#436171}
==========

[commit-bot: I haz the power, 2016-12-03 21:05:50]

Patchset 3 (id:??) landed as
https://crrev.com/f7c765c5d213236ea8e76b6d6cee1f81c1982fc6
Cr-Commit-Position: refs/heads/master@{#436171}