[webkit-reviews] review denied: [Bug 57154] [Qt] Make OpenGL symbol resolver transparent : [Attachment 101209] use OpenGLShims resolver implementation

[bugzilla-daemon at webkit.org]

Noam Rosenthal <noam.rosenthal at nokia.com> has denied Andrew Wason
<rectalogic at rectalogic.com>'s request for review:
Bug 57154: [Qt] Make OpenGL symbol resolver transparent
https://bugs.webkit.org/show_bug.cgi?id=57154

Attachment 101209: use OpenGLShims resolver implementation
https://bugs.webkit.org/attachment.cgi?id=101209&action=review

------- Additional Comments from Noam Rosenthal <noam.rosenthal at nokia.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=101209&action=review


> Source/WebCore/WebCore.pro:3619
> +    CONFIG += opengl-shims

Indentation seems a bit off.

> Source/WebCore/platform/graphics/cairo/OpenGLShims.cpp:20
> +#if ENABLE(WEBGL) || (PLATFORM(QT) && USE(TEXTURE_MAPPER_GL))

This is a bit awkward. Maybe add a define QT_OPENGL_SHIMS inside the
contains(CONFIG...) clause? Otherwise we repeat the build-time logic that
decides whether to use it twice.

> Source/WebCore/platform/graphics/cairo/OpenGLShims.cpp:34
> +#define ASSIGN_FUNCTION_TABLE_ENTRY(FunctionName, success) \

Strange to use uppercase for FunctionName and lowercase for success.

> Source/WebCore/platform/graphics/cairo/OpenGLShims.cpp:46
> +    return
QGLContext::currentContext()->getProcAddress(QLatin1String(procName));

This doesn't buy you anything. getProcAddress(procName) is essentially the same
thing and is somewhat more readable.