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

Issue 847998 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: May 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Windows
Pri: 1
Type: Bug-Regression

Blocked on:
issue 830046

Blocking:
issue 842747
issue 844308



Sign in to add a comment

WebglConformance_conformance2_state_gl_object_get_calls and canvas tests flaky with passthrough command buffer

Project Member Reported by jmad...@chromium.org, May 30 2018

Issue description

Example flaky builds:

https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/Win10%20FYI%20Release%20%28NVIDIA%29/1247

https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/Win10%20FYI%20Release%20%28NVIDIA%29/1249

I see messages like this:

6108:7216:0530/054856.599:WARNING:angle_platform_impl.cc(59)] checkStatus(618): GL framebuffer returned incomplete.

[6108:7216:0530/054856.603:ERROR:gles2_command_buffer_stub.cc(334)] Failed to initialize decoder.
[6108:7216:0530/054856.603:ERROR:gpu_channel.cc(634)] GpuChannel::CreateCommandBuffer(): failed to initialize CommandBufferStub
[6720:2500:0530/054856.604:ERROR:command_buffer_proxy_impl.cc(137)] Failure processing GpuChannelMsg_CreateCommandBuffer.
[6720:2500:0530/054856.604:ERROR:context_provider_command_buffer.cc(235)] GpuChannelHost failed to create command buffer.
[6108:7216:0530/054856.646:WARNING:angle_platform_impl.cc(59)] checkStatus(618): GL framebuffer returned incomplete.
[6108:7216:0530/054856.720:ERROR:angle_platform_impl.cc(54)] LogGLDebugMessage(145): 
	Source: OpenGL
	Type: Error

Possibly some recent change went in that is breaking something to do with Framebuffers or Surfaces. Also breaking many of the runs on the ANGLE CQ. The ones that pass are usually due to the test failing with and without patch:

https://ci.chromium.org/p/chromium/builders/luci.chromium.try/win_angle_rel_ng/1142
https://ci.chromium.org/p/chromium/builders/luci.chromium.try/win_angle_rel_ng/1140
https://ci.chromium.org/p/chromium/builders/luci.chromium.try/win_angle_rel_ng/1139

etc

 
Project Member

Comment 1 by bugdroid1@chromium.org, May 30 2018

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

commit cea7bce789fb2ea47dcf4d302cee074099c5b5e7
Author: Jamie Madill <jmadill@chromium.org>
Date: Wed May 30 20:28:52 2018

Revert "Fix bugs for blitFramebuffer when we limit src/dst rect size."

This reverts commit 32cf029fdc88c543b7d2e6a362874ca522cc7728.

Reason for revert: Seems to cause the subsequent tests to fail on the 
passthrough command buffer. Example:

https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/Win10%20FYI%20Release%20%28NVIDIA%29/1247

See issue for more details.

Bug:  847998 

Original change's description:
> Fix bugs for blitFramebuffer when we limit src/dst rect size.
> 
> According to WebGL 2 spec. the width/height of src/dst blitting area
> should not exceed the max value that can be stored in an integer.
> 
> Bug:  844308 , 830046
> 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: I65833dfacabacc46c5cd3602233926e3e31aa6cf
> Reviewed-on: https://chromium-review.googlesource.com/1061053
> Commit-Queue: Yunchao He <yunchao.he@intel.com>
> Reviewed-by: Kai Ninomiya <kainino@chromium.org>
> Reviewed-by: Kenneth Russell <kbr@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#562748}

TBR=kbr@chromium.org,yunchao.he@intel.com,kainino@chromium.org

Change-Id: I7f351aa0534b4962012b27d29245eab14a07ec24
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  844308 , 830046
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
Reviewed-on: https://chromium-review.googlesource.com/1079797
Reviewed-by: Jamie Madill <jmadill@chromium.org>
Commit-Queue: Jamie Madill <jmadill@chromium.org>
Cr-Commit-Position: refs/heads/master@{#562978}
[modify] https://crrev.com/cea7bce789fb2ea47dcf4d302cee074099c5b5e7/content/test/gpu/gpu_tests/webgl2_conformance_expectations.py
[modify] https://crrev.com/cea7bce789fb2ea47dcf4d302cee074099c5b5e7/gpu/command_buffer/service/gles2_cmd_decoder.cc

Blocking: 844308
Owner: yunchao...@intel.com
Status: Assigned (was: Untriaged)
Yuly diagnosed this. He noticed that the previous test was WebglConformance_conformance2_rendering_blitframebuffer_size_overflow and this was previously skipped. It seems this test messes things up. He also noticed that the GL + passthrough version of the WebGL 2 CTS is not run on the optional bots. We should increase coverage for the passthrough. See issue 842747 for adding more testing for this config to the CQ.
The code in command buffer should be OK. It seems that we need to add one item in the webgl2_conformance_expectation.py to suppress this failure for NV + Win + GL backend + Passthrough for this test. I will submit a new patchset to do think on my original patch at https://chromium-review.googlesource.com/c/chromium/src/+/1061053 (please tell me if it is better to submit a new patch)

The fix for this flaky might be in ANGLE. I guess that ANGLE's implementation for blitFramebuffer doesn't cover all the code in command buffer service side. Otherwise, it can't explain why the test fails when running with passthrough command buffer. 

BTW, I don't see this flaky as such a high priority bug (P1 issue). What do you think?  
Cc: yunchao...@intel.com
Owner: ----
Status: Available (was: Assigned)
The falky of WebglConformance_conformance2_state_gl_object_get_calls and WebglConformance_conformance2_textures_canvas_sub_rectangle_tex_2d_r16f_red_half_float should be not related to my patch for blitFramebuffer, but I will have a check. 

Since all flaky are NV specific, it is not appropriate for me to own this bug. 

Comment 5 by kbr@chromium.org, May 31 2018

Blocking: 842747
Owner: jmad...@chromium.org
Status: Fixed (was: Available)
jmadill@: I'm not sure we have the hardware resources to add a second copy of the WebGL 2.0 conformance suite to the optional trybots which uses the passthrough command decoder. See Issue 848084; just by adding the optional trybots to a high-traffic Chromium subdirectory we ended up swamping the GPU bots, Linux in particular.

yunchao@: these test flakes were affecting ANGLE's commit queue (CQ). Anything blocking the CQ is P1 because it affects other developers.

It's unfortunate that the fix for  Issue 844308  is still causing problems. Maybe we should consider a WebGL 2.0 specification change which will allow the browser to generate an OpenGL error in these situations where drivers aren't robust.

Closing this as Fixed by jmadill@'s revert. Another fix for  Issue 844308  will have to be attempted.

Blockedon: 830046
Cc: oetu...@nvidia.com
kbr@ and jmadill@, I think the flakes were caused by context lost from blitframebuffer. If we skip blitframebuffer for windows + NV + GL + passthrough config, the flakes will disappear. 

The context lost issue for blitframebuffer can be elegantly handled by command buffer service side code. But it is not done well for context lost in ANGLE. So I think it might be an ANGLE issue. And this issue has been reported already at https://bugs.chromium.org/p/chromium/issues/detail?id=830046. I suppose it can be handled in ANGLE, but it is not. Note that patches to fix issue crbug/830046 doesn't run the updated test blitframebuffer-size-overflow.html, because we have not roll the updated test at that time. So it is not reliable to be marked as fixed. 

Another similar issue was reported at https://bugs.chromium.org/p/chromium/issues/detail?id=709320 for Linux NV, see the comment #1 at  crbug.com/709320 . I suppose the root cause of context lost is a driver bug in NV Windows/Linux GL driver. 
OK, maybe it makes sense to reopen issue 830046 then. I'm fine with adding a skip to the test for this config for now.
See the patch at https://chromium-review.googlesource.com/c/chromium/src/+/1080003. As I said, if we skip blitFramebuffer for NV + Win + GL, these two flakes reported by ANGLE bots were gone.

Comment 9 by kbr@chromium.org, May 31 2018

Mozilla has given an opinion about this issue as well. It may not be reasonable to assume that all OpenGL drivers out there can handle enormous blits. If we can modify the WebGL specification to restrict the maximum blitFramebuffer width or height to something like 16x the maximum texture width/height, or something similar, and if that works around these driver issues, both Mozilla and Google are in agreement that we should make that change.

Project Member

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

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

commit 71a5107a7fa3567b43841b4c95a55ff247bb7790
Author: Yunchao He <yunchao.he@intel.com>
Date: Sat Jun 02 01:39:18 2018

Fix bugs for blitFramebuffer when we limit src/dst rect size.

According to WebGL 2 spec. the width/height of src/dst blitting area
should not exceed the max value that can be stored in an integer.

https://chromium-review.googlesource.com/c/chromium/src/+/1061053
was the original patch, but it was reverted because it broke ANGLE
CQ (win_angle_rel_ng bot). This new patch simply skipped the updated test
blitframebuffer-size-overflow.html on win/linux + NV + gl + passthrough,
because it still lead to context lost for this config and make subsequent
webgl tests fail. After we skip the test as what it was in the
expectation file, ANGLE CQ like win_angle_rel_ng turns green.

Bug:  844308 , 830046,  847998 
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: Ib934814a63a77e2e8a2e7e327bfe500b56e3c63d
Reviewed-on: https://chromium-review.googlesource.com/1080003
Commit-Queue: Yunchao He <yunchao.he@intel.com>
Reviewed-by: Kenneth Russell <kbr@chromium.org>
Reviewed-by: Jamie Madill <jmadill@chromium.org>
Cr-Commit-Position: refs/heads/master@{#563906}
[modify] https://crrev.com/71a5107a7fa3567b43841b4c95a55ff247bb7790/content/test/gpu/gpu_tests/webgl2_conformance_expectations.py
[modify] https://crrev.com/71a5107a7fa3567b43841b4c95a55ff247bb7790/gpu/command_buffer/service/gles2_cmd_decoder.cc

Sign in to add a comment