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

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

Can't Edit
Can't Publish+Mail
Start Review
Created:
2 years, 12 months ago by Zhenyao Mo
Modified:
2 years, 12 months 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.
2 years, 12 months ago #1
kbr1
On 2011/04/21 00:36:08, Zhenyao Mo wrote: > Please review. include/GLSLANG/ShaderLang.h needs to be updated with ...
2 years, 12 months ago #2
Zhenyao Mo
Oops, forget to upload ShaderLang.h. Uploaded. Alok and Nicolas, please review.
2 years, 12 months 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 ...
2 years, 12 months 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 ...
2 years, 12 months ago #5
Zhenyao Mo
Revised, please review again. Changes from previous CL: 1) Cache mapped name in TSymbol so ...
2 years, 12 months 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: ...
2 years, 12 months 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 ...
2 years, 12 months ago #8
Zhenyao Mo
Fixed the issue in the original code that some TIntermSymbols' ids are zero. So now ...
2 years, 12 months 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 ...
2 years, 12 months 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, ...
2 years, 12 months ago #11
kbr1
The changes to src/compiler/glslang_tab.cpp are problematic, because that file is autogenerated by flex/bison. The right ...
2 years, 12 months ago #12
Zhenyao Mo
Please review again.
2 years, 12 months ago #13
kbr1
2 years, 12 months 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