Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(421)

Issue 4428058: Implement shader identifier name mapping. (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
3 years ago by Zhenyao Mo
Modified:
3 years ago
CC:
angleproject-review_googlegroups.com
Base URL:
http://angleproject.googlecode.com/svn/trunk/
Visibility:
Public.

Description

Implement shader identifier name mapping.

The name mapping happens when an identifier is longer than 32 characters.  The
name mapping is behind a flag, so it won't happen by default.  Also, functions
to query the mapped names are added.

The purpose of this CL is for the drivers that can't handle long names.  For
example, linux NVIDIA driver can't handle 256 character name, whereas WebGL spec
requires that.

This CL also fixes the issue that some of the TIntermSymbols' ids are 0s.

ANGLEBUG=144
TEST=test manually with shaders with long identifier names.
Committed: http://code.google.com/p/angleproject/source/detail?r=619

Patch Set 1 : '' #

Patch Set 2 : '' #

Total comments: 1

Patch Set 3 : '' #

Patch Set 4 : '' #

Total comments: 11

Patch Set 5 : '' #

Patch Set 6 : '' #

Total comments: 4

Patch Set 7 : '' #

Patch Set 8 : '' #

Unified diffs Side-by-side diffs Delta from patch set Stats (+258 lines, -67 lines) Patch
M include/GLSLANG/ShaderLang.h View 1 2 3 4 5 6 5 chunks +27 lines, -11 lines 0 comments Download
M src/build_angle.gyp View 1 2 3 4 5 6 1 chunk +2 lines, -0 lines 0 comments Download
M src/common/version.h View 1 2 3 4 5 6 1 chunk +1 line, -1 line 0 comments Download
M src/compiler/Compiler.cpp View 1 2 3 4 5 3 chunks +21 lines, -3 lines 0 comments Download
A src/compiler/MapLongVariableNames.h View 1 2 3 4 5 1 chunk +34 lines, -0 lines 0 comments Download
A src/compiler/MapLongVariableNames.cpp View 1 2 3 4 5 1 chunk +61 lines, -0 lines 0 comments Download
M src/compiler/ParseHelper.h View 1 2 3 4 5 1 chunk +1 line, -1 line 0 comments Download
M src/compiler/ParseHelper.cpp View 1 2 3 4 5 1 chunk +3 lines, -2 lines 0 comments Download
M src/compiler/ShHandle.h View 1 2 3 4 5 2 chunks +3 lines, -0 lines 0 comments Download
M src/compiler/ShaderLang.cpp View 1 2 3 4 5 5 chunks +13 lines, -6 lines 0 comments Download
M src/compiler/VariableInfo.h View 1 2 3 4 5 3 chunks +6 lines, -1 line 0 comments Download
M src/compiler/VariableInfo.cpp View 1 2 3 4 5 6 4 chunks +16 lines, -6 lines 0 comments Download
M src/compiler/glslang.y View 1 2 3 4 5 6 7 8 chunks +24 lines, -10 lines 0 comments Download
M src/compiler/glslang_tab.h View 1 2 3 4 5 6 7 1 chunk +1 line, -1 line 0 comments Download
M src/compiler/glslang_tab.cpp View 1 2 3 4 5 6 7 12 chunks +38 lines, -24 lines 0 comments Download
M src/compiler/intermediate.h View 1 2 3 4 5 1 chunk +7 lines, -1 line 0 comments Download

Messages

Total messages: 14
Zhenyao Mo
Please review.
3 years ago #1
kbr1
On 2011/04/21 00:36:08, Zhenyao Mo wrote: > Please review. include/GLSLANG/ShaderLang.h needs to be updated with ...
3 years ago #2
Zhenyao Mo
Oops, forget to upload ShaderLang.h. Uploaded. Alok and Nicolas, please review.
3 years ago #3
Alok Priyadarshi
http://codereview.appspot.com/4428058/diff/6001/src/compiler/MapLongVariableNames.cpp File src/compiler/MapLongVariableNames.cpp (right): http://codereview.appspot.com/4428058/diff/6001/src/compiler/MapLongVariableNames.cpp#newcode116 src/compiler/MapLongVariableNames.cpp:116: if (list[ii].name == originalName.c_str()) { I do not see ...
3 years ago #4
Zhenyao Mo
On 2011/04/21 15:54:25, Alok Priyadarshi wrote: > http://codereview.appspot.com/4428058/diff/6001/src/compiler/MapLongVariableNames.cpp > File src/compiler/MapLongVariableNames.cpp (right): > > http://codereview.appspot.com/4428058/diff/6001/src/compiler/MapLongVariableNames.cpp#newcode116 ...
3 years ago #5
Zhenyao Mo
Revised, please review again. Changes from previous CL: 1) Cache mapped name in TSymbol so ...
3 years ago #6
Alok Priyadarshi
http://codereview.appspot.com/4428058/diff/6003/src/compiler/Compiler.cpp File src/compiler/Compiler.cpp (right): http://codereview.appspot.com/4428058/diff/6003/src/compiler/Compiler.cpp#newcode151 src/compiler/Compiler.cpp:151: // mapLongVariableNames() needs to be called before collectAttribsUniforms(). nit: ...
3 years ago #7
Zhenyao Mo
http://codereview.appspot.com/4428058/diff/6003/src/compiler/Compiler.cpp File src/compiler/Compiler.cpp (right): http://codereview.appspot.com/4428058/diff/6003/src/compiler/Compiler.cpp#newcode151 src/compiler/Compiler.cpp:151: // mapLongVariableNames() needs to be called before collectAttribsUniforms(). On ...
3 years ago #8
Zhenyao Mo
Fixed the issue in the original code that some TIntermSymbols' ids are zero. So now ...
3 years ago #9
Alok Priyadarshi
lgtm http://codereview.appspot.com/4428058/diff/12002/src/compiler/ParseHelper.cpp File src/compiler/ParseHelper.cpp (right): http://codereview.appspot.com/4428058/diff/12002/src/compiler/ParseHelper.cpp#newcode899 src/compiler/ParseHelper.cpp:899: if (voidErrorCheck(line, identifier, type)) do you need to ...
3 years ago #10
Zhenyao Mo
Upload the final CL, for the records. http://codereview.appspot.com/4428058/diff/12002/src/compiler/ParseHelper.cpp File src/compiler/ParseHelper.cpp (right): http://codereview.appspot.com/4428058/diff/12002/src/compiler/ParseHelper.cpp#newcode899 src/compiler/ParseHelper.cpp:899: if (voidErrorCheck(line, ...
3 years ago #11
kbr1
The changes to src/compiler/glslang_tab.cpp are problematic, because that file is autogenerated by flex/bison. The right ...
3 years ago #12
Zhenyao Mo
Please review again.
3 years ago #13
kbr1
3 years ago #14
Fantastic. LGTM
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 1278:e6ce13d99bf5