New issue
Advanced search Search tips

Issue 735747 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jul 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug

Blocking:
issue 735662



Sign in to add a comment

Replace ImageHijackCanvas with an interface that allows replacing PaintImages.

Project Member Reported by khushals...@chromium.org, Jun 22 2017

Issue description

Currently we use the hijack canvas to provide the image to use for raster given the painted image, but its a canvas override so we only see the SkImage.

In order to select the SkImage based on the frame index we need this step to have a PaintImage. I think in general there are a lot of things we do with images during playback for raster (SkipImageCanvas, ImageHijackCanvas, skipping checkered images in the hijack canvas), and it would be nice to have a single interface that is used by the PaintOpBuffer directly instead of going through a canvas override.
 
Components: Internals>Compositing>Images
Labels: I
Owner: khushals...@chromium.org
Status: Started (was: Available)
Project Member

Comment 2 by bugdroid1@chromium.org, Jul 26 2017

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

commit d880a1511779ef31871522ef88605911848f4282
Author: Khushal <khushalsagar@chromium.org>
Date: Wed Jul 26 23:48:04 2017

cc: Eliminate canvas overrides for replacing images.

Currently we use the ImageHijackCanvas for replacing lazy generated
images with cached raster images from the image decode cache. Its a
level of indirection that is unnecessary since this can be performed
during PaintOpBuffer playback.

Performing it at a later stage also means we don't get to inspect the
PaintImage that wraps this SkImage, which is important to be able to
select frame index to use when playing back a buffer containing an
animated image.

Note: FunWithFlags.

Bug:  735747 
Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel
Change-Id: I7bb20cbf8cfee9be6cafd80905cf972652adf287
Reviewed-on: https://chromium-review.googlesource.com/582330
Reviewed-by: enne <enne@chromium.org>
Commit-Queue: Khushal <khushalsagar@chromium.org>
Cr-Commit-Position: refs/heads/master@{#489801}
[modify] https://crrev.com/d880a1511779ef31871522ef88605911848f4282/cc/BUILD.gn
[modify] https://crrev.com/d880a1511779ef31871522ef88605911848f4282/cc/benchmarks/rasterize_and_record_benchmark_impl.cc
[modify] https://crrev.com/d880a1511779ef31871522ef88605911848f4282/cc/layers/picture_layer_impl.cc
[modify] https://crrev.com/d880a1511779ef31871522ef88605911848f4282/cc/output/software_renderer.cc
[modify] https://crrev.com/d880a1511779ef31871522ef88605911848f4282/cc/paint/BUILD.gn
[rename] https://crrev.com/d880a1511779ef31871522ef88605911848f4282/cc/paint/decoded_draw_image.cc
[rename] https://crrev.com/d880a1511779ef31871522ef88605911848f4282/cc/paint/decoded_draw_image.h
[modify] https://crrev.com/d880a1511779ef31871522ef88605911848f4282/cc/paint/discardable_image_map.cc
[modify] https://crrev.com/d880a1511779ef31871522ef88605911848f4282/cc/paint/display_item_list.cc
[modify] https://crrev.com/d880a1511779ef31871522ef88605911848f4282/cc/paint/display_item_list.h
[modify] https://crrev.com/d880a1511779ef31871522ef88605911848f4282/cc/paint/image_id.h
[add] https://crrev.com/d880a1511779ef31871522ef88605911848f4282/cc/paint/image_provider.h
[modify] https://crrev.com/d880a1511779ef31871522ef88605911848f4282/cc/paint/paint_flags.h
[modify] https://crrev.com/d880a1511779ef31871522ef88605911848f4282/cc/paint/paint_image.h
[modify] https://crrev.com/d880a1511779ef31871522ef88605911848f4282/cc/paint/paint_op_buffer.cc
[modify] https://crrev.com/d880a1511779ef31871522ef88605911848f4282/cc/paint/paint_op_buffer.h
[modify] https://crrev.com/d880a1511779ef31871522ef88605911848f4282/cc/paint/paint_op_buffer_fuzzer.cc
[modify] https://crrev.com/d880a1511779ef31871522ef88605911848f4282/cc/paint/paint_op_buffer_unittest.cc
[modify] https://crrev.com/d880a1511779ef31871522ef88605911848f4282/cc/paint/paint_shader.h
[modify] https://crrev.com/d880a1511779ef31871522ef88605911848f4282/cc/paint/record_paint_canvas.cc
[modify] https://crrev.com/d880a1511779ef31871522ef88605911848f4282/cc/paint/solid_color_analyzer.cc
[modify] https://crrev.com/d880a1511779ef31871522ef88605911848f4282/cc/raster/gpu_raster_buffer_provider.cc
[delete] https://crrev.com/1eb2db5b44e5076cac3d634ffd5332cb96b92ea3/cc/raster/image_hijack_canvas.cc
[delete] https://crrev.com/1eb2db5b44e5076cac3d634ffd5332cb96b92ea3/cc/raster/image_hijack_canvas.h
[delete] https://crrev.com/1eb2db5b44e5076cac3d634ffd5332cb96b92ea3/cc/raster/image_hijack_canvas_unittest.cc
[add] https://crrev.com/d880a1511779ef31871522ef88605911848f4282/cc/raster/playback_image_provider.cc
[add] https://crrev.com/d880a1511779ef31871522ef88605911848f4282/cc/raster/playback_image_provider.h
[add] https://crrev.com/d880a1511779ef31871522ef88605911848f4282/cc/raster/playback_image_provider_unittest.cc
[modify] https://crrev.com/d880a1511779ef31871522ef88605911848f4282/cc/raster/raster_buffer_provider_unittest.cc
[modify] https://crrev.com/d880a1511779ef31871522ef88605911848f4282/cc/raster/raster_source.cc
[modify] https://crrev.com/d880a1511779ef31871522ef88605911848f4282/cc/raster/raster_source.h
[modify] https://crrev.com/d880a1511779ef31871522ef88605911848f4282/cc/raster/raster_source_unittest.cc
[delete] https://crrev.com/1eb2db5b44e5076cac3d634ffd5332cb96b92ea3/cc/raster/skip_image_canvas.cc
[delete] https://crrev.com/1eb2db5b44e5076cac3d634ffd5332cb96b92ea3/cc/raster/skip_image_canvas.h
[add] https://crrev.com/d880a1511779ef31871522ef88605911848f4282/cc/test/stub_decode_cache.cc
[add] https://crrev.com/d880a1511779ef31871522ef88605911848f4282/cc/test/stub_decode_cache.h
[modify] https://crrev.com/d880a1511779ef31871522ef88605911848f4282/cc/test/test_skcanvas.h
[modify] https://crrev.com/d880a1511779ef31871522ef88605911848f4282/cc/tiles/checker_image_tracker.cc
[modify] https://crrev.com/d880a1511779ef31871522ef88605911848f4282/cc/tiles/image_controller.h
[modify] https://crrev.com/d880a1511779ef31871522ef88605911848f4282/cc/tiles/image_decode_cache.h
[modify] https://crrev.com/d880a1511779ef31871522ef88605911848f4282/cc/tiles/software_image_decode_cache.h
[modify] https://crrev.com/d880a1511779ef31871522ef88605911848f4282/cc/tiles/tile_manager.cc

Labels: -I
Status: Fixed (was: Started)
Project Member

Comment 4 by bugdroid1@chromium.org, Aug 1 2017

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

commit 4480385f68985000880a3dd9e056731aa9a73a43
Author: Khushal <khushalsagar@chromium.org>
Date: Tue Aug 01 07:39:56 2017

cc: Follow up changes to ImageProvider in PaintOpBuffer.

1) Replace DecodedImageHolder with ScopedDecodedDrawImage to return a
scoped object for managing the decode lifetime in the ImageDecodeCache.

2) Use base::Optional to limit the use of the ImageProvider during
raster instead of a unique_ptr.

Bug:  735747 
Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel
Change-Id: I4b23103760a7edf96ea782df0015dacd0bb79943
Reviewed-on: https://chromium-review.googlesource.com/594639
Commit-Queue: Khushal <khushalsagar@chromium.org>
Reviewed-by: Vladimir Levin <vmpstr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#490901}
[modify] https://crrev.com/4480385f68985000880a3dd9e056731aa9a73a43/cc/paint/BUILD.gn
[modify] https://crrev.com/4480385f68985000880a3dd9e056731aa9a73a43/cc/paint/decoded_draw_image.cc
[modify] https://crrev.com/4480385f68985000880a3dd9e056731aa9a73a43/cc/paint/decoded_draw_image.h
[add] https://crrev.com/4480385f68985000880a3dd9e056731aa9a73a43/cc/paint/image_provider.cc
[modify] https://crrev.com/4480385f68985000880a3dd9e056731aa9a73a43/cc/paint/image_provider.h
[modify] https://crrev.com/4480385f68985000880a3dd9e056731aa9a73a43/cc/paint/paint_image.h
[modify] https://crrev.com/4480385f68985000880a3dd9e056731aa9a73a43/cc/paint/paint_op_buffer.cc
[modify] https://crrev.com/4480385f68985000880a3dd9e056731aa9a73a43/cc/paint/paint_op_buffer_unittest.cc
[modify] https://crrev.com/4480385f68985000880a3dd9e056731aa9a73a43/cc/raster/playback_image_provider.cc
[modify] https://crrev.com/4480385f68985000880a3dd9e056731aa9a73a43/cc/raster/playback_image_provider.h
[modify] https://crrev.com/4480385f68985000880a3dd9e056731aa9a73a43/cc/raster/playback_image_provider_unittest.cc
[modify] https://crrev.com/4480385f68985000880a3dd9e056731aa9a73a43/cc/tiles/tile_manager.cc

Sign in to add a comment