[webkit-reviews] review granted: [Bug 31517] [GTK] WebGL support : [Attachment 87394] Patch adding missing shim table entry

[bugzilla-daemon at webkit.org]

Gustavo Noronha (kov) <gns at gnome.org> has granted Martin Robinson
<mrobinson at webkit.org>'s request for review:
Bug 31517: [GTK] WebGL support
https://bugs.webkit.org/show_bug.cgi?id=31517

Attachment 87394: Patch adding missing shim table entry
https://bugs.webkit.org/attachment.cgi?id=87394&action=review

------- Additional Comments from Gustavo Noronha (kov) <gns at gnome.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=87394&action=review

It looks good to me. Like I've noted before I am no expert in OpenGL, but given
this has been looked over by people who are, and it's essentially glue code
I'll say go ahead after fixing/clarifying my comments =)

> Source/WebCore/platform/graphics/gtk/GraphicsContext3DInternal.cpp:60
> +	   addedAtExitHandler = false;

Looks like you want true here? I guess this raises the pressure on us to find a
cross-platform way of cleaning up on exit heh. Didn't we have something added
for the icon database at some point?

> Source/WebCore/platform/graphics/gtk/GraphicsContext3DInternal.cpp:102
> +    static bool initialized = false;
> +    static bool success = true;
> +    if (!initialized) {
> +	   success = initializeOpenGLShims();
> +	   initialized = true;
> +    }
> +    if (!success)
> +	   return 0;

Just to make sure I understand this - if we fail to initialize the shims we're
essentially out of luck and won't be able to support webgl, and there's no
point in trying to initialize them again, so we cache the failure, that sounds
good yeah.

> Source/WebKit/gtk/webkit/webkitwebsettings.cpp:924
> +    * Since: 1.3.7

Needs to be 1.3.14, or would it be 1.3.15? =)