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

Issue 594021 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Mar 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug

Blocked on:
issue 481184

Blocking:
issue 295792
issue 598930



Sign in to add a comment

Primitive restart not working in WebGL 2.0

Project Member Reported by kbr@chromium.org, Mar 11 2016

Issue description

The behavior defined in https://www.khronos.org/registry/webgl/specs/latest/2.0/#5.18 , that the PRIMITIVE_RESTART_FIXED_INDEX state is always enabled, is not currently implemented. At least a couple of changes are needed:

1. WebGL2RenderingContextBase needs to enable the capability.
2. The command buffer's index validation code needs to be updated to take primitive restart into account. GetMaxValue in src/gpu/command_buffer/service/buffer_manager.cc needs to be conditionalized on whether primitive restart is enabled, and filter out the maximum index value for the type. Either the range_set_ in the Buffer class needs to be cleared whenever the PRIMITIVE_RESTART_FIXED_INDEX enable bit is changed, or it needs to be duplicated, one cache for when primitive restart is enabled, and one for when it's disabled. (Since the command buffer's ES 3.0 support is mainly intended for WebGL 2.0, just clearing range_set_ when the enable bit changes may be best, since it'll incur the least memory consumption.)
3. PRIMITIVE_RESTART_FIXED_INDEX needs to be emulated on desktop OpenGL as described in the TODOs for  Issue 481184 .

Perhaps this should just be folded into  Issue 481184  but for the time being I'm filing it separately.
 

Comment 1 by kbr@chromium.org, Mar 11 2016

Blocking: 295792

Comment 2 by kbr@chromium.org, Mar 11 2016

Cc: yunchao...@intel.com qiankun....@intel.com
Yunchao, Qiankun: this might be a good fairly self-contained small project.
Cc: zmo@chromium.org bajones@chromium.org
Owner: yunchao...@intel.com
Yes, I have been working on this feature these two days. :) 

The deqp test primitiverestart.html can expose this feature is not enabled in Chromium. 

Comment 4 by kbr@chromium.org, Mar 14 2016

Status: Assigned (was: Available)
Thanks Yunchao. Looking forward to your patches.

Project Member

Comment 5 by bugdroid1@chromium.org, Mar 22 2016

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

commit 0b677e15ebe36546b0d946af78657be3d8b3f0b0
Author: kbr <kbr@chromium.org>
Date: Tue Mar 22 03:23:24 2016

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

Review URL: https://codereview.chromium.org/1813163003

Cr-Commit-Position: refs/heads/master@{#382497}

[modify] https://crrev.com/0b677e15ebe36546b0d946af78657be3d8b3f0b0/gpu/command_buffer/service/buffer_manager.cc
[modify] https://crrev.com/0b677e15ebe36546b0d946af78657be3d8b3f0b0/gpu/command_buffer/service/buffer_manager.h
[modify] https://crrev.com/0b677e15ebe36546b0d946af78657be3d8b3f0b0/gpu/command_buffer/service/buffer_manager_unittest.cc
[modify] https://crrev.com/0b677e15ebe36546b0d946af78657be3d8b3f0b0/gpu/command_buffer/service/gles2_cmd_decoder.cc
[modify] https://crrev.com/0b677e15ebe36546b0d946af78657be3d8b3f0b0/third_party/WebKit/Source/modules/webgl/WebGLRenderingContextBase.cpp

Project Member

Comment 6 by bugdroid1@chromium.org, Mar 23 2016

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

commit 557d64542a3d0eda9c8f4cbd445a9ecfd88c8622
Author: kbr <kbr@chromium.org>
Date: Wed Mar 23 00:14:36 2016

Revert of Fix PRIMITIVE_RESTART_FIXED_INDEX handling in command buffer and WebGL 2.0. (patchset #4 id:60001 of https://codereview.chromium.org/1813163003/ )

Reason for revert:
Broke WebGL 2.0 conformance tests on Mac OS X. Needs https://codereview.chromium.org/1822643002 and possibly more.

Original issue's description:
> 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}

TBR=zmo@chromium.org,bajones@chromium.org,piman@chromium.org,yunchao.he@intel.com
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG= 594021 ,  295792 

Review URL: https://codereview.chromium.org/1827693002

Cr-Commit-Position: refs/heads/master@{#382742}

[modify] https://crrev.com/557d64542a3d0eda9c8f4cbd445a9ecfd88c8622/gpu/command_buffer/service/buffer_manager.cc
[modify] https://crrev.com/557d64542a3d0eda9c8f4cbd445a9ecfd88c8622/gpu/command_buffer/service/buffer_manager.h
[modify] https://crrev.com/557d64542a3d0eda9c8f4cbd445a9ecfd88c8622/gpu/command_buffer/service/buffer_manager_unittest.cc
[modify] https://crrev.com/557d64542a3d0eda9c8f4cbd445a9ecfd88c8622/gpu/command_buffer/service/gles2_cmd_decoder.cc
[modify] https://crrev.com/557d64542a3d0eda9c8f4cbd445a9ecfd88c8622/third_party/WebKit/Source/modules/webgl/WebGLRenderingContextBase.cpp

Comment 7 by kbr@chromium.org, Mar 30 2016

Blocking: 598930
Project Member

Comment 8 by bugdroid1@chromium.org, Mar 30 2016

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

commit 56aae8327a6d2e3fa4863339be9f8abeaea47680
Author: yunchao.he <yunchao.he@intel.com>
Date: Wed Mar 30 08:32:32 2016

[Command buffer] Enable primitive restart for WebGL 2.
This change also fixed bugs in primitiverestart.html in WebGL deqp tests.

Some code come from kbr@chromium.org's
https://codereview.chromium.org/1813163003/

BUG= 594021 ,  295792 ,  598930 
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

Review URL: https://codereview.chromium.org/1822643002

Cr-Commit-Position: refs/heads/master@{#383931}

[modify] https://crrev.com/56aae8327a6d2e3fa4863339be9f8abeaea47680/content/test/gpu/gpu_tests/webgl2_conformance_expectations.py
[modify] https://crrev.com/56aae8327a6d2e3fa4863339be9f8abeaea47680/gpu/command_buffer/service/buffer_manager.cc
[modify] https://crrev.com/56aae8327a6d2e3fa4863339be9f8abeaea47680/gpu/command_buffer/service/buffer_manager.h
[modify] https://crrev.com/56aae8327a6d2e3fa4863339be9f8abeaea47680/gpu/command_buffer/service/buffer_manager_unittest.cc
[modify] https://crrev.com/56aae8327a6d2e3fa4863339be9f8abeaea47680/gpu/command_buffer/service/context_state.cc
[modify] https://crrev.com/56aae8327a6d2e3fa4863339be9f8abeaea47680/gpu/command_buffer/service/gles2_cmd_decoder.cc
[modify] https://crrev.com/56aae8327a6d2e3fa4863339be9f8abeaea47680/third_party/WebKit/Source/modules/webgl/WebGLRenderingContextBase.cpp
[modify] https://crrev.com/56aae8327a6d2e3fa4863339be9f8abeaea47680/ui/gl/generate_bindings.py
[modify] https://crrev.com/56aae8327a6d2e3fa4863339be9f8abeaea47680/ui/gl/gl_bindings_api_autogen_gl.h
[modify] https://crrev.com/56aae8327a6d2e3fa4863339be9f8abeaea47680/ui/gl/gl_bindings_autogen_gl.cc
[modify] https://crrev.com/56aae8327a6d2e3fa4863339be9f8abeaea47680/ui/gl/gl_bindings_autogen_gl.h
[modify] https://crrev.com/56aae8327a6d2e3fa4863339be9f8abeaea47680/ui/gl/gl_bindings_autogen_mock.cc
[modify] https://crrev.com/56aae8327a6d2e3fa4863339be9f8abeaea47680/ui/gl/gl_bindings_autogen_mock.h
[modify] https://crrev.com/56aae8327a6d2e3fa4863339be9f8abeaea47680/ui/gl/gl_mock_autogen_gl.h

Project Member

Comment 9 by bugdroid1@chromium.org, Mar 30 2016

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

commit 56aae8327a6d2e3fa4863339be9f8abeaea47680
Author: yunchao.he <yunchao.he@intel.com>
Date: Wed Mar 30 08:32:32 2016

[Command buffer] Enable primitive restart for WebGL 2.
This change also fixed bugs in primitiverestart.html in WebGL deqp tests.

Some code come from kbr@chromium.org's
https://codereview.chromium.org/1813163003/

BUG= 594021 ,  295792 ,  598930 
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

Review URL: https://codereview.chromium.org/1822643002

Cr-Commit-Position: refs/heads/master@{#383931}

[modify] https://crrev.com/56aae8327a6d2e3fa4863339be9f8abeaea47680/content/test/gpu/gpu_tests/webgl2_conformance_expectations.py
[modify] https://crrev.com/56aae8327a6d2e3fa4863339be9f8abeaea47680/gpu/command_buffer/service/buffer_manager.cc
[modify] https://crrev.com/56aae8327a6d2e3fa4863339be9f8abeaea47680/gpu/command_buffer/service/buffer_manager.h
[modify] https://crrev.com/56aae8327a6d2e3fa4863339be9f8abeaea47680/gpu/command_buffer/service/buffer_manager_unittest.cc
[modify] https://crrev.com/56aae8327a6d2e3fa4863339be9f8abeaea47680/gpu/command_buffer/service/context_state.cc
[modify] https://crrev.com/56aae8327a6d2e3fa4863339be9f8abeaea47680/gpu/command_buffer/service/gles2_cmd_decoder.cc
[modify] https://crrev.com/56aae8327a6d2e3fa4863339be9f8abeaea47680/third_party/WebKit/Source/modules/webgl/WebGLRenderingContextBase.cpp
[modify] https://crrev.com/56aae8327a6d2e3fa4863339be9f8abeaea47680/ui/gl/generate_bindings.py
[modify] https://crrev.com/56aae8327a6d2e3fa4863339be9f8abeaea47680/ui/gl/gl_bindings_api_autogen_gl.h
[modify] https://crrev.com/56aae8327a6d2e3fa4863339be9f8abeaea47680/ui/gl/gl_bindings_autogen_gl.cc
[modify] https://crrev.com/56aae8327a6d2e3fa4863339be9f8abeaea47680/ui/gl/gl_bindings_autogen_gl.h
[modify] https://crrev.com/56aae8327a6d2e3fa4863339be9f8abeaea47680/ui/gl/gl_bindings_autogen_mock.cc
[modify] https://crrev.com/56aae8327a6d2e3fa4863339be9f8abeaea47680/ui/gl/gl_bindings_autogen_mock.h
[modify] https://crrev.com/56aae8327a6d2e3fa4863339be9f8abeaea47680/ui/gl/gl_mock_autogen_gl.h

Comment 10 by kbr@chromium.org, Mar 30 2016

Status: Fixed (was: Assigned)
Thanks Yunchao for persevering with this fix!

Sign in to add a comment