[webkit-reviews] review denied: [Bug 39230] [OpenVG] Implement ImageBuffer using (EGL pbuffer) surfaces : [Attachment 56263] Patch

[bugzilla-daemon at webkit.org]

Dirk Schulze <krit at webkit.org> has denied Jakob Petsovits
<jpetsovits at rim.com>'s request for review:
Bug 39230: [OpenVG] Implement ImageBuffer using (EGL pbuffer) surfaces
https://bugs.webkit.org/show_bug.cgi?id=39230

Attachment 56263: Patch
https://bugs.webkit.org/attachment.cgi?id=56263&action=review

------- Additional Comments from Dirk Schulze <krit at webkit.org>
> +#ifdef WTF_PLATFORM_BIG_ENDIAN
> +#define IMAGEBUFFER_A 0
> +#define IMAGEBUFFER_R 1
> +#define IMAGEBUFFER_G 2
> +#define IMAGEBUFFER_B 3
> +#else
> +#define IMAGEBUFFER_A 3
> +#define IMAGEBUFFER_R 2
> +#define IMAGEBUFFER_G 1
> +#define IMAGEBUFFER_B 0
> +#endif
Not sure about our policy on define's. Don't we use static constants here? 

WebCore/platform/graphics/openvg/ImageBufferData.h:44
 +	void transformColorSpace(const Vector<int>& lookUpTable);
OpenVG doesn't need transformColorSpace. It is used to transform the colorSpace
of an ImageBuffer from linearRGB to sRGB (DeviceRGB) and back. OpenVG can do
that itself. We also don't transform an ImageBuffer after it is created (or at
least it's not the intention of this method).

WebCore/platform/graphics/openvg/ImageBufferOpenVG.cpp:70
 +	    m_format = VG_sARGB_8888_PRE; break;
Newlines after ';'.

WebCore/platform/graphics/openvg/ImageBufferOpenVG.cpp:123
 +	    rect.x(), rect.y(), rect.width(), rect.height());
line up with 'data' please. Think I saw this before. Please fix it everywhere.