Bug 89293 - [TextureMapper] Reduce boilerplate code in the TextureMapperShaderManager
Summary: [TextureMapper] Reduce boilerplate code in the TextureMapperShaderManager
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Platform (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Martin Robinson
URL:
Keywords:
Depends on:
Blocks: 89317
  Show dependency tree / graph
 
Reported: 2012-06-16 17:24 PDT by Martin Robinson
Modified: 2012-06-17 23:36 PDT (History)
3 users (show)

See Also:

Attachments
Patch (25.41 KB, patch)
2012-06-17 20:43 PDT, Martin Robinson 		no flags Details | Formatted Diff | Diff
Patch (25.41 KB, patch)
2012-06-17 21:06 PDT, Martin Robinson 		no flags Details | Formatted Diff | Diff
Patch (25.68 KB, patch)
2012-06-17 22:09 PDT, Martin Robinson 		no flags Details | Formatted Diff | Diff
Show Obsolete (2) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Martin Robinson 2012-06-16 17:24:28 PDT 
The amount of boilerplate code in TextureMapperShaderManager could be reduced quite a bit to make the code more readable.
Comment 1 Martin Robinson 2012-06-17 20:43:25 PDT 
Created attachment 148043 [details]
Patch
Comment 2 Martin Robinson 2012-06-17 21:06:00 PDT 
Created attachment 148044 [details]
Patch
Comment 3 Noam Rosenthal 2012-06-17 21:36:51 PDT 
Comment on attachment 148044 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=148044&action=review

Awesome!
Though let's use >= 0 or an enum { InvalidLocation = -1 } instead of using -1 as a magic number.

> Source/WebCore/platform/graphics/texmap/TextureMapperGL.cpp:397
> +    if (program->opacityLocation() != -1)

How about >= 0

> Source/WebCore/platform/graphics/texmap/TextureMapperGL.cpp:434
> +    if (program->opacityLocation() != -1)

>= 0
Comment 4 Martin Robinson 2012-06-17 22:09:40 PDT 
Created attachment 148052 [details]
Patch
Comment 5 Martin Robinson 2012-06-17 22:10:35 PDT 
Makes sense! I've replaced the test for != -1 with a call to the new static method,  TextureMapperShaderProgram::isValidUniformLocation.
Comment 6 WebKit Review Bot 2012-06-17 23:36:24 PDT 
Comment on attachment 148052 [details]
Patch

Clearing flags on attachment: 148052

Committed r120572: <http://trac.webkit.org/changeset/120572>
Comment 7 WebKit Review Bot 2012-06-17 23:36:29 PDT 
All reviewed patches have been landed.  Closing bug.