Bug 154965

Summary: [TextureMapper] Use RGBA format for textures attached to framebuffers
Product: WebKit Reporter: Miguel Gomez <magomez>
Component: WebCore Misc.Assignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: cmarcelo, commit-queue, kondapallykalyan, luiz, noam, peavo
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch none

Description Miguel Gomez 2016-03-03 03:50:42 PST
BitmapTextureGL internal textures always use BGRA format when the GL_EXT_texture_format_BGRA8888 extension is available, which is good for uploading data to them. But if those textures are not used to upload data, but to be attached to framebuffers, using BGRA can be a problem in some platforms (for example the RPi, where currently a BGRA texture can't be attached to a framebuffer). So, in general, using RGBA for attachment textures would be more standard and platform-friendly.
Comment 1 Miguel Gomez 2016-03-03 04:14:18 PST
Created attachment 272751 [details]
Patch
Comment 2 WebKit Commit Bot 2016-03-03 04:15:22 PST
Attachment 272751 [details] did not pass style-queue:


ERROR: Source/WebCore/platform/graphics/texmap/BitmapTexturePool.cpp:79:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/WebCore/platform/graphics/texmap/BitmapTexturePool.cpp:91:  More than one command on the same line  [whitespace/newline] [4]
Total errors found: 2 in 8 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 WebKit Commit Bot 2016-03-03 09:09:40 PST
Comment on attachment 272751 [details]
Patch

Clearing flags on attachment: 272751

Committed r197506: <http://trac.webkit.org/changeset/197506>
Comment 4 WebKit Commit Bot 2016-03-03 09:09:44 PST
All reviewed patches have been landed.  Closing bug.
Comment 5 peavo 2016-03-07 00:18:35 PST
There might be a small typo in line 95 in BitmapTexturePool.cpp.
Should we be getting the size of m_attachmentTextures instead of m_textures?

93	        for (size_t i = 0; i < m_attachmentTextures.size(); ++i) {
94	            if (m_attachmentTextures[i].m_lastUsedTime < minUsedTime) {
95	                m_attachmentTextures.remove(i, m_textures.size() - i);
96	                break;
97	            }
Comment 6 Miguel Gomez 2016-03-07 01:21:12 PST
> There might be a small typo in line 95 in BitmapTexturePool.cpp.
> Should we be getting the size of m_attachmentTextures instead of m_textures?
> 
> 93	        for (size_t i = 0; i < m_attachmentTextures.size(); ++i) {
> 94	            if (m_attachmentTextures[i].m_lastUsedTime < minUsedTime) {
> 95	                m_attachmentTextures.remove(i, m_textures.size() - i);
> 96	                break;
> 97	            }

You're totally right! I've created a follow up in https://bugs.webkit.org/show_bug.cgi?id=155105 and I'll send a fix in a while.