[Aura] Use GLHelper support to validate format before processing texture

We now have the GLHelperReadBackSupport, this should tell us
if some format is supported or not.IsReadbackConfigSupported
takes care of validating this.

BUG=376769, 415131
Committed: https://crrev.com/07cf029821008ce25812489bd6121f82bec90e92
Cr-Commit-Position: refs/heads/master@{#295656}

[sivag, 2014-09-17 13:35:39]

siva.gunturi@samsung.com changed reviewers:
+ piman@chromium.org

[sivag, 2014-09-17 18:02:31]

siva.gunturi@samsung.com changed reviewers:
+ danakj@chromium.org, sievers@chromium.org

[sivag, 2014-09-17 18:57:00]

Ptal...

[piman, 2014-09-17 19:52:31]

https://codereview.chromium.org/570003002/diff/200001/content/common/gpu/clie...
File content/common/gpu/client/gl_helper_readback_support.cc (right):

https://codereview.chromium.org/570003002/diff/200001/content/common/gpu/clie...
content/common/gpu/client/gl_helper_readback_support.cc:148: case
kAlpha_8_SkColorType:
I think we need more logic to handle this.

In the GLHelper::ReadbackTextureSync or Async path, there's no logic to move the
alpha components into a decimated RGBA texture, like there is in
CropScaleReadbackAndCleanTexture. So if we return GL_RGBA in that case, it will
do the wrong thing.

I don't think we want to encode the decimation logic into this class, which
should be strictly about glReadPixels capabilities. Note that
CropScaleReadbackAndCleanTexture calls this with kRGBA_8888_SkColorType for the
alpha case.


That said, glReadPixels does support reading back alpha only - see
https://www.khronos.org/opengles/sdk/docs/man/xhtml/glReadPixels.xml - so we
could support that case.

[sivag, 2014-09-18 13:19:04]

ptal..
On mac and linux trybots are failing due to this change.
I posted the call stack in the bug.

https://codereview.chromium.org/570003002/diff/200001/content/common/gpu/clie...
File content/common/gpu/client/gl_helper_readback_support.cc (right):

https://codereview.chromium.org/570003002/diff/200001/content/common/gpu/clie...
content/common/gpu/client/gl_helper_readback_support.cc:148: case
kAlpha_8_SkColorType:
On 2014/09/17 19:52:31, piman (Very slow to review) wrote:
> I think we need more logic to handle this.
> 
> In the GLHelper::ReadbackTextureSync or Async path, there's no logic to move
the
> alpha components into a decimated RGBA texture, like there is in
> CropScaleReadbackAndCleanTexture. So if we return GL_RGBA in that case, it
will
> do the wrong thing.
> 
> I don't think we want to encode the decimation logic into this class, which
> should be strictly about glReadPixels capabilities. Note that
> CropScaleReadbackAndCleanTexture calls this with kRGBA_8888_SkColorType for
the
> alpha case.
> 
> 
> That said, glReadPixels does support reading back alpha only - see
> https://www.khronos.org/opengles/sdk/docs/man/xhtml/glReadPixels.xml - so we
> could support that case.

Done.

[sivag, 2014-09-18 19:12:17]

@piman, added TODO comments ptal..

[piman, 2014-09-18 19:53:49]

lgtm

[sivag, 2014-09-18 19:58:38]

The CQ bit was checked by siva.gunturi@samsung.com

[commit-bot: I haz the power, 2014-09-18 19:59:56]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/570003002/240001

[commit-bot: I haz the power, 2014-09-18 22:01:39]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-09-18 22:01:40]

Exceeded time limit waiting for builds to trigger.

[sivag, 2014-09-19 03:23:53]

The CQ bit was checked by siva.gunturi@samsung.com

[commit-bot: I haz the power, 2014-09-19 03:24:58]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/570003002/240001

[commit-bot: I haz the power, 2014-09-19 03:53:28]

Committed patchset #13 (id:240001) as 785575f14e34dd1be4571bec76aa78f90bce9efc

[commit-bot: I haz the power, 2014-09-19 03:54:06]

Patchset 13 (id:??) landed as
https://crrev.com/07cf029821008ce25812489bd6121f82bec90e92
Cr-Commit-Position: refs/heads/master@{#295656}