We used to check if GL_EXT_texture_format_BGRA8888 is listed in the extensions to assume that the driver supports swizzling from GL_BGRA to the GL_RGBA texture format. Unfortunately this does not work reliably with all chipsets.
r110703 removed the swizzling for GLES.
From http://www.khronos.org/opengles/sdk/docs/man/xhtml/glTexSubImage2D.xml it appears that the format passed to glTexSubImage2D is supposed to be the same: "GL_INVALID_OPERATION is generated if the texture array has not been defined by a previous glTexImage2D or glCopyTexImage2D operation whose internalformat matches the format of glTexSubImage2D."
Another interesting point: <k5kangas> You might also want to quote OpenGLES2 spec, v2.0.25, page 72, "Each of the TexSubImage commands <k5kangas> interprets and processes pixel groups in exactly the manner of its TexImage coun- <k5kangas> terpart, except that the assignment of R, G, B, and A pixel group values to the <k5kangas> texture components is controlled by the internalformat of the texture array, not <k5kangas> by an argument to the command.
(In reply to comment #3) > Another interesting point: > > <k5kangas> You might also want to quote OpenGLES2 spec, v2.0.25, page 72, "Each of the TexSubImage commands > <k5kangas> interprets and processes pixel groups in exactly the manner of its TexImage coun- > <k5kangas> terpart, except that the assignment of R, G, B, and A pixel group values to the > <k5kangas> texture components is controlled by the internalformat of the texture array, not > <k5kangas> by an argument to the command. Interesting. I wonder why this is the only thing that had worked for desktop and Harmattan then :)
Created attachment 186167 [details] Patch Proposed patch
Comment on attachment 186167 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=186167&action=review > Source/WebCore/platform/graphics/texmap/TextureMapperGL.cpp:589 > + return context->getExtensions()->supports("GL_EXT_texture_format_BGRA8888"); It is called so many times. I doubt that swizzling support can change during app execution and would cache result here. Something like this: static bool canSwizzle = context->getExtensions()->supports("GL_EXT_texture_format_BGRA8888"); return canSwizzle;
Created attachment 186175 [details] Patch V2 Proposed patch.
Created attachment 186178 [details] Patch v3 Fix left over from other patch.
Comment on attachment 186178 [details] Patch v3 View in context: https://bugs.webkit.org/attachment.cgi?id=186178&action=review I remember this kind of patch had a tendency to break stuff :) Can someone validate it on more than one GLES2 hardware? > Source/WebCore/platform/graphics/texmap/TextureMapperGL.cpp:591 > #if defined(TEXMAP_OPENGL_ES_2) > - // FIXME: Implement reliable detection. See also https://bugs.webkit.org/show_bug.cgi?id=81103. > - return false; > + static bool supportsExternalTextureBGRA = context->getExtensions()->supports("GL_EXT_texture_format_BGRA8888"); > + return supportsExternalTextureBGRA; > #else I think we shoul switch here to isGLES2Compliant() instead of an #ifdef
(In reply to comment #9) > (From update of attachment 186178 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=186178&action=review > > I remember this kind of patch had a tendency to break stuff :) > Can someone validate it on more than one GLES2 hardware? I tested in mesa3d llvmpipe, intel and mali t400. The reverted implementation(changeset r110703) was slightly different. Basically it was just using BGRA in the external format, however in the GLESv2 case, when BGRA is used it also needs to be used in the internal texture format. > > > Source/WebCore/platform/graphics/texmap/TextureMapperGL.cpp:591 > > #if defined(TEXMAP_OPENGL_ES_2) > > - // FIXME: Implement reliable detection. See also https://bugs.webkit.org/show_bug.cgi?id=81103. > > - return false; > > + static bool supportsExternalTextureBGRA = context->getExtensions()->supports("GL_EXT_texture_format_BGRA8888"); > > + return supportsExternalTextureBGRA; > > #else > > I think we shoul switch here to isGLES2Compliant() instead of an #ifdef
(In reply to comment #10) > (In reply to comment #9) > > (From update of attachment 186178 [details] [details]) > > View in context: https://bugs.webkit.org/attachment.cgi?id=186178&action=review > > > > I remember this kind of patch had a tendency to break stuff :) > > Can someone validate it on more than one GLES2 hardware? > > I tested in mesa3d llvmpipe, intel and mali t400. > The reverted implementation(changeset r110703) was slightly different. Basically it was just using BGRA in the external format, however in the GLESv2 case, when BGRA is used it also needs to be used in the internal texture format. Yes, in the N9 case that's the only thing that worked (and N9 was the only GLESv2 device running TextureMapper at the time). But I think this should be ok, if you can address the other comment :)
Comment on attachment 186178 [details] Patch v3 See previous comment.
Created attachment 186427 [details] Patch Patch for landing.
Comment on attachment 186427 [details] Patch Clearing flags on attachment: 186427 Committed r141807: <http://trac.webkit.org/changeset/141807>
All reviewed patches have been landed. Closing bug.