Issue 10247043: Use new symbol table initization for vertex shader.

Issue 10247043: Use new symbol table initization for vertex shader. (Closed)

Can't Edit
Can't Publish+Mail
Start Review

Created:
10 years, 10 months ago by Alan Leung Chromium

Modified:
10 years, 10 months ago

Reviewers:
Alok Priyadarshi

CC:
angleproject-review_googlegroups.com

Base URL:
https://angleproject.googlecode.com/svn/trunk

Visibility:
Public.

More Reviews

Description

Use new symbol table initization for vertex shader. BUG= R=alokp@chromium.org Committed: https://code.google.com/p/angleproject/source/detail?r=2426

Patch Set 2 : remove pool allocate changes #

Total comments: 12

Patch Set 3 : address comments #

Created: 10 years, 10 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+1849 lines, -1801 lines)			Patch
M	src/compiler/Compiler.cpp	View	1 2	1 chunk	+14 lines, -1 line	0 comments	Download
M	src/compiler/Initialize.cpp	View		2 chunks	+0 lines, -26 lines	0 comments	Download
M	src/compiler/builtin_symbol_table.h	View	1 2	1 chunk	+2 lines, -1 line	0 comments	Download
M	src/compiler/builtin_symbol_table.cpp	View	1 2	3 chunks	+11 lines, -1 line	0 comments	Download
M	src/compiler/builtin_symbols.json	View	1 2	1 chunk	+1790 lines, -1750 lines	0 comments	Download
M	src/compiler/generate_builtin_symbol_table.py	View	1 2	2 chunks	+32 lines, -22 lines	0 comments	Download

Messages

Total messages: 5

Expand All Messages | Collapse All Messages

Alan Leung Chromium

Moved the vertex shader builtin to use the new initialization system.

10 years, 10 months ago (2013-06-12 23:09:12 UTC) #2

Alok Priyadarshi

lgtm https://codereview.appspot.com/10247043/diff/2001/src/compiler/Compiler.cpp File src/compiler/Compiler.cpp (right): https://codereview.appspot.com/10247043/diff/2001/src/compiler/Compiler.cpp#newcode133 src/compiler/Compiler.cpp:133: please delete this line. https://codereview.appspot.com/10247043/diff/2001/src/compiler/builtin_symbol_table.cpp File src/compiler/builtin_symbol_table.cpp (right): ...

10 years, 10 months ago (2013-06-13 17:20:43 UTC) #3

Alan Leung Chromium

Committed patchset #3 manually as r2426 (presubmit successful).

10 years, 10 months ago (2013-06-13 19:36:40 UTC) #4

Alan Leung Chromium

10 years, 10 months ago (2013-06-13 19:36:55 UTC) #5

Message was sent while issue was closed.

https://codereview.appspot.com/10247043/diff/2001/src/compiler/Compiler.cpp
File src/compiler/Compiler.cpp (right):

https://codereview.appspot.com/10247043/diff/2001/src/compiler/Compiler.cpp#n...
src/compiler/Compiler.cpp:133: 
On 2013/06/13 17:20:43, Alok Priyadarshi wrote:
> please delete this line.

Done.

https://codereview.appspot.com/10247043/diff/2001/src/compiler/builtin_symbol...
File src/compiler/builtin_symbol_table.cpp (right):

https://codereview.appspot.com/10247043/diff/2001/src/compiler/builtin_symbol...
src/compiler/builtin_symbol_table.cpp:283: void
InsertBuiltInFunctionsVertex(const ShBuiltInResources& resources, TSymbolTable *
t) {
On 2013/06/13 17:20:43, Alok Priyadarshi wrote:
> nit: get rid of space between TSymbolTable and '*'.

Done.

https://codereview.appspot.com/10247043/diff/2001/src/compiler/builtin_symbol...
File src/compiler/builtin_symbol_table.h (right):

https://codereview.appspot.com/10247043/diff/2001/src/compiler/builtin_symbol...
src/compiler/builtin_symbol_table.h:14: extern void
InsertBuiltInFunctionsVertex(const ShBuiltInResources& resources, TSymbolTable *
t);
On 2013/06/13 17:20:43, Alok Priyadarshi wrote:
> here too.

Done.

https://codereview.appspot.com/10247043/diff/2001/src/compiler/builtin_symbol...
File src/compiler/builtin_symbols.json (right):

https://codereview.appspot.com/10247043/diff/2001/src/compiler/builtin_symbol...
src/compiler/builtin_symbols.json:1778: {
On 2013/06/13 17:20:43, Alok Priyadarshi wrote:
> nit: indent the array entries:
> "vertex": [
>   {
>     "name": "foo",
>     ...
>   },
>   {
>   },
>   ...
> ]

Done.

https://codereview.appspot.com/10247043/diff/2001/src/compiler/generate_built...
File src/compiler/generate_builtin_symbol_table.py (right):

https://codereview.appspot.com/10247043/diff/2001/src/compiler/generate_built...
src/compiler/generate_builtin_symbol_table.py:67: # A function that parse the
JSON representation of a list of builtin functions.
On 2013/06/13 17:20:43, Alok Priyadarshi wrote:
> parse -> parses

Done.

https://codereview.appspot.com/10247043/diff/2001/src/compiler/generate_built...
src/compiler/generate_builtin_symbol_table.py:69: for func in builtin[name]:
On 2013/06/13 17:20:43, Alok Priyadarshi wrote:
> nit: 4-space indent

Done.

Expand All Messages | Collapse All Messages