Summary: | [GTK] Initial build support for WebGL | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Martin Robinson <mrobinson> | ||||||
Component: | WebKitGTK | Assignee: | Nobody <webkit-unassigned> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | Keywords: | Gtk | ||||||
Priority: | P3 | ||||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | PC | ||||||||
OS: | Linux | ||||||||
Bug Depends on: | |||||||||
Bug Blocks: | 31517 | ||||||||
Attachments: |
|
Description
Martin Robinson
2010-12-29 11:24:57 PST
Created attachment 77633 [details]
Initial WebGL build support
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... Created attachment 77697 [details]
Patch which tries to remove duplicate typedefs
(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 on attachment 77697 [details]
Patch which tries to remove duplicate typedefs
LGTM.
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> All reviewed patches have been landed. Closing bug. |