90567 – Make GC3D and E3D more maintainable for GLES platforms

RESOLVED FIXED90567

Make GC3D and E3D more maintainable for GLES platforms

https://bugs.webkit.org/show_bug.cgi?id=90567

Summary Make GC3D and E3D more maintainable for GLES platforms

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	Details Formatted Diff Diff
Patch (60.32 KB, patch) 2012-07-04 14:45 PDT, Joshua Netterfield	no flags	Details Formatted Diff Diff
Patch (59.79 KB, patch) 2012-07-04 14:59 PDT, Joshua Netterfield	no flags	Details Formatted Diff Diff
Patch (62.15 KB, patch) 2012-07-05 10:33 PDT, Joshua Netterfield	no flags	Details Formatted Diff Diff
Patch (64.03 KB, patch) 2012-07-05 11:21 PDT, Joshua Netterfield	no flags	Details Formatted Diff Diff
Patch (64.00 KB, patch) 2012-07-05 11:40 PDT, Joshua Netterfield	no flags	Details Formatted Diff Diff
Patch (64.00 KB, patch) 2012-07-05 11:58 PDT, Joshua Netterfield	no flags	Details Formatted Diff Diff
Patch (64.06 KB, patch) 2012-07-05 12:52 PDT, Joshua Netterfield	no flags	Details Formatted Diff Diff
Patch (64.63 KB, patch) 2012-07-05 13:19 PDT, Joshua Netterfield	no flags	Details Formatted Diff Diff
Patch (64.63 KB, patch) 2012-07-05 13:30 PDT, Joshua Netterfield	no flags	Details Formatted Diff Diff
Patch (64.38 KB, patch) 2012-07-05 14:06 PDT, Joshua Netterfield	no flags	Details Formatted Diff Diff
Patch (65.71 KB, patch) 2012-07-09 08:20 PDT, Joshua Netterfield	no flags	Details Formatted Diff Diff
Patch (65.66 KB, patch) 2012-07-09 08:46 PDT, Joshua Netterfield	no flags	Details Formatted Diff Diff
Patch (67.79 KB, patch) 2012-07-10 10:51 PDT, Joshua Netterfield	no flags	Details Formatted Diff Diff
Show Obsolete (13) View All Add attachment proposed patch, testcase, etc.

Joshua Netterfield

Comment 1 2012-07-04 11:23:41 PDT

Created attachment 150826 [details] Patch

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

Comment on attachment 150826 [details] Patch Attachment 150826 [details] did not pass mac-ews (mac): Output: http://queues.webkit.org/results/13136799

Joshua Netterfield

Comment 4 2012-07-04 14:45:07 PDT

Created attachment 150844 [details] Patch

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

Created attachment 150848 [details] Patch

Build Bot

Comment 7 2012-07-04 15:11:10 PDT

Comment on attachment 150848 [details] Patch Attachment 150848 [details] did not pass mac-ews (mac): Output: http://queues.webkit.org/results/13134880

Joshua Netterfield

Comment 8 2012-07-05 10:33:58 PDT

Created attachment 150956 [details] Patch

Build Bot

Comment 9 2012-07-05 10:45:45 PDT

Comment on attachment 150956 [details] Patch Attachment 150956 [details] did not pass mac-ews (mac): Output: http://queues.webkit.org/results/13154067

Joshua Netterfield

Comment 10 2012-07-05 11:21:00 PDT

Created attachment 150965 [details] Patch

Joshua Netterfield

Comment 11 2012-07-05 11:40:03 PDT

Created attachment 150966 [details] Patch

Joshua Netterfield

Comment 12 2012-07-05 11:58:19 PDT

Created attachment 150969 [details] Patch

Build Bot

Comment 13 2012-07-05 12:33:18 PDT

Comment on attachment 150969 [details] Patch Attachment 150969 [details] did not pass mac-ews (mac): Output: http://queues.webkit.org/results/13155041

Joshua Netterfield

Comment 14 2012-07-05 12:52:39 PDT

Created attachment 150974 [details] Patch

Joshua Netterfield

Comment 15 2012-07-05 13:19:50 PDT

Created attachment 150976 [details] Patch

Joshua Netterfield

Comment 16 2012-07-05 13:30:02 PDT

Created attachment 150977 [details] Patch

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

Created attachment 150982 [details] Patch

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

Created attachment 151250 [details] Patch

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

Created attachment 151255 [details] Patch

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

Mac build fix landed in http://trac.webkit.org/changeset/122119.

Filip Pizlo

Comment 27 2012-07-09 13:34:43 PDT

This broke five WebGL tests on Mac: http://build.webkit.org/results/Apple%20Lion%20Debug%20WK1%20(Tests)/r122124%20(759)/results.html

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

Rolled out in http://trac.webkit.org/changeset/122156

Joshua Netterfield

Comment 30 2012-07-10 10:51:15 PDT

Created attachment 151485 [details] Patch

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.

Status RESOLVED

Resolution FIXED

Priority P2

Severity Normal

Classification Unclassified

Version 528+ (Nightly build)

Hardware Unspecified

OS Unspecified

Product WebKit

Component WebGL

Assignee

Nobody

Reported

2012-07-04 09:42 PDT

Modified

2012-07-10 23:29 PDT History

CC List

11 users Show

URL

Keywords

Depends on

90943

Blocks

Dependencies

tree graph