Bug 51716 - [GTK] Initial build support for WebGL
Summary: [GTK] Initial build support for WebGL
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKitGTK (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC Linux
: P3 Normal
Assignee: Nobody
URL:
Keywords: Gtk
Depends on:
Blocks: 31517
  Show dependency tree / graph
 
Reported: 2010-12-29 11:24 PST by Martin Robinson
Modified: 2011-01-06 16:24 PST (History)
0 users

See Also:

Attachments
Initial WebGL build support (8.24 KB, patch)
2010-12-29 11:43 PST, Martin Robinson 		no flags Details | Formatted Diff | Diff
Patch which tries to remove duplicate typedefs (9.30 KB, patch)
2010-12-30 13:18 PST, Martin Robinson 		no flags Details | Formatted Diff | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Martin Robinson 2010-12-29 11:24:57 PST 
There are some very simple tasks for bringing WebGL to GTK+. This bug tracks adding the initial build support for WebGL.
Comment 1 Martin Robinson 2010-12-29 11:43:54 PST 
Created attachment 77633 [details]
Initial WebGL build support
Comment 2 Eric Seidel (no email) 2010-12-29 18:36:04 PST 
Comment on attachment 77633 [details]
Initial WebGL build support

View in context: https://bugs.webkit.org/attachment.cgi?id=77633&action=review

In general looks fine, but needs more explanation (in code) as to what is permenant and what is temporary? (if any).  In general looks fine.

> WebCore/platform/graphics/GraphicsContext3D.h:70
> +#undef VERSION
> +#undef None

Huh?

> WebCore/platform/graphics/GraphicsContext3D.h:74
> +typedef unsigned int GLuint;
> +typedef void* PlatformGraphicsContext3D;
> +const PlatformGraphicsContext3D NullPlatformGraphicsContext3D = 0;
> +typedef GLuint Platform3DObject;

Should these be shared between implementations?  Seems they all define he same...
Comment 3 Martin Robinson 2010-12-30 13:18:36 PST 
Created attachment 77697 [details]
Patch which tries to remove duplicate typedefs
Comment 4 Martin Robinson 2010-12-30 13:21:50 PST 
(In reply to comment #2)

Thanks for the review!

> In general looks fine, but needs more explanation (in code) as to what is permenant and what is temporary? (if any).  In general looks fine.

Everything in this patch is permanent, barring unforseen circumstances. :)

> > WebCore/platform/graphics/GraphicsContext3D.h:70
> > +#undef VERSION
> > +#undef None

I've dropped a comment about this in the second version of this patch and removed the #undef None, which is not necessary yet.

> > WebCore/platform/graphics/GraphicsContext3D.h:74
> > +typedef unsigned int GLuint;
> > +typedef void* PlatformGraphicsContext3D;
> > +const PlatformGraphicsContext3D NullPlatformGraphicsContext3D = 0;
> > +typedef GLuint Platform3DObject;
> Should these be shared between implementations?  Seems they all define he same...

I posted a new version of the patch, which consolidates the similar typedefs. This sacrifices a little bit of clarity for less repitition, but it's a decent tradeoff in my opinion. Let me know which version of the patch you prefer. Thanks!
Comment 5 Eric Seidel (no email) 2011-01-06 15:22:03 PST 
Comment on attachment 77697 [details]
Patch which tries to remove duplicate typedefs

LGTM.
Comment 6 Martin Robinson 2011-01-06 16:24:11 PST 
Comment on attachment 77697 [details]
Patch which tries to remove duplicate typedefs

Clearing flags on attachment: 77697

Committed r75209: <http://trac.webkit.org/changeset/75209>
Comment 7 Martin Robinson 2011-01-06 16:24:15 PST 
All reviewed patches have been landed.  Closing bug.