RESOLVED FIXED 44682
[CHROMIUM] Canvas2D shaders should have their own classes 
https://bugs.webkit.org/show_bug.cgi?id=44682
Summary [CHROMIUM] Canvas2D shaders should have their own classes
Stephen White
Reported Thursday, August 26, 2010 2:18:40 PM UTC
GLES2Canvas currently contains all the shaders for 2D <canvas>. They should be refactored out into their own class hierarchy.
Attachments
Patch (24.29 KB, patch)
2010-08-26 06:52 PDT, Stephen White 		kbr: review+
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Stephen White
Comment 1 Thursday, August 26, 2010 2:52:52 PM UTC
Created attachment 65555 [details] Patch
Kenneth Russell
Comment 2 Thursday, August 26, 2010 7:37:55 PM UTC
Comment on attachment 65555 [details] Patch This refactoring looks good overall but I'm concerned about putting a name as generic as Shader into the WebCore namespace. I will start sending out patches today which introduce a gpu2d sub-namespace into which I'm also planning to put some shader-related classes that will need to be merged with your work. Once that is in place I think it would be best to scope Shader (and the other accelerated 2D canvas classes) within that namespace. I wonder whether you can hold off this commit for a day or two. If not, I'll r+ it and we can refactor things later. Up to you.
Stephen White
Comment 3 Thursday, August 26, 2010 8:14:54 PM UTC
(In reply to comment #2) > (From update of attachment 65555 [details]) > This refactoring looks good overall but I'm concerned about putting a name as generic as Shader into the WebCore namespace. I will start sending out patches today which introduce a gpu2d sub-namespace into which I'm also planning to put some shader-related classes that will need to be merged with your work. Once that is in place I think it would be best to scope Shader (and the other accelerated 2D canvas classes) within that namespace. I wonder whether you can hold off this commit for a day or two. If not, I'll r+ it and we can refactor things later. Up to you. According to the EWS bots, it doesn't seem to conflict with anything yet. I don't mind refactoring after your stuff goes in, as long as this change doesn't slow you down too much.
Kenneth Russell
Comment 4 Thursday, August 26, 2010 9:06:27 PM UTC
Comment on attachment 65555 [details] Patch r+'ing after offline discussion.
Stephen White
Comment 5 Thursday, August 26, 2010 9:33:38 PM UTC
Committed r66130: <http://trac.webkit.org/changeset/66130>
WebKit Review Bot
Comment 6 Thursday, August 26, 2010 10:12:28 PM UTC
http://trac.webkit.org/changeset/66130 might have broken Chromium Mac Release
Note You need to log in before you can comment on or make changes to this bug.