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

Issue 735784 link

Starred by 1 user
Status: Fixed
Owner:
zmo@chromium.org
Closed: Jun 2017
Cc:
█ kbr@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 2
Type: Bug



Sign in to add a comment

gl.detachShader after shader program linking causes failures in gl.getUniformBlockIndex

Reported by thothone...@gmail.com, Jun 22 2017 
UserAgent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/59.0.3071.104 Safari/537.36

Steps to reproduce the problem:
Attached use case is a modified khronos webgl conformance test (WebGL2).

Essentially it does the following:
1. Create a shader program
2. Create and compile both a vertex and fragment shader that uses a uniform block.
3. Attach the shaders to the program, and link the program.
4. Detach the shaders from the program - since they are unused at this point, they should have no effect on the linked program.
5. Try to get the index of the uniform block.

What is the expected behavior?
The index should be returned properly.

What went wrong?
It returns INVALID_INDEX. Inspecting the shader program, it reveals all of the internal identifier (webgl_* ) instead of the names that were part of the shader.

Removing the lines which detaches the shaders (step 4), chrome returns the proper index of the uniform block.

Did this work before? N/A 

Does this work in other browsers? Yes

Chrome version: 59.0.3071.104  Channel: stable
OS Version: OpenSUSE Tumbleweed
Flash Version: 

Spec clarification (though oddly enough i cant find the text in the published version) : https://github.com/KhronosGroup/WebGL/commit/d7be2a37e8157e4d18593efd81590cae9bae75e5

Tested in firefox which works, chrome 59[stable], 60[beta], 61[unstable] fails as specified, all under linux. Haven't tested in other browsers.
uniform-buffers-detach.html
3.3 KB View Download

Comment 1 by kbr@chromium.org, Jun 23 2017

Cc: zmo@chromium.org
Status: Available (was: Unconfirmed)
Reproducible on a Retina MBP with NVIDIA GPU running macOS 10.12.5 too.

Here's a slightly modified version of the test case which dumps the translated shader. Yes, the uniform block name is mangled, along with the other identifiers.

The problem appears to be that detaching the shaders causes this to actually be performed on the service side, and so Program::GetInterfaceBlockInfo has no attached shaders against which to query the hashed name.

Given that program_manager.cc caches information about attributes and uniforms, it should probably also eagerly cache information about the uniform blocks so it isn't necessary to go back to the attached shaders.
uniform-buffers-detach.html
3.5 KB View Download

Comment 2 by zmo@chromium.org, Jun 23 2017

Cc: -zmo@chromium.org kbr@chromium.org
Owner: zmo@chromium.org
Status: Started (was: Available)
Take this
Project Member

Comment 3 by bugdroid1@chromium.org, Jun 27 2017 

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

commit 8e4a133d94d88ad3fcf2705fb45f16ab72b58092
Author: zmo <zmo@chromium.org>
Date: Tue Jun 27 00:03:46 2017

Fix a bug in shader/program relationship.

A program can have two shaders attached in ES2/3. However, a linked program's
shaders may not be the current attached shaders to that program. Consider the
following sequence.

1) attach Shader1 and Shader2 to Program
2) link Program, succeed
3) detach Shader1 and Shader2 from Program
4) attach Shader3 and Shader4 to program

So after step 3, no shaders are attached to program. After step 4, Shader3 and
Shader4 are attached to Program, not Shader1 and Shader2. However, if we want
to use shaders to access information from Program, we still need to use Shader1
and Shader2.

BUG= 735784 
TEST=conformance2/uniforms/query-uniform-blocks-after-shader-detach.html
R=kbr@chromium.org
NOTRY=true
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/2958763003
Cr-Commit-Position: refs/heads/master@{#482477}

[modify] https://crrev.com/8e4a133d94d88ad3fcf2705fb45f16ab72b58092/gpu/command_buffer/service/program_manager.cc
[modify] https://crrev.com/8e4a133d94d88ad3fcf2705fb45f16ab72b58092/gpu/command_buffer/service/program_manager.h

Comment 4 by zmo@chromium.org, Jun 27 2017

Status: Fixed (was: Started)
Project Member

Comment 5 by bugdroid1@chromium.org, Jul 7 2017 

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

commit a58703a885159856b5bce37a4c7798a7c481ae39
Author: Kenneth Russell <kbr@chromium.org>
Date: Fri Jul 07 04:31:43 2017

Roll WebGL 9ffa3b5..5e57726

https://chromium.googlesource.com/external/khronosgroup/webgl.git/+log/9ffa3b5..5e57726

BUG= 731324 ,  735784 ,  736499 ,  739604 ,  angleproject:2095 
TBR=kainino@chromium.org
TEST=bots

CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.win:win_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.android:android_optional_gpu_tests_rel

Change-Id: Ie2ef09eaf2087059d1c3dd190c4098993cd7ec61
Reviewed-on: https://chromium-review.googlesource.com/562857
Commit-Queue: Kenneth Russell <kbr@chromium.org>
Reviewed-by: Kenneth Russell <kbr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#484832}
[modify] https://crrev.com/a58703a885159856b5bce37a4c7798a7c481ae39/DEPS
[modify] https://crrev.com/a58703a885159856b5bce37a4c7798a7c481ae39/content/test/gpu/gpu_tests/webgl2_conformance_expectations.py
[modify] https://crrev.com/a58703a885159856b5bce37a4c7798a7c481ae39/content/test/gpu/gpu_tests/webgl_conformance_expectations.py
[modify] https://crrev.com/a58703a885159856b5bce37a4c7798a7c481ae39/content/test/gpu/gpu_tests/webgl_conformance_revision.txt
►

Sign in to add a comment