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

Issue metadata

Status: Fixed
Owner:
Last visit 15 days ago
Closed: Sep 2012
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 1
Type: Bug

Restricted
  • Only users with EditIssue permission may comment.



Sign in to add a comment
link

Issue 142705: Mini Ninjas shaders fails on M21

Reported by bradchen@chromium.org, Aug 14 2012 Project Member

Issue description

Version: M21
OS: observed on both Windows 7 and MacOS

What steps will reproduce the problem?
1. Enable Native Client in about:flags
2. navigate to testing.coreonline.com
3. play Mini Ninjas

What is the expected output? What do you see instead?
"Skinned characters don't render."

Suspect shader program attached.

Please use labels and text to provide additional information.
 
skin30.txt
13.2 KB View Download

Comment 1 by bradchen@chromium.org, Aug 14 2012

May be related to  crbug.com/142477 .

Comment 2 by nfullagar@chromium.org, Aug 14 2012

More info:
OS: win7
Video Card: ATI HD7700

With Chrome Stable 21.0.1180.79, I do not see the skinned character.
With Chrome Canary 23.0.1235.0, I do see the skinned character.

Comment 3 by alokp@chromium.org, Aug 14 2012

 crbug.com/142477  is linux only. If it is observed on other platforms, then it is not related.

Comment 4 by tingo...@gmail.com, Aug 17 2012

In chrome 21 there is a bug somewhere in the glGetUniformLocation() function. The bug is happening in the code that matches up strings so that when you have similar names it sometimes gives you the wrong location.

Comment 5 by alokp@chromium.org, Aug 17 2012

Cc: zmo@chromium.org
@tingo256: Thanks for the tip. A reduced test-case will help a lot.

+zmo: Could this be related to any name shortening changes in ANGLE?

Comment 6 by tingo...@gmail.com, Aug 20 2012

We have only managed to reproduce it when using uniform arrays:

Example shader:
...
uniform mat4 u_mvpNameCollision[8];  // the smaller array goes earlier
uniform mat4 u_mvpName[16];          // then goes bigger (there is no difference in order they are declared)
...

C++ code:
...
  GLint uniformsNum;
  glGetProgramiv(shader_program_object_, GL_ACTIVE_UNIFORMS, &uniformsNum);

  printf("Uniforms num = %d\n\n", uniformsNum);

  for (int i=0; i<uniformsNum; ++i)
  {
    int Size,NameLength;
    GLenum Type;
    GLchar uniformName[256];
    glGetActiveUniform(shader_program_object_, i, 256, &NameLength, &Size, &Type, uniformName);

    printf("Uniform %d: '%s' id = %d\n", i, uniformName, glGetUniformLocation(shader_program_object_, uniformName));
  }
...

Output:
Uniforms num = 3

Uniform 0: 'u_mvpMatrix' 		id = 0
Uniform 1: 'u_mvpNameCollision[0]' 	id = 2 
Uniform 2: 'u_mvpName[0]'  		id = 2

As you see, "u_mvpName" id is invalid because it already taken by "mvpNameCollision".

In this example is important that "mvpNameCollision" goes earlier than "mvpName".
It seems that the algorithm goes through uniform vectors and searches possible entries using strstr() or memcmp() without checking the lengths of elements at first.

i have also attached an example reproducing the bug
tumbler.7z
839 KB Download

Comment 7 by zmo@chromium.org, Aug 20 2012

These names are not affected by name shortening.  tingo256's guess is very likely: there is a bug in our name comparison code.

Comment 8 by zmo@chromium.org, Aug 20 2012

Cc: kbr@chromium.org
Owner: zmo@chromium.org
Taking this bug

Comment 9 by gman@chromium.org, Aug 20 2012

Here's a test but it doesn't repo on OSX

Put it in sdk/tests/conformance/glsl/misc in the WeBGL conformance tests.
glsl-bug-01.html
2.8 KB View Download

Comment 10 by zmo@chromium.org, Aug 20 2012

I can reproduce with M21 on my win7 (ATI).  I am bisecting to see where this gets fixed.

Comment 11 by zmo@chromium.org, Aug 20 2012

Status: Started
crrev.com/142879 breaks this, and crrev.com/144070 fixes the bug, so it only affects M21.

Comment 12 by zmo@chromium.org, Aug 20 2012

Owner: gman@chromium.org
Per discussion with bradchen and Karen, we will try to get a small fix and try to merge it back to M21 and push out an update.

Gregg will get the fix for M21.

Comment 13 by gman@chromium.org, Aug 20 2012

Cc: karen@chromium.org
Labels: Merge-Requested Mstone-21

Comment 14 by gman@chromium.org, Aug 21 2012

So here's a CL for m21
https://chromiumcodereview.appspot.com/10861035

That CL puts the behavior back to what it was in m20.

Comment 15 by gman@chromium.org, Aug 21 2012

This will sound confusing but... the real bug is somewhere else. I'll fix that for M22.  For M21 the smallest change is just to put it back to what it was doing in M20. That's what the CL above does.

Comment 16 by ddrew@chromium.org, Aug 23 2012

Labels: OS-All

Comment 17 by kareng@google.com, Aug 23 2012

Labels: -Merge-Requested Merge-Approved

Comment 18 by gman@chromium.org, Aug 24 2012

The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=153103

------------------------------------------------------------------------
r153103 | gman@chromium.org | Thu Aug 23 17:09:25 PDT 2012

Changed paths:
 M http://src.chromium.org/viewvc/chrome/branches/1180/src/gpu/command_buffer/service/program_manager.cc?r1=153103&r2=153102&pathrev=153103

Reverts the behavior of looking up a uniform location 

There's a real bug I'll fix in M22. This CL puts
M21 back to the behavior we've had for many releases
which is the smalled change to fix this issue.

Review URL: https://chromiumcodereview.appspot.com/10861035

Comment 19 by mbollu@chromium.org, Aug 27 2012

Tested using url http://beta.coreonline.com/mini-ninjas/demo/ on Win7, Mac 10.7.4, Linux Ubuntu 10.04. 

Skin characters renders on Win7, Mac 10.7.4. Unable to launch Mini Ninajas on Linux. It gets directed back to chrome webstore.

URL testing.coreonline.com doens't work.

Comment 20 by mbollu@chromium.org, Aug 27 2012

Cc: mbollu@chromium.org

Comment 21 by mbollu@chromium.org, Aug 28 2012

Tested with url given by Christian. Fixed on Win7, Mac 10.7.4 but behaves the same on Linux Ubuntu. Cannot launch Mini Ninjas and it's directed to chrome web store.

Comment 22 by bradchen@chromium.org, Aug 28 2012

I believe the behavior on Linux/ChromeOS is as intended by the developer. Last I checked this app had explicitly disabled these systems.

Comment 23 by cstefansen@google.com, Sep 11 2012

Status: Fixed
I believe the CL was merged and the issue has not been seen since. (The app does not work on Linux; redirecting to CWS is intended behavior by the app developer.)

Closing. Please re-open if I missed something.

Comment 24 by bugdroid1@chromium.org, Oct 14 2012

Project Member
Labels: Restrict-AddIssueComment-Commit
This issue has been closed for some time. No one will pay attention to new comments.
If you are seeing this bug or have new data, please click New Issue to start a new bug.

Comment 25 by k...@google.com, Jan 14 2013

Labels: -Merge-Approved Merge-Merged

Comment 26 by bugdroid1@chromium.org, Mar 10 2013

Project Member
Labels: -Area-Internals -Feature-GPU -Feature-NaCl -Mstone-21 Cr-Internals-GPU Cr-Platform-NaCl M-21 Cr-Internals

Comment 27 by bugdroid1@chromium.org, Mar 14 2013

Project Member
Labels: -Restrict-AddIssueComment-Commit Restrict-AddIssueComment-EditIssue

Sign in to add a comment