Avoid EGLImage target texture reuse on PowerVR

Avoid EGLImage target texture reuse on PowerVR

This workarounds a memory corruption bug in PowerVR drivers
that cause memory corruption. The bug happens when
EGLImageTargetTexture2DOES is called on an already
defined texture object.

Workaround is to always create a new texture id each time.
This only works if there are no other code saving the
old texture id, and only AsyncPixelTransferDelegateIdle
needed to be updated to not do this.

BUG=480992
Committed: https://crrev.com/3ad5e7fd1f05b4febb1bcd6837b77ffdb700a6bb
Cr-Commit-Position: refs/heads/master@{#327425}

[boliu, 2015-04-28 22:59:07]

boliu@chromium.org changed reviewers:
+ sievers@chromium.org

[boliu, 2015-04-28 22:59:08]

ptal \o/

[no sievers, 2015-04-28 23:23:18]

lgtm

https://codereview.chromium.org/1105153003/diff/60001/gpu/command_buffer/serv...
File gpu/command_buffer/service/async_pixel_transfer_manager_idle.cc (right):

https://codereview.chromium.org/1105153003/diff/60001/gpu/command_buffer/serv...
gpu/command_buffer/service/async_pixel_transfer_manager_idle.cc:324:
&shared_state_,
Can you put a comment why we are doing this instead of passing the texture_id,
i.e. that it might change (maybe reference the workaround).

https://codereview.chromium.org/1105153003/diff/60001/gpu/command_buffer/serv...
gpu/command_buffer/service/async_pixel_transfer_manager_idle.cc:325:
base::Bind(&gles2::TextureRef::service_id, base::Unretained(ref)),
Can you put a comment similar to the one in
GLES2CmdDecoderImpl::HandleAsyncTexImage2D() why Unretained() is ok here?

[boliu, 2015-04-29 00:02:43]

comments added

[boliu, 2015-04-29 00:02:49]

The CQ bit was checked by boliu@chromium.org

[boliu, 2015-04-29 00:02:49]

The patchset sent to the CQ was uploaded after l-g-t-m from sievers@chromium.org
Link to the patchset: https://codereview.chromium.org/1105153003/#ps80001
(title: "comments")

[commit-bot: I haz the power, 2015-04-29 00:05:37]

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

[commit-bot: I haz the power, 2015-04-29 02:49:40]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2015-04-29 02:50:29]

Patchset 5 (id:??) landed as
https://crrev.com/3ad5e7fd1f05b4febb1bcd6837b77ffdb700a6bb
Cr-Commit-Position: refs/heads/master@{#327425}

[no sievers, 2015-04-29 17:21:30]

https://codereview.chromium.org/1105153003/diff/80001/gpu/command_buffer/serv...
File gpu/command_buffer/service/texture_definition.cc (right):

https://codereview.chromium.org/1105153003/diff/80001/gpu/command_buffer/serv...
gpu/command_buffer/service/texture_definition.cc:415: 
Sorry I missed this though we talked about it - but we have to fix it:
Need to handle the case where old_service_id is currently bound. I think it's
simple, we just bind the new texture if that is the case.

I'm also not fully sure about what if the texture is bound to an FBO.
On the one hand it's the same buffer (egl image), so in that regard it doesn't
really matter. But maybe I'm missing something that can go wrong, because the
old and not the new texture really remains the attachment.

[boliu, 2015-04-29 18:22:36]

https://codereview.chromium.org/1105153003/diff/80001/gpu/command_buffer/serv...
File gpu/command_buffer/service/texture_definition.cc (right):

https://codereview.chromium.org/1105153003/diff/80001/gpu/command_buffer/serv...
gpu/command_buffer/service/texture_definition.cc:415: 
On 2015/04/29 17:21:30, sievers wrote:
> Sorry I missed this though we talked about it - but we have to fix it:
> Need to handle the case where old_service_id is currently bound. I think it's
> simple, we just bind the new texture if that is the case.

SG, I'll make the patch.

> 
> I'm also not fully sure about what if the texture is bound to an FBO.
> On the one hand it's the same buffer (egl image), so in that regard it doesn't
> really matter. But maybe I'm missing something that can go wrong, because the
> old and not the new texture really remains the attachment.

So... do nothing for now?