about:gpu is attached

Yuri, any ideas what's going on?

For nearly all of the images, it seems that the stride (bytes per row) is incorrect. We should confirm the VideoFrame::stride(...each of kYPlane, kUPlane, kVPlane...) is the same as that client-side in the DevTools code.

Yuri, I didn't notice any discrepancies between strides client-side and service-side. I noticed that this issue does not happen when software composting is on, so I suspect there is a bug in GlRendererCopier. Do you have any guesses?

Or maybe it's a command buffer issue because when viz is on we use a different type of command buffer. Kyle, any ideas?

I'll see if I can repro on my end.

samans: There are a lot of differences between InProcessCommandBuffer and GLES2CommandBufferStub. I'm not sure if something different about how the GL surface or GL context gets created could cause this.

miu: Does the weird behavior in the video, where the highlighted regions shift color when the highlight is removed, also indicate an incorrect stride?

I was able to reproduce on a Windows machine with Intel graphics using canary FYI.

miu: Do you have any idea what kind of bug can cause this problem?

https://bugs.chromium.org/p/chromium/issues/attachment?aid=331367&signed_aid=2DoSvcn1ehf3fPXTEC9fcA==&inline=1

A wrong stride can explain some of these artifacts but this looks like a different issue to me.

By the way, this problem happens pretty consistently on Windows. Any help with debugging would be appreciated!

I think the stride problem is not because FrameSinkVideoCapturer has the wrong stride. Seems like when using RGBA readback, each CopyOutputRequest returns the result of the previous CopyOutputRequest (or a combination of previous and the current result). See  crbug.com/825861 . During resize, we expect the right stride but because the previous result was returned the stride doesn't match.

This bug happens only on Windows when Viz is enabled and hardware acceleration is on.

Turns out the snapshot is not always the result of the previous CopyOutputRequest; it can also be a combination of previous and current results. Either RenderResultTexture does not create the result properly or we don't wait for the result texture to be ready before reading from it. However, I've tried adding delays before the readback / calling glFlush and glFinish before the readback and the issue persists.

I've also confirmed that the snapshot is already broken in FrameSinkVideoCapturerImpl by dumping the bitmap into a file. Also filling the VideoFrame with black before populating it makes no difference.

I'm able to reproduce this issue on ToT *without* Viz when I switch the video capture pixel format from I420 to RGBA.

0) Modify FrameSinkVideoCapturerImpl's code so it always uses RGBA readbacks. https://cs.chromium.org/chromium/src/components/viz/service/frame_sinks/video_capture/frame_sink_video_capturer_impl.cc?rcl=72390a4295cd68d32796b9ae2e3dd44ffd71b1d2&l=492

1) Open two tabs playing YouTube videos in the *same* window (this is important because they need to share the same display / GlRenderer / GlRendererCopier) 

2) Start tab capture in both tabs.

You will notice that sometimes the video of one tab contains a frame of the other tab. I believe this has the same root cause as other artifacts we are seeing: a CopyOutputRequest is returning the result of an older CopyOutputRequest.

I wasn't able to reproduce this with I420 readback without viz, but when viz is enabled both I420 and RGBA are broken.

Please see the attached videos.

Deleted: no viz rgba.mp4 2.1 MB

	no viz rgba.mp4 2.1 MB View Download

Deleted: viz i420.mp4 1.7 MB

	viz i420.mp4 1.7 MB View Download

I'm getting increasingly more confident that this is an issue with the command buffer and not any code in GlRendererCopier. What I did is, for every other CopyOutputRequest I drew one rectangle on framebuffer_texture at the beginning of CopyFromTextureOrFrameBuffer and one rectangle on source_texture in StartReadbackFromTexture. If something in between is returning old result (maybe RenderResultTexture?) we would see these rectangles appear in different screenshots, otherwise both rectangles will appear in the same screenshot. What I saw was the latter, one screenshot has two rectangles and the next one has none. Maybe glReadPixels is doing something wrong, or glMapBufferChromium returns a pointer at the wrong offset?

I just confirmed that by adding a 300 milliseconds wait before calling glMapBufferCHROMIUM in GLPixelBufferRGBAResult this problem goes away.

If this is windows-only, there's a good chance it might be only on the pass-through command buffers. Can you check if you can repro when running with --use-cmd-decoder=validating ?

Thank you piman. I verified that this bug does not happen with --use-cmd-decoder=validating.

geofflang, can you please take a look?

 Issue 825861  has been merged into this issue.

Thanks, I'll take a look.

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

commit 8d6d5225272cd89e7c61ce07f6725c1af5ec7ae2
Author: kylechar <kylechar@chromium.org>
Date: Mon May 14 20:27:47 2018

RELAND: Add viz_screenshot_sync_tests.

Add a new GPU test target that runs screenshot_sync a second time with
--enable-features=VizDisplayCompositor. This test will verify that GPU
compositing is working for OOP-D. The test only runs on Windows and
Linux bots.

Originally landed in https://crrev.com/c/1028613 but was reverted
because it failed on Windows 7 waterfall bots. There is some bug in the
interaction between the passthrough command decoder, OOP-D and frame
capture. Disable viz_screenshot_sync on Windows 7 FYI bots for now.

Bug:  812385 ,  825906 
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel
Change-Id: I96ad1b49313bf5bbcdca0f464da4b185ee74722a
Reviewed-on: https://chromium-review.googlesource.com/1047146
Commit-Queue: kylechar <kylechar@chromium.org>
Reviewed-by: Kenneth Russell <kbr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#558443}
[modify] https://crrev.com/8d6d5225272cd89e7c61ce07f6725c1af5ec7ae2/content/test/gpu/generate_buildbot_json.py
[modify] https://crrev.com/8d6d5225272cd89e7c61ce07f6725c1af5ec7ae2/testing/buildbot/chromium.gpu.fyi.json
[modify] https://crrev.com/8d6d5225272cd89e7c61ce07f6725c1af5ec7ae2/testing/buildbot/chromium.gpu.json

Steps to reproduce

1. Run chrome (Windows ToT or Canary).
  $ .\chrome.exe --use-features=VizDisplayCompositor --use-cmd-decoder=passthrough
1. Add the "Tab Capture Example" extension.
  a. Visit chrome://extensions.
  b. Turn on developer mode.
  c. Click on "Load Unpacked".
  d. Select src/chrome/common/extensions/docs/examples/api/tabCapture/ in your source tree on your local machine. 
3. Visit something like https://en.wikipedia.org/.
4. Click on Tab Capture Example extension icon to start casting to another tab. Move that tab to another Window so you can see both source and casted version at once.
5. Select all on the website, unselect all, select all, etc. The colors will be wrong for the selected area momentarily.

Whoops. Command line is wrong, it's enable-features not use-features.

$ .\chrome.exe --enable-features=VizDisplayCompositor --use-cmd-decoder=passthrough

You can check in chrome://gpu that Viz Display Compositor is enabled.

Also resizing the window will almost certainly cause artifacts.

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

commit 85287e1e619d8c64b74412e54a0526e87df71315
Author: Geoff Lang <geofflang@chromium.org>
Date: Tue May 22 17:26:40 2018

Implement the OnSyncQuery API with the passthrough command decoder.

Log info when the error cases are hit to make it easier to debug
synchronization issues.

BUG= 825906 

Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel
Change-Id: I5c783a68116ca227e1f2b45329f495d07277fb2e
Reviewed-on: https://chromium-review.googlesource.com/1066176
Reviewed-by: Antoine Labour <piman@chromium.org>
Commit-Queue: Geoff Lang <geofflang@chromium.org>
Cr-Commit-Position: refs/heads/master@{#560674}
[modify] https://crrev.com/85287e1e619d8c64b74412e54a0526e87df71315/gpu/command_buffer/service/decoder_context.h
[modify] https://crrev.com/85287e1e619d8c64b74412e54a0526e87df71315/gpu/command_buffer/service/gles2_cmd_decoder.cc
[modify] https://crrev.com/85287e1e619d8c64b74412e54a0526e87df71315/gpu/command_buffer/service/gles2_cmd_decoder_mock.h
[modify] https://crrev.com/85287e1e619d8c64b74412e54a0526e87df71315/gpu/command_buffer/service/gles2_cmd_decoder_passthrough.cc
[modify] https://crrev.com/85287e1e619d8c64b74412e54a0526e87df71315/gpu/command_buffer/service/gles2_cmd_decoder_passthrough.h
[modify] https://crrev.com/85287e1e619d8c64b74412e54a0526e87df71315/gpu/command_buffer/service/gles2_cmd_decoder_passthrough_doers.cc
[modify] https://crrev.com/85287e1e619d8c64b74412e54a0526e87df71315/gpu/command_buffer/service/raster_decoder.cc
[modify] https://crrev.com/85287e1e619d8c64b74412e54a0526e87df71315/gpu/command_buffer/service/raster_decoder_mock.h
[modify] https://crrev.com/85287e1e619d8c64b74412e54a0526e87df71315/gpu/ipc/in_process_command_buffer.cc
[modify] https://crrev.com/85287e1e619d8c64b74412e54a0526e87df71315/gpu/ipc/service/command_buffer_stub.cc

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

commit 540303afca917ffe0b7889e02c6e74e98a64fefe
Author: kylechar <kylechar@chromium.org>
Date: Fri May 25 19:51:12 2018

Enable viz_screenshot_sync_tests on Win7.

The test failures from using GLES2DecoderPassthroughImpl have been
fixed. Re-enable viz_screenshot_sync_tests on Win7 GPU FYI bots now that
it passes.

Bug:  825906 
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel
Change-Id: I0da7271ecdfa65de3c767dffbda96fa6b80979c4
Reviewed-on: https://chromium-review.googlesource.com/1070329
Reviewed-by: Kenneth Russell <kbr@chromium.org>
Commit-Queue: kylechar <kylechar@chromium.org>
Cr-Commit-Position: refs/heads/master@{#561979}
[modify] https://crrev.com/540303afca917ffe0b7889e02c6e74e98a64fefe/content/test/gpu/generate_buildbot_json.py
[modify] https://crrev.com/540303afca917ffe0b7889e02c6e74e98a64fefe/testing/buildbot/chromium.gpu.fyi.json