New issue
Advanced search Search tips

Issue 853978 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jun 2018
Components:
EstimatedDays: ----
NextAction: ----
OS: Windows
Pri: 2
Type: Bug-Regression

Blocked on:
issue 696345



Sign in to add a comment

Allow drawing with buffers bound to TRANSFORM_FEEDBACK_BUFFER generic bind point

Reported by reo...@gmail.com, Jun 18 2018

Issue description

UserAgent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/67.0.3396.87 Safari/537.36

Steps to reproduce the problem:
1. Go to http://webglsamples.org/WebGL2Samples/#transform_feedback_instanced, it's demo with transform feedback and instanced
2. Webgl errors in console

What is the expected behavior?
No webgl errors

What went wrong?
[.Offscreen-For-WebGL-0000018902CF6B70]GL ERROR :GL_INVALID_OPERATION : glDrawArraysInstancedANGLE: attached to enabled attrib 1 : buffer is bound for transform feedback and other use simultaneously

Did this work before? Yes Chrome 66

Does this work in other browsers? Yes

Chrome version: 67.0.3396.87  Channel: stable
OS Version: 10.0
Flash Version: 

It works in Firefox 60.
 
Owner: jdarpinian@chromium.org
Status: Assigned (was: Unconfirmed)
jdarpinian: do you know what this is, or do we need a chrome://gpu report?
Status: WontFix (was: Assigned)
The error message is correct. The sample is also broken in Firefox 62 (Nightly) with a similar error message. The sample will need to be updated to unbind transform feedback buffers before attempting to draw with them.
reporter: would you mind filing an issue against https://github.com/WebGLSamples/WebGL2Samples/issues instead?
Thanks!

Comment 4 by reo...@gmail.com, Jun 20 2018

Could explain little bit more please?
Because I call gl.bindTransformFeedback(gl.TRANSFORM_FEEDBACK, null) before draw from second VAO.
Hmm, I see the problem. It's a little tricky. 

TL;DR You have to also call gl.bindBuffer(gl.TRANSFORM_FEEDBACK_BUFFER, null);

There are two different types of buffer bindings with the name gl.TRANSFORM_FEEDBACK_BUFFER: the generic binding point and the indexed binding points. bindBuffer/getParameter use the generic binding point and bindBufferBase/getIndexedParameter use the indexed binding points. You might expect that the generic binding point is the same as the 0th indexed binding point, but it is not. You can have different buffers bound to the generic binding point and the 0th indexed binding point. Also, the generic binding point is *not* part of the transform feedback object, and switching transform feedback objects does *not* change the generic binding point. (this is a recent spec change to align with driver behavior)

// Set the 0th indexed binding point.
gl.bindBufferBase(gl.TRANSFORM_FEEDBACK_BUFFER, 0, buffer1);
// Set the generic binding point.
gl.bindBuffer(gl.TRANSFORM_FEEDBACK_BUFFER, buffer2);

gl.getIndexedParameter(gl.TRANSFORM_FEEDBACK_BUFFER_BINDING, 0); // returns buffer1
gl.getParameter(gl.TRANSFORM_FEEDBACK_BUFFER_BINDING); // returns buffer2

// Changing the transform feedback binding does not unbind the generic bind point.
gl.bindTransformFeedback(gl.createTransformFeedback());
gl.getIndexedParameter(gl.TRANSFORM_FEEDBACK_BUFFER_BINDING, 0); // returns null
gl.getParameter(gl.TRANSFORM_FEEDBACK_BUFFER_BINDING); // Still returns buffer2!

When a buffer is bound to the generic binding point it is still considered bound for transform feedback and cannot be used for drawing. There's one more trick too, which is that calling bindBufferBase sets *both* the indexed binding point *and* the generic binding point. So whatever buffer you last passed to bindBufferBase is stuck in the generic binding point and it won't be unbound when you switch the transform feedback object. To clear the generic binding point you must call gl.bindBuffer(gl.TRANSFORM_FEEDBACK_BUFFER, null).

Comment 6 Deleted

Comment 7 by reo...@gmail.com, Jun 21 2018

Thank you so much, it works.
Status: Assigned (was: WontFix)
Summary: Allow drawing with buffers bound to TRANSFORM_FEEDBACK_BUFFER generic bind point (was: Webgl 2 transform_feedback_instanced was broken)
This behavior is unfortunate. We discussed it at the WebGL working group meeting and I think we can fix it so that buffers bound to the transform feedback generic binding point can still be used for drawing.
Project Member

Comment 9 by bugdroid1@chromium.org, Jun 27 2018

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

commit f17993fb50f2631fdea37777d833ef03619b1432
Author: James Darpinian <jdarpinian@chromium.org>
Date: Wed Jun 27 06:02:12 2018

WebGL: Treat TF generic binding point specially

The transform feedback generic binding point is not part of the
transform feedback object and is not used for transform feedback. Only
the indexed binding points are used. A buffer that is bound to the
generic binding point should be usable for either transform feedback or
non-transform-feedback purposes.

Bug:  853978 
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: I4da8501cb21168258742c337c8c97d593f9323a1
Reviewed-on: https://chromium-review.googlesource.com/1112846
Reviewed-by: Kenneth Russell <kbr@chromium.org>
Commit-Queue: James Darpinian <jdarpinian@chromium.org>
Cr-Commit-Position: refs/heads/master@{#570672}
[modify] https://crrev.com/f17993fb50f2631fdea37777d833ef03619b1432/content/test/gpu/gpu_tests/webgl2_conformance_expectations.py
[modify] https://crrev.com/f17993fb50f2631fdea37777d833ef03619b1432/gpu/command_buffer/service/buffer_manager.cc
[modify] https://crrev.com/f17993fb50f2631fdea37777d833ef03619b1432/gpu/command_buffer/service/buffer_manager.h
[modify] https://crrev.com/f17993fb50f2631fdea37777d833ef03619b1432/gpu/command_buffer/service/context_state.cc
[modify] https://crrev.com/f17993fb50f2631fdea37777d833ef03619b1432/gpu/command_buffer/service/indexed_buffer_binding_host.cc
[modify] https://crrev.com/f17993fb50f2631fdea37777d833ef03619b1432/gpu/command_buffer/service/vertex_attrib_manager.cc
[modify] https://crrev.com/f17993fb50f2631fdea37777d833ef03619b1432/gpu/command_buffer/service/vertex_attrib_manager.h

Project Member

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

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

commit 09303e448d1786fb7c14347a8ae523c3bd7668db
Author: James Darpinian <jdarpinian@chromium.org>
Date: Wed Jun 27 06:54:40 2018

Treat transform feedback generic binding point specially

The transform feedback generic binding point is not part of the
transform feedback object and is not used for transform feedback. Only
the indexed binding points are used. A buffer that is bound to the
generic binding point should be usable for either transform feedback or
non-transform-feedback purposes.

Bug:  853978 
Change-Id: I5b730212c65524188134ac34645328328664f0a4
Reviewed-on: https://chromium-review.googlesource.com/1112841
Commit-Queue: James Darpinian <jdarpinian@chromium.org>
Reviewed-by: Geoff Lang <geofflang@chromium.org>

[modify] https://crrev.com/09303e448d1786fb7c14347a8ae523c3bd7668db/src/libANGLE/Buffer.cpp
[modify] https://crrev.com/09303e448d1786fb7c14347a8ae523c3bd7668db/src/libANGLE/TransformFeedback.cpp
[modify] https://crrev.com/09303e448d1786fb7c14347a8ae523c3bd7668db/src/libANGLE/State.cpp
[modify] https://crrev.com/09303e448d1786fb7c14347a8ae523c3bd7668db/src/libANGLE/VertexAttribute.cpp
[modify] https://crrev.com/09303e448d1786fb7c14347a8ae523c3bd7668db/src/libANGLE/Buffer.h
[modify] https://crrev.com/09303e448d1786fb7c14347a8ae523c3bd7668db/src/libANGLE/VertexArray.cpp

Project Member

Comment 11 by bugdroid1@chromium.org, Jun 27 2018

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

commit d35028ff07556bd43bfb89670ebc0f7d31f8868b
Author: angle-chromium-autoroll <angle-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Date: Wed Jun 27 10:55:15 2018

Roll src/third_party/angle 82af620e0559..09303e448d17 (1 commits)

https://chromium.googlesource.com/angle/angle.git/+log/82af620e0559..09303e448d17


git log 82af620e0559..09303e448d17 --date=short --no-merges --format='%ad %ae %s'
2018-06-27 jdarpinian@chromium.org Treat transform feedback generic binding point specially


Created with:
  gclient setdep -r src/third_party/angle@09303e448d17

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

BUG= chromium:853978 
TBR=ynovikov@chromium.org

Change-Id: I56f408668abcca4b6ef3c1a407ae912f8013178d
Reviewed-on: https://chromium-review.googlesource.com/1116558
Reviewed-by: angle-chromium-autoroll <angle-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Commit-Queue: angle-chromium-autoroll <angle-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Cr-Commit-Position: refs/heads/master@{#570726}
[modify] https://crrev.com/d35028ff07556bd43bfb89670ebc0f7d31f8868b/DEPS

Project Member

Comment 12 by bugdroid1@chromium.org, Jun 28 2018

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

commit a32faf137bbffbabf44ba29e50417d46c805ee19
Author: James Darpinian <jdarpinian@chromium.org>
Date: Thu Jun 28 23:46:23 2018

Roll WebGL 5e83981..a5c263c

https://chromium.googlesource.com/external/khronosgroup/webgl.git/+log/5e83981..a5c263c

Bug:  853978 ,  857303 
Change-Id: I9cf554d30f5f51bcdca25f7fa6b1027312de0e77
Cq-Include-Trybots: luci.chromium.try:win_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_angle_rel_ng;luci.chromium.try:win_angle_rel_ng
Reviewed-on: https://chromium-review.googlesource.com/1117894
Reviewed-by: Kenneth Russell <kbr@chromium.org>
Commit-Queue: James Darpinian <jdarpinian@chromium.org>
Cr-Commit-Position: refs/heads/master@{#571327}
[modify] https://crrev.com/a32faf137bbffbabf44ba29e50417d46c805ee19/DEPS
[modify] https://crrev.com/a32faf137bbffbabf44ba29e50417d46c805ee19/content/test/gpu/gpu_tests/webgl2_conformance_expectations.py
[modify] https://crrev.com/a32faf137bbffbabf44ba29e50417d46c805ee19/content/test/gpu/gpu_tests/webgl_conformance_revision.txt

Status: Fixed (was: Assigned)
Fixes are submitted for Chrome 69 which should reach stable in September. You can remove the workaround then. Thanks for the report!

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

Blockedon: 696345
Great work James following up so thoroughly on this!

Linking to the bug where the new behavior was introduced.

Sign in to add a comment