Bug 111333 – [EFL] Ensure right format is used in SnapshotImageGL.

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit EFL

[Kalyan]

In SnapshotImageGL we use GL_BGRA as the format with glReadPixels. This is not the standard format on OpenGLES2.0. With GLES we should use GL_RGBA as the format and convert the result to BGRA.

[Kalyan]

Created attachment 191264 [details]
patch

[Kenneth Rohde Christiansen]

Comment on attachment 191264 [details]
patch

View in context: https://bugs.webkit.org/attachment.cgi?id=191264&action=review

> Source/WebKit2/UIProcess/API/efl/SnapshotImageGL.cpp:60
> +
> +        for (int i = 0; i < totalBytes; i += 4)
> +            std::swap(buf[i], buf[i + 2]);

why this indentation?

[Kalyan]

Created attachment 191328 [details]
patchv2

[Kalyan]

(In reply to comment #2)
> (From update of attachment 191264 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=191264&action=review
> 
> > +            std::swap(buf[i], buf[i + 2]);
> 
> why this indentation?

oops, fixed it now.

[Benjamin Poulain]

Comment on attachment 191328 [details]
patchv2

View in context: https://bugs.webkit.org/attachment.cgi?id=191328&action=review

> Source/WebKit2/UIProcess/API/efl/SnapshotImageGL.cpp:62
> +#if USE(OPENGL_ES_2)
> +    // Convert to BGRA.
> +    int totalBytes = width * height * 4;
> +
> +    for (int i = 0; i < totalBytes; i += 4)
> +        std::swap(buf[i], buf[i + 2]);
> +#endif
> +

It is hard to believe you must do something so inefficient. :(

Here: std::swap(buf[i], buf[i + 2]);
You swap to 32bits pixels, not 2 * 8 bits component.

[Kalyan]

Created attachment 191629 [details]
patchv3

[Kalyan]

(In reply to comment #5)
> (From update of attachment 191328 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=191328&action=review
> It is hard to believe you must do something so inefficient. :(
> 
> Here: std::swap(buf[i], buf[i + 2]);
> You swap to 32bits pixels, not 2 * 8 bits component.

Thanks for highlighting this. Fixed it now.

[WebKit Review Bot]

Comment on attachment 191629 [details]
patchv3

Clearing flags on attachment: 191629

Committed r144887: <http://trac.webkit.org/changeset/144887>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.