Bug 104708

Summary: [Texmap] Slightly improve readability of the antialias shader
Product: WebKit Reporter: Noam Rosenthal <noam>
Component: New BugsAssignee: Noam Rosenthal <noam>
Status: RESOLVED FIXED    
Severity: Normal CC: mrobinson, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Patch for landing
none
Patch for landing
none
Patch for landing none

Noam Rosenthal
Reported 2012-12-11 13:20:12 PST
[Texmap] Slightly improve readability of the antialias shader
Attachments
Patch (5.24 KB, patch)
2012-12-11 13:23 PST, Noam Rosenthal 		no flags
DetailsFormatted DiffDiff
Patch (5.28 KB, patch)
2012-12-11 15:35 PST, Noam Rosenthal 		no flags
DetailsFormatted DiffDiff
Patch for landing (20.08 KB, patch)
2012-12-11 23:46 PST, Noam Rosenthal 		no flags
DetailsFormatted DiffDiff
Patch for landing (20.07 KB, patch)
2012-12-11 23:49 PST, Noam Rosenthal 		no flags
DetailsFormatted DiffDiff
Patch for landing (15.86 KB, patch)
2012-12-12 00:22 PST, Noam Rosenthal 		no flags
DetailsFormatted DiffDiff
Show Obsolete (4) View All Add attachment proposed patch, testcase, etc.
Noam Rosenthal
Comment 1 2012-12-11 13:23:41 PST
Created attachment 178860 [details] Patch
Martin Robinson
Comment 2 2012-12-11 15:28:40 PST
Comment on attachment 178860 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=178860&action=review I like how you packed some of the text of the comments into variable names. I have a couple questions though. > Source/WebCore/platform/graphics/texmap/TextureMapperShaderManager.cpp:201 > + float antialiasQuad(vec3 coef1, vec3 coef2, vec3 coef3, vec3 coef4) I think I would prefer avoiding abbreviations, as we typically do in WebKit. Isn't it useful to describe where the technique originates from? > Source/WebCore/platform/graphics/texmap/TextureMapperShaderManager.cpp:220 > + , u_expandedQuadEdgesInScreenSpace[5] > + , u_expandedQuadEdgesInScreenSpace[6] > + , u_expandedQuadEdgesInScreenSpace[7]); > + That's a strange use of commas. I think they typically go at the end of the previous line.
Noam Rosenthal
Comment 3 2012-12-11 15:35:20 PST
Created attachment 178895 [details] Patch
Martin Robinson
Comment 4 2012-12-11 15:45:46 PST
Comment on attachment 178895 [details] Patch Nice.
WebKit Review Bot
Comment 5 2012-12-11 17:19:12 PST
Comment on attachment 178895 [details] Patch Clearing flags on attachment: 178895 Committed r137394: <http://trac.webkit.org/changeset/137394>
WebKit Review Bot
Comment 6 2012-12-11 17:19:16 PST
All reviewed patches have been landed. Closing bug.
Noam Rosenthal
Comment 7 2012-12-11 23:46:38 PST
Reopening to attach new patch.
Noam Rosenthal
Comment 8 2012-12-11 23:46:41 PST
Created attachment 178980 [details] Patch for landing
Noam Rosenthal
Comment 9 2012-12-11 23:49:33 PST
Created attachment 178982 [details] Patch for landing
WebKit Review Bot
Comment 10 2012-12-12 00:11:45 PST
Comment on attachment 178982 [details] Patch for landing Clearing flags on attachment: 178982 Committed r137428: <http://trac.webkit.org/changeset/137428>
WebKit Review Bot
Comment 11 2012-12-12 00:11:48 PST
All reviewed patches have been landed. Closing bug.
Noam Rosenthal
Comment 12 2012-12-12 00:22:04 PST
Reopening to attach new patch.
Noam Rosenthal
Comment 13 2012-12-12 00:22:06 PST
Created attachment 178986 [details] Patch for landing
Note You need to log in before you can comment on or make changes to this bug.