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

Issue 696345 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Apr 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 2
Type: Bug


Sign in to add a comment

Incorrect Transform Feedback Implementation in Chrome

Project Member Reported by gman@chromium.org, Feb 26 2017

Issue description

Chrome Version: 58.0.3024.0 (Official Build) canary (64-bit)
OS: ALL? (OSX)

What steps will reproduce the problem?
(1) https://jsfiddle.net/greggman/7bt2m8sc/

What is the expected result?

It should print "TRANSFORM FEEDBACK:"

What happens instead?

It prints "TRANSFORM FEEDBACK: [object WebGLBuffer]


Transform feedback is new to me so I could be wrong but AFAICT all the state of buffer bindings using `gl.bindBufferBase` or `gl.bindBufferRange` are part of the currently bound transform feedback. They are NOT global state.

So, if I bind aa transformfeedback

    gl.bindTransformFeedback(gl.TRANSFORM_FEEDBACK, tf); 

Then bind a buffer to gl.TRANSFORM_FEEDBACK_BUFFER using bindBufferBase

    gl.useProgram(prg);
    gl.bindBufferBase(gl.TRANSFORM_FEEDBACK_BUFFER, idIndex, buf);

Then unbind the transform feedback

    gl.bindTransformFeedback(gl.TRANSFORM_FEEDBACK, null);

If I then check which buffer is bound

    console.log("TRANSFORM_FEEDBACK:", gl.getIndexedParameter(gl.TRANSFORM_FEEDBACK_BUFFER_BINDING, 0));

It should return NULL (or undefined or whatever) because gl.TRANSFORM_FEEDBACK_BUFFER_BINDING is not global state

In section 2.15.1 of the spec it says

> The resulting transform feedback object is a new state vector, comprising
> all the state and with the same initial values listed in table 6.24.

And table 6.24 lists all this state

> TRANSFORM FEEDBACK BUFFER BINDING
> TRANSFORM FEEDBACK BUFFER START
> TRANSFORM FEEDBACK BUFFER SIZE
> TRANSFORM FEEDBACK PAUSED
> TRANSFORM FEEDBACK ACTIVE

In other words if I understand correctly, a Transform Feedback Object is very much like a Vertex Array Object. A Vertex Array Object keeps a set of attributes, a Transform Feedback Object keep the set of buffers to receive transform feedback.

This comes up with more than just the query. When I unbind the transform feedback no buffers should be bound anymore but if I try to bind any of the buffers on to ARRAY_BUFFER Chrome complains the buffer is already bound to TRANSFORM_FEEDBACK 

> WebGL: INVALID_OPERATION: bindBuffer: a buffer bound to TRANSFORM_FEEDBACK_BUFFER can not be bound to any other targets

but, it's NOT because I unbound the TransformFeedback Object that contained the binding. 


In any case, whether or not my interpretation is correct, Firefox seems to be doing what I expect. I also threw together a native C test on OSX that seemed to confirm my understanding

Whichever is correct, either Chrome or Firefox should be fixed and tests should be added to test


 

Comment 1 by kbr@chromium.org, Feb 27 2017

Labels: -Pri-3 Pri-2
Status: Available (was: Untriaged)
Summary: Incorrect Transform Feedback Implementation in Chrome (was: Incorrect Transform Feedback Implementation in Chrome?)
Thanks Gregg for catching this. Chrome's behavior is incorrect. (The command buffer is correct, but the code in WebGL2RenderingContextBase is wrong.) There aren't adequate tests in the WebGL 2.0 conformance suite covering these queries.

Transform feedback's indexed binding points are incorrectly maintained in WebGL2RenderingContextBase. They should be in WebGLTransformFeedback.

Comment 2 by gman@chromium.org, Mar 19 2017

I just wanted to make it clear this bug will prevent lots of webassembly OpenGL ES 3.0 apps from running in Chrome

The issue is not the query (which is broken but uncommon). The issue is the bocus binding errors from chrome. Standard transform feedback usage won't work in chrome because it incorrectly generates errors

Just pointing that out because it's been a month and no updates on this issue.


Comment 3 by zmo@chromium.org, Mar 20 2017

Cc: kainino@chromium.org
Owner: zmo@chromium.org
Status: Assigned (was: Available)
OK, I thought it's a minor query bug that no one will run into.

If it blocks contents, then let's fix it now.

Comment 4 by kbr@chromium.org, Mar 20 2017

Owner: kbr@chromium.org
Let me take this from Mo.

Comment 5 by zmo@chromium.org, Apr 17 2017

Currently we check conflicts at buffer binding time, and we only check with the currently bound ARRAY_BUFFER. However, this is not enough.  We also need to check the currently enabled vertex attrib's VERTEX_ATTRIB_ARRAY_BUFFER_BINDING. This is missing in the current command buffer implementation.
Project Member

Comment 6 by bugdroid1@chromium.org, Apr 24 2017

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

commit 772c234787651b285f4bee7d5a714fadcf7c0bd4
Author: kbr <kbr@chromium.org>
Date: Mon Apr 24 21:13:44 2017

Fix state management of transform feedback buffers.

New test being added in https://github.com/KhronosGroup/WebGL/pull/2345 .

WebGLTransformFeedback was incorrectly implemented as WebGLSharedObject.
Fixed by changing to WebGLContextObject.

BUG= 696345 
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;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/2773843002
Cr-Commit-Position: refs/heads/master@{#466766}

[modify] https://crrev.com/772c234787651b285f4bee7d5a714fadcf7c0bd4/third_party/WebKit/Source/modules/webgl/WebGL2RenderingContextBase.cpp
[modify] https://crrev.com/772c234787651b285f4bee7d5a714fadcf7c0bd4/third_party/WebKit/Source/modules/webgl/WebGL2RenderingContextBase.h
[modify] https://crrev.com/772c234787651b285f4bee7d5a714fadcf7c0bd4/third_party/WebKit/Source/modules/webgl/WebGLSharedObject.h
[modify] https://crrev.com/772c234787651b285f4bee7d5a714fadcf7c0bd4/third_party/WebKit/Source/modules/webgl/WebGLTransformFeedback.cpp
[modify] https://crrev.com/772c234787651b285f4bee7d5a714fadcf7c0bd4/third_party/WebKit/Source/modules/webgl/WebGLTransformFeedback.h

Comment 7 by kbr@chromium.org, Jun 5 2017

Blocking: angleproject:2051

Comment 8 by kbr@chromium.org, Dec 5 2017

Owner: jdarpinian@chromium.org
The issues are described more clearly in this Github issue:
https://github.com/KhronosGroup/WebGL/issues/2346

To try to summarize:

- It needs to be illegal to have a buffer bound to the active transform feedback in the context, and to any other binding point in the context (or, really, active vertex array object) which is going to read from or write to that buffer in a given API call.

- API calls that are going to require checking include:
-- texImage (because of reads from PBOs)
-- readPixels (because of writes to PBOs)
-- draw calls (because of vertex fetches)
-- CopyBufferSubData

- These will all generate INVALID_OPERATION if an illegal state is found, and this behavior will be documented in the spec.

- The restrictions on buffer binding related to transform feedback will be removed from the spec.

In discussions with new team member jdarpinian@ he is going to pick up this bug and drive it to completion (thanks!).

Comment 9 by kbr@chromium.org, Dec 5 2017

See also pull request https://github.com/KhronosGroup/WebGL/pull/2345 . This needs to be redone in terms of the new rules.

Project Member

Comment 10 by bugdroid1@chromium.org, Jan 24 2018

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

commit 079bcdc14e4056ba9e2d2e8b28667baea72acc8f
Author: James Darpinian <jdarpinian@chromium.org>
Date: Wed Jan 24 03:47:18 2018

Stop caching GL_TRANSFORM_FEEDBACK_BUFFER incorrectly.

The cached value for GL_TRANSFORM_FEEDBACK_BUFFER was incorrect after
glBindTransformFeedback was called. Correctly updating the cache would
require tracking a ton more state which we don't want to do at this
level, so instead this change simply removes the cache.

Bug:  696345 
Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel;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
Change-Id: I2a539cdead4c1f3417301d49d0407c9f3041db1d
Reviewed-on: https://chromium-review.googlesource.com/879857
Commit-Queue: James Darpinian <jdarpinian@chromium.org>
Reviewed-by: Zhenyao Mo <zmo@chromium.org>
Reviewed-by: Kenneth Russell <kbr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#531418}
[modify] https://crrev.com/079bcdc14e4056ba9e2d2e8b28667baea72acc8f/gpu/command_buffer/client/gles2_implementation.cc
[modify] https://crrev.com/079bcdc14e4056ba9e2d2e8b28667baea72acc8f/gpu/command_buffer/client/gles2_implementation.h
[modify] https://crrev.com/079bcdc14e4056ba9e2d2e8b28667baea72acc8f/gpu/command_buffer/client/gles2_implementation_unittest.cc

https://github.com/KhronosGroup/WebGL/pull/2560

I've updated the WebGL spec to change how transform feedback binding restrictions are enforced. When a buffer is bound to transform feedback and non-transform feedback binding points simultaneously, an error will no longer be generated at bind time. Instead the error is generated when the buffer is used. I will shortly be submitting Chrome and ANGLE patches for the new behavior.

Separately, while investigating this I discovered that the GL_TRANSFORM_FEEDBACK_BUFFER_BINDING generic bind point (as opposed to the indexed bind points) was modeled incorrectly in many drivers. According to the spec, the generic bind point should be part of the transform feedback object, which means it should be affected when glBindTransformFeedback is called. A majority of tested drivers instead treat it as part of the context, and glBindTransformFeedback never affects it.

I brought the issue to the Khronos OpenGL working group (https://gitlab.khronos.org/opengl/API/issues/66) and they agreed to change the OpenGL spec. The GL_TRANSFORM_FEEDBACK_BUFFER_BINDING generic bind point will now be part of the context and it will not change when glBindTransformFeedback is called, matching what the majority of drivers already do. I am in the process of making a deqp test and a WebGL conformance test for the new behavior. My Chrome and ANGLE patches fix this too, but SwiftShader will also need to be updated.
Project Member

Comment 13 by bugdroid1@chromium.org, Mar 9 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/angle/angle/+/e8a93c6ed6feda4e0f22bd034cf95dd4aae3618d

commit e8a93c6ed6feda4e0f22bd034cf95dd4aae3618d
Author: James Darpinian <jdarpinian@chromium.org>
Date: Fri Mar 09 00:50:24 2018

New transform feedback buffer binding rules

Detects undefined behavior when a buffer is bound to a transform feedback
binding point and a non transform feedback binding point at the same time.

Also moves the transform feedback buffer generic binding point out of the
transform feedback object and into the context's global state, to match
driver behavior. This way binding a new transform feedback object does not
affect GL_TRANSFORM_FEEDBACK_BUFFER_BINDING which is similar to how VAOs
work with GL_ARRAY_BUFFER_BINDING.

Bug:  696345 
Change-Id: If3b9306cde7cd2197a8ce35e10c3af9ee58da0b8
Reviewed-on: https://chromium-review.googlesource.com/853130
Commit-Queue: James Darpinian <jdarpinian@chromium.org>
Reviewed-by: Jamie Madill <jmadill@chromium.org>
Reviewed-by: Geoff Lang <geofflang@chromium.org>

[modify] https://crrev.com/e8a93c6ed6feda4e0f22bd034cf95dd4aae3618d/src/libANGLE/RefCountObject.h
[modify] https://crrev.com/e8a93c6ed6feda4e0f22bd034cf95dd4aae3618d/src/libANGLE/Buffer.cpp
[modify] https://crrev.com/e8a93c6ed6feda4e0f22bd034cf95dd4aae3618d/src/libANGLE/VertexArray.h
[modify] https://crrev.com/e8a93c6ed6feda4e0f22bd034cf95dd4aae3618d/src/libANGLE/TransformFeedback.cpp
[modify] https://crrev.com/e8a93c6ed6feda4e0f22bd034cf95dd4aae3618d/src/libANGLE/State.cpp
[modify] https://crrev.com/e8a93c6ed6feda4e0f22bd034cf95dd4aae3618d/src/libANGLE/VertexAttribute.cpp
[modify] https://crrev.com/e8a93c6ed6feda4e0f22bd034cf95dd4aae3618d/src/libANGLE/Context.cpp
[modify] https://crrev.com/e8a93c6ed6feda4e0f22bd034cf95dd4aae3618d/src/libANGLE/TransformFeedback_unittest.cpp
[modify] https://crrev.com/e8a93c6ed6feda4e0f22bd034cf95dd4aae3618d/src/libANGLE/ErrorStrings.h
[modify] https://crrev.com/e8a93c6ed6feda4e0f22bd034cf95dd4aae3618d/src/libANGLE/validationES.cpp
[modify] https://crrev.com/e8a93c6ed6feda4e0f22bd034cf95dd4aae3618d/src/libANGLE/validationES3.cpp
[modify] https://crrev.com/e8a93c6ed6feda4e0f22bd034cf95dd4aae3618d/src/libANGLE/Buffer.h
[modify] https://crrev.com/e8a93c6ed6feda4e0f22bd034cf95dd4aae3618d/src/libANGLE/VertexAttribute.h
[modify] https://crrev.com/e8a93c6ed6feda4e0f22bd034cf95dd4aae3618d/src/tests/gl_tests/TransformFeedbackTest.cpp
[modify] https://crrev.com/e8a93c6ed6feda4e0f22bd034cf95dd4aae3618d/src/libANGLE/renderer/gl/VertexArrayGL.cpp
[modify] https://crrev.com/e8a93c6ed6feda4e0f22bd034cf95dd4aae3618d/src/libANGLE/renderer/gl/StateManagerGL.cpp
[modify] https://crrev.com/e8a93c6ed6feda4e0f22bd034cf95dd4aae3618d/src/libANGLE/State.h
[modify] https://crrev.com/e8a93c6ed6feda4e0f22bd034cf95dd4aae3618d/src/libANGLE/validationES2.cpp
[modify] https://crrev.com/e8a93c6ed6feda4e0f22bd034cf95dd4aae3618d/src/libANGLE/TransformFeedback.h
[modify] https://crrev.com/e8a93c6ed6feda4e0f22bd034cf95dd4aae3618d/src/libANGLE/VertexArray.cpp
[modify] https://crrev.com/e8a93c6ed6feda4e0f22bd034cf95dd4aae3618d/src/libANGLE/Context.h

Blockedon: angleproject:2398
Project Member

Comment 15 by bugdroid1@chromium.org, Mar 9 2018

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

commit 61c71162b81037b9db2b09d03d940b424a5b5eb9
Author: James Darpinian <jdarpinian@chromium.org>
Date: Fri Mar 09 23:25:42 2018

Implement new transform feedback buffer binding rules

Bug:  696345 
Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel;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
Change-Id: Ia471817b9eaaf3ea25c773c5292e2b21fafbcb94
Reviewed-on: https://chromium-review.googlesource.com/826491
Commit-Queue: James Darpinian <jdarpinian@chromium.org>
Reviewed-by: Kai Ninomiya <kainino@chromium.org>
Reviewed-by: Zhenyao Mo <zmo@chromium.org>
Reviewed-by: Kenneth Russell <kbr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#542276}
[modify] https://crrev.com/61c71162b81037b9db2b09d03d940b424a5b5eb9/content/test/gpu/gpu_tests/webgl2_conformance_expectations.py
[modify] https://crrev.com/61c71162b81037b9db2b09d03d940b424a5b5eb9/gpu/command_buffer/service/buffer_manager.cc
[modify] https://crrev.com/61c71162b81037b9db2b09d03d940b424a5b5eb9/gpu/command_buffer/service/buffer_manager.h
[modify] https://crrev.com/61c71162b81037b9db2b09d03d940b424a5b5eb9/gpu/command_buffer/service/context_state.cc
[modify] https://crrev.com/61c71162b81037b9db2b09d03d940b424a5b5eb9/gpu/command_buffer/service/gles2_cmd_decoder.cc
[modify] https://crrev.com/61c71162b81037b9db2b09d03d940b424a5b5eb9/gpu/command_buffer/service/indexed_buffer_binding_host.cc
[modify] https://crrev.com/61c71162b81037b9db2b09d03d940b424a5b5eb9/gpu/command_buffer/service/indexed_buffer_binding_host.h
[modify] https://crrev.com/61c71162b81037b9db2b09d03d940b424a5b5eb9/gpu/command_buffer/service/indexed_buffer_binding_host_unittest.cc
[modify] https://crrev.com/61c71162b81037b9db2b09d03d940b424a5b5eb9/gpu/command_buffer/service/texture_manager.cc
[modify] https://crrev.com/61c71162b81037b9db2b09d03d940b424a5b5eb9/gpu/command_buffer/service/transform_feedback_manager.cc
[modify] https://crrev.com/61c71162b81037b9db2b09d03d940b424a5b5eb9/gpu/command_buffer/service/transform_feedback_manager.h
[modify] https://crrev.com/61c71162b81037b9db2b09d03d940b424a5b5eb9/gpu/command_buffer/service/vertex_array_manager.cc
[modify] https://crrev.com/61c71162b81037b9db2b09d03d940b424a5b5eb9/gpu/command_buffer/service/vertex_array_manager.h
[modify] https://crrev.com/61c71162b81037b9db2b09d03d940b424a5b5eb9/gpu/command_buffer/service/vertex_array_manager_unittest.cc
[modify] https://crrev.com/61c71162b81037b9db2b09d03d940b424a5b5eb9/gpu/command_buffer/service/vertex_attrib_manager.cc
[modify] https://crrev.com/61c71162b81037b9db2b09d03d940b424a5b5eb9/gpu/command_buffer/service/vertex_attrib_manager.h
[modify] https://crrev.com/61c71162b81037b9db2b09d03d940b424a5b5eb9/gpu/command_buffer/service/vertex_attrib_manager_unittest.cc
[modify] https://crrev.com/61c71162b81037b9db2b09d03d940b424a5b5eb9/gpu/command_buffer/tests/gl_map_buffer_range_unittest.cc
[modify] https://crrev.com/61c71162b81037b9db2b09d03d940b424a5b5eb9/third_party/WebKit/Source/modules/webgl/WebGL2RenderingContextBase.cpp
[modify] https://crrev.com/61c71162b81037b9db2b09d03d940b424a5b5eb9/third_party/WebKit/Source/modules/webgl/WebGL2RenderingContextBase.h
[modify] https://crrev.com/61c71162b81037b9db2b09d03d940b424a5b5eb9/third_party/WebKit/Source/modules/webgl/WebGLProgram.cpp
[modify] https://crrev.com/61c71162b81037b9db2b09d03d940b424a5b5eb9/third_party/WebKit/Source/modules/webgl/WebGLProgram.h
[modify] https://crrev.com/61c71162b81037b9db2b09d03d940b424a5b5eb9/third_party/WebKit/Source/modules/webgl/WebGLRenderingContextBase.h
[modify] https://crrev.com/61c71162b81037b9db2b09d03d940b424a5b5eb9/third_party/WebKit/Source/modules/webgl/WebGLTransformFeedback.cpp
[modify] https://crrev.com/61c71162b81037b9db2b09d03d940b424a5b5eb9/third_party/WebKit/Source/modules/webgl/WebGLTransformFeedback.h

Project Member

Comment 16 by bugdroid1@chromium.org, Mar 10 2018

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

commit c164831e262a8b9e8d8e6e3ea3659fd69d795061
Author: James Darpinian <jdarpinian@chromium.org>
Date: Sat Mar 10 01:54:05 2018

Move transform feedback generic bind point out of TF object

Bug:  696345 
Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel;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
Change-Id: Id534cd0169d02311c808cd7803a86d63c5e1329b
Reviewed-on: https://chromium-review.googlesource.com/924825
Commit-Queue: James Darpinian <jdarpinian@chromium.org>
Reviewed-by: Kenneth Russell <kbr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#542311}
[modify] https://crrev.com/c164831e262a8b9e8d8e6e3ea3659fd69d795061/gpu/command_buffer/client/gles2_implementation.cc
[modify] https://crrev.com/c164831e262a8b9e8d8e6e3ea3659fd69d795061/gpu/command_buffer/client/gles2_implementation.h
[modify] https://crrev.com/c164831e262a8b9e8d8e6e3ea3659fd69d795061/gpu/command_buffer/client/gles2_implementation_unittest.cc
[modify] https://crrev.com/c164831e262a8b9e8d8e6e3ea3659fd69d795061/gpu/command_buffer/service/context_state.cc
[modify] https://crrev.com/c164831e262a8b9e8d8e6e3ea3659fd69d795061/gpu/command_buffer/service/gles2_cmd_decoder.cc
[modify] https://crrev.com/c164831e262a8b9e8d8e6e3ea3659fd69d795061/gpu/command_buffer/service/gles2_cmd_decoder_unittest.cc
[modify] https://crrev.com/c164831e262a8b9e8d8e6e3ea3659fd69d795061/gpu/command_buffer/service/indexed_buffer_binding_host.cc
[modify] https://crrev.com/c164831e262a8b9e8d8e6e3ea3659fd69d795061/gpu/command_buffer/service/indexed_buffer_binding_host.h
[modify] https://crrev.com/c164831e262a8b9e8d8e6e3ea3659fd69d795061/gpu/command_buffer/service/transform_feedback_manager.cc
[modify] https://crrev.com/c164831e262a8b9e8d8e6e3ea3659fd69d795061/gpu/command_buffer/service/transform_feedback_manager.h
[modify] https://crrev.com/c164831e262a8b9e8d8e6e3ea3659fd69d795061/third_party/WebKit/Source/modules/webgl/WebGL2RenderingContextBase.cpp
[modify] https://crrev.com/c164831e262a8b9e8d8e6e3ea3659fd69d795061/third_party/WebKit/Source/modules/webgl/WebGL2RenderingContextBase.h
[modify] https://crrev.com/c164831e262a8b9e8d8e6e3ea3659fd69d795061/third_party/WebKit/Source/modules/webgl/WebGLTransformFeedback.cpp
[modify] https://crrev.com/c164831e262a8b9e8d8e6e3ea3659fd69d795061/third_party/WebKit/Source/modules/webgl/WebGLTransformFeedback.h

Blockedon: 828579
Project Member

Comment 18 by bugdroid1@chromium.org, Apr 11 2018

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

commit 779f7a5ddec09f15fb601e3a2b9e98e75d350b68
Author: James Darpinian <jdarpinian@chromium.org>
Date: Wed Apr 11 08:32:19 2018

Fix UnmapBufferHelper to use the correct binding

UnmapBufferHelper assumed that the mapped buffer was still bound to the same
binding point that it was first bound to, which isn't guaranteed. This
fixes the WebGL conformance test transform_feedback/switching-objects.html

Bug:  696345 
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;master.tryserver.chromium.win:win_optional_gpu_tests_rel
Change-Id: Ia06b8e3b2b4a7a7f1a38c4bcdb241a039dc36a80
Reviewed-on: https://chromium-review.googlesource.com/998845
Commit-Queue: James Darpinian <jdarpinian@chromium.org>
Reviewed-by: Zhenyao Mo <zmo@chromium.org>
Reviewed-by: Kenneth Russell <kbr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#549825}
[modify] https://crrev.com/779f7a5ddec09f15fb601e3a2b9e98e75d350b68/gpu/command_buffer/service/gles2_cmd_decoder.cc
[modify] https://crrev.com/779f7a5ddec09f15fb601e3a2b9e98e75d350b68/gpu/command_buffer/tests/gl_map_buffer_range_unittest.cc

Project Member

Comment 19 by bugdroid1@chromium.org, Apr 11 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/angle/angle/+/78acf5b5488481bba37d76b45fba427081677fa0

commit 78acf5b5488481bba37d76b45fba427081677fa0
Author: James Darpinian <jdarpinian@chromium.org>
Date: Wed Apr 11 11:52:15 2018

D3D11: Fix primitive topology dirty bit tracking.

syncPrimitiveTopology checks the transform feedback state, so it needs
to be called whenever the transform feedback state changes. This fixes
flakes in the WebGL 2 conformance test
transform_feedback/simultaneous_binding.html

Bug:  696345 
Change-Id: I4e17bbf60b4a387cc23dc55bd5a051f5da9fa66e
Reviewed-on: https://chromium-review.googlesource.com/1006489
Commit-Queue: James Darpinian <jdarpinian@chromium.org>
Commit-Queue: Jamie Madill <jmadill@chromium.org>
Reviewed-by: Jamie Madill <jmadill@chromium.org>

[modify] https://crrev.com/78acf5b5488481bba37d76b45fba427081677fa0/src/libANGLE/renderer/d3d/d3d11/StateManager11.cpp

Project Member

Comment 20 by bugdroid1@chromium.org, Apr 12 2018

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

commit cd2b9676d2a45d84d784da927bbd75ee0e62f0fa
Author: angle-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com <angle-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Date: Thu Apr 12 01:01:30 2018

Roll src/third_party/angle/ ebd6e2df2..14f4817c4 (7 commits)

https://chromium.googlesource.com/angle/angle.git/+log/ebd6e2df290a..14f4817c4dad

$ git log ebd6e2df2..14f4817c4 --date=short --no-merges --format='%ad %ae %s'
2018-04-11 lucferron Vulkan: Simplify viewport / scissor updates
2018-04-11 cwallez cq.cfg: equivalent_to to 100% for Windows tryservers.
2018-04-11 jmadill D3D11: Fix inactive attrib VAO perf regression.
2018-03-31 jmadill Texture: Pass ImageIndex to relevant methods.
2018-04-08 jmadill Vulkan: Rename vk::LineLoopHelper.
2018-04-10 lucferron Vulkan: Cleanup some TODOs in TextureTest.cpp
2018-04-10 jdarpinian D3D11: Fix primitive topology dirty bit tracking.

Created with:
  roll-dep src/third_party/angle
BUG=chromium:815092, chromium:829906 , chromium:696345 


The AutoRoll server is located here: https://angle-chromium-roll.skia.org

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+/master/autoroll/README.md

If the roll is causing failures, please contact the current sheriff, who should
be CC'd on the roll, and stop the roller if necessary.


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
TBR=ynovikov@chromium.org

Change-Id: Ib53f6489642caaae9657a7f69d4bd28c60527e19
Reviewed-on: https://chromium-review.googlesource.com/1008728
Reviewed-by: angle-chromium-autoroll <angle-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Commit-Queue: Jamie Madill <jmadill@chromium.org>
Cr-Commit-Position: refs/heads/master@{#549981}
[modify] https://crrev.com/cd2b9676d2a45d84d784da927bbd75ee0e62f0fa/DEPS

Project Member

Comment 21 by bugdroid1@chromium.org, Apr 12 2018

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

commit d400565ddce087a38f25df0cfd023a3f93cb7d38
Author: James Darpinian <jdarpinian@chromium.org>
Date: Thu Apr 12 05:38:07 2018

Transform feedback fixes are in, remove test failure expectations.

Bug:  696345 , 818383 , 820639 
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;master.tryserver.chromium.win:win_optional_gpu_tests_rel
Change-Id: Iaacb3bbaf738e994ea6caaad7b83c5a8bbf8e964
Reviewed-on: https://chromium-review.googlesource.com/993393
Reviewed-by: Kenneth Russell <kbr@chromium.org>
Commit-Queue: James Darpinian <jdarpinian@chromium.org>
Cr-Commit-Position: refs/heads/master@{#550025}
[modify] https://crrev.com/d400565ddce087a38f25df0cfd023a3f93cb7d38/content/test/gpu/gpu_tests/webgl2_conformance_expectations.py

Project Member

Comment 22 by bugdroid1@chromium.org, Apr 12 2018

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

commit bac7b5cc6ca384e48ec88b1286960c80283e3288
Author: Jamie Madill <jmadill@chromium.org>
Date: Thu Apr 12 17:53:38 2018

Suppress two flaky XFB tests on ANGLE/GL.

The failure expectations are restored for two of the three new
tests. Marking as flaky - if they continue to fail despite the
flaky suppression we will need to mark them as failing.

Bug:  696345 
Bug:  818383 
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: I8ac5db7e2c75f2c50cbea42523767336e5c431ea
Tbr: geofflang@chromium.org
Tbr: jdarpinian@chromium.org
Tbr: kbr@chromium.org
No-try: True
Reviewed-on: https://chromium-review.googlesource.com/1010770
Reviewed-by: Jamie Madill <jmadill@chromium.org>
Reviewed-by: Kenneth Russell <kbr@chromium.org>
Commit-Queue: Jamie Madill <jmadill@chromium.org>
Cr-Commit-Position: refs/heads/master@{#550263}
[modify] https://crrev.com/bac7b5cc6ca384e48ec88b1286960c80283e3288/content/test/gpu/gpu_tests/webgl2_conformance_expectations.py

WebglConformance_conformance2_transform_feedback_switching_objects still fails here:
https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/Linux%20FYI%20Release%20%28AMD%20R7%20240%29/497
Going to update expectations accordingly.
Status: Fixed (was: Assigned)
Fixes are in! The remaining low priority test failures will be debugged in bug 832238.

Comment 25 by kbr@chromium.org, Apr 12 2018

Blocking: 832238

Comment 26 by kbr@chromium.org, Apr 12 2018

Great work James! Linked these bugs together so they can be found more easily.

Project Member

Comment 27 by bugdroid1@chromium.org, Apr 13 2018

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

commit 8d537d0fcb070f469cc0cf4d3185875964f1f0b6
Author: Yuly Novikov <ynovikov@chromium.org>
Date: Fri Apr 13 00:06:22 2018

Mark one WebGL2 test Fail

conformance2/transform_feedback/switching-objects.html
on Linux AMD R7 240, no_angle

BUG= 696345 
TBR=kbr@chromium.org

No-try: True
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: I55e92720c7d129b3ca6a249fefb517cb33318137
Reviewed-on: https://chromium-review.googlesource.com/1010520
Commit-Queue: Yuly Novikov <ynovikov@chromium.org>
Reviewed-by: Kenneth Russell <kbr@chromium.org>
Reviewed-by: Yuly Novikov <ynovikov@chromium.org>
Cr-Commit-Position: refs/heads/master@{#550423}
[modify] https://crrev.com/8d537d0fcb070f469cc0cf4d3185875964f1f0b6/content/test/gpu/gpu_tests/webgl2_conformance_expectations.py

Project Member

Comment 28 by bugdroid1@chromium.org, Apr 17 2018

Labels: merge-merged-testbranch
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/bac7b5cc6ca384e48ec88b1286960c80283e3288

commit bac7b5cc6ca384e48ec88b1286960c80283e3288
Author: Jamie Madill <jmadill@chromium.org>
Date: Thu Apr 12 17:53:38 2018

Suppress two flaky XFB tests on ANGLE/GL.

The failure expectations are restored for two of the three new
tests. Marking as flaky - if they continue to fail despite the
flaky suppression we will need to mark them as failing.

Bug:  696345 
Bug:  818383 
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: I8ac5db7e2c75f2c50cbea42523767336e5c431ea
Tbr: geofflang@chromium.org
Tbr: jdarpinian@chromium.org
Tbr: kbr@chromium.org
No-try: True
Reviewed-on: https://chromium-review.googlesource.com/1010770
Reviewed-by: Jamie Madill <jmadill@chromium.org>
Reviewed-by: Kenneth Russell <kbr@chromium.org>
Commit-Queue: Jamie Madill <jmadill@chromium.org>
Cr-Commit-Position: refs/heads/master@{#550263}
[modify] https://crrev.com/bac7b5cc6ca384e48ec88b1286960c80283e3288/content/test/gpu/gpu_tests/webgl2_conformance_expectations.py

Project Member

Comment 29 by bugdroid1@chromium.org, Apr 17 2018

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

commit 8d537d0fcb070f469cc0cf4d3185875964f1f0b6
Author: Yuly Novikov <ynovikov@chromium.org>
Date: Fri Apr 13 00:06:22 2018

Mark one WebGL2 test Fail

conformance2/transform_feedback/switching-objects.html
on Linux AMD R7 240, no_angle

BUG= 696345 
TBR=kbr@chromium.org

No-try: True
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: I55e92720c7d129b3ca6a249fefb517cb33318137
Reviewed-on: https://chromium-review.googlesource.com/1010520
Commit-Queue: Yuly Novikov <ynovikov@chromium.org>
Reviewed-by: Kenneth Russell <kbr@chromium.org>
Reviewed-by: Yuly Novikov <ynovikov@chromium.org>
Cr-Commit-Position: refs/heads/master@{#550423}
[modify] https://crrev.com/8d537d0fcb070f469cc0cf4d3185875964f1f0b6/content/test/gpu/gpu_tests/webgl2_conformance_expectations.py

Comment 30 by kbr@chromium.org, Jun 29 2018

Blocking: 853978

Sign in to add a comment