https://chromium-review.googlesource.com/c/567271/ enabled hardware overlay by default on eve.

Are you testing this on eve?

What version of chrome and cros are you testing this on?

Today test image of both eve and reef (with command_line)
ChromeOS-test-R61-9743.0.0-reef/eve.tar.xz

Kristian found this issue 1 week ago.
Please install "Crossy Road" from google play.

There are two ways to test it.
1. click window bar of "Crossy Road", and shake it crazy.

2. Run "Crossy Road" on the top of browser, on the top of google play as the video shows. Try to hover mouse on google play to update it.

This happens if I move the android app around (1.) and we continue to switch between only primary, and primary plus one overlay.

I can't reproduce 2., Do you also have a WebGL aquarium going on in the background?

We don't need WebGL aquarium.

ARC++
on the partial browser
on the fullscreen ARC++

In the above setting, fullscreen ARC++ bleeding to the partial browser. It must be a fault by cc (i.e. gpu)

I remove the link in the description because it may reveal something....

I record video again using reef. https://photos.app.goo.gl/EgdBec3vV3TpsiBa2

Do you mind checking if you can reproduce this on eve?

#6 - today 3 of intel engineers (including me) checked this issue on eve separately.

I see. I can't still reproduce 2. on eve.

I doubt this is a cc issue though. If you run a trace and you enable the category gpu.debug, you get a snapshot of the primary framebuffer for every swapbuffer.
You can see that when a buffer is promoted to an overlay, it is transparent where the buffer should be.

None of the snapshots I see contain any artifact like the one showed in the video, while the artifacts were definitely visible on the screen while tracing.

It seems this issue causes flickering during resizing ARC windows. See more here: b/63870135. To reproduce resize while the window left coordinate is less than screenHeight - windowHeight.

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

commit 7a2ac6c75aaeea4e3a2546b82cd358b4088cf036
Author: Dongseong Hwang <dongseong.hwang@intel.com>
Date: Fri Jul 21 23:17:37 2017

cros, drm: fix errata to set src_y of plane to drm atomic

TEST=run chrome on youtube using eve
BUG= 743277 

Change-Id: Ie096276779faafed71309372d4fa44ac07054c20
Reviewed-on: https://chromium-review.googlesource.com/581880
Commit-Queue: Dongseong Hwang <dongseong.hwang@intel.com>
Reviewed-by: Dongseong Hwang <dongseong.hwang@intel.com>
Reviewed-by: Daniel Nicoara <dnicoara@chromium.org>
Reviewed-by: Daniele Castagna <dcastagna@chromium.org>
Cr-Commit-Position: refs/heads/master@{#488783}
[modify] https://crrev.com/7a2ac6c75aaeea4e3a2546b82cd358b4088cf036/ui/ozone/platform/drm/gpu/hardware_display_plane_atomic.cc

dongseong.hwang@: Did you have a chance to test if this issue reproduces with an upstream kernel?

#15 - kevin.strasser@ is working on it. It may take time.

There are several issues when running chrome with the upstream kernel.
* With upstream kernel, cros login doesn't work. He resolved it.
* There are many random crash on reef. He's moving to cave
* We are not sure ARC++ work with the upstream kernel. He's figuring out.

dcastagna@, reveman@, would you happen to have some tips about #3?

#15: regarding 3, you don't need to get ARC++ working. You can easily reproduce the issue building a Wayland client and running it on the test device:
# ninja -C out_$BOARD/Release wayland_rects_client
# scp out_$BOARD/Release/wayland_rects_client $HOST:///tmp

and on the device after you login:
# XDG_RUNTIME_DIR=/var/run/chrome /tmp/wayland_rects_client --use-drm --scale=2 --size=1024x1024 --fullscreen --num-rects=40


Btw, we investigated a little bit and it seems the issue is with GLRenderer and viz::BufferQueue, and it is not a problem of HW overlays on Intel.

When moving the window around, we're basically mostly doing compositing, overlays are not involved. The background shows up because the primary plane buffer is being scanned out while we're drawing to it. This would also explain why if you have any intensive GPU work going on the artifacts are more visible (Android game and/or WebGL running in the background.)
Getting a snapshot of the fb works fine because the readpixel is serialized.

The BufferQueue seems to enter a bad state when a few swapbuffers with an empty damage rects are called (this usually happens with android apps) and the bad state persists when we do compositing after that.

To confirm this, the issue shows up only if partial update is enabled, if you disable it everything works fine.

We're working on a fix and we'll update soon.

#17 - thank you for sharing how to reproduce using wayland client!!!
We reproduced this glitch with upstream kernel on the following setup
1. today eve image on eve device with upstream kernel (4.13)
2. today amd64-generic image with upstream kernel built by me

As Daniele mentioned, we agree on it maybe cc issue.

To make sure it's not mesa issue, we reproduced this glitch on eve with upstream mesa.

P.S. I also reproduced it using wayland_rects_client yesterday night. I thought it may be cc issue so tried to reproduce it using my 1.3 years ago CL to prove this issue's description; http://crrev.com/1752723003/ and then found wayland_rects_client, which was landed 7 months ago. I like the demo. :)

DS: https://chromium-review.googlesource.com/c/587461/ fixes it and is about to land. You're cced to it.

#19 - Thank you for fixing it. I verified it fixing arc++ glitch on eve and reef.
now we identified all overlay issues and don't need to disable overlay. :)

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

commit b5168e0d1731a1e4b8e6da22bdac6d08f6ab41a2
Author: Daniele Castagna <dcastagna@chromium.org>
Date: Thu Jul 27 17:15:53 2017

viz: Align OutputSurface and BufferQueue in flight buffers.

In crrev.com/2829543003 we landed support for empty swap on surfaceless
output surfaces.

When the damage rect of the swap buffer was empty, the OutputSurface
would avoid calling BindFramebuffer and SwapBuffers on the BufferQueue.
It would still call BufferQueue::PageFlipComplete though, causing
an inconsistency between the BufferQueue::in_flight_surfaces_ and
the actual in flight surfaces scheduled by the GLRenderer.

In particular, in case of an empty damage rect, the renderer would
reschedule the same surface, without calling BufferQueue::SwapBuffers,
BufferQueue::in_flight_surfaces_ would contain only one surface, when
the actual scheduled surfaces for scanout would be two (even if the
same buffer).

After PageFlipComplete, BufferQueue would think there were no more
in_flight_surfaces_ and it would soon recycle the surface, and the
renderer would end up drawing to the surface while still in use for
scanout by the display controller ( crbug.com/743277  for symptoms).

This CL makes sure that the number of time we call
Additionally, it makes the buffer queue advance by a surface every
time a current surface is needed and is not available. In this way we
can call SwapBuffers, GetCurrentTextureIdtwice followed by another
SwapBuffers without having to call BindFramebuffer to have a current
surface.
 

BufferQueue: :SwapBuffers matches BufferQueue::PageFlipComplete.
Bug:  743277 
Change-Id: Ibc970c8beea9edc3d901d165f0c0479718c6e43e
Reviewed-on: https://chromium-review.googlesource.com/587461
Reviewed-by: Dongseong Hwang <dongseong.hwang@intel.com>
Reviewed-by: Robert Kroeger <rjkroege@chromium.org>
Reviewed-by: David Reveman <reveman@chromium.org>
Reviewed-by: John Bauman <jbauman@chromium.org>
Commit-Queue: Daniele Castagna <dcastagna@chromium.org>
Cr-Commit-Position: refs/heads/master@{#490001}
[modify] https://crrev.com/b5168e0d1731a1e4b8e6da22bdac6d08f6ab41a2/components/viz/service/display_embedder/buffer_queue.cc
[modify] https://crrev.com/b5168e0d1731a1e4b8e6da22bdac6d08f6ab41a2/components/viz/service/display_embedder/buffer_queue.h
[modify] https://crrev.com/b5168e0d1731a1e4b8e6da22bdac6d08f6ab41a2/components/viz/service/display_embedder/display_output_surface_ozone.cc
[modify] https://crrev.com/b5168e0d1731a1e4b8e6da22bdac6d08f6ab41a2/content/browser/compositor/gpu_surfaceless_browser_compositor_output_surface.cc

Your change meets the bar and is auto-approved for M61. Please go ahead and merge the CL to branch 3163 manually. Please contact milestone owner if you have questions.
Owners: amineer@(Android), cmasso@(iOS), ketakid @(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

This issue has been approved for a merge. Please merge the fix to any appropriate branches as soon as possible!

If all merges have been completed, please remove any remaining Merge-Approved labels from this issue.

Thanks for your time! To disable nags, add the Disable-Nags label.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

This issue has been approved for a merge. Please merge the fix to any appropriate branches as soon as possible!

If all merges have been completed, please remove any remaining Merge-Approved labels from this issue.

Thanks for your time! To disable nags, add the Disable-Nags label.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

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

commit c4a98f0576d29d5328d125917eb4b04457aec29b
Author: Daniele Castagna <dcastagna@chromium.org>
Date: Tue Aug 08 23:50:17 2017

viz: Align OutputSurface and BufferQueue in flight buffers.

In crrev.com/2829543003 we landed support for empty swap on surfaceless
output surfaces.

When the damage rect of the swap buffer was empty, the OutputSurface
would avoid calling BindFramebuffer and SwapBuffers on the BufferQueue.
It would still call BufferQueue::PageFlipComplete though, causing
an inconsistency between the BufferQueue::in_flight_surfaces_ and
the actual in flight surfaces scheduled by the GLRenderer.

In particular, in case of an empty damage rect, the renderer would
reschedule the same surface, without calling BufferQueue::SwapBuffers,
BufferQueue::in_flight_surfaces_ would contain only one surface, when
the actual scheduled surfaces for scanout would be two (even if the
same buffer).

After PageFlipComplete, BufferQueue would think there were no more
in_flight_surfaces_ and it would soon recycle the surface, and the
renderer would end up drawing to the surface while still in use for
scanout by the display controller ( crbug.com/743277  for symptoms).

This CL makes sure that the number of time we call
Additionally, it makes the buffer queue advance by a surface every
time a current surface is needed and is not available. In this way we
can call SwapBuffers, GetCurrentTextureIdtwice followed by another
SwapBuffers without having to call BindFramebuffer to have a current
surface.

TBR=dcastagna@chromium.org

(cherry picked from commit b5168e0d1731a1e4b8e6da22bdac6d08f6ab41a2)

BufferQueue: :SwapBuffers matches BufferQueue::PageFlipComplete.
Bug:  743277 
Change-Id: Ibc970c8beea9edc3d901d165f0c0479718c6e43e
Reviewed-on: https://chromium-review.googlesource.com/587461
Reviewed-by: Dongseong Hwang <dongseong.hwang@intel.com>
Reviewed-by: Robert Kroeger <rjkroege@chromium.org>
Reviewed-by: David Reveman <reveman@chromium.org>
Reviewed-by: John Bauman <jbauman@chromium.org>
Commit-Queue: Daniele Castagna <dcastagna@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#490001}
Reviewed-on: https://chromium-review.googlesource.com/607188
Reviewed-by: Daniele Castagna <dcastagna@chromium.org>
Cr-Commit-Position: refs/branch-heads/3163@{#394}
Cr-Branched-From: ff259bab28b35d242e10186cd63af7ed404fae0d-refs/heads/master@{#488528}
[modify] https://crrev.com/c4a98f0576d29d5328d125917eb4b04457aec29b/components/viz/service/display_embedder/buffer_queue.cc
[modify] https://crrev.com/c4a98f0576d29d5328d125917eb4b04457aec29b/components/viz/service/display_embedder/buffer_queue.h
[modify] https://crrev.com/c4a98f0576d29d5328d125917eb4b04457aec29b/components/viz/service/display_embedder/display_output_surface_ozone.cc
[modify] https://crrev.com/c4a98f0576d29d5328d125917eb4b04457aec29b/content/browser/compositor/gpu_surfaceless_browser_compositor_output_surface.cc

Does the bug fixed in #14 exist in M61? I suspect it to cause b/64769491. If so, can we merge?

I was under the impression we were always passing a crop_rect with origin in 0,0. So, while the bug was there, you shouldn't really be hitting it.

mtomasz@ confirmed offline that they're now setting the cropping rect and he's testing if the patch fixes the issue.

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

commit fb378c49bb8c36ef7d3a451549203794d4e797f0
Author: Daniele Castagna <dcastagna@chromium.org>
Date: Thu Aug 17 02:57:14 2017

cros, drm: fix errata to set src_y of plane to drm atomic

TEST=run chrome on youtube using eve
BUG= 743277 
TBR=dongseong.hwang@intel.com

(cherry picked from commit 7a2ac6c75aaeea4e3a2546b82cd358b4088cf036)

Change-Id: Ie096276779faafed71309372d4fa44ac07054c20
Reviewed-on: https://chromium-review.googlesource.com/581880
Commit-Queue: Dongseong Hwang <dongseong.hwang@intel.com>
Reviewed-by: Dongseong Hwang <dongseong.hwang@intel.com>
Reviewed-by: Daniel Nicoara <dnicoara@chromium.org>
Reviewed-by: Daniele Castagna <dcastagna@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#488783}
Reviewed-on: https://chromium-review.googlesource.com/618487
Cr-Commit-Position: refs/branch-heads/3163@{#625}
Cr-Branched-From: ff259bab28b35d242e10186cd63af7ed404fae0d-refs/heads/master@{#488528}
[modify] https://crrev.com/fb378c49bb8c36ef7d3a451549203794d4e797f0/ui/ozone/platform/drm/gpu/hardware_display_plane_atomic.cc

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

commit 28c6ec2bd253f0a6409bd31f7ae3abb01eb71063
Author: Dongseong Hwang <dongseong.hwang@intel.com>
Date: Thu Aug 17 17:20:52 2017

viz: optimize empty swap.

Do nothing for empty swap as much as possible.
This CL improves
1. BufferQueue::SwapBuffers() doesn't copy redundantly
2. Only BufferQueue::BindFramebuffer() makes |current_surface_| advance, which
makes it easy to understand |current_surface_| lifecycle.
3. Make empty swap use the same buffer. It may help the underline display
hardware reuse cache effectively. (e.g. Framebuffer compression [1])

[1] p145 https://01.org/sites/default/files/documentation/intel-gfx-prm-osrc-kbl-vol12-display.pdf

TEST=XDG_RUNTIME_DIR=/var/run/chrome ./wayland_rects_client --use-drm --size=512x512 --fullscreen
viz_unittests BufferQueueTest.CheckEmptySwap

Bug:  743277 ,  750919 
Change-Id: I73c9380f61c21bfea390584f1cc493e615681548
Reviewed-on: https://chromium-review.googlesource.com/590155
Commit-Queue: Dongseong Hwang <dongseong.hwang@intel.com>
Reviewed-by: David Reveman <reveman@chromium.org>
Reviewed-by: Dongseong Hwang <dongseong.hwang@intel.com>
Cr-Commit-Position: refs/heads/master@{#495205}
[modify] https://crrev.com/28c6ec2bd253f0a6409bd31f7ae3abb01eb71063/components/viz/service/display_embedder/buffer_queue.cc
[modify] https://crrev.com/28c6ec2bd253f0a6409bd31f7ae3abb01eb71063/components/viz/service/display_embedder/buffer_queue.h
[modify] https://crrev.com/28c6ec2bd253f0a6409bd31f7ae3abb01eb71063/components/viz/service/display_embedder/buffer_queue_unittest.cc

Requesting merge of #33.

This bug requires manual review: M61 has already been promoted to the beta branch, so this requires manual review
Please contact the milestone owner if you have questions.
Owners: amineer@(Android), cmasso@(iOS), ketakid@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Approving merge to M61 Chrome OS.

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

commit 686bdb21cf78f55928bd812f6a6b67d037f5b48a
Author: Daniele Castagna <dcastagna@chromium.org>
Date: Thu Aug 31 20:02:48 2017

viz: optimize empty swap.

Do nothing for empty swap as much as possible.
This CL improves
1. BufferQueue::SwapBuffers() doesn't copy redundantly
2. Only BufferQueue::BindFramebuffer() makes |current_surface_| advance, which
makes it easy to understand |current_surface_| lifecycle.
3. Make empty swap use the same buffer. It may help the underline display
hardware reuse cache effectively. (e.g. Framebuffer compression [1])

[1] p145 https://01.org/sites/default/files/documentation/intel-gfx-prm-osrc-kbl-vol12-display.pdf

TEST=XDG_RUNTIME_DIR=/var/run/chrome ./wayland_rects_client --use-drm --size=512x512 --fullscreen
viz_unittests BufferQueueTest.CheckEmptySwap

TBR=dongseong.hwang@intel.com

(cherry picked from commit 28c6ec2bd253f0a6409bd31f7ae3abb01eb71063)

Bug:  743277 ,  750919 
Change-Id: I73c9380f61c21bfea390584f1cc493e615681548
Reviewed-on: https://chromium-review.googlesource.com/590155
Commit-Queue: Dongseong Hwang <dongseong.hwang@intel.com>
Reviewed-by: David Reveman <reveman@chromium.org>
Reviewed-by: Dongseong Hwang <dongseong.hwang@intel.com>
Cr-Original-Commit-Position: refs/heads/master@{#495205}
Reviewed-on: https://chromium-review.googlesource.com/646657
Reviewed-by: Daniele Castagna <dcastagna@chromium.org>
Cr-Commit-Position: refs/branch-heads/3163@{#1042}
Cr-Branched-From: ff259bab28b35d242e10186cd63af7ed404fae0d-refs/heads/master@{#488528}
[modify] https://crrev.com/686bdb21cf78f55928bd812f6a6b67d037f5b48a/components/viz/service/display_embedder/buffer_queue.cc
[modify] https://crrev.com/686bdb21cf78f55928bd812f6a6b67d037f5b48a/components/viz/service/display_embedder/buffer_queue.h
[modify] https://crrev.com/686bdb21cf78f55928bd812f6a6b67d037f5b48a/components/viz/service/display_embedder/buffer_queue_unittest.cc