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

Miguel Gomez
Reported 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.
Attachments
Patch (12.34 KB, patch)
2016-03-03 04:14 PST, Miguel Gomez 		no flags
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Miguel Gomez
Comment 1 2016-03-03 04:14:18 PST
Created attachment 272751 [details] Patch
WebKit Commit Bot
Comment 2 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.
WebKit Commit Bot
Comment 3 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>
WebKit Commit Bot
Comment 4 2016-03-03 09:09:44 PST
All reviewed patches have been landed. Closing bug.
peavo
Comment 5 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 }
Miguel Gomez
Comment 6 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.
Note You need to log in before you can comment on or make changes to this bug.