Pixel unpack buffer bindings in WebGL code cause unrelated TexImage2D calls to error |
||||||||||||
Issue descriptionVersion: 55.0.2853.0 at 66f753c OS: Ubuntu 16.04, NVIDIA What steps will reproduce the problem? (1) Use a debug build (this works in release as well, but much less frequently) (2) Run expando-loss-2, getextension-while-pbo-bound-stability (new), or conformance2/rendering/canvas-resizing-with-pbo-bound (attached). What is the expected output? No crash. What do you see instead? Failed TexImage2D calls (because a pbo is still bound but it should have been saved, cleared, and later restored).
,
Sep 7 2016
This is related to the incomplete initialization of virtual GL contexts in Chrome as discussed in person with a few folks. Qiankun's https://codereview.chromium.org/2302323002/ attempts to fix this but it's not clear that would be the best place to put this initialization. It looks like a three-phase initialization (allocate the virtual GL context, initialize the command decoder, and make the virtual GL context current again) is undesirable. The problematic code is in https://cs.chromium.org/chromium/src/gpu/ipc/service/gpu_command_buffer_stub.cc?q=gpu_command_buffer_stub.cc&sq=package:chromium&l=556 . Note that the GLContextVirtual's initialization is always short-circuited because the command decoder isn't initialized yet.
,
Sep 7 2016
sievers@ may be most familiar with this original code. My initial thought is that it is correct to initialize the state at decoder initialization time. Here's the rationale: 1) Until the decoder is initialized, the virtual context is unusable. The first MakeVirtuallyCurrent should only make the real GL context current, but not set any state. 2) Next the decoder is initialized, which should set all initial GL state. 3) Subsequent runs of MakeVirtuallyCurrent deal with both making the real GL context current, and calling into the decoder to restore it's state.
,
Sep 7 2016
I hadn't realized that in GLES2DecoderImpl::Initialize there is this comment: // Set all the default state because some GL drivers get it wrong. This does imply that resetting just this particular piece of state is the right thing to do, though perhaps there's a more principled way of doing it. It's unfortunate that the allocation of the black textures in the TextureManager during the early call to ContextGroup::Initialize() is what's breaking, when later on the ContextState would be used to zero out all of this state anyway.
,
Sep 7 2016
Do we still need black textures in ES3, where the unpack buffer is introduced? If not, we can simply make it conditional to ES2.
,
Sep 7 2016
Good point - but if there's even one ES 3.0 driver out there which doesn't correctly handle incomplete textures, this code path would have to be enabled via a driver bug workaround. So at a minimum this has to work.
,
Sep 8 2016
A suppression is needed on Linux for the new WebglConformance_conformance2_misc_getextension_while_pbo_bound_stability test. It's flaking on the linux_optional_gpu_test_rel tryserver per https://bugs.chromium.org/p/chromium/issues/detail?id=619264#c56 .
,
Sep 8 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/4506eb54b0c0165ec11eaec9e5143d5eaa612fe1 commit 4506eb54b0c0165ec11eaec9e5143d5eaa612fe1 Author: kbr <kbr@chromium.org> Date: Thu Sep 08 04:10:27 2016 Skip getextension-while-pbo-bound-stability on Linux. There are known bugs affecting this test and it's failing intermittently on linux_optional_gpu_tests_rel. BUG= 644572 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel;master.tryserver.chromium.android:android_optional_gpu_tests_rel TBR=zmo@chromium.org NOTRY=true Review-Url: https://codereview.chromium.org/2320033002 Cr-Commit-Position: refs/heads/master@{#417188} [modify] https://crrev.com/4506eb54b0c0165ec11eaec9e5143d5eaa612fe1/content/test/gpu/gpu_tests/webgl2_conformance_expectations.py
,
Sep 8 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/4506eb54b0c0165ec11eaec9e5143d5eaa612fe1 commit 4506eb54b0c0165ec11eaec9e5143d5eaa612fe1 Author: kbr <kbr@chromium.org> Date: Thu Sep 08 04:10:27 2016 Skip getextension-while-pbo-bound-stability on Linux. There are known bugs affecting this test and it's failing intermittently on linux_optional_gpu_tests_rel. BUG= 644572 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel;master.tryserver.chromium.android:android_optional_gpu_tests_rel TBR=zmo@chromium.org NOTRY=true Review-Url: https://codereview.chromium.org/2320033002 Cr-Commit-Position: refs/heads/master@{#417188} [modify] https://crrev.com/4506eb54b0c0165ec11eaec9e5143d5eaa612fe1/content/test/gpu/gpu_tests/webgl2_conformance_expectations.py
,
Sep 9 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/17ad0f1ca0367fbb6d28cd9ed3d4d60a44e0e655 commit 17ad0f1ca0367fbb6d28cd9ed3d4d60a44e0e655 Author: qiankun.miao <qiankun.miao@intel.com> Date: Fri Sep 09 20:14:05 2016 Reset PIXEL_UNPACK_BUFFER at texture manager initialization time Otherwise, there will be some weird GL errors on some GL drivers. BUG= 641643 , 644572 TEST=conformance2/misc/getextension-while-pbo-bound-stability.html CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel Review-Url: https://codereview.chromium.org/2302323002 Cr-Commit-Position: refs/heads/master@{#417685} [modify] https://crrev.com/17ad0f1ca0367fbb6d28cd9ed3d4d60a44e0e655/content/test/gpu/gpu_tests/webgl2_conformance_expectations.py [modify] https://crrev.com/17ad0f1ca0367fbb6d28cd9ed3d4d60a44e0e655/gpu/command_buffer/service/test_helper.cc [modify] https://crrev.com/17ad0f1ca0367fbb6d28cd9ed3d4d60a44e0e655/gpu/command_buffer/service/test_helper.h [modify] https://crrev.com/17ad0f1ca0367fbb6d28cd9ed3d4d60a44e0e655/gpu/command_buffer/service/texture_manager.cc [modify] https://crrev.com/17ad0f1ca0367fbb6d28cd9ed3d4d60a44e0e655/gpu/command_buffer/service/texture_manager_unittest.cc
,
Sep 9 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/17ad0f1ca0367fbb6d28cd9ed3d4d60a44e0e655 commit 17ad0f1ca0367fbb6d28cd9ed3d4d60a44e0e655 Author: qiankun.miao <qiankun.miao@intel.com> Date: Fri Sep 09 20:14:05 2016 Reset PIXEL_UNPACK_BUFFER at texture manager initialization time Otherwise, there will be some weird GL errors on some GL drivers. BUG= 641643 , 644572 TEST=conformance2/misc/getextension-while-pbo-bound-stability.html CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel Review-Url: https://codereview.chromium.org/2302323002 Cr-Commit-Position: refs/heads/master@{#417685} [modify] https://crrev.com/17ad0f1ca0367fbb6d28cd9ed3d4d60a44e0e655/content/test/gpu/gpu_tests/webgl2_conformance_expectations.py [modify] https://crrev.com/17ad0f1ca0367fbb6d28cd9ed3d4d60a44e0e655/gpu/command_buffer/service/test_helper.cc [modify] https://crrev.com/17ad0f1ca0367fbb6d28cd9ed3d4d60a44e0e655/gpu/command_buffer/service/test_helper.h [modify] https://crrev.com/17ad0f1ca0367fbb6d28cd9ed3d4d60a44e0e655/gpu/command_buffer/service/texture_manager.cc [modify] https://crrev.com/17ad0f1ca0367fbb6d28cd9ed3d4d60a44e0e655/gpu/command_buffer/service/texture_manager_unittest.cc
,
Sep 10 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/2c847da844d0f3f802069754b840bc36c3366c87 commit 2c847da844d0f3f802069754b840bc36c3366c87 Author: kainino <kainino@chromium.org> Date: Sat Sep 10 03:33:05 2016 Save/restore PBO bindings in DrawingBuffer DrawingBuffer::prepareTextureMailboxInternal This prevents TexImage2D from being called internally with a leaked pixel unpack buffer object bound. BUG= 644572 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.win:win_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel Review-Url: https://codereview.chromium.org/2329523002 Cr-Commit-Position: refs/heads/master@{#417815} [modify] https://crrev.com/2c847da844d0f3f802069754b840bc36c3366c87/third_party/WebKit/Source/modules/webgl/WebGLRenderingContextBase.cpp [modify] https://crrev.com/2c847da844d0f3f802069754b840bc36c3366c87/third_party/WebKit/Source/platform/graphics/gpu/DrawingBuffer.cpp [modify] https://crrev.com/2c847da844d0f3f802069754b840bc36c3366c87/third_party/WebKit/Source/platform/graphics/gpu/DrawingBuffer.h
,
Sep 10 2016
|
||||||||||||
►
Sign in to add a comment |
||||||||||||
Comment 1 by qiankun....@intel.com
, Sep 7 2016