Bug 90567

Summary:

Make GC3D and E3D more maintainable for GLES platforms

Product:

WebKit

Reporter:

Joshua Netterfield <jnetterfield>

Component:

WebGL

Assignee:

Nobody <webkit-unassigned>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

dglazkov, efidler, fpizlo, gustavo, kbr, ossy, philn, rakuco, rniwa, webkit.review.bot, xan.lopez

Priority:

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	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.