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

Issue 869161 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Aug 7
Cc:
Components:
EstimatedDays: ----
NextAction: 2018-08-16
OS: Chrome
Pri: 3
Type: Bug

Blocking:
issue 839970



Sign in to add a comment

Canvas 2D low latency on Chrome OS Intel does not turn into overlay

Project Member Reported by mcasas@chromium.org, Jul 30

Issue description

E.g. [1] (scribbling on a 2D canvas) with the flag
  chrome://flags#enable-experimental-web-platform-features
set to "Enabled".

The canvas 2d low latency code does a:

  sk_sp<SkImage> image = GetSkSurface()->makeImageSnapshot();

that then is copied into a GpuMemoryBuffer. 

Since the origin of coordinates for canvas and for overlays is
different, the DrawQuad is marked as upside down (y_flipped) 
and not promoted to overlay.

What would be the best way to use Skia to draw upside down?



[1] https://codepen.io/miguelao/full/ZjJNNw
[2] https://cs.chromium.org/chromium/src/third_party/blink/renderer/platform/graphics/canvas_resource_provider.cc?type=cs&q=canvasresource+makeimagesnapshot&sq=package:chromium&g=0&l=183
 
Blocking: 839970
Cc: brianosman@chromium.org bsalomon@chromium.org
brianosman@, bsalomon@, this is probably trivial but, what would be
the best way?
Cc: andrescj@chromium.org
Owner: mcasas@chromium.org
Status: Assigned (was: Available)
Project Member

Comment 5 by bugdroid1@chromium.org, Aug 1

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

commit 86d4fd13113a1daf835d15bee3879486cdfc1185
Author: Miguel Casas <mcasas@chromium.org>
Date: Wed Aug 01 18:18:38 2018

CanvasResourceDispatcher: correct viz::TextureDrawQuad size

This CL fixes the Size we SetAll() in the viz::TextureDrawQuad
from an empty one to the appropriate CanvasResource Size -- this
is needed to indicate to e.g. Ozone the |resource_size_in_pixels|
of the overlay candidate, but it's also more correct in general.

Also removes an unused variable and adds {} in a few needed places.

Bug:  869161 
Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_slimming_paint_v2;master.tryserver.blink:linux_trusty_blink_rel
Change-Id: Ice28fb8c9d7f9a4eee29a22f8634b08c9dfb2e36
Reviewed-on: https://chromium-review.googlesource.com/1157232
Commit-Queue: Miguel Casas <mcasas@chromium.org>
Reviewed-by: Daniele Castagna <dcastagna@chromium.org>
Reviewed-by: Fernando Serboncini <fserb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#579864}
[modify] https://crrev.com/86d4fd13113a1daf835d15bee3879486cdfc1185/third_party/blink/renderer/platform/graphics/canvas_resource_dispatcher.cc

Part of the problem is (TIL) [1]: 
"The CanvasRenderingContext2D.clearRect() method of the Canvas 2D API 
sets all pixels in the rectangle defined by starting point (x, y) and 
size (width, height) to transparent black, erasing any previously 
drawn content" 
-- which means that on RockChip Scarlet, the canvas 2d in the codepen
[2] was being promoted to overlay, but we could see the GL tint of the
background. I corrected the said pen to not use clearRect(), which
should improve the situation.


[1] https://developer.mozilla.org/en-US/docs/Web/API/CanvasRenderingContext2D/clearRect
[2] https://codepen.io/miguelao/full/ZjJNNw
Project Member

Comment 7 by bugdroid1@chromium.org, Aug 2

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

commit d6b707904a7ad2f93a4636b5c0a289a3d2702ae4
Author: Miguel Casas <mcasas@chromium.org>
Date: Thu Aug 02 18:09:29 2018

OverlayCandidate: simplify logic and correct style

This CL has a few tiny cleanups:
- Uses {} in conditionals if either the body or the condition
 is >1 line.
- base::ContainsValue() for ease of reading.
- Uses early return.

No logical changes intended.

Bug:  869161 
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel
Change-Id: I1085ba8084982c2e78aa57052c4b8d05b3f72dc7
Reviewed-on: https://chromium-review.googlesource.com/1160763
Reviewed-by: ccameron <ccameron@chromium.org>
Commit-Queue: Miguel Casas <mcasas@chromium.org>
Cr-Commit-Position: refs/heads/master@{#580247}
[modify] https://crrev.com/d6b707904a7ad2f93a4636b5c0a289a3d2702ae4/components/viz/service/display/overlay_candidate.cc

Project Member

Comment 8 by bugdroid1@chromium.org, Aug 3

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

commit a2f973dea12aed352e6426790186d5f908320171
Author: Miguel Casas <mcasas@chromium.org>
Date: Fri Aug 03 20:09:27 2018

CanvasResourceDispatcher: add interesting TRACE_EVENT0()

This CL adds TRACE_EVENT0() to some important CanvasResourceDispatcher
methods: DispatchFrame{Sync}() and PrepareFrame(), which are now used
more frequently, for lowLatency cases.

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

Project Member

Comment 9 by bugdroid1@chromium.org, Aug 7

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

commit 7e961a4ba3703e88845effae936e652b7172377e
Author: Miguel Casas <mcasas@chromium.org>
Date: Tue Aug 07 18:40:37 2018

canvas 2D lowLatency ✏️: correct CreateSkSurface() orientation

This CL does two things to support the correct SkSurface
orientation for low latency canvas 2D contexts to get promoted
to overlay:

1. CanvasRenderingContext::IsOriginTopLeft() is extended in 2D
canvases to return false if IsAccelerated() (because those refer
to the bottom left) -- but true if it IsSingleBuffered(). This
is needed to support overlays on Intel, which can only be top
left (and other platforms/archs like ARM RockChip are OK with it
anyway).

2. CanvasResourceProvider::Create() gets a new parameter, namely
|is_origin_top_left|, which is by default true (since that's how
Canvases work by default). The callsites where true is not the
right thing are corrected.

Bug:  869161 
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_layout_tests_slimming_paint_v2;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel;master.tryserver.blink:linux_trusty_blink_rel
Change-Id: Ia798bd674b3f9e42063e5ae8b4017bfdd9e60d48
Reviewed-on: https://chromium-review.googlesource.com/1160774
Reviewed-by: Fernando Serboncini <fserb@chromium.org>
Reviewed-by: Kenneth Russell <kbr@chromium.org>
Commit-Queue: Miguel Casas <mcasas@chromium.org>
Cr-Commit-Position: refs/heads/master@{#581294}
[modify] https://crrev.com/7e961a4ba3703e88845effae936e652b7172377e/third_party/blink/renderer/core/html/canvas/canvas_rendering_context_host.cc
[modify] https://crrev.com/7e961a4ba3703e88845effae936e652b7172377e/third_party/blink/renderer/core/imagebitmap/image_bitmap.cc
[modify] https://crrev.com/7e961a4ba3703e88845effae936e652b7172377e/third_party/blink/renderer/core/offscreencanvas/offscreen_canvas.cc
[modify] https://crrev.com/7e961a4ba3703e88845effae936e652b7172377e/third_party/blink/renderer/modules/canvas/canvas2d/canvas_rendering_context_2d.cc
[modify] https://crrev.com/7e961a4ba3703e88845effae936e652b7172377e/third_party/blink/renderer/modules/canvas/canvas2d/canvas_rendering_context_2d.h
[modify] https://crrev.com/7e961a4ba3703e88845effae936e652b7172377e/third_party/blink/renderer/modules/webgl/webgl_rendering_context_base.cc
[modify] https://crrev.com/7e961a4ba3703e88845effae936e652b7172377e/third_party/blink/renderer/platform/graphics/canvas_resource_provider.cc
[modify] https://crrev.com/7e961a4ba3703e88845effae936e652b7172377e/third_party/blink/renderer/platform/graphics/canvas_resource_provider.h

bsalomon@ thanks for your pointers in #3, #9 is based on it \o/
NextAction: 2018-08-16
Status: Fixed (was: Assigned)
This should be fixed by C9, let's verify independently on ToT in a few days

Commit 7e961a4b... initially landed in 70.0.3516.0


The NextAction date has arrived: 2018-08-16

Sign in to add a comment