New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 839970 link

Starred by 7 users

Issue metadata

Status: Assigned
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 2
Type: Task


Sign in to add a comment

Enable 2D canvas and Webgl GpuMemoryBuffer code paths on ChromeOS

Project Member Reported by junov@chromium.org, May 4 2018

Issue description

CanvasResourceProviderGpuMemoryBuffer currently only works with CALayers (MacOS).  We should extend this to ChromeOS which has the necessary HW+driver requirements to allow canvases to be presented via HW overlay buffers.  This should result in measurable battery power savings.
 
Cc: marc...@chromium.org dcasta...@chromium.org
Labels: OS-Chrome
For the record, WebGL is already using GMBs on drm_atomic boards, look at https://chromium.googlesource.com/chromiumos/platform2/+/master/libchromeos-ui/chromeos/ui/chromium_command_builder.cc#552
#2: correct, this comment applies to canvas2d [1]:

  bool enable_canvas_2d_image_chromium = false;

[1] https://cs.chromium.org/chromium/src/content/child/runtime_features.cc?type=cs&q=kCanvas2DImageChromium&l=154
Project Member

Comment 4 by bugdroid1@chromium.org, May 8 2018

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

commit 0d36c93505c63193f39f9d49c4ccdf3e43a61fad
Author: Miguel Casas <mcasas@chromium.org>
Date: Tue May 08 19:08:17 2018

canvas: move CanvasResource recycling methods to derived class

A few methods and members of CanvasResourceProvider deal with
CanvasResource recycling; these only apply to a derived class
(CanvasResourceProviderTextureGpuMemoryBuffer); this CL moves
them there, deeper in the class hierarchy.

Asides from that, this CL addresses a few more nits:

- Changes the protected methods of a final class to private.
- |msaa_sample_count_| can be const.
- CanvasResourceProviderBitmap doesn't need a |surface_| member
because it inherits it from CanvasResource.
- Removes a few {} from one-line if()s.

Bug: 839970
Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
Change-Id: I7d2e2e82fa500b1b1c9c667a4f36c48d5a57bada
Reviewed-on: https://chromium-review.googlesource.com/1047807
Reviewed-by: Justin Novosad <junov@chromium.org>
Commit-Queue: Miguel Casas <mcasas@chromium.org>
Cr-Commit-Position: refs/heads/master@{#556911}
[modify] https://crrev.com/0d36c93505c63193f39f9d49c4ccdf3e43a61fad/third_party/blink/renderer/platform/graphics/canvas_resource_provider.cc
[modify] https://crrev.com/0d36c93505c63193f39f9d49c4ccdf3e43a61fad/third_party/blink/renderer/platform/graphics/canvas_resource_provider.h

Project Member

Comment 5 by bugdroid1@chromium.org, May 9 2018

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

commit b0ce001299eb49dde55d0bc520bf1d233ea0b6c4
Author: Miguel Casas <mcasas@chromium.org>
Date: Wed May 09 19:33:59 2018

canvas: use GetPlatformSpecificTextureTarget() for GMB CanvasResources

CanvasResourceGpuMemoryBuffer always uses GL_TEXTURE_RECTANGLE_ARB as texture
target [0] when it should use gpu::GetPlatformSpecificTextureTarget() (which
boils down to the said GL_TEXTURE_RECTANGLE_ARB on Mac [1] which is the only
enabled platform [2]).  This change is needed to use CanvasResourceGMBs on
CrOs.

Unittests extended (I just wanted to mock & expect BindTexture but somehow
that needed adding other expectations).

[0] https://cs.chromium.org/chromium/src/third_party/blink/renderer/platform/graphics/canvas_resource.cc?type=cs&q=canvas_resource.cc+GL_TEXTURE_RECTANGLE_ARB&sq=package:chromium&l=207
[1] https://cs.chromium.org/chromium/src/gpu/command_buffer/common/gpu_memory_buffer_support.cc?type=cs&q=GetPlatformSpecificTextureTarget&sq=package:chromium&l=169
[2] https://cs.chromium.org/chromium/src/content/child/runtime_features.cc?dr=CSs&q=runtime_features.cc&sq=package:chromium&l=154

Bug: 839970
Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
Change-Id: Iaa1150e95f83d8ab91464b0d8d385556ba5a9ed6
Reviewed-on: https://chromium-review.googlesource.com/1049832
Reviewed-by: Justin Novosad <junov@chromium.org>
Reviewed-by: Daniele Castagna <dcastagna@chromium.org>
Commit-Queue: Miguel Casas <mcasas@chromium.org>
Cr-Commit-Position: refs/heads/master@{#557277}
[modify] https://crrev.com/b0ce001299eb49dde55d0bc520bf1d233ea0b6c4/third_party/blink/renderer/platform/graphics/canvas_resource.cc
[modify] https://crrev.com/b0ce001299eb49dde55d0bc520bf1d233ea0b6c4/third_party/blink/renderer/platform/graphics/canvas_resource_test.cc

Project Member

Comment 6 by bugdroid1@chromium.org, May 15 2018

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

commit 16b5675d433ff43b918e7595a142ff4b47b2aaab
Author: Miguel Casas <mcasas@chromium.org>
Date: Tue May 15 14:08:58 2018

blink canvas: don't BindTextures() to delete them

This CL removes unnecessary BindTexture() calls from
CanvasResourceGpuMemoryBuffer::TearDown().

Bug: 839970
Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
Change-Id: I0281c30d5c8f18ce95a563767c15e174ced4b3ed
Reviewed-on: https://chromium-review.googlesource.com/1058152
Reviewed-by: Daniele Castagna <dcastagna@chromium.org>
Reviewed-by: Justin Novosad <junov@chromium.org>
Commit-Queue: Miguel Casas <mcasas@chromium.org>
Cr-Commit-Position: refs/heads/master@{#558693}
[modify] https://crrev.com/16b5675d433ff43b918e7595a142ff4b47b2aaab/third_party/blink/renderer/platform/graphics/canvas_resource.cc

Project Member

Comment 7 by bugdroid1@chromium.org, May 15 2018

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

commit 2045a10ce6683f35bafc971f4ca85da1aa5943fe
Author: Miguel Casas <mcasas@chromium.org>
Date: Tue May 15 14:23:35 2018

canvas: use InSequence and s/const/constexpr/ in some unit tests

This CL introduces InSequence and s/const/constexpr/ in the GMB-related
Canvas2DLayerBridgeTest cases.

This is a minor preparation for crrev.com/c/1056168 where having the
expectations in a sequence makes things clearer.

Bug: 839970
Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
Change-Id: I8b43341af1e7bbae50cf9a117e20867bf2628e73
Reviewed-on: https://chromium-review.googlesource.com/1058018
Reviewed-by: Justin Novosad <junov@chromium.org>
Commit-Queue: Miguel Casas <mcasas@chromium.org>
Cr-Commit-Position: refs/heads/master@{#558697}
[modify] https://crrev.com/2045a10ce6683f35bafc971f4ca85da1aa5943fe/third_party/blink/renderer/platform/graphics/canvas_2d_layer_bridge_test.cc

Project Member

Comment 8 by bugdroid1@chromium.org, May 15 2018

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

commit c41a36a374e1d340ef97250d60a4e3098218d824
Author: Miguel Casas <mcasas@chromium.org>
Date: Tue May 15 15:29:44 2018

canvas: simplify |enable_canvas_2d_image_chromium| logic

Simplify logic around |enable_canvas_2d_image_chromium| in runtime_features.cc.

Bug: 839970
Change-Id: I7371c6ce22e7a2c73013b75c8545e5fc91b983c7
Reviewed-on: https://chromium-review.googlesource.com/1057388
Reviewed-by: Daniele Castagna <dcastagna@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Commit-Queue: Miguel Casas <mcasas@chromium.org>
Cr-Commit-Position: refs/heads/master@{#558711}
[modify] https://crrev.com/c41a36a374e1d340ef97250d60a4e3098218d824/content/child/runtime_features.cc

Comment 9 by mcasas@chromium.org, Jun 14 2018

Blocking: 788439
Project Member

Comment 10 by bugdroid1@chromium.org, Jun 28 2018

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

commit 454d9cf79519221e6365159067a41f8a2ce75ff0
Author: Miguel Casas <mcasas@chromium.org>
Date: Thu Jun 28 16:25:00 2018

Canvas: expose kCanvas2DImageChromium to chrome:flags

kCanvas2DImageChromium [0] is a FeatureFlag enabled by default in
Mac and CrOS allowing for using GL CHROMIUM_image extension for HTML
Canvas Contexts.  This CL exposes this flag to chrome:flags so it
can be used for debugging e.g. crrev.com/c/1056168 etc.

In CrOs, the flag goes unused now [1], so nothing changes there.

[0] https://cs.chromium.org/chromium/src/content/public/common/content_features.cc?type=cs&q=kCanvas2DImageChromium&g=0&l=83
[1] https://cs.chromium.org/chromium/src/content/child/runtime_features.cc?type=cs&q=enable_canvas_2d_image_chromium&g=0&l=156

Bug: 839970
Change-Id: I94a3646e996b996f6901d9414734801d1aa2bfab
Reviewed-on: https://chromium-review.googlesource.com/1117328
Commit-Queue: Miguel Casas <mcasas@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Reviewed-by: Daniele Castagna <dcastagna@chromium.org>
Cr-Commit-Position: refs/heads/master@{#571157}
[modify] https://crrev.com/454d9cf79519221e6365159067a41f8a2ce75ff0/chrome/browser/about_flags.cc
[modify] https://crrev.com/454d9cf79519221e6365159067a41f8a2ce75ff0/chrome/browser/flag_descriptions.cc
[modify] https://crrev.com/454d9cf79519221e6365159067a41f8a2ce75ff0/chrome/browser/flag_descriptions.h
[modify] https://crrev.com/454d9cf79519221e6365159067a41f8a2ce75ff0/tools/metrics/histograms/enums.xml

Project Member

Comment 11 by bugdroid1@chromium.org, Jul 9

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

commit 41a4da646e906962d816087850e7fd68c8d5bdf9
Author: Miguel Casas <mcasas@chromium.org>
Date: Mon Jul 09 23:43:21 2018

canvas: use GpuMemoryBuffers CanvasResource on CrOS

This CL connects |enable_canvas_2d_image_chromium| on CrOs, enabling
use of GpuMemoryBuffers for blitting the Skia produced textures onto.

GMBs on CrOs are GL_TEXTURE_EXTERNAL_OES, which can't be used to
CopyTextureCHROMIUM() onto, so CanvasResourceGpuMemoryBuffer gets
a new member (a 2D texture ID), connected to the same GMB, to be
used for that purpose.

Also as pointed during the review, there's no need to hold on to a
Image2DChromium as long as it's bound to a texture (to which we
hold on to), so removing the current |image_id_|.

Bug: 839970
Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_slimming_paint_v2;master.tryserver.blink:linux_trusty_blink_rel
Change-Id: I375451cca8f4e29e8fd40a4e6aaadc1a49bcd398
Reviewed-on: https://chromium-review.googlesource.com/1056168
Commit-Queue: Miguel Casas <mcasas@chromium.org>
Reviewed-by: Daniele Castagna <dcastagna@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Reviewed-by: Justin Novosad <junov@chromium.org>
Cr-Commit-Position: refs/heads/master@{#573530}
[modify] https://crrev.com/41a4da646e906962d816087850e7fd68c8d5bdf9/content/child/runtime_features.cc
[modify] https://crrev.com/41a4da646e906962d816087850e7fd68c8d5bdf9/content/public/common/content_features.cc
[modify] https://crrev.com/41a4da646e906962d816087850e7fd68c8d5bdf9/third_party/blink/renderer/platform/graphics/canvas_2d_layer_bridge_test.cc
[modify] https://crrev.com/41a4da646e906962d816087850e7fd68c8d5bdf9/third_party/blink/renderer/platform/graphics/canvas_resource.cc
[modify] https://crrev.com/41a4da646e906962d816087850e7fd68c8d5bdf9/third_party/blink/renderer/platform/graphics/canvas_resource.h
[modify] https://crrev.com/41a4da646e906962d816087850e7fd68c8d5bdf9/third_party/blink/renderer/platform/graphics/canvas_resource_test.cc

Blockedon: 867025 867601
Project Member

Comment 13 by bugdroid1@chromium.org, Jul 26

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

commit 0ae550c11d1b4cb0e27b0338add6964926cbe6e2
Author: Miguel Casas <mcasas@chromium.org>
Date: Thu Jul 26 00:08:29 2018

canvas: homogeneise style and simplify methods logic.

This CL homogeneises the if-else bracketing style in said file: these
blocks get {} if either the condition or the body in either branch is
> 1 line.  Otherwise, no brackets.

Simplified the logic of a few service methods too, and extracted
the calculation of CanvasResourceProvider::ResourceUsage |usage|
in GetOrCreateCanvasResourceProvider() so that it looks similar\
on both sides of the branch.

No new functionality intended.

Bug: 839970
Change-Id: I6e275a330f145aea861ae9344aa172ffc5840a27
Reviewed-on: https://chromium-review.googlesource.com/1150437
Commit-Queue: Miguel Casas <mcasas@chromium.org>
Reviewed-by: Justin Novosad <junov@chromium.org>
Cr-Commit-Position: refs/heads/master@{#578135}
[modify] https://crrev.com/0ae550c11d1b4cb0e27b0338add6964926cbe6e2/third_party/blink/renderer/core/html/canvas/canvas_rendering_context_host.cc
[modify] https://crrev.com/0ae550c11d1b4cb0e27b0338add6964926cbe6e2/third_party/blink/renderer/core/html/canvas/html_canvas_element.cc

Blockedon: 869161
Labels: LowLatency
Blockedon: 869922
Project Member

Comment 17 by bugdroid1@chromium.org, Aug 8

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

commit 2a7b492663d25f505f4d02db2dbcafe5df512e69
Author: Miguel Casas <mcasas@chromium.org>
Date: Wed Aug 08 18:20:57 2018

OffscreenCanvas: Remove unused method and member variable

This CL removes the member var |previous_image_release_callback_|
and method DrawingBuffer::SwapPreviousFrameCallback(), because
they are not used in a meaningful way.

It's extracted from junov@'s crrev.com/c/1103011.

Bug: 839970
Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_slimming_paint_v2;luci.chromium.try:win_optional_gpu_tests_rel;master.tryserver.blink:linux_trusty_blink_rel
Change-Id: I0c410d5181ceb973843f74bc7e419e7e8305baf7
Reviewed-on: https://chromium-review.googlesource.com/1165833
Reviewed-by: Fernando Serboncini <fserb@chromium.org>
Commit-Queue: Miguel Casas <mcasas@chromium.org>
Cr-Commit-Position: refs/heads/master@{#581632}
[modify] https://crrev.com/2a7b492663d25f505f4d02db2dbcafe5df512e69/third_party/blink/renderer/modules/canvas/offscreencanvas/OffscreenCanvas-commit.md
[modify] https://crrev.com/2a7b492663d25f505f4d02db2dbcafe5df512e69/third_party/blink/renderer/platform/graphics/gpu/drawing_buffer.cc
[modify] https://crrev.com/2a7b492663d25f505f4d02db2dbcafe5df512e69/third_party/blink/renderer/platform/graphics/gpu/drawing_buffer.h

Project Member

Comment 18 by bugdroid1@chromium.org, Aug 9

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

commit 83ee4594f33d5e84da4e8573238b8ae5420f3565
Author: Miguel Casas <mcasas@chromium.org>
Date: Thu Aug 09 14:47:12 2018

Simplify OffscreenCanvasResourceProvider

This CL simplifies OffscreenCanvasResourceProvider:

- Removes the unused member variables |width_|, |height_|
 and |frame_provider_|.
- Removes superfluous method Reshape().
- Forward declares a few data types ISO including files.
- Uses early return for easier indenting.

This is inspired by junov@'s crrev.com/c/1103011 (in the follow
up CL crrev.com/c/1167907 I'll fold OffscreenCanvasResourceProvider
into CanvasResourceDispatcher, but it's easier to review in two
steps).

Bug: 839970
Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_slimming_paint_v2;master.tryserver.blink:linux_trusty_blink_rel
Change-Id: Ie41059f647a57a85427ce5a8398a73d708503b37
Reviewed-on: https://chromium-review.googlesource.com/1167474
Reviewed-by: Fernando Serboncini <fserb@chromium.org>

[modify] https://crrev.com/83ee4594f33d5e84da4e8573238b8ae5420f3565/third_party/blink/renderer/platform/graphics/canvas_resource_dispatcher.cc
[modify] https://crrev.com/83ee4594f33d5e84da4e8573238b8ae5420f3565/third_party/blink/renderer/platform/graphics/offscreen_canvas_placeholder.cc
[modify] https://crrev.com/83ee4594f33d5e84da4e8573238b8ae5420f3565/third_party/blink/renderer/platform/graphics/offscreen_canvas_resource_provider.cc
[modify] https://crrev.com/83ee4594f33d5e84da4e8573238b8ae5420f3565/third_party/blink/renderer/platform/graphics/offscreen_canvas_resource_provider.h

Project Member

Comment 19 by bugdroid1@chromium.org, Aug 9

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

commit bc078224f82b5839fc17baf3096cb5aded8e893d
Author: Andrii Shyshkalov <tandrii@chromium.org>
Date: Thu Aug 09 19:17:03 2018

Revert "Simplify OffscreenCanvasResourceProvider"

This reverts commit 83ee4594f33d5e84da4e8573238b8ae5420f3565.

Reason for revert: Due to Gerrit outage  http://crbug.com/872722 , we are reverting this CL. Please, re-land it after all clear is given. If you have questions, please ask on the bug. Sorry for the inconvenience.

Original change's description:
> Simplify OffscreenCanvasResourceProvider
> 
> This CL simplifies OffscreenCanvasResourceProvider:
> 
> - Removes the unused member variables |width_|, |height_|
>  and |frame_provider_|.
> - Removes superfluous method Reshape().
> - Forward declares a few data types ISO including files.
> - Uses early return for easier indenting.
> 
> This is inspired by junov@'s crrev.com/c/1103011 (in the follow
> up CL crrev.com/c/1167907 I'll fold OffscreenCanvasResourceProvider
> into CanvasResourceDispatcher, but it's easier to review in two
> steps).
> 
> Bug: 839970
> Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_slimming_paint_v2;master.tryserver.blink:linux_trusty_blink_rel
> Change-Id: Ie41059f647a57a85427ce5a8398a73d708503b37
> Reviewed-on: https://chromium-review.googlesource.com/1167474
> Reviewed-by: Fernando Serboncini <fserb@chromium.org>

TBR=mcasas@chromium.org,fserb@chromium.org

Change-Id: I74fc6a1bd5d79330777960508276e75f97a281c4
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 839970
Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_slimming_paint_v2;master.tryserver.blink:linux_trusty_blink_rel
Reviewed-on: https://chromium-review.googlesource.com/1169846
Reviewed-by: Andrii Shyshkalov <tandrii@chromium.org>

[modify] https://crrev.com/bc078224f82b5839fc17baf3096cb5aded8e893d/third_party/blink/renderer/platform/graphics/canvas_resource_dispatcher.cc
[modify] https://crrev.com/bc078224f82b5839fc17baf3096cb5aded8e893d/third_party/blink/renderer/platform/graphics/offscreen_canvas_placeholder.cc
[modify] https://crrev.com/bc078224f82b5839fc17baf3096cb5aded8e893d/third_party/blink/renderer/platform/graphics/offscreen_canvas_resource_provider.cc
[modify] https://crrev.com/bc078224f82b5839fc17baf3096cb5aded8e893d/third_party/blink/renderer/platform/graphics/offscreen_canvas_resource_provider.h

Project Member

Comment 20 by bugdroid1@chromium.org, Aug 10

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

commit a0a93b4596c19a5d95830e29e997c73538f91da9
Author: Miguel Casas <mcasas@chromium.org>
Date: Fri Aug 10 00:30:26 2018

RELAND: Simplify OffscreenCanvasResourceProvider

This is just a reland of crrev.com/c/1167474; it got reverted
due to infra issues (Gerit outage), see  crbug.com/872722  and/or
https://groups.google.com/a/chromium.org/forum/#!topic/chromium-dev/YnUEOtzCrzs

TBR=fserb@chromium.org

Original CL description ------------------------------------------------

This CL simplifies OffscreenCanvasResourceProvider:

- Removes the unused member variables |width_|, |height_|
 and |frame_provider_|.
- Removes superfluous method Reshape().
- Forward declares a few data types ISO including files.
- Uses early return for easier indenting.

This is inspired by junov@'s crrev.com/c/1103011 (in the follow
up CL crrev.com/c/1167907 I'll fold OffscreenCanvasResourceProvider
into CanvasResourceDispatcher, but it's easier to review in two
steps).

Bug: 839970
Change-Id: Ic4d3a6002cec32c01e8914a11d3c392981dae8b5
Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_slimming_paint_v2;master.tryserver.blink:linux_trusty_blink_rel
Reviewed-on: https://chromium-review.googlesource.com/1167474
Reviewed-by: Fernando Serboncini <fserb@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/1169829
Reviewed-by: Miguel Casas <mcasas@chromium.org>
Commit-Queue: Miguel Casas <mcasas@chromium.org>
Cr-Commit-Position: refs/heads/master@{#581981}
[modify] https://crrev.com/a0a93b4596c19a5d95830e29e997c73538f91da9/third_party/blink/renderer/platform/graphics/canvas_resource_dispatcher.cc
[modify] https://crrev.com/a0a93b4596c19a5d95830e29e997c73538f91da9/third_party/blink/renderer/platform/graphics/offscreen_canvas_placeholder.cc
[modify] https://crrev.com/a0a93b4596c19a5d95830e29e997c73538f91da9/third_party/blink/renderer/platform/graphics/offscreen_canvas_resource_provider.cc
[modify] https://crrev.com/a0a93b4596c19a5d95830e29e997c73538f91da9/third_party/blink/renderer/platform/graphics/offscreen_canvas_resource_provider.h

Project Member

Comment 21 by bugdroid1@chromium.org, Aug 10

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

commit 79c70d39731925261ae6a5930bbc8b45e95aeab9
Author: Miguel Casas-Sanchez <mcasas@chromium.org>
Date: Fri Aug 10 17:11:17 2018

Fold OffscreenCanvasResourceProvider into CanvasResourceDispatcher

OffscreenCanvasResourceProvider is only used for accountancy of
viz::{Transferable/Returned}Resource; moreover it's not only used
for Offscreen canvases (but also for e.g. lowLatency ones). This
CL folds the logic into its only client, CanvasResourceDispatcher.

Also, the resource recycling inside OffscreenCRP is removed. This
is (all) inspired by junov@'s crrev.com/c/1103011 and follows up
on crrev.com/c/1167474.

CQ-DEPEND=CL:1169829

Bug: 839970
Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_slimming_paint_v2;master.tryserver.blink:linux_trusty_blink_rel
Change-Id: Ifed24992a6aded5c48f5e808cd61986906b64f7a
Reviewed-on: https://chromium-review.googlesource.com/1167907
Commit-Queue: Miguel Casas <mcasas@chromium.org>
Reviewed-by: Fernando Serboncini <fserb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#582217}
[modify] https://crrev.com/79c70d39731925261ae6a5930bbc8b45e95aeab9/third_party/blink/renderer/platform/BUILD.gn
[modify] https://crrev.com/79c70d39731925261ae6a5930bbc8b45e95aeab9/third_party/blink/renderer/platform/graphics/canvas_resource_dispatcher.cc
[modify] https://crrev.com/79c70d39731925261ae6a5930bbc8b45e95aeab9/third_party/blink/renderer/platform/graphics/canvas_resource_dispatcher.h
[modify] https://crrev.com/79c70d39731925261ae6a5930bbc8b45e95aeab9/third_party/blink/renderer/platform/graphics/offscreen_canvas_frame_dispatcher_test.cc
[delete] https://crrev.com/793ff51fe9dd802b273896cebc056b81e6413419/third_party/blink/renderer/platform/graphics/offscreen_canvas_resource_provider.cc
[delete] https://crrev.com/793ff51fe9dd802b273896cebc056b81e6413419/third_party/blink/renderer/platform/graphics/offscreen_canvas_resource_provider.h

Project Member

Comment 22 by bugdroid1@chromium.org, Aug 13

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

commit a38cd556307ceb4ac81d1c0bec65624ab748bea5
Author: Miguel Casas <mcasas@chromium.org>
Date: Mon Aug 13 15:24:51 2018

CanvasResourceDispatcher: s/image/canvas_resource/

This CL renames /image/canvas_resource/ in some method parameter
names.  This is prob a leftover of earlier times when
CanvasResourceDispatcher used a StaticBitmapImage -- now it uses
a CanvasResource.

Bug: 839970
Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_slimming_paint_v2;master.tryserver.blink:linux_trusty_blink_rel
Change-Id: I0575eafd326738e8a52e5801e689b5280f3ff0ec
Reviewed-on: https://chromium-review.googlesource.com/1167911
Reviewed-by: Fernando Serboncini <fserb@chromium.org>
Commit-Queue: Miguel Casas <mcasas@chromium.org>
Cr-Commit-Position: refs/heads/master@{#582582}
[modify] https://crrev.com/a38cd556307ceb4ac81d1c0bec65624ab748bea5/third_party/blink/renderer/platform/graphics/canvas_resource_dispatcher.cc

Blockedon: 869228
Project Member

Comment 24 by bugdroid1@chromium.org, Aug 14

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

commit b34b7be96c8e4026ab5f17598b4c0682b70b532f
Author: Miguel Casas <mcasas@chromium.org>
Date: Tue Aug 14 14:47:21 2018

lowLatency canvas2D: fix hibernation tab crash

lowLatency 2D contexts would crash the tab when sent to the background;
this is because of the callstack
  HTMLCanvasElement::PageVisibilityChanged()
   CanvasRenderingContext2D::SetIsHidden()
     canvas()->GetCanvas2DLayerBridge()->SetIsHidden(hidden)
     ...
       Canvas2DLayerBridge::Hibernate()

and that tries to destroy its Layer, which has not been created
(because in lowLatency mode we don't use it).

This CL fixes the issue by cutting at SetIsHidden() if there is
no |layer_|.

Test: open https://codepen.io/miguelao/full/ZjJNNw (or any other
trivial sample canvas lowLatency demo), draw something , then
open a new tab. The former tab crashes on ToT, doesn't with this CL.

Bug: 839970,  870873 
Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_slimming_paint_v2;master.tryserver.blink:linux_trusty_blink_rel
Change-Id: I6657a72009438cb31fac2b2d874c807b1719d16a
Reviewed-on: https://chromium-review.googlesource.com/1173344
Reviewed-by: Fernando Serboncini <fserb@chromium.org>
Commit-Queue: Miguel Casas <mcasas@chromium.org>
Cr-Commit-Position: refs/heads/master@{#582913}
[modify] https://crrev.com/b34b7be96c8e4026ab5f17598b4c0682b70b532f/third_party/blink/renderer/modules/canvas/canvas2d/canvas_rendering_context_2d.cc
[modify] https://crrev.com/b34b7be96c8e4026ab5f17598b4c0682b70b532f/third_party/blink/renderer/platform/graphics/canvas_2d_layer_bridge.cc

Labels: M-70
Blockedon: 878405
Blockedon: 883362
Blockedon: 878801
Blockedon: 888122
Project Member

Comment 30 by bugdroid1@chromium.org, Oct 4

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

commit 64d3813c79e904f024b15cb77fe151853e11c4c4
Author: Miguel Casas <mcasas@chromium.org>
Date: Thu Oct 04 19:08:29 2018

Update Blink lowlatency/canvas OWNERS

Comment out OWNERS that left, add myself for lowLatency canvas stuff.

Bug: 839970
Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_slimming_paint_v2;master.tryserver.blink:linux_trusty_blink_rel
Change-Id: I16f585423393083f31c154cbba4a0e0a16006218
Reviewed-on: https://chromium-review.googlesource.com/c/1237181
Commit-Queue: Miguel Casas <mcasas@chromium.org>
Reviewed-by: Stephen Chenney <schenney@chromium.org>
Reviewed-by: Fernando Serboncini <fserb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#596791}
[modify] https://crrev.com/64d3813c79e904f024b15cb77fe151853e11c4c4/third_party/blink/renderer/modules/canvas/OWNERS
[modify] https://crrev.com/64d3813c79e904f024b15cb77fe151853e11c4c4/third_party/blink/renderer/platform/graphics/OWNERS

Blockedon: 895551
Blockedon: 895528
Project Member

Comment 33 by bugdroid1@chromium.org, Nov 2

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

commit 7eb68f62e89a4f00b565baa087de6f601d410205
Author: Miguel Casas <mcasas@chromium.org>
Date: Fri Nov 02 14:34:31 2018

canvas: clean up start/stop listening for DidProcessTask

Bit of a cleanup accesory to crrev.com/c/1308043: a new pair
of private methods (StartListeningForDidProcessTask() and
StopListeningForDidProcessTask() ) are added. They work together
with the associate renamed flag
s/finalize_frame_scheduled_/listening_for_did_process_task_/
to listen for ProcessTask events (WillProcessTask/DidProcessTask).

This is a cleanup of things in preparation for the next CL
in the chain: crrev.com/c/1308043.

No new functionality intended.

Bug: 839970
Change-Id: I0bad9e3bfc80b02865c34ba4c20d37f71ecb83a9
Reviewed-on: https://chromium-review.googlesource.com/c/1308754
Reviewed-by: Fernando Serboncini <fserb@chromium.org>
Commit-Queue: Miguel Casas <mcasas@chromium.org>
Cr-Commit-Position: refs/heads/master@{#604912}
[modify] https://crrev.com/7eb68f62e89a4f00b565baa087de6f601d410205/third_party/blink/renderer/core/html/canvas/canvas_rendering_context.cc
[modify] https://crrev.com/7eb68f62e89a4f00b565baa087de6f601d410205/third_party/blink/renderer/core/html/canvas/canvas_rendering_context.h
[modify] https://crrev.com/7eb68f62e89a4f00b565baa087de6f601d410205/third_party/blink/renderer/modules/canvas/canvas2d/base_rendering_context_2d.h

Project Member

Comment 34 by bugdroid1@chromium.org, Nov 6

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

commit 53944574d38b201edd9fd7ca3ab148364d8c0c77
Author: Miguel Casas <mcasas@chromium.org>
Date: Tue Nov 06 19:28:29 2018

CanvasResourceProvider: simplify CanvasResource mgmt

This CL simplifies the resource management in CRProvider:
there's no need to keep |is_single_buffered_| since it's
always the opposite of |resource_recycling_enabled_|,
and there's no need to keep a |single_buffer_| since
there's already a vector of |canvas_resources_|, that can
be limited to having only one when IsSingleBuffered().

Most of the changes are in NewOrRecycledResource(), which
has to accommodate this new simpler mode (note that there
are unit tests covering it).

Two more side things:
- Renames: s/recycled_resources_/canvas_resources_/
- A few comments are updated/corrected.

Change-Id: Ib12db2fd7c2ba7eeb2f9144c49b061114c395e97
Bug: 839970
Reviewed-on: https://chromium-review.googlesource.com/c/1308043
Reviewed-by: Stephen White <senorblanco@chromium.org>
Reviewed-by: Fernando Serboncini <fserb@chromium.org>
Commit-Queue: Miguel Casas <mcasas@chromium.org>
Cr-Commit-Position: refs/heads/master@{#605784}
[modify] https://crrev.com/53944574d38b201edd9fd7ca3ab148364d8c0c77/third_party/blink/renderer/platform/graphics/canvas_resource_provider.cc
[modify] https://crrev.com/53944574d38b201edd9fd7ca3ab148364d8c0c77/third_party/blink/renderer/platform/graphics/canvas_resource_provider.h

Project Member

Comment 35 by bugdroid1@chromium.org, Nov 6

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

commit 69f4102d098fa48f4f7f97688fa38c5a6796f48e
Author: Miguel Casas <mcasas@chromium.org>
Date: Tue Nov 06 21:02:39 2018

CanvasResourceProvider: Move CanvasImageProvider to .cc file

CanvasImageProvider is an inner class of CanvasResourceProvider,
that only needs to be defined on the header for the base::Optional
use of it. This CL changes that to std::unique_ptr<> like its sister
member |canvas_|, moves its implementation to the .cc file and
inlines the 1-line methods.

Also I replaced a base::Unretained() with a WeakPtr() since
I didn't see why wouldn't it work.

No new code intended beyond that, just cleaning up what there is.

Bug: 839970
Change-Id: I8f3b299c12662a7a83c72ac7bc3eae1b8c1f27ea
Reviewed-on: https://chromium-review.googlesource.com/c/1318816
Commit-Queue: Miguel Casas <mcasas@chromium.org>
Reviewed-by: enne <enne@chromium.org>
Reviewed-by: Mohammad Reza Zakerinasab <zakerinasab@chromium.org>
Reviewed-by: Khushal <khushalsagar@chromium.org>
Cr-Commit-Position: refs/heads/master@{#605819}
[modify] https://crrev.com/69f4102d098fa48f4f7f97688fa38c5a6796f48e/third_party/blink/renderer/platform/graphics/canvas_resource_provider.cc
[modify] https://crrev.com/69f4102d098fa48f4f7f97688fa38c5a6796f48e/third_party/blink/renderer/platform/graphics/canvas_resource_provider.h

Blockedon: 902585
Blockedon: 902951
Project Member

Comment 38 by bugdroid1@chromium.org, Nov 9

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

commit cfb51ccdee421f37d7ed1ec897dc7aea504b6ef6
Author: Miguel Casas <mcasas@chromium.org>
Date: Fri Nov 09 19:28:28 2018

CanvasResourceGpuMemoryBuffer: remove |image_id_| member

This CL follows crrev.com/c/1056168 and removes |image_id_| from
CanvasResourceGpuMemoryBuffer, since it's not needed (it was
mentioned in that CL description but finally not removed).

Bug: 839970, 903837
Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_slimming_paint_v2;master.tryserver.blink:linux_trusty_blink_rel
Change-Id: I7540363f131e3a33609c945e68c8106992e0ba77
Reviewed-on: https://chromium-review.googlesource.com/c/1130170
Reviewed-by: Fernando Serboncini <fserb@chromium.org>
Reviewed-by: Justin Novosad <junov@chromium.org>
Commit-Queue: Miguel Casas <mcasas@chromium.org>
Cr-Commit-Position: refs/heads/master@{#606927}
[modify] https://crrev.com/cfb51ccdee421f37d7ed1ec897dc7aea504b6ef6/third_party/WebKit/LayoutTests/TestExpectations
[modify] https://crrev.com/cfb51ccdee421f37d7ed1ec897dc7aea504b6ef6/third_party/blink/renderer/platform/graphics/canvas_2d_layer_bridge_test.cc
[modify] https://crrev.com/cfb51ccdee421f37d7ed1ec897dc7aea504b6ef6/third_party/blink/renderer/platform/graphics/canvas_resource.cc
[modify] https://crrev.com/cfb51ccdee421f37d7ed1ec897dc7aea504b6ef6/third_party/blink/renderer/platform/graphics/canvas_resource.h

Blockedon: 920626

Sign in to add a comment