Bug 71502 - [chromium] Test for GL_EXT_bgra in addition to GL_EXT_texture_format_BGRA8888/GL_EXT_read_format_bgra for accelerated paining
Summary: [chromium] Test for GL_EXT_bgra in addition to GL_EXT_texture_format_BGRA8888...
Status: RESOLVED WONTFIX
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-11-03 13:44 PDT by Antoine Labour
Modified: 2011-11-03 17:36 PDT (History)
4 users (show)

See Also:


Attachments
Patch (2.21 KB, patch)
2011-11-03 13:46 PDT, Antoine Labour
no flags Details | Formatted Diff | Diff
Patch (1.96 KB, patch)
2011-11-03 16:36 PDT, Antoine Labour
kbr: review-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Antoine Labour 2011-11-03 13:44:44 PDT
[chromium] Test for GL_EXT_bgra in addition to GL_EXT_texture_format_BGRA8888/GL_EXT_read_format_bgra for accelerated paining
Comment 1 Antoine Labour 2011-11-03 13:46:22 PDT
Created attachment 113553 [details]
Patch
Comment 2 Antoine Labour 2011-11-03 13:50:12 PDT
GL_EXT_texture_format_BGRA8888/GL_EXT_read_format_bgra are GLES2-only extensions. GL_EXT_bgra is a superset of them, on desktop GL, but generally GL drivers don't explicitly expose GL_EXT_texture_format_BGRA8888/GL_EXT_read_format_bgra.
Comment 3 Nat Duca 2011-11-03 13:58:37 PDT
Comment on attachment 113553 [details]
Patch

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

Unofficial lgtm

> Source/WebCore/platform/graphics/chromium/LayerRendererChromium.cpp:118
> +        if (extensions->supports("GL_EXT_texture_format_BGRA8888"))

is a cleaner way to write this

elseif supports->x && supports->y
  ensureENabled x
  ensureEnabledY
 return true
else
  return false
Comment 4 Antoine Labour 2011-11-03 16:36:14 PDT
Created attachment 113577 [details]
Patch
Comment 5 Antoine Labour 2011-11-03 16:36:41 PDT
Comment on attachment 113553 [details]
Patch

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

>> Source/WebCore/platform/graphics/chromium/LayerRendererChromium.cpp:118
>> +        if (extensions->supports("GL_EXT_texture_format_BGRA8888"))
> 
> is a cleaner way to write this
> 
> elseif supports->x && supports->y
>   ensureENabled x
>   ensureEnabledY
>  return true
> else
>   return false

Done.
Comment 6 Kenneth Russell 2011-11-03 16:59:53 PDT
Comment on attachment 113577 [details]
Patch

GraphicsContext3D and Extensions3D have followed the functionality and naming conventions in OpenGL ES 2.0 and extensions. The most consistent way to handle this would be to have the underlying implementation report support for GL_EXT_texture_format_BGRA8888 when running on desktop GL supporting GL_EXT_bgra. This is how Chromium's command buffer implementation works. See src/gpu/command_buffer/service/feature_info.cc. For this reason I'm marking this r-. If you really want to pursue this direction then minimally you need to update the documentation on Extensions3D.h indicating which extensions it supports.
Comment 7 Antoine Labour 2011-11-03 17:36:48 PDT
That's fair, I'm doing this chrome-side in http://codereview.chromium.org/8460001/ then. We can close this.