Bug 90567

Summary: Make GC3D and E3D more maintainable for GLES platforms
Product: WebKit Reporter: Joshua Netterfield <jnetterfield>
Component: WebGLAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: dglazkov, efidler, fpizlo, gustavo, kbr, ossy, philn, rakuco, rniwa, webkit.review.bot, xan.lopez
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 90943    
Bug Blocks:    
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch
none
Patch
none
Patch
none
Patch
none
Patch
none
Patch
none
Patch
none
Patch
none
Patch
none
Patch
none
Patch none

Joshua Netterfield
Reported 2012-07-04 09:42:35 PDT
The current structure of GraphicsContext3DOpenGL* and Extensions3DOpenGL makes improvements to GLES-specific code difficult. Extensions3DOpenGL should be split into Extensions3DOpenGLCommon, Extensions3DOpenGLES, and Extensions3DOpenGL mirroring the structure of GC3D. GraphicsContext3DOpenGLCommon should contain as much code as possible while limiting the amount of "#if"s
Attachments
Patch (57.20 KB, patch)
2012-07-04 11:23 PDT, Joshua Netterfield
no flags
Patch (60.32 KB, patch)
2012-07-04 14:45 PDT, Joshua Netterfield
no flags
Patch (59.79 KB, patch)
2012-07-04 14:59 PDT, Joshua Netterfield
no flags
Patch (62.15 KB, patch)
2012-07-05 10:33 PDT, Joshua Netterfield
no flags
Patch (64.03 KB, patch)
2012-07-05 11:21 PDT, Joshua Netterfield
no flags
Patch (64.00 KB, patch)
2012-07-05 11:40 PDT, Joshua Netterfield
no flags
Patch (64.00 KB, patch)
2012-07-05 11:58 PDT, Joshua Netterfield
no flags
Patch (64.06 KB, patch)
2012-07-05 12:52 PDT, Joshua Netterfield
no flags
Patch (64.63 KB, patch)
2012-07-05 13:19 PDT, Joshua Netterfield
no flags
Patch (64.63 KB, patch)
2012-07-05 13:30 PDT, Joshua Netterfield
no flags
Patch (64.38 KB, patch)
2012-07-05 14:06 PDT, Joshua Netterfield
no flags
Patch (65.71 KB, patch)
2012-07-09 08:20 PDT, Joshua Netterfield
no flags
Patch (65.66 KB, patch)
2012-07-09 08:46 PDT, Joshua Netterfield
no flags
Patch (67.79 KB, patch)
2012-07-10 10:51 PDT, Joshua Netterfield
no flags
Joshua Netterfield
Comment 1 2012-07-04 11:23:41 PDT
WebKit Review Bot
Comment 2 2012-07-04 11:49:07 PDT
Comment on attachment 150826 [details] Patch Attachment 150826 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/13131874
Build Bot
Comment 3 2012-07-04 12:10:48 PDT
Joshua Netterfield
Comment 4 2012-07-04 14:45:07 PDT
WebKit Review Bot
Comment 5 2012-07-04 14:48:58 PDT
Attachment 150844 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1 Source/WebCore/platform/graphics/cairo/GraphicsContext3DCairo.cpp:37: Alphabetical sorting problem. [build/include_order] [4] Total errors found: 1 in 19 files If any of these errors are false positives, please file a bug against check-webkit-style.
Joshua Netterfield
Comment 6 2012-07-04 14:59:26 PDT
Build Bot
Comment 7 2012-07-04 15:11:10 PDT
Joshua Netterfield
Comment 8 2012-07-05 10:33:58 PDT
Build Bot
Comment 9 2012-07-05 10:45:45 PDT
Joshua Netterfield
Comment 10 2012-07-05 11:21:00 PDT
Joshua Netterfield
Comment 11 2012-07-05 11:40:03 PDT
Joshua Netterfield
Comment 12 2012-07-05 11:58:19 PDT
Build Bot
Comment 13 2012-07-05 12:33:18 PDT
Joshua Netterfield
Comment 14 2012-07-05 12:52:39 PDT
Joshua Netterfield
Comment 15 2012-07-05 13:19:50 PDT
Joshua Netterfield
Comment 16 2012-07-05 13:30:02 PDT
Rob Buis
Comment 17 2012-07-05 14:04:41 PDT
Comment on attachment 150977 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=150977&action=review Looks good in general, can be cleaned up a bit more. > Source/WebCore/platform/graphics/opengl/Extensions3DOpenGLCommon.h:2 > + * Copyright (C) 2011 Google Inc. All rights reserved. Should be RIM copyright I think. > Source/WebCore/platform/graphics/opengl/GraphicsContext3DOpenGL.cpp:185 > + No need for empty line. > Source/WebCore/platform/graphics/opengl/GraphicsContext3DOpenGL.cpp:267 > + Ditto. > Source/WebCore/platform/graphics/opengl/GraphicsContext3DOpenGL.cpp:277 > + Ditto.
Joshua Netterfield
Comment 18 2012-07-05 14:06:34 PDT
Kenneth Russell
Comment 19 2012-07-06 11:59:48 PDT
Comment on attachment 150982 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=150982&action=review This patch looks fine to me overall; I'll leave it to Rob or someone who works on one of the other affected ports to r+ it. One comment. > Source/WebCore/platform/graphics/OpenGLESShims.h:2 > + * Copyright (C) 2012 Research In Motion Limited. All rights reserved. This file should use the two-clause BSD license in Source/WebKit/LICENSE.
Joshua Netterfield
Comment 20 2012-07-09 08:20:50 PDT
Rob Buis
Comment 21 2012-07-09 08:42:48 PDT
Comment on attachment 151250 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=151250&action=review Looks good, but needs the ChangeLog fix. > Source/WebCore/ChangeLog:868 > +2012-07-04 Joshua Netterfield <jnetterfield@rim.com> Update date and make sure this is top of the ChangeLog. > Source/WebCore/platform/graphics/opengl/Extensions3DOpenGLES.h:2 > + * Copyright (C) 2011 Google Inc. All rights reserved. Could this use RIM copyright?
Joshua Netterfield
Comment 22 2012-07-09 08:46:27 PDT
Rob Buis
Comment 23 2012-07-09 09:51:18 PDT
Comment on attachment 151255 [details] Patch LGTM.
WebKit Review Bot
Comment 24 2012-07-09 11:00:30 PDT
Comment on attachment 151255 [details] Patch Clearing flags on attachment: 151255 Committed r122116: <http://trac.webkit.org/changeset/122116>
WebKit Review Bot
Comment 25 2012-07-09 11:00:42 PDT
All reviewed patches have been landed. Closing bug.
Ryosuke Niwa
Comment 26 2012-07-09 11:32:25 PDT
Filip Pizlo
Comment 27 2012-07-09 13:34:43 PDT
Filip Pizlo
Comment 28 2012-07-09 13:54:16 PDT
Reopening because it broke tests.
Filip Pizlo
Comment 29 2012-07-09 14:15:36 PDT
Joshua Netterfield
Comment 30 2012-07-10 10:51:15 PDT
Rob Buis
Comment 31 2012-07-10 11:49:25 PDT
Comment on attachment 151485 [details] Patch Looks great, and Joshua verified on OS X SL that the fast/canvas/webgl tests pass with this patch.
WebKit Review Bot
Comment 32 2012-07-10 12:52:54 PDT
Comment on attachment 151485 [details] Patch Clearing flags on attachment: 151485 Committed r122250: <http://trac.webkit.org/changeset/122250>
WebKit Review Bot
Comment 33 2012-07-10 12:53:01 PDT
All reviewed patches have been landed. Closing bug.
Csaba Osztrogonác
Comment 34 2012-07-10 23:29:43 PDT
(In reply to comment #32) > (From update of attachment 151485 [details]) > Clearing flags on attachment: 151485 > > Committed r122250: <http://trac.webkit.org/changeset/122250> It broke USE(3D_GRAPHICS)=1 and ENABLE(WEBGL)=0 builds - https://bugs.webkit.org/show_bug.cgi?id=90943 Could you check and fix it please?
Note You need to log in before you can comment on or make changes to this bug.