New issue
Advanced search Search tips

Issue 628394 link

Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Closed: Dec 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 2
Type: Feature



Sign in to add a comment

Redesign RasterSource to avoid exposing ImageDecodeController

Project Member Reported by ericrk@chromium.org, Jul 14 2016

Issue description

To work around issues with the MultiPictureDraw approach used in GPU raster, RasterSource exposes its ImageDecodeController, allowing GpuRasterBufferProvider to create/manage its own ImageHijackCanvas. This can probably be handled in a nicer way be restructuring RasterSource or GpuRasterBufferProvider.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Jul 15 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/1259d6e73e7f8904243aa3451bc9071a18f93cbd

commit 1259d6e73e7f8904243aa3451bc9071a18f93cbd
Author: ericrk <ericrk@chromium.org>
Date: Fri Jul 15 00:47:32 2016

Fix MultiPictureDraw issues with GpuImageDecodeController

Currently, the GpuImageDecodeController (GPU IDC) is used during the
initial playback of a RasterSource. Unfortunately, the initial playback
is just used to generate an SkPicture, not to actually raster. Actual
rasterization happens later, via MultiPictureDraw. Because of this the
calls to GetDecodedImageForDraw and DrawWithImageFinished on the GPU IDC
do not scope Skia's usage of an image, as was intended. This can lead
to lifetime issues, especially with large, software-backed images.

This change moves the usage of ImageHijackCanvas and the GPU IDC to the
later MultiPictureDraw phase, allowing lifetimes to line up as expected.

R=vmpstr@chromium.org
BUG=612329, 628394 
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_blink_rel

Review-Url: https://codereview.chromium.org/2149843003
Cr-Commit-Position: refs/heads/master@{#405649}

[modify] https://crrev.com/1259d6e73e7f8904243aa3451bc9071a18f93cbd/cc/layers/picture_layer_impl.cc
[modify] https://crrev.com/1259d6e73e7f8904243aa3451bc9071a18f93cbd/cc/playback/raster_source.cc
[modify] https://crrev.com/1259d6e73e7f8904243aa3451bc9071a18f93cbd/cc/playback/raster_source.h
[modify] https://crrev.com/1259d6e73e7f8904243aa3451bc9071a18f93cbd/cc/playback/raster_source_unittest.cc
[modify] https://crrev.com/1259d6e73e7f8904243aa3451bc9071a18f93cbd/cc/raster/gpu_raster_buffer_provider.cc
[modify] https://crrev.com/1259d6e73e7f8904243aa3451bc9071a18f93cbd/cc/raster/raster_buffer_provider_unittest.cc
[modify] https://crrev.com/1259d6e73e7f8904243aa3451bc9071a18f93cbd/cc/test/skia_common.cc
[modify] https://crrev.com/1259d6e73e7f8904243aa3451bc9071a18f93cbd/cc/trees/layer_tree_host_unittest.cc

Project Member

Comment 2 by bugdroid1@chromium.org, Jul 18 2016

Labels: merge-merged-2785
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f546781498cdb5648210484fc4e9ade4127f17a5

commit f546781498cdb5648210484fc4e9ade4127f17a5
Author: Eric Karl <ericrk@chromium.org>
Date: Mon Jul 18 20:01:21 2016

Fix MultiPictureDraw issues with GpuImageDecodeController

Currently, the GpuImageDecodeController (GPU IDC) is used during the
initial playback of a RasterSource. Unfortunately, the initial playback
is just used to generate an SkPicture, not to actually raster. Actual
rasterization happens later, via MultiPictureDraw. Because of this the
calls to GetDecodedImageForDraw and DrawWithImageFinished on the GPU IDC
do not scope Skia's usage of an image, as was intended. This can lead
to lifetime issues, especially with large, software-backed images.

This change moves the usage of ImageHijackCanvas and the GPU IDC to the
later MultiPictureDraw phase, allowing lifetimes to line up as expected.

R=vmpstr@chromium.org
BUG=612329, 628394 
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_blink_rel

Review-Url: https://codereview.chromium.org/2149843003
Cr-Commit-Position: refs/heads/master@{#405649}
(cherry picked from commit 1259d6e73e7f8904243aa3451bc9071a18f93cbd)

Review URL: https://codereview.chromium.org/2160683003 .

Cr-Commit-Position: refs/branch-heads/2785@{#193}
Cr-Branched-From: 68623971be0cfc492a2cb0427d7f478e7b214c24-refs/heads/master@{#403382}

[modify] https://crrev.com/f546781498cdb5648210484fc4e9ade4127f17a5/cc/layers/picture_layer_impl.cc
[modify] https://crrev.com/f546781498cdb5648210484fc4e9ade4127f17a5/cc/playback/raster_source.cc
[modify] https://crrev.com/f546781498cdb5648210484fc4e9ade4127f17a5/cc/playback/raster_source.h
[modify] https://crrev.com/f546781498cdb5648210484fc4e9ade4127f17a5/cc/playback/raster_source_unittest.cc
[modify] https://crrev.com/f546781498cdb5648210484fc4e9ade4127f17a5/cc/raster/gpu_raster_buffer_provider.cc
[modify] https://crrev.com/f546781498cdb5648210484fc4e9ade4127f17a5/cc/raster/raster_buffer_provider_unittest.cc
[modify] https://crrev.com/f546781498cdb5648210484fc4e9ade4127f17a5/cc/test/skia_common.cc
[modify] https://crrev.com/f546781498cdb5648210484fc4e9ade4127f17a5/cc/trees/layer_tree_host_unittest.cc

Project Member

Comment 3 by bugdroid1@chromium.org, Jul 18 2016

Labels: merge-merged-2743
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/0701df60264a6aa83032e19f49c6040ee4c4042d

commit 0701df60264a6aa83032e19f49c6040ee4c4042d
Author: Eric Karl <ericrk@chromium.org>
Date: Mon Jul 18 21:39:26 2016

Fix MultiPictureDraw issues with GpuImageDecodeController

Currently, the GpuImageDecodeController (GPU IDC) is used during the
initial playback of a RasterSource. Unfortunately, the initial playback
is just used to generate an SkPicture, not to actually raster. Actual
rasterization happens later, via MultiPictureDraw. Because of this the
calls to GetDecodedImageForDraw and DrawWithImageFinished on the GPU IDC
do not scope Skia's usage of an image, as was intended. This can lead
to lifetime issues, especially with large, software-backed images.

This change moves the usage of ImageHijackCanvas and the GPU IDC to the
later MultiPictureDraw phase, allowing lifetimes to line up as expected.

TBR=vmpstr@chromium.org
BUG=612329, 628394 
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_blink_rel

Review-Url: https://codereview.chromium.org/2149843003
Cr-Commit-Position: refs/heads/master@{#405649}
(cherry picked from commit 1259d6e73e7f8904243aa3451bc9071a18f93cbd)

Review URL: https://codereview.chromium.org/2161633003 .

Cr-Commit-Position: refs/branch-heads/2743@{#666}
Cr-Branched-From: 2b3ae3b8090361f8af5a611712fc1a5ab2de53cb-refs/heads/master@{#394939}

[modify] https://crrev.com/0701df60264a6aa83032e19f49c6040ee4c4042d/cc/layers/picture_layer_impl.cc
[modify] https://crrev.com/0701df60264a6aa83032e19f49c6040ee4c4042d/cc/playback/raster_source.cc
[modify] https://crrev.com/0701df60264a6aa83032e19f49c6040ee4c4042d/cc/playback/raster_source.h
[modify] https://crrev.com/0701df60264a6aa83032e19f49c6040ee4c4042d/cc/playback/raster_source_unittest.cc
[modify] https://crrev.com/0701df60264a6aa83032e19f49c6040ee4c4042d/cc/raster/gpu_rasterizer.cc
[modify] https://crrev.com/0701df60264a6aa83032e19f49c6040ee4c4042d/cc/raster/raster_buffer_provider_unittest.cc
[modify] https://crrev.com/0701df60264a6aa83032e19f49c6040ee4c4042d/cc/test/skia_common.cc
[modify] https://crrev.com/0701df60264a6aa83032e19f49c6040ee4c4042d/cc/trees/layer_tree_host_unittest.cc

Project Member

Comment 4 by bugdroid1@chromium.org, Dec 3 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f7c765c5d213236ea8e76b6d6cee1f81c1982fc6

commit f7c765c5d213236ea8e76b6d6cee1f81c1982fc6
Author: vmiura <vmiura@chromium.org>
Date: Sat Dec 03 21:02:32 2016

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

Review-Url: https://codereview.chromium.org/2535153002
Cr-Commit-Position: refs/heads/master@{#436171}

[modify] https://crrev.com/f7c765c5d213236ea8e76b6d6cee1f81c1982fc6/cc/playback/raster_source.h
[modify] https://crrev.com/f7c765c5d213236ea8e76b6d6cee1f81c1982fc6/cc/raster/gpu_raster_buffer_provider.cc

Comment 5 by ericrk@chromium.org, Dec 19 2016

Status: Fixed (was: Available)
This is fixed with the change in #4

Sign in to add a comment