Issue metadata
Sign in to add a comment
|
HTML canvas elements will repeatedly re-color-convert non-sRGB images |
||||||||||||||||||||||
Issue descriptionConsider the attached color-profile.html and no-color-profile.html (in canvas-re-color-convert.zip). The color-profile.html example, an image with a color profile is drawn with rAF. We will hit SkImage_Raster/Gpu::onMakeColorSpace every single frame for this image. This is* because images in the HTML canvas element do not hit the cc::ImageDecodeCache, and have no place where they will cache color converted images. *(sometimes this is because of the semi-related issue 791821).
,
Dec 5 2017
This sounds like a good plan. Some background of something else we discussed off thread in this space. SkColorSpaceXformer keeps temporary caches of color transformed SkImages/SkColorFilters/SkImageFilters. Caches: https://cs.chromium.org/chromium/src/third_party/skia/src/core/SkColorSpaceXformer.h?sq=package:chromium&dr=C&l=59 These caches are temporary, scoped to the life of one draw, and were intended to improve performance for certain known filter graphs that re-use the same image in a single draw. It doesn't cover the cases described above. If Skia were to cache these images in the global SkResourceCache it would cover this issue and issue 791821, until we have full support for the images as described in #1. That would be similar to other features such as SkPictureShaders where today we rely on Skia's caching for performance. The long term goal would be to shift this all to CC caching. I'm not sure if SkColorSpaceXformer has other users than Chrome, but it's part of Skia core.
,
Dec 5 2017
,
Dec 5 2017
(SkColorSpaceXformer / SkColorSpaceXformCanvas are also used by Flutter for basically the same purpose as Chrome, color management with non-linear blending.)
,
Dec 5 2017
I discussed this issue with khushalsagar, vmpstr, ccameron and ericrk. We agreed that the most feasible way to get cc-owned image decode caching for canvas is to have a separate cache instance for canvas that is owned on the main thread in LayerTreeHost, and then is plumbed to Canvas2DLayerBridge.cpp for rastering. It's hard to share the other cc decode cache because (a) canvas uses a different gl context thank non-canvas content, and (b) canvas rasters on the main thread, and the image decode cache is not tracked by a thread-safe refcounted object at present. In the future when canvas raster no longer happens on the main thread (e.g. OOPR world), we can change this. Khushal is cool with implementing this approach, sounds simple.
,
Dec 9 2017
A word of caution that we need to watch how we stage this change as I'm also working on adapting Gpu rasterization to the RasterInterface for OOP-Raster, which will include changing GpuImageDecodeCache to also use the RasterInterface. (see Issue 757607 ) If we introduce ImageDecodeCache to 2D Canvas we'll also either have to enable Raster Interface on the 2D Canvas context, or support both modes in GpuImageDecodeCache.
,
Dec 11 2017
It makes sense to have the context used by canvas have Raster interface enabled, since it will use OOP raster eventually as well.
,
Jan 19 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/60916ba1169d9207e048e2eca290c946ba37fcad commit 60916ba1169d9207e048e2eca290c946ba37fcad Author: Khushal <khushalsagar@chromium.org> Date: Fri Jan 19 19:47:47 2018 blink/canvas: Switch Canvas to use cc's ImageDecodeCache. Currently rasterization for canvas uses skia's image decode cache which does not cache color converted images, causing an expensive conversion performed for every frame. This replaces this with cc's ImageDecodeCache which does cache the result of color conversion. This change only does this for accelerated canvas, follow up changes should also set up a cc::SoftwareImageDecodeCache in blink for the non-accelerated case. In addition, this also provides an easy hook that canvas can use to serialize PaintRecords for OOP raster, since the PaintOpBufferSerializer requires the same ImageProvider/Cache for image serialization. R=chrishtr@chromium.org, ccameron@chromium.org TBR=sadrul@chromium.org Bug: 791828 Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2 Change-Id: Ief9aa958fd174e1d0ea1165600c1405968f271c2 Reviewed-on: https://chromium-review.googlesource.com/809758 Commit-Queue: Khushal <khushalsagar@chromium.org> Reviewed-by: Chris Harrelson <chrishtr@chromium.org> Reviewed-by: Eric Karl <ericrk@chromium.org> Reviewed-by: Antoine Labour <piman@chromium.org> Reviewed-by: Justin Novosad <junov@chromium.org> Cr-Commit-Position: refs/heads/master@{#530586} [modify] https://crrev.com/60916ba1169d9207e048e2eca290c946ba37fcad/cc/paint/skia_paint_canvas.cc [modify] https://crrev.com/60916ba1169d9207e048e2eca290c946ba37fcad/cc/paint/skia_paint_canvas.h [modify] https://crrev.com/60916ba1169d9207e048e2eca290c946ba37fcad/cc/raster/raster_buffer_provider_unittest.cc [modify] https://crrev.com/60916ba1169d9207e048e2eca290c946ba37fcad/cc/test/test_context_provider.cc [modify] https://crrev.com/60916ba1169d9207e048e2eca290c946ba37fcad/cc/tiles/gpu_image_decode_cache.cc [modify] https://crrev.com/60916ba1169d9207e048e2eca290c946ba37fcad/cc/tiles/gpu_image_decode_cache.h [modify] https://crrev.com/60916ba1169d9207e048e2eca290c946ba37fcad/cc/tiles/gpu_image_decode_cache_unittest.cc [modify] https://crrev.com/60916ba1169d9207e048e2eca290c946ba37fcad/cc/trees/layer_tree_host_unittest.cc [modify] https://crrev.com/60916ba1169d9207e048e2eca290c946ba37fcad/components/viz/common/gpu/raster_context_provider.h [modify] https://crrev.com/60916ba1169d9207e048e2eca290c946ba37fcad/content/renderer/render_thread_impl.cc [modify] https://crrev.com/60916ba1169d9207e048e2eca290c946ba37fcad/content/renderer/renderer_blink_platform_impl.cc [modify] https://crrev.com/60916ba1169d9207e048e2eca290c946ba37fcad/content/renderer/webgraphicscontext3d_provider_impl.cc [modify] https://crrev.com/60916ba1169d9207e048e2eca290c946ba37fcad/content/renderer/webgraphicscontext3d_provider_impl.h [modify] https://crrev.com/60916ba1169d9207e048e2eca290c946ba37fcad/services/ui/public/cpp/gpu/context_provider_command_buffer.cc [modify] https://crrev.com/60916ba1169d9207e048e2eca290c946ba37fcad/third_party/WebKit/LayoutTests/fast/canvas/canvas-createImageBitmap-blob-in-workers.html [modify] https://crrev.com/60916ba1169d9207e048e2eca290c946ba37fcad/third_party/WebKit/LayoutTests/fast/canvas/canvas-createImageBitmap-drawImage-video.html [modify] https://crrev.com/60916ba1169d9207e048e2eca290c946ba37fcad/third_party/WebKit/LayoutTests/fast/canvas/canvas-createImageBitmap-from-canvas-toBlob.html [modify] https://crrev.com/60916ba1169d9207e048e2eca290c946ba37fcad/third_party/WebKit/LayoutTests/fast/canvas/canvas-createImageBitmap-video-resize.html [modify] https://crrev.com/60916ba1169d9207e048e2eca290c946ba37fcad/third_party/WebKit/LayoutTests/fast/canvas/downsample-quality.html [rename] https://crrev.com/60916ba1169d9207e048e2eca290c946ba37fcad/third_party/WebKit/LayoutTests/platform/linux/virtual/gpu/fast/canvas/canvas-drawImage-video-expected.png [add] https://crrev.com/60916ba1169d9207e048e2eca290c946ba37fcad/third_party/WebKit/LayoutTests/platform/mac/virtual/gpu/fast/canvas/downsample-quality-expected.png [add] https://crrev.com/60916ba1169d9207e048e2eca290c946ba37fcad/third_party/WebKit/LayoutTests/platform/win/virtual/gpu/fast/canvas/downsample-quality-expected.png [delete] https://crrev.com/b0e3c370f735edef8128ae377f6fe163e02df2b0/third_party/WebKit/LayoutTests/platform/win/virtual/gpu/fast/canvas/yuv-video-on-accelerated-canvas-expected.png [modify] https://crrev.com/60916ba1169d9207e048e2eca290c946ba37fcad/third_party/WebKit/LayoutTests/virtual/gpu/fast/canvas/canvas-drawImage-video-expected.png [modify] https://crrev.com/60916ba1169d9207e048e2eca290c946ba37fcad/third_party/WebKit/LayoutTests/virtual/gpu/fast/canvas/yuv-video-on-accelerated-canvas-expected.png [modify] https://crrev.com/60916ba1169d9207e048e2eca290c946ba37fcad/third_party/WebKit/Source/platform/graphics/Canvas2DLayerBridgeTest.cpp [modify] https://crrev.com/60916ba1169d9207e048e2eca290c946ba37fcad/third_party/WebKit/Source/platform/graphics/CanvasResourceProvider.cpp [modify] https://crrev.com/60916ba1169d9207e048e2eca290c946ba37fcad/third_party/WebKit/Source/platform/graphics/CanvasResourceProvider.h [modify] https://crrev.com/60916ba1169d9207e048e2eca290c946ba37fcad/third_party/WebKit/Source/platform/graphics/VideoFrameSubmitterTest.cpp [modify] https://crrev.com/60916ba1169d9207e048e2eca290c946ba37fcad/third_party/WebKit/Source/platform/graphics/gpu/DrawingBufferTestHelpers.h [modify] https://crrev.com/60916ba1169d9207e048e2eca290c946ba37fcad/third_party/WebKit/Source/platform/graphics/gpu/SharedGpuContext.cpp [modify] https://crrev.com/60916ba1169d9207e048e2eca290c946ba37fcad/third_party/WebKit/Source/platform/graphics/test/FakeWebGraphicsContext3DProvider.h [modify] https://crrev.com/60916ba1169d9207e048e2eca290c946ba37fcad/third_party/WebKit/public/platform/Platform.h [modify] https://crrev.com/60916ba1169d9207e048e2eca290c946ba37fcad/third_party/WebKit/public/platform/WebGraphicsContext3DProvider.h [modify] https://crrev.com/60916ba1169d9207e048e2eca290c946ba37fcad/ui/compositor/test/in_process_context_provider.cc
,
Feb 1 2018
Need to set up a software cache as well: https://chromium-review.googlesource.com/c/chromium/src/+/858570.
,
Feb 2 2018
IIUC the plan is to move away from using SkColorSpaceXformCanvas. In that works, I would suspect that - for GPU raster we will want to decode images only in their original space, and do color conversion on the fly (cause it's nearly-free) - for CPU raster we will probably still want to cache converted results (cause color conversion on-the-fly is not cheap)
,
Apr 19 2018
Issue 823335 has been merged into this issue.
,
Apr 19 2018
@Chris, do the M-67 and Beta-Blocker labels from merged Issue 823335 apply to this too?
,
Apr 20 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/2b8ee37329e17ee4c76ef3dbbe2a503583fcefda commit 2b8ee37329e17ee4c76ef3dbbe2a503583fcefda Author: Christopher Cameron <ccameron@chromium.org> Date: Fri Apr 20 00:30:39 2018 cc: Cache color conversion results in software raster No sense in re-converting the same image over and over. This can introduce a problem where one content source (e.g, a canvas) can create new frames that then pollute the cache. The source is identified by a stable id, and each version has a new content id. To avoid this problem, when allocating a new cache entry, ensure that we don't have cache entries with more than 2 different content ids for a given stable id. This is similar to what was done in the GpuImageDecodeCache in crrev.com/551078. Add the stable id to the software CacheKey, to enable the bookkeeping necessary for the above fix. Consolidate the code to delete a cache entry. Delete all references to NotifyImageUnused because it's unused and complicates adding the new deletion path. Bug: 791828 Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;master.tryserver.blink:linux_trusty_blink_rel Change-Id: I5632afa03c6f5ba9885d6474d4666780d39ad340 Reviewed-on: https://chromium-review.googlesource.com/1016063 Reviewed-by: ccameron <ccameron@chromium.org> Reviewed-by: Khushal <khushalsagar@chromium.org> Commit-Queue: ccameron <ccameron@chromium.org> Cr-Commit-Position: refs/heads/master@{#552214} [modify] https://crrev.com/2b8ee37329e17ee4c76ef3dbbe2a503583fcefda/cc/test/stub_decode_cache.h [modify] https://crrev.com/2b8ee37329e17ee4c76ef3dbbe2a503583fcefda/cc/tiles/gpu_image_decode_cache.cc [modify] https://crrev.com/2b8ee37329e17ee4c76ef3dbbe2a503583fcefda/cc/tiles/gpu_image_decode_cache.h [modify] https://crrev.com/2b8ee37329e17ee4c76ef3dbbe2a503583fcefda/cc/tiles/gpu_image_decode_cache_unittest.cc [modify] https://crrev.com/2b8ee37329e17ee4c76ef3dbbe2a503583fcefda/cc/tiles/image_decode_cache.h [modify] https://crrev.com/2b8ee37329e17ee4c76ef3dbbe2a503583fcefda/cc/tiles/software_image_decode_cache.cc [modify] https://crrev.com/2b8ee37329e17ee4c76ef3dbbe2a503583fcefda/cc/tiles/software_image_decode_cache.h [modify] https://crrev.com/2b8ee37329e17ee4c76ef3dbbe2a503583fcefda/cc/tiles/software_image_decode_cache_unittest.cc [modify] https://crrev.com/2b8ee37329e17ee4c76ef3dbbe2a503583fcefda/cc/tiles/software_image_decode_cache_utils.cc [modify] https://crrev.com/2b8ee37329e17ee4c76ef3dbbe2a503583fcefda/cc/tiles/software_image_decode_cache_utils.h
,
Apr 20 2018
Pls update the bug with canary result and request a merge to M67 & M66 if needed. Thank you.
,
Apr 20 2018
The NextAction date has arrived: 2018-04-20
,
Apr 20 2018
Verified the patch in #13 on canary. Its working as expected. Here is a trace. Requesting a merge to 67.
,
Apr 20 2018
,
Apr 21 2018
Your change meets the bar and is auto-approved for M67. Please go ahead and merge the CL to branch 3396 manually. Please contact milestone owner if you have questions. Owners: cmasso@(Android), cmasso@(iOS), kbleicher@(ChromeOS), govind@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Apr 21 2018
Pls merge your change to M67 branch 3396 by 1:00 PM PT, Monday (04/23) so we can pick it up next M67 Dev/Beta release. Thank you.
,
Apr 23 2018
Issue 821020 has been merged into this issue.
,
Apr 23 2018
The patch in #13 is causing lots of crashes, see issue 835272.
,
Apr 23 2018
Thank you ccameron@. khushalsagar@, rejecting merge to M67 based on comment #21. Pls re-request merge to M67 once safe fix is ready to be merged. Thank you.
,
Apr 23 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/9c1b6321c1dc93753fdaffc81c44d0bbb729ade5 commit 9c1b6321c1dc93753fdaffc81c44d0bbb729ade5 Author: Christopher Cameron <ccameron@chromium.org> Date: Mon Apr 23 07:30:24 2018 Revert parts of "cc: Cache color conversion results in software raster" This reverts only the additional content id tracking part of the patch, so color converted results will still be cached, but run-away canvas conversions can still pollute the cache. TBR=khushalsagar Bug: 835272, 791828 Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;master.tryserver.blink:linux_trusty_blink_rel Change-Id: I5aa00135ae51ad77d102474aae3d85a77c21d90f Reviewed-on: https://chromium-review.googlesource.com/1023658 Reviewed-by: ccameron <ccameron@chromium.org> Commit-Queue: ccameron <ccameron@chromium.org> Cr-Commit-Position: refs/heads/master@{#552650} [modify] https://crrev.com/9c1b6321c1dc93753fdaffc81c44d0bbb729ade5/cc/tiles/software_image_decode_cache.cc [modify] https://crrev.com/9c1b6321c1dc93753fdaffc81c44d0bbb729ade5/cc/tiles/software_image_decode_cache.h [modify] https://crrev.com/9c1b6321c1dc93753fdaffc81c44d0bbb729ade5/cc/tiles/software_image_decode_cache_unittest.cc
,
Apr 25 2018
Making another merge request. The revert has fixed the crash that was introduced, while still fixing the original bug. I plan to merge the change in #13, with the partial revert from #23. A bit more detail into what was reverted. The sw cache has no limit to the amount of unlocked discardable it can keep, it is assumed to be free since it can be purged by the system. But with these canvas2d cases, we cache the color conversion result of each canvas2d frame which is never re-used. The patch included an optimization to actively purge these entries. That optimization can still be landed in a follow up.
,
Apr 25 2018
This bug requires manual review: Reverts referenced in bugdroid comments after merge request. Please contact the milestone owner if you have questions. Owners: cmasso@(Android), cmasso@(iOS), kbleicher@(ChromeOS), govind@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Apr 25 2018
This bug has been present since December so exists on M65 & M66 as well. Can't this wait until M68? I'm little afraid to take cl listed at #13 as it caused stability regression although there is a partial revert at #23.
,
Apr 25 2018
Discussed offline with govind@. The minimal fix now (with the revert in #23) is less risky but given that the regression has been there since 65, its not critical enough to warrant a merge. We'll wait on M68 for the fix.
,
Apr 26 2018
I'd still vaguely prefer to merge the fixes to 67, but if it's preferred not to, I can wait.
,
Apr 26 2018
I'm fine taking it in if both of you're fully confident that this will be a completely safe merge.
,
Apr 27 2018
I'm confident in their safety -- I haven't seen any fallout.
,
Apr 27 2018
Approving merge to M67 branch 3396 based on comment #30. Please merge ASAP. Thank you.
,
Apr 27 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/f05ae8c731e8fd5a0b3c30eedd090c685684b09a commit f05ae8c731e8fd5a0b3c30eedd090c685684b09a Author: Christopher Cameron <ccameron@chromium.org> Date: Fri Apr 27 17:35:58 2018 cc: Cache color conversion results in software raster No sense in re-converting the same image over and over. This can introduce a problem where one content source (e.g, a canvas) can create new frames that then pollute the cache. The source is identified by a stable id, and each version has a new content id. To avoid this problem, when allocating a new cache entry, ensure that we don't have cache entries with more than 2 different content ids for a given stable id. This is similar to what was done in the GpuImageDecodeCache in crrev.com/551078. Add the stable id to the software CacheKey, to enable the bookkeeping necessary for the above fix. Consolidate the code to delete a cache entry. Delete all references to NotifyImageUnused because it's unused and complicates adding the new deletion path. TBR=ccameron@chromium.org (cherry picked from commit 2b8ee37329e17ee4c76ef3dbbe2a503583fcefda) Bug: 791828 Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;master.tryserver.blink:linux_trusty_blink_rel Change-Id: I5632afa03c6f5ba9885d6474d4666780d39ad340 Reviewed-on: https://chromium-review.googlesource.com/1016063 Reviewed-by: ccameron <ccameron@chromium.org> Reviewed-by: Khushal <khushalsagar@chromium.org> Commit-Queue: ccameron <ccameron@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#552214} Reviewed-on: https://chromium-review.googlesource.com/1032995 Cr-Commit-Position: refs/branch-heads/3396@{#349} Cr-Branched-From: 9ef2aa869bc7bc0c089e255d698cca6e47d6b038-refs/heads/master@{#550428} [modify] https://crrev.com/f05ae8c731e8fd5a0b3c30eedd090c685684b09a/cc/test/stub_decode_cache.h [modify] https://crrev.com/f05ae8c731e8fd5a0b3c30eedd090c685684b09a/cc/tiles/gpu_image_decode_cache.cc [modify] https://crrev.com/f05ae8c731e8fd5a0b3c30eedd090c685684b09a/cc/tiles/gpu_image_decode_cache.h [modify] https://crrev.com/f05ae8c731e8fd5a0b3c30eedd090c685684b09a/cc/tiles/gpu_image_decode_cache_unittest.cc [modify] https://crrev.com/f05ae8c731e8fd5a0b3c30eedd090c685684b09a/cc/tiles/image_decode_cache.h [modify] https://crrev.com/f05ae8c731e8fd5a0b3c30eedd090c685684b09a/cc/tiles/software_image_decode_cache.cc [modify] https://crrev.com/f05ae8c731e8fd5a0b3c30eedd090c685684b09a/cc/tiles/software_image_decode_cache.h [modify] https://crrev.com/f05ae8c731e8fd5a0b3c30eedd090c685684b09a/cc/tiles/software_image_decode_cache_unittest.cc [modify] https://crrev.com/f05ae8c731e8fd5a0b3c30eedd090c685684b09a/cc/tiles/software_image_decode_cache_utils.cc [modify] https://crrev.com/f05ae8c731e8fd5a0b3c30eedd090c685684b09a/cc/tiles/software_image_decode_cache_utils.h
,
Apr 27 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/0a647ceb3b316675d3be9cb7cabf94f9bd67a20d commit 0a647ceb3b316675d3be9cb7cabf94f9bd67a20d Author: Christopher Cameron <ccameron@chromium.org> Date: Fri Apr 27 17:40:48 2018 Revert parts of "cc: Cache color conversion results in software raster" This reverts only the additional content id tracking part of the patch, so color converted results will still be cached, but run-away canvas conversions can still pollute the cache. TBR=ccameron@chromium.org, khushalsagar (cherry picked from commit 9c1b6321c1dc93753fdaffc81c44d0bbb729ade5) Bug: 835272, 791828 Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;master.tryserver.blink:linux_trusty_blink_rel Change-Id: I5aa00135ae51ad77d102474aae3d85a77c21d90f Reviewed-on: https://chromium-review.googlesource.com/1023658 Reviewed-by: ccameron <ccameron@chromium.org> Commit-Queue: ccameron <ccameron@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#552650} Reviewed-on: https://chromium-review.googlesource.com/1032939 Cr-Commit-Position: refs/branch-heads/3396@{#350} Cr-Branched-From: 9ef2aa869bc7bc0c089e255d698cca6e47d6b038-refs/heads/master@{#550428} [modify] https://crrev.com/0a647ceb3b316675d3be9cb7cabf94f9bd67a20d/cc/tiles/software_image_decode_cache.cc [modify] https://crrev.com/0a647ceb3b316675d3be9cb7cabf94f9bd67a20d/cc/tiles/software_image_decode_cache.h [modify] https://crrev.com/0a647ceb3b316675d3be9cb7cabf94f9bd67a20d/cc/tiles/software_image_decode_cache_unittest.cc
,
Apr 27 2018
Merged the two parts in. We still want to re-do the part that was reverted, eventually, but not for 67.
,
Apr 27 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/df897ec8e47058ff9948460e56bf09eb4f3b1419 commit df897ec8e47058ff9948460e56bf09eb4f3b1419 Author: Christopher Cameron <ccameron@chromium.org> Date: Fri Apr 27 18:18:12 2018 cc: Add content id accessor to PaintImage This was added in crrev.com/551078, and is needed by the merge of crrev.com/552214. Merging just this line to branch 3396. TBR=khushalsagar Bug: 791828 Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;master.tryserver.blink:linux_trusty_blink_rel Change-Id: Ifc2ef08084939ea1567a85baac0003a426e4f18e Reviewed-on: https://chromium-review.googlesource.com/1032941 Reviewed-by: ccameron <ccameron@chromium.org> Cr-Commit-Position: refs/branch-heads/3396@{#353} Cr-Branched-From: 9ef2aa869bc7bc0c089e255d698cca6e47d6b038-refs/heads/master@{#550428} [modify] https://crrev.com/df897ec8e47058ff9948460e56bf09eb4f3b1419/cc/paint/paint_image.h
,
Apr 30 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/461799f8b75cbb840c04da63a68569e45ae65670 commit 461799f8b75cbb840c04da63a68569e45ae65670 Author: Anita Woodruff <awdf@chromium.org> Date: Mon Apr 30 10:58:26 2018 Revert "Revert parts of "cc: Cache color conversion results in software raster"" This reverts commit 0a647ceb3b316675d3be9cb7cabf94f9bd67a20d. Reason for revert: Compile failure in software_image_decode_cache_unittest.cc: "no matching function for call to 'CreateDiscardablePaintImage' PaintImage paint_image = CreateDiscardablePaintImage(" BUG=838059 Original change's description: > Revert parts of "cc: Cache color conversion results in software raster" > > This reverts only the additional content id tracking part of the patch, > so color converted results will still be cached, but run-away canvas > conversions can still pollute the cache. > > TBR=ccameron@chromium.org, khushalsagar > > (cherry picked from commit 9c1b6321c1dc93753fdaffc81c44d0bbb729ade5) > > Bug: 835272, 791828 > Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;master.tryserver.blink:linux_trusty_blink_rel > Change-Id: I5aa00135ae51ad77d102474aae3d85a77c21d90f > Reviewed-on: https://chromium-review.googlesource.com/1023658 > Reviewed-by: ccameron <ccameron@chromium.org> > Commit-Queue: ccameron <ccameron@chromium.org> > Cr-Original-Commit-Position: refs/heads/master@{#552650} > Reviewed-on: https://chromium-review.googlesource.com/1032939 > Cr-Commit-Position: refs/branch-heads/3396@{#350} > Cr-Branched-From: 9ef2aa869bc7bc0c089e255d698cca6e47d6b038-refs/heads/master@{#550428} TBR=ccameron@chromium.org,khushalsagar@chromium.org # Not skipping CQ checks because original CL landed > 1 day ago. Bug: 835272, 791828 Change-Id: I3ac3f1623316beea4eb12607f956d0c0982cd4c1 Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;master.tryserver.blink:linux_trusty_blink_rel Reviewed-on: https://chromium-review.googlesource.com/1034535 Reviewed-by: Anita Woodruff <awdf@chromium.org> Cr-Commit-Position: refs/branch-heads/3396@{#372} Cr-Branched-From: 9ef2aa869bc7bc0c089e255d698cca6e47d6b038-refs/heads/master@{#550428} [modify] https://crrev.com/461799f8b75cbb840c04da63a68569e45ae65670/cc/tiles/software_image_decode_cache.cc [modify] https://crrev.com/461799f8b75cbb840c04da63a68569e45ae65670/cc/tiles/software_image_decode_cache.h [modify] https://crrev.com/461799f8b75cbb840c04da63a68569e45ae65670/cc/tiles/software_image_decode_cache_unittest.cc
,
Apr 30 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/c0b0b86df49decc8bd822810aef27f236537eb96 commit c0b0b86df49decc8bd822810aef27f236537eb96 Author: Anita Woodruff <awdf@chromium.org> Date: Mon Apr 30 11:07:14 2018 Revert "cc: Cache color conversion results in software raster" This reverts commit f05ae8c731e8fd5a0b3c30eedd090c685684b09a. Reason for revert: Compile failure in software_image_decode_cache_unittest.cc: "no matching function for call to 'CreateDiscardablePaintImage' PaintImage paint_image = CreateDiscardablePaintImage(" Original change's description: > cc: Cache color conversion results in software raster > > No sense in re-converting the same image over and over. > > This can introduce a problem where one content source (e.g, > a canvas) can create new frames that then pollute the cache. > The source is identified by a stable id, and each version has > a new content id. To avoid this problem, when allocating a new > cache entry, ensure that we don't have cache entries with more > than 2 different content ids for a given stable id. This is > similar to what was done in the GpuImageDecodeCache in > crrev.com/551078. > > Add the stable id to the software CacheKey, to enable the > bookkeeping necessary for the above fix. > > Consolidate the code to delete a cache entry. > > Delete all references to NotifyImageUnused because it's unused > and complicates adding the new deletion path. > > TBR=ccameron@chromium.org > > (cherry picked from commit 2b8ee37329e17ee4c76ef3dbbe2a503583fcefda) > > Bug: 791828 > Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;master.tryserver.blink:linux_trusty_blink_rel > Change-Id: I5632afa03c6f5ba9885d6474d4666780d39ad340 > Reviewed-on: https://chromium-review.googlesource.com/1016063 > Reviewed-by: ccameron <ccameron@chromium.org> > Reviewed-by: Khushal <khushalsagar@chromium.org> > Commit-Queue: ccameron <ccameron@chromium.org> > Cr-Original-Commit-Position: refs/heads/master@{#552214} > Reviewed-on: https://chromium-review.googlesource.com/1032995 > Cr-Commit-Position: refs/branch-heads/3396@{#349} > Cr-Branched-From: 9ef2aa869bc7bc0c089e255d698cca6e47d6b038-refs/heads/master@{#550428} TBR=ccameron@chromium.org,khushalsagar@chromium.org # Not skipping CQ checks because original CL landed > 1 day ago. Bug: 791828 Change-Id: I47034fef6ba91be675d3937a3c4c3a858899efcb Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;master.tryserver.blink:linux_trusty_blink_rel Reviewed-on: https://chromium-review.googlesource.com/1033674 Reviewed-by: Anita Woodruff <awdf@chromium.org> Cr-Commit-Position: refs/branch-heads/3396@{#373} Cr-Branched-From: 9ef2aa869bc7bc0c089e255d698cca6e47d6b038-refs/heads/master@{#550428} [modify] https://crrev.com/c0b0b86df49decc8bd822810aef27f236537eb96/cc/test/stub_decode_cache.h [modify] https://crrev.com/c0b0b86df49decc8bd822810aef27f236537eb96/cc/tiles/gpu_image_decode_cache.cc [modify] https://crrev.com/c0b0b86df49decc8bd822810aef27f236537eb96/cc/tiles/gpu_image_decode_cache.h [modify] https://crrev.com/c0b0b86df49decc8bd822810aef27f236537eb96/cc/tiles/gpu_image_decode_cache_unittest.cc [modify] https://crrev.com/c0b0b86df49decc8bd822810aef27f236537eb96/cc/tiles/image_decode_cache.h [modify] https://crrev.com/c0b0b86df49decc8bd822810aef27f236537eb96/cc/tiles/software_image_decode_cache.cc [modify] https://crrev.com/c0b0b86df49decc8bd822810aef27f236537eb96/cc/tiles/software_image_decode_cache.h [modify] https://crrev.com/c0b0b86df49decc8bd822810aef27f236537eb96/cc/tiles/software_image_decode_cache_unittest.cc [modify] https://crrev.com/c0b0b86df49decc8bd822810aef27f236537eb96/cc/tiles/software_image_decode_cache_utils.cc [modify] https://crrev.com/c0b0b86df49decc8bd822810aef27f236537eb96/cc/tiles/software_image_decode_cache_utils.h
,
Apr 30 2018
I had thought that this had cycled green on the official waterfall -- at least on linux64 -- this is the build for the patch in #35: https://uberchromegw.corp.google.com/i/official.desktop.continuous/builders/linux64%20beta/builds/3917 Was the linux64 build not compiling these lines https://cs.chromium.org/chromium/src/cc/tiles/software_image_decode_cache_unittest.cc?rcl=50cce18b90b688e08c7d84c64d1e11abc2521231&l=1747 At this point I'm no-longer-inclined to keep trying to merge this into 67. Anyone disagree?
,
Apr 30 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/33a4d8e116d8cb383fd8599503e3fc7bea734c26 commit 33a4d8e116d8cb383fd8599503e3fc7bea734c26 Author: Khushal <khushalsagar@chromium.org> Date: Mon Apr 30 17:32:28 2018 cc: Cache color conversion results in software raster No sense in re-converting the same image over and over. Cherry-picked from: https://chromium-review.googlesource.com/c/chromium/src/+/1016063 TBR=ccameron@chromium.org Bug: 791828 Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;master.tryserver.blink:linux_trusty_blink_rel Change-Id: Idf14cd79607364974301bfe92ec5242db6c79f69 Reviewed-on: https://chromium-review.googlesource.com/1035523 Reviewed-by: Khushal <khushalsagar@chromium.org> Cr-Commit-Position: refs/branch-heads/3396@{#379} Cr-Branched-From: 9ef2aa869bc7bc0c089e255d698cca6e47d6b038-refs/heads/master@{#550428} [modify] https://crrev.com/33a4d8e116d8cb383fd8599503e3fc7bea734c26/cc/tiles/software_image_decode_cache.cc
,
May 2 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/49fc8a9c4a9eb46faa2258daea5bdbee48da7dcb commit 49fc8a9c4a9eb46faa2258daea5bdbee48da7dcb Author: Khushal <khushalsagar@chromium.org> Date: Wed May 02 05:55:12 2018 blink: Set up a cc::SoftwareImageDecodeCache for canvas image decodes. Add an instance of SoftwareImageDecodeCache for image decoding for non-accelerated canvas. This can potentially be used for any other use-case requiring image decoding on the main thread in blink. R=chrishtr@chromium.org Bug: 791828 Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2 Change-Id: I1df793ea00e8e5df26878d6ca8e0769a9ca0c500 Reviewed-on: https://chromium-review.googlesource.com/858570 Commit-Queue: Khushal <khushalsagar@chromium.org> Reviewed-by: Justin Novosad <junov@chromium.org> Reviewed-by: ccameron <ccameron@chromium.org> Cr-Commit-Position: refs/heads/master@{#555318} [modify] https://crrev.com/49fc8a9c4a9eb46faa2258daea5bdbee48da7dcb/third_party/WebKit/LayoutTests/fast/canvas/canvas-drawImage-video-imageSmoothingEnabled.html [modify] https://crrev.com/49fc8a9c4a9eb46faa2258daea5bdbee48da7dcb/third_party/WebKit/LayoutTests/fast/canvas/canvas-imageSmoothingQuality.html [modify] https://crrev.com/49fc8a9c4a9eb46faa2258daea5bdbee48da7dcb/third_party/WebKit/LayoutTests/fast/canvas/downsample-quality-expected.png [modify] https://crrev.com/49fc8a9c4a9eb46faa2258daea5bdbee48da7dcb/third_party/WebKit/LayoutTests/images/color-profile-image-canvas-expected.png [modify] https://crrev.com/49fc8a9c4a9eb46faa2258daea5bdbee48da7dcb/third_party/WebKit/LayoutTests/images/color-profile-image-canvas-pattern-expected.png [modify] https://crrev.com/49fc8a9c4a9eb46faa2258daea5bdbee48da7dcb/third_party/WebKit/LayoutTests/images/jpeg-yuv-progressive-canvas-expected.png [modify] https://crrev.com/49fc8a9c4a9eb46faa2258daea5bdbee48da7dcb/third_party/WebKit/LayoutTests/platform/mac/virtual/exotic-color-space/images/color-profile-image-canvas-expected.png [modify] https://crrev.com/49fc8a9c4a9eb46faa2258daea5bdbee48da7dcb/third_party/WebKit/LayoutTests/platform/mac/virtual/exotic-color-space/images/color-profile-image-canvas-pattern-expected.png [modify] https://crrev.com/49fc8a9c4a9eb46faa2258daea5bdbee48da7dcb/third_party/WebKit/LayoutTests/platform/mac/virtual/exotic-color-space/images/jpeg-yuv-progressive-canvas-expected.png [modify] https://crrev.com/49fc8a9c4a9eb46faa2258daea5bdbee48da7dcb/third_party/WebKit/LayoutTests/platform/mac/virtual/gpu-rasterization/images/color-profile-image-canvas-expected.png [modify] https://crrev.com/49fc8a9c4a9eb46faa2258daea5bdbee48da7dcb/third_party/WebKit/LayoutTests/platform/mac/virtual/gpu-rasterization/images/color-profile-image-canvas-pattern-expected.png [modify] https://crrev.com/49fc8a9c4a9eb46faa2258daea5bdbee48da7dcb/third_party/WebKit/LayoutTests/platform/mac/virtual/gpu-rasterization/images/jpeg-yuv-progressive-canvas-expected.png [modify] https://crrev.com/49fc8a9c4a9eb46faa2258daea5bdbee48da7dcb/third_party/WebKit/LayoutTests/platform/win/virtual/exotic-color-space/images/color-profile-image-canvas-expected.png [modify] https://crrev.com/49fc8a9c4a9eb46faa2258daea5bdbee48da7dcb/third_party/WebKit/LayoutTests/platform/win/virtual/exotic-color-space/images/color-profile-image-canvas-pattern-expected.png [modify] https://crrev.com/49fc8a9c4a9eb46faa2258daea5bdbee48da7dcb/third_party/WebKit/LayoutTests/platform/win/virtual/exotic-color-space/images/jpeg-yuv-progressive-canvas-expected.png [modify] https://crrev.com/49fc8a9c4a9eb46faa2258daea5bdbee48da7dcb/third_party/WebKit/LayoutTests/platform/win/virtual/gpu-rasterization/images/color-profile-image-canvas-expected.png [modify] https://crrev.com/49fc8a9c4a9eb46faa2258daea5bdbee48da7dcb/third_party/WebKit/LayoutTests/platform/win/virtual/gpu-rasterization/images/color-profile-image-canvas-pattern-expected.png [modify] https://crrev.com/49fc8a9c4a9eb46faa2258daea5bdbee48da7dcb/third_party/WebKit/LayoutTests/platform/win/virtual/gpu-rasterization/images/jpeg-yuv-progressive-canvas-expected.png [modify] https://crrev.com/49fc8a9c4a9eb46faa2258daea5bdbee48da7dcb/third_party/blink/renderer/modules/canvas/canvas2d/base_rendering_context_2d.cc [modify] https://crrev.com/49fc8a9c4a9eb46faa2258daea5bdbee48da7dcb/third_party/blink/renderer/platform/graphics/canvas_2d_layer_bridge_test.cc [modify] https://crrev.com/49fc8a9c4a9eb46faa2258daea5bdbee48da7dcb/third_party/blink/renderer/platform/graphics/canvas_resource_provider.cc [modify] https://crrev.com/49fc8a9c4a9eb46faa2258daea5bdbee48da7dcb/third_party/blink/renderer/platform/graphics/gpu/drawing_buffer_test_helpers.h [modify] https://crrev.com/49fc8a9c4a9eb46faa2258daea5bdbee48da7dcb/third_party/blink/renderer/platform/graphics/image.cc [modify] https://crrev.com/49fc8a9c4a9eb46faa2258daea5bdbee48da7dcb/third_party/blink/renderer/platform/graphics/image.h [modify] https://crrev.com/49fc8a9c4a9eb46faa2258daea5bdbee48da7dcb/third_party/blink/renderer/platform/graphics/test/fake_web_graphics_context_3d_provider.h
,
May 3 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/66888879b7b06742e48dce9da2ba5bd4e4642212 commit 66888879b7b06742e48dce9da2ba5bd4e4642212 Author: Khushal <khushalsagar@chromium.org> Date: Thu May 03 20:02:16 2018 cc: Use decode/upload task for uploading images in GpuImageDecodeCache. Ensure that we use upload tasks for uploading bitmaps in the Gpu image cache. This allows the image to be locked across multiple tasks. R=ericrk@chromium.org Bug: 791828 Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;master.tryserver.blink:linux_trusty_blink_rel Change-Id: I7f82f2001846df02469e9976d12581706c1673b7 Reviewed-on: https://chromium-review.googlesource.com/1041525 Commit-Queue: Khushal <khushalsagar@chromium.org> Reviewed-by: Eric Karl <ericrk@chromium.org> Cr-Commit-Position: refs/heads/master@{#555849} [modify] https://crrev.com/66888879b7b06742e48dce9da2ba5bd4e4642212/cc/tiles/gpu_image_decode_cache.cc [modify] https://crrev.com/66888879b7b06742e48dce9da2ba5bd4e4642212/cc/tiles/gpu_image_decode_cache_unittest.cc
,
May 4 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/ce15d2f4607d6db583533c55999c8610ebb5e31f commit ce15d2f4607d6db583533c55999c8610ebb5e31f Author: Khushal <khushalsagar@chromium.org> Date: Fri May 04 23:47:41 2018 cc: Cache color converted bitmaps > max_texture_size in GpuCache. Ensure that we cache the result of color conversion in the GpuImageDecodeCache for images larger than max_texture_size, which are color converted on the cpu during scaling. R=ericrk@chromium.org Bug: 791828 , 759188 Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;master.tryserver.blink:linux_trusty_blink_rel Change-Id: I0940e516ef6b36ccac961c1c15057eee2c73d64e Reviewed-on: https://chromium-review.googlesource.com/1041465 Commit-Queue: Khushal <khushalsagar@chromium.org> Reviewed-by: Eric Karl <ericrk@chromium.org> Cr-Commit-Position: refs/heads/master@{#556257} [modify] https://crrev.com/ce15d2f4607d6db583533c55999c8610ebb5e31f/cc/tiles/gpu_image_decode_cache.cc [modify] https://crrev.com/ce15d2f4607d6db583533c55999c8610ebb5e31f/cc/tiles/gpu_image_decode_cache.h [modify] https://crrev.com/ce15d2f4607d6db583533c55999c8610ebb5e31f/cc/tiles/gpu_image_decode_cache_unittest.cc
,
May 4 2018
I'm following up on some perf regressions from the change in #40. Otherwise, nothing more left to do here. |
|||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||
Comment 1 by ccameron@chromium.org
, Dec 5 2017549 KB
549 KB Download