[webkit-reviews] review granted: [Bug 90881] [Qt][WK2] Implement GraphicsSurface for Linux/GLX. : [Attachment 151974] patch for review. - rebased.

[bugzilla-daemon at webkit.org]

Noam Rosenthal <noam.rosenthal at nokia.com> has granted Zeno Albisser
<zeno at webkit.org>'s request for review:
Bug 90881: [Qt][WK2] Implement GraphicsSurface for Linux/GLX.
https://bugs.webkit.org/show_bug.cgi?id=90881

Attachment 151974: patch for review. - rebased.
https://bugs.webkit.org/attachment.cgi?id=151974&action=review

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


LGTM, see nitpick.

> Source/WebCore/platform/graphics/surfaces/qt/GraphicsSurfaceGLX.cpp:91
> +    spec[i++] = GLX_LEVEL;			       spec[i++] = 0;
> +    spec[i++] = GLX_DRAWABLE_TYPE;		       spec[i++] =
GLX_PIXMAP_BIT | GLX_WINDOW_BIT;
> +    spec[i++] = GLX_BIND_TO_TEXTURE_TARGETS_EXT;    spec[i++] =
GLX_TEXTURE_2D_BIT_EXT;
> +    spec[i++] = GLX_BIND_TO_TEXTURE_RGB_EXT;        spec[i++] = TRUE;

I'd rather just use a static array
static const int glxSpec[] = {
...
};

> Source/WebCore/platform/graphics/surfaces/qt/GraphicsSurfaceGLX.cpp:163
> +	   QVector<int> attribList;
> +	   attribList.append(GLX_TEXTURE_FORMAT_EXT);
> +	   attribList.append(GLX_TEXTURE_FORMAT_RGB_EXT);
> +	   attribList.append(GLX_TEXTURE_TARGET_EXT);
> +	   attribList.append(GLX_TEXTURE_2D_EXT);
> +	   attribList.append(0);

I'd rather use a static const array.