Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1813163003:
Fix PRIMITIVE_RESTART_FIXED_INDEX handling in command buffer and WebGL 2.0. (Closed)
|
Created:
4 years, 9 months ago by Ken Russell (switch to Gerrit) Modified:
4 years, 9 months ago CC:
chromium-reviews, blink-reviews, piman+watch_chromium.org, qiankun Base URL:
https://chromium.googlesource.com/chromium/src.git@master Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionFix PRIMITIVE_RESTART_FIXED_INDEX handling in command buffer and WebGL 2.0.
Adapted from yunchao.he@intel.com's
https://codereview.chromium.org/1783763002 .
This fixes the command buffer's index validation and WebGL 2.0's
implicit enabling of this capability. Emulation of this feature on top
of earlier desktop OpenGL versions will be implemented in a later CL.
BUG=594021, 295792
TEST=deqp/functional/gles3/primitiverestart.html
CQ_INCLUDE_TRYBOTS=tryserver.chromium.win:win_optional_gpu_tests_rel
Committed: https://crrev.com/0b677e15ebe36546b0d946af78657be3d8b3f0b0
Cr-Commit-Position: refs/heads/master@{#382497}
Patch Set 1 #
Total comments: 13
Patch Set 2 : Addressed review feedback from piman. #Patch Set 3 : Addressed more review feedback from piman. #Patch Set 4 : Refactored and expanded unittests. #
Messages
Total messages: 35 (13 generated)
Description was changed from ========== Fix PRIMITIVE_RESTART_FIXED_INDEX handling in command buffer and WebGL 2.0. Adapted from yunchao.he@intel.com's https://codereview.chromium.org/1783763002 . This fixes the command buffer's index validation and WebGL 2.0's implicit enabling of this capability. Emulation of this feature on top of earlier desktop OpenGL versions will be implemented in a later CL. BUG=594021, 295792 TEST=deqp/functional/gles3/primitiverestart.html ========== to ========== Fix PRIMITIVE_RESTART_FIXED_INDEX handling in command buffer and WebGL 2.0. Adapted from yunchao.he@intel.com's https://codereview.chromium.org/1783763002 . This fixes the command buffer's index validation and WebGL 2.0's implicit enabling of this capability. Emulation of this feature on top of earlier desktop OpenGL versions will be implemented in a later CL. BUG=594021, 295792 TEST=deqp/functional/gles3/primitiverestart.html CQ_INCLUDE_TRYBOTS=tryserver.chromium.win:win_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel ==========
kbr@chromium.org changed reviewers: + bajones@chromium.org, piman@chromium.org, zmo@chromium.org
This is based on yunchao.he@intel.com's https://codereview.chromium.org/1783763002/ and implements my suggestion from that CL. I think it's a fair bit simpler. It passes the deqp/functional/gles3/primitiverestart.html tests on my Linux workstation (which supports OpenGL 4.5). The emulation of PRIMITIVE_RESTART_FIXED_INDEX on OpenGL versions less than 4.3 will be done in another bug ID.
https://codereview.chromium.org/1813163003/diff/1/gpu/command_buffer/service/... File gpu/command_buffer/service/buffer_manager.cc (right): https://codereview.chromium.org/1813163003/diff/1/gpu/command_buffer/service/... gpu/command_buffer/service/buffer_manager.cc:223: this->initial_target() == GL_ELEMENT_ARRAY_BUFFER ? true : false; nit: "? true : false" is redundant https://codereview.chromium.org/1813163003/diff/1/gpu/command_buffer/service/... gpu/command_buffer/service/buffer_manager.cc:223: this->initial_target() == GL_ELEMENT_ARRAY_BUFFER ? true : false; What is this check for? This is only used for things that will be used as indices, it doesn't really matter which target this was originally created for - we still would want to avoid false-negatives if primitive restart is enabled. https://codereview.chromium.org/1813163003/diff/1/gpu/command_buffer/service/... File gpu/command_buffer/service/buffer_manager.h (right): https://codereview.chromium.org/1813163003/diff/1/gpu/command_buffer/service/... gpu/command_buffer/service/buffer_manager.h:110: primitive_restart_enabled ? GL_TRUE : GL_FALSE) { nit: keep primitive_restart_enabled_ as a bool to avoid the churn? https://codereview.chromium.org/1813163003/diff/1/gpu/command_buffer/service/... File gpu/command_buffer/service/gles2_cmd_decoder.cc (right): https://codereview.chromium.org/1813163003/diff/1/gpu/command_buffer/service/... gpu/command_buffer/service/gles2_cmd_decoder.cc:8286: state_.GetEnabled(GL_PRIMITIVE_RESTART_FIXED_INDEX), nit: state_.enable_flags.primitive_restart_fixed_index, saves a switch. https://codereview.chromium.org/1813163003/diff/1/gpu/command_buffer/service/... gpu/command_buffer/service/gles2_cmd_decoder.cc:8386: state_.GetEnabled(GL_PRIMITIVE_RESTART_FIXED_INDEX), This is used by the client side, for client-side buffers. We're kind of baking in the behavior of the client side, that only calls this before drawing - so this state is set. Worth at least a comment. Otherwise, the slightly cleaner way is to pass this bit from the client side, so that it can describe how it wants to interpret the buffer values.
https://codereview.chromium.org/1813163003/diff/1/gpu/command_buffer/service/... File gpu/command_buffer/service/buffer_manager.cc (right): https://codereview.chromium.org/1813163003/diff/1/gpu/command_buffer/service/... gpu/command_buffer/service/buffer_manager.cc:223: this->initial_target() == GL_ELEMENT_ARRAY_BUFFER ? true : false; On 2016/03/19 00:56:11, piman wrote: > What is this check for? This is only used for things that will be used as > indices, it doesn't really matter which target this was originally created for - > we still would want to avoid false-negatives if primitive restart is enabled. Good point -- I copied this code from the original CL without thinking it through. It seems unnecessary. Removed and re-tested; all tests in deqp/functional/gles3/primitiverestart.html still pass. https://codereview.chromium.org/1813163003/diff/1/gpu/command_buffer/service/... File gpu/command_buffer/service/buffer_manager.h (right): https://codereview.chromium.org/1813163003/diff/1/gpu/command_buffer/service/... gpu/command_buffer/service/buffer_manager.h:110: primitive_restart_enabled ? GL_TRUE : GL_FALSE) { On 2016/03/19 00:56:11, piman wrote: > nit: keep primitive_restart_enabled_ as a bool to avoid the churn? It was more straightforward in my opinion to turn it into an int-like type for easier comparison in the Less operator below. https://codereview.chromium.org/1813163003/diff/1/gpu/command_buffer/service/... File gpu/command_buffer/service/gles2_cmd_decoder.cc (right): https://codereview.chromium.org/1813163003/diff/1/gpu/command_buffer/service/... gpu/command_buffer/service/gles2_cmd_decoder.cc:8286: state_.GetEnabled(GL_PRIMITIVE_RESTART_FIXED_INDEX), On 2016/03/19 00:56:11, piman wrote: > nit: state_.enable_flags.primitive_restart_fixed_index, saves a switch. Done here and below. https://codereview.chromium.org/1813163003/diff/1/gpu/command_buffer/service/... gpu/command_buffer/service/gles2_cmd_decoder.cc:8386: state_.GetEnabled(GL_PRIMITIVE_RESTART_FIXED_INDEX), On 2016/03/19 00:56:11, piman wrote: > This is used by the client side, for client-side buffers. We're kind of baking > in the behavior of the client side, that only calls this before drawing - so > this state is set. Worth at least a comment. Otherwise, the slightly cleaner way > is to pass this bit from the client side, so that it can describe how it wants > to interpret the buffer values. Added a comment about how this value is used and why the current code is correct.
yunchao.he@intel.com changed reviewers: + yunchao.he@intel.com
https://codereview.chromium.org/1813163003/diff/1/gpu/command_buffer/service/... File gpu/command_buffer/service/buffer_manager.cc (right): https://codereview.chromium.org/1813163003/diff/1/gpu/command_buffer/service/... gpu/command_buffer/service/buffer_manager.cc:223: this->initial_target() == GL_ELEMENT_ARRAY_BUFFER ? true : false; On 2016/03/19 04:06:52, Ken Russell wrote: > On 2016/03/19 00:56:11, piman wrote: > > What is this check for? This is only used for things that will be used as > > indices, it doesn't really matter which target this was originally created for > - > > we still would want to avoid false-negatives if primitive restart is enabled. > > Good point -- I copied this code from the original CL without thinking it > through. It seems unnecessary. Removed and re-tested; all tests in > deqp/functional/gles3/primitiverestart.html still pass. I am not familiar with GetMaxValueInBufferCHROMIUM. We expose GetMaxValueInBufferCHROMIUM in command buffer, which calls into GetMaxValueForRange. Does this API only access to index buffer? Is it valid to access to other buffers? Please considering this situation, we have 0xFF in color buffer, and try to get the max value in that buffer in WebGL 2 (primitive restart is always enabled in WebGL 2).
https://codereview.chromium.org/1813163003/diff/1/gpu/command_buffer/service/... File gpu/command_buffer/service/buffer_manager.cc (right): https://codereview.chromium.org/1813163003/diff/1/gpu/command_buffer/service/... gpu/command_buffer/service/buffer_manager.cc:223: this->initial_target() == GL_ELEMENT_ARRAY_BUFFER ? true : false; On 2016/03/21 15:11:35, yunchao wrote: > On 2016/03/19 04:06:52, Ken Russell wrote: > > On 2016/03/19 00:56:11, piman wrote: > > > What is this check for? This is only used for things that will be used as > > > indices, it doesn't really matter which target this was originally created > for > > - > > > we still would want to avoid false-negatives if primitive restart is > enabled. > > > > Good point -- I copied this code from the original CL without thinking it > > through. It seems unnecessary. Removed and re-tested; all tests in > > deqp/functional/gles3/primitiverestart.html still pass. > > I am not familiar with GetMaxValueInBufferCHROMIUM. > We expose GetMaxValueInBufferCHROMIUM in command buffer, which calls into > GetMaxValueForRange. Does this API only access to index buffer? Is it valid to > access to other buffers? Please considering this situation, we have 0xFF in > color buffer, and try to get the max value in that buffer in WebGL 2 (primitive > restart is always enabled in WebGL 2). GetMaxValueInBufferCHROMIUM is used to emulate client-side vertex arrays. It returns the largest index accessed by the index buffer for a given draw call so the client side can upload just those vertices into a buffer object behind the scenes. It won't affect the fetching of values from non-element-array buffer objects.
lgtm https://codereview.chromium.org/1813163003/diff/1/gpu/command_buffer/service/... File gpu/command_buffer/service/buffer_manager.h (right): https://codereview.chromium.org/1813163003/diff/1/gpu/command_buffer/service/... gpu/command_buffer/service/buffer_manager.h:110: primitive_restart_enabled ? GL_TRUE : GL_FALSE) { On 2016/03/19 04:06:53, Ken Russell wrote: > On 2016/03/19 00:56:11, piman wrote: > > nit: keep primitive_restart_enabled_ as a bool to avoid the churn? > > It was more straightforward in my opinion to turn it into an int-like type for > easier comparison in the Less operator below. I don't care all that much, but FYI you can compare bools (true > false).
https://codereview.chromium.org/1813163003/diff/1/gpu/command_buffer/service/... File gpu/command_buffer/service/buffer_manager.h (right): https://codereview.chromium.org/1813163003/diff/1/gpu/command_buffer/service/... gpu/command_buffer/service/buffer_manager.h:110: primitive_restart_enabled ? GL_TRUE : GL_FALSE) { On 2016/03/21 19:14:41, piman wrote: > On 2016/03/19 04:06:53, Ken Russell wrote: > > On 2016/03/19 00:56:11, piman wrote: > > > nit: keep primitive_restart_enabled_ as a bool to avoid the churn? > > > > It was more straightforward in my opinion to turn it into an int-like type for > > easier comparison in the Less operator below. > > I don't care all that much, but FYI you can compare bools (true > false). Thanks, I didn't know that and should have tested it myself. Cleaned up.
P.S. rebased and re-ran all tests. CQ'ing.
The CQ bit was checked by kbr@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from piman@chromium.org Link to the patchset: https://codereview.chromium.org/1813163003/#ps40001 (title: "Addressed more review feedback from piman.")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1813163003/40001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1813163003/40001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: android_chromium_gn_compile_dbg on tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/android_chro...) mac_chromium_gn_rel on tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_gn_r...)
Refactored and expanded the unittests. Didn't know whether ::testing::TestWithParam<bool> could be brought in via multiple inheritance, so made the bodies of the affected tests methods on the class.
The CQ bit was checked by kbr@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from piman@chromium.org Link to the patchset: https://codereview.chromium.org/1813163003/#ps60001 (title: "Refactored and expanded unittests.")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1813163003/60001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1813163003/60001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: mac_optional_gpu_tests_rel on tryserver.chromium.mac (JOB_TIMED_OUT, no build URL)
Description was changed from ========== Fix PRIMITIVE_RESTART_FIXED_INDEX handling in command buffer and WebGL 2.0. Adapted from yunchao.he@intel.com's https://codereview.chromium.org/1783763002 . This fixes the command buffer's index validation and WebGL 2.0's implicit enabling of this capability. Emulation of this feature on top of earlier desktop OpenGL versions will be implemented in a later CL. BUG=594021, 295792 TEST=deqp/functional/gles3/primitiverestart.html CQ_INCLUDE_TRYBOTS=tryserver.chromium.win:win_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel ========== to ========== Fix PRIMITIVE_RESTART_FIXED_INDEX handling in command buffer and WebGL 2.0. Adapted from yunchao.he@intel.com's https://codereview.chromium.org/1783763002 . This fixes the command buffer's index validation and WebGL 2.0's implicit enabling of this capability. Emulation of this feature on top of earlier desktop OpenGL versions will be implemented in a later CL. BUG=594021, 295792 TEST=deqp/functional/gles3/primitiverestart.html CQ_INCLUDE_TRYBOTS=tryserver.chromium.win:win_optional_gpu_tests_rel ==========
On 2016/03/22 01:22:05, commit-bot: I haz the power wrote: > Try jobs failed on following builders: > mac_optional_gpu_tests_rel on tryserver.chromium.mac (JOB_TIMED_OUT, no build > URL) That's http://crbug.com/596250 . Skipping this bot.
The CQ bit was checked by kbr@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1813163003/60001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1813163003/60001
Message was sent while issue was closed.
Description was changed from ========== Fix PRIMITIVE_RESTART_FIXED_INDEX handling in command buffer and WebGL 2.0. Adapted from yunchao.he@intel.com's https://codereview.chromium.org/1783763002 . This fixes the command buffer's index validation and WebGL 2.0's implicit enabling of this capability. Emulation of this feature on top of earlier desktop OpenGL versions will be implemented in a later CL. BUG=594021, 295792 TEST=deqp/functional/gles3/primitiverestart.html CQ_INCLUDE_TRYBOTS=tryserver.chromium.win:win_optional_gpu_tests_rel ========== to ========== Fix PRIMITIVE_RESTART_FIXED_INDEX handling in command buffer and WebGL 2.0. Adapted from yunchao.he@intel.com's https://codereview.chromium.org/1783763002 . This fixes the command buffer's index validation and WebGL 2.0's implicit enabling of this capability. Emulation of this feature on top of earlier desktop OpenGL versions will be implemented in a later CL. BUG=594021, 295792 TEST=deqp/functional/gles3/primitiverestart.html CQ_INCLUDE_TRYBOTS=tryserver.chromium.win:win_optional_gpu_tests_rel ==========
Message was sent while issue was closed.
Committed patchset #4 (id:60001)
Message was sent while issue was closed.
Description was changed from ========== Fix PRIMITIVE_RESTART_FIXED_INDEX handling in command buffer and WebGL 2.0. Adapted from yunchao.he@intel.com's https://codereview.chromium.org/1783763002 . This fixes the command buffer's index validation and WebGL 2.0's implicit enabling of this capability. Emulation of this feature on top of earlier desktop OpenGL versions will be implemented in a later CL. BUG=594021, 295792 TEST=deqp/functional/gles3/primitiverestart.html CQ_INCLUDE_TRYBOTS=tryserver.chromium.win:win_optional_gpu_tests_rel ========== to ========== Fix PRIMITIVE_RESTART_FIXED_INDEX handling in command buffer and WebGL 2.0. Adapted from yunchao.he@intel.com's https://codereview.chromium.org/1783763002 . This fixes the command buffer's index validation and WebGL 2.0's implicit enabling of this capability. Emulation of this feature on top of earlier desktop OpenGL versions will be implemented in a later CL. BUG=594021, 295792 TEST=deqp/functional/gles3/primitiverestart.html CQ_INCLUDE_TRYBOTS=tryserver.chromium.win:win_optional_gpu_tests_rel Committed: https://crrev.com/0b677e15ebe36546b0d946af78657be3d8b3f0b0 Cr-Commit-Position: refs/heads/master@{#382497} ==========
Message was sent while issue was closed.
Patchset 4 (id:??) landed as https://crrev.com/0b677e15ebe36546b0d946af78657be3d8b3f0b0 Cr-Commit-Position: refs/heads/master@{#382497}
Message was sent while issue was closed.
On 2016/03/22 03:27:34, commit-bot: I haz the power wrote: > Patchset 4 (id:??) landed as > https://crrev.com/0b677e15ebe36546b0d946af78657be3d8b3f0b0 > Cr-Commit-Position: refs/heads/master@{#382497} Failing the Mac GPU FYI webgl2_conformance_tests: https://build.chromium.org/p/chromium.gpu.fyi/builders/Mac%2010.10%20Release%... WebglConformance.deqp_data_gles3_shaders_arrays WebglConformance.deqp_data_gles3_shaders_conditionals WebglConformance.deqp_data_gles3_shaders_constants WebglConformance.deqp_data_gles3_shaders_constant_expressions WebglConformance.deqp_data_gles3_shaders_conversions WebglConformance.deqp_data_gles3_shaders_declarations WebglConformance.deqp_data_gles3_shaders_fragdata WebglConformance.deqp_data_gles3_shaders_invalid_texture_functions WebglConformance.deqp_data_gles3_shaders_keywords WebglConformance.deqp_data_gles3_shaders_negative WebglConformance.deqp_data_gles3_shaders_switch You actually had a mac job fail with these failures in your try job list above too: http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_optional_gpu_...
Message was sent while issue was closed.
A revert of this CL (patchset #4 id:60001) has been created in https://codereview.chromium.org/1827693002/ by kbr@chromium.org. The reason for reverting is: Broke WebGL 2.0 conformance tests on Mac OS X. Needs https://codereview.chromium.org/1822643002 and possibly more. .
Message was sent while issue was closed.
On 2016/03/22 13:50:36, Jamie Madill wrote: > On 2016/03/22 03:27:34, commit-bot: I haz the power wrote: > > Patchset 4 (id:??) landed as > > https://crrev.com/0b677e15ebe36546b0d946af78657be3d8b3f0b0 > > Cr-Commit-Position: refs/heads/master@{#382497} > > Failing the Mac GPU FYI webgl2_conformance_tests: > > https://build.chromium.org/p/chromium.gpu.fyi/builders/Mac%2010.10%20Release%... > > WebglConformance.deqp_data_gles3_shaders_arrays > WebglConformance.deqp_data_gles3_shaders_conditionals > WebglConformance.deqp_data_gles3_shaders_constants > WebglConformance.deqp_data_gles3_shaders_constant_expressions > WebglConformance.deqp_data_gles3_shaders_conversions > WebglConformance.deqp_data_gles3_shaders_declarations > WebglConformance.deqp_data_gles3_shaders_fragdata > WebglConformance.deqp_data_gles3_shaders_invalid_texture_functions > WebglConformance.deqp_data_gles3_shaders_keywords > WebglConformance.deqp_data_gles3_shaders_negative > WebglConformance.deqp_data_gles3_shaders_switch > > You actually had a mac job fail with these failures in your try job list above > too: > > http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_optional_gpu_... Sorry, I didn't realize the failures were caused by this CL. I received a notification that mac_optional_gpu_tests_rel had timed out with no build, and assumed incorrectly that the previous failure was also infrastructure-related.
Message was sent while issue was closed.
On 2016/03/23 00:15:30, Ken Russell wrote: > On 2016/03/22 13:50:36, Jamie Madill wrote: > > On 2016/03/22 03:27:34, commit-bot: I haz the power wrote: > > > Patchset 4 (id:??) landed as > > > https://crrev.com/0b677e15ebe36546b0d946af78657be3d8b3f0b0 > > > Cr-Commit-Position: refs/heads/master@{#382497} > > > > Failing the Mac GPU FYI webgl2_conformance_tests: > > > > > https://build.chromium.org/p/chromium.gpu.fyi/builders/Mac%2010.10%20Release%... > > > > WebglConformance.deqp_data_gles3_shaders_arrays > > WebglConformance.deqp_data_gles3_shaders_conditionals > > WebglConformance.deqp_data_gles3_shaders_constants > > WebglConformance.deqp_data_gles3_shaders_constant_expressions > > WebglConformance.deqp_data_gles3_shaders_conversions > > WebglConformance.deqp_data_gles3_shaders_declarations > > WebglConformance.deqp_data_gles3_shaders_fragdata > > WebglConformance.deqp_data_gles3_shaders_invalid_texture_functions > > WebglConformance.deqp_data_gles3_shaders_keywords > > WebglConformance.deqp_data_gles3_shaders_negative > > WebglConformance.deqp_data_gles3_shaders_switch > > > > You actually had a mac job fail with these failures in your try job list above > > too: > > > > > http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_optional_gpu_... > > Sorry, I didn't realize the failures were caused by this CL. I received a > notification that mac_optional_gpu_tests_rel had timed out with no build, and > assumed incorrectly that the previous failure was also infrastructure-related. I have noticed this error in my previous patch: https://codereview.chromium.org/1783763002/. Just skip the primitive restart on mac (It is OGL 4.1? ). You can refer to my original patch.
Message was sent while issue was closed.
On 2016/03/23 00:25:49, yunchao wrote: > On 2016/03/23 00:15:30, Ken Russell wrote: > > On 2016/03/22 13:50:36, Jamie Madill wrote: > > > On 2016/03/22 03:27:34, commit-bot: I haz the power wrote: > > > > Patchset 4 (id:??) landed as > > > > https://crrev.com/0b677e15ebe36546b0d946af78657be3d8b3f0b0 > > > > Cr-Commit-Position: refs/heads/master@{#382497} > > > > > > Failing the Mac GPU FYI webgl2_conformance_tests: > > > > > > > > > https://build.chromium.org/p/chromium.gpu.fyi/builders/Mac%2010.10%20Release%... > > > > > > WebglConformance.deqp_data_gles3_shaders_arrays > > > WebglConformance.deqp_data_gles3_shaders_conditionals > > > WebglConformance.deqp_data_gles3_shaders_constants > > > WebglConformance.deqp_data_gles3_shaders_constant_expressions > > > WebglConformance.deqp_data_gles3_shaders_conversions > > > WebglConformance.deqp_data_gles3_shaders_declarations > > > WebglConformance.deqp_data_gles3_shaders_fragdata > > > WebglConformance.deqp_data_gles3_shaders_invalid_texture_functions > > > WebglConformance.deqp_data_gles3_shaders_keywords > > > WebglConformance.deqp_data_gles3_shaders_negative > > > WebglConformance.deqp_data_gles3_shaders_switch > > > > > > You actually had a mac job fail with these failures in your try job list > above > > > too: > > > > > > > > > http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_optional_gpu_... > > > > Sorry, I didn't realize the failures were caused by this CL. I received a > > notification that mac_optional_gpu_tests_rel had timed out with no build, and > > assumed incorrectly that the previous failure was also infrastructure-related. > > I have noticed this error in my previous patch: > https://codereview.chromium.org/1783763002/. Just skip the primitive restart on > mac (It is OGL 4.1? ). You can refer to my original patch. In the function DoDisable() and DoEnable() in gles_cmd_decoder.cc in my original patch, I add a TODO and avoid to enable primitive restart for OLD desktop OGL. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
