Right now TextureMapperGL is hard-coded to use the BGRA extension, which is: 1. not always present; 2. not always needed, if the platform has native support for BGRA. TextureMapperGL should support both cases.
Created attachment 102693 [details] Patch
Created attachment 102695 [details] Patch
Comment on attachment 102695 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=102695&action=review > Source/WebCore/platform/graphics/opengl/TextureMapperGL.cpp:539 > + // FIXME: use shaders for RGBA->BGRA swap if this becomes a performance issue. > + m_buffer->swapRGB(); I think it would be better to invert the color components direclty in a shader. swapRGB() is doomed to be slow.
Comment on attachment 102695 [details] Patch Yep, thinking about it, that should really be done on the shader. That way we avoid adding swapRGB() at all on the texture mapper. You still did not explain why your assign a ARGB buffer as BGRA. My guess is because you are targetting little endian so the component are inverted when assigned in 32 bits compared to passed as byte here??? But in that case you would have to check the endianess in order to check the format. Am I missing something? :)
Created attachment 102716 [details] Patch
(In reply to comment #4) > (From update of attachment 102695 [details]) > Yep, thinking about it, that should really be done on the shader. That way we avoid adding swapRGB() at all on the texture mapper. As you wish. > You still did not explain why your assign a ARGB buffer as BGRA. My guess is because you are targetting little endian so the component are inverted when assigned in 32 bits compared to passed as byte here??? But in that case you would have to check the endianess in order to check the format. Am I missing something? :) I believe endianness is not an issue (see http://www.opengl.org/wiki/Pixel_Transfer). The problem is that both OpenGL and QImage are endian-agnostic, but in a non-compatible way. Qt is natively compatible with what OpenGL refers to as BGRA, which is not always available. What would really "fix" the problem is adding QImage::Format_ABGR32_Premultiplied, but that's far from trivial.
(In reply to comment #3) > (From update of attachment 102695 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=102695&action=review > > > Source/WebCore/platform/graphics/opengl/TextureMapperGL.cpp:539 > > + // FIXME: use shaders for RGBA->BGRA swap if this becomes a performance issue. > > + m_buffer->swapRGB(); > > I think it would be better to invert the color components direclty in a shader. > swapRGB() is doomed to be slow. I thought about this again and I disagree. It's better to do this once in software while uploading, than to do it for every paint, albeit in hardware. That's how QtGL does it, I think it's the right choice. I still believe that the second patch is good; Let's try to resolve this.
Comment on attachment 102695 [details] Patch r=me for correctness now.
Created attachment 108648 [details] Patch
Comment on attachment 108648 [details] Patch Clearing flags on attachment: 108648 Committed r95939: <http://trac.webkit.org/changeset/95939>
All reviewed patches have been landed. Closing bug.