aw: Workaround qualcomm driver bug

aw: Workaround qualcomm driver bug

This is a small webview-specific workaround that affects all drivers.
Will need a better driver/version specific workaround later.

BUG=434570
Committed: https://crrev.com/18abdf1afb8034be81213b23d9e05d34e92bc8d3
Cr-Commit-Position: refs/heads/master@{#305056}

[boliu, 2014-11-19 01:52:34]

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

[boliu, 2014-11-19 01:52:35]

So how do you feel a hacky but small workaround first so it's easy for merging?

[no sievers, 2014-11-19 02:17:45]

Given that this fixes it, looks more like our bug.

[boliu, 2014-11-19 02:24:07]

On 2014/11/19 02:17:45, sievers wrote:
> Given that this fixes it, looks more like our bug.

Hmm? What makes you say that? Can you look over at the internal bug b/18386625 ?

[no sievers, 2014-11-19 19:20:30]

Did you try restoring to 0 in the destructor? Does that also fix the corruption?
Seems like we are not restoring renderbuffer bindings.
Either way, somebody must be changing the binding to something else from 0
somewhere for this to trigger some sort of problem.

[boliu, 2014-11-19 19:27:48]

On 2014/11/19 19:20:30, sievers wrote:
> Did you try restoring to 0 in the destructor? Does that also fix the
corruption?

Yes I did try it. No it does *not* fix the corruption.

> Seems like we are not restoring renderbuffer bindings.
> Either way, somebody must be changing the binding to something else from 0
> somewhere for this to trigger some sort of problem.

The first thing I tried was saving and restoring the renderbuffer binding from
the app, and that definitely did not help. But yes, we should add
saving/restoring renderbuffer binding in trunk in a follow up.

I also logged what the render buffer *would* have been restored to in these
apps, if done synchronously, and it's 0. Nothing in chrome or framework uses
renderbuffer afaict.

[no sievers, 2014-11-19 19:45:50]

On 2014/11/19 19:27:48, boliu wrote:
> On 2014/11/19 19:20:30, sievers wrote:
> > Did you try restoring to 0 in the destructor? Does that also fix the
> corruption?
> 
> Yes I did try it. No it does *not* fix the corruption.
> 
> > Seems like we are not restoring renderbuffer bindings.
> > Either way, somebody must be changing the binding to something else from 0
> > somewhere for this to trigger some sort of problem.
> 
> The first thing I tried was saving and restoring the renderbuffer binding from
> the app, and that definitely did not help. But yes, we should add
> saving/restoring renderbuffer binding in trunk in a follow up.
> 
> I also logged what the render buffer *would* have been restored to in these
> apps, if done synchronously, and it's 0. Nothing in chrome or framework uses
> renderbuffer afaict.

Can you enable GL tracing so we can see that nobody changes it from 0 (after we
initially set it)?
It's weird since it's not a problem in Chrome. If nobody changed it, it really
shouldn't make a difference.

On the other hand, if somebody changed it then it shouldn't make a difference if
you are not issuing a call that is affected by the current binding.
So if it affects unrelated rendering to an FBO, for example, then this would be
an interesting driver bug that we have to deal with. Because otherwise a webgl
app could corrupt all rendering in the presence of context virtualization.

[no sievers, 2014-11-19 19:47:17]

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

[no sievers, 2014-11-19 19:48:06]

On 2014/11/19 19:45:50, sievers wrote:
> On 2014/11/19 19:27:48, boliu wrote:
> > On 2014/11/19 19:20:30, sievers wrote:
> > > Did you try restoring to 0 in the destructor? Does that also fix the
> > corruption?
> > 
> > Yes I did try it. No it does *not* fix the corruption.
> > 
> > > Seems like we are not restoring renderbuffer bindings.
> > > Either way, somebody must be changing the binding to something else from 0
> > > somewhere for this to trigger some sort of problem.
> > 
> > The first thing I tried was saving and restoring the renderbuffer binding
from
> > the app, and that definitely did not help. But yes, we should add
> > saving/restoring renderbuffer binding in trunk in a follow up.
> > 
> > I also logged what the render buffer *would* have been restored to in these
> > apps, if done synchronously, and it's 0. Nothing in chrome or framework uses
> > renderbuffer afaict.
> 
> Can you enable GL tracing so we can see that nobody changes it from 0 (after
we
> initially set it)?
> It's weird since it's not a problem in Chrome. If nobody changed it, it really
> shouldn't make a difference.
> 
> On the other hand, if somebody changed it then it shouldn't make a difference
if
> you are not issuing a call that is affected by the current binding.
> So if it affects unrelated rendering to an FBO, for example, then this would
be
> an interesting driver bug that we have to deal with. Because otherwise a webgl
> app could corrupt all rendering in the presence of context virtualization.

+vmiura also FYI. See my previous comment. Looks like your change to restore
renderbuffer bindings lazily triggered some weird bug.
But not fully understood yet how the problem works.

[boliu, 2014-11-19 19:50:52]

On 2014/11/19 19:48:06, sievers wrote:
> On 2014/11/19 19:45:50, sievers wrote:
> > On 2014/11/19 19:27:48, boliu wrote:
> > > On 2014/11/19 19:20:30, sievers wrote:
> > > > Did you try restoring to 0 in the destructor? Does that also fix the
> > > corruption?
> > > 
> > > Yes I did try it. No it does *not* fix the corruption.
> > > 
> > > > Seems like we are not restoring renderbuffer bindings.
> > > > Either way, somebody must be changing the binding to something else from
0
> > > > somewhere for this to trigger some sort of problem.
> > > 
> > > The first thing I tried was saving and restoring the renderbuffer binding
> from
> > > the app, and that definitely did not help. But yes, we should add
> > > saving/restoring renderbuffer binding in trunk in a follow up.
> > > 
> > > I also logged what the render buffer *would* have been restored to in
these
> > > apps, if done synchronously, and it's 0. Nothing in chrome or framework
uses
> > > renderbuffer afaict.
> > 
> > Can you enable GL tracing so we can see that nobody changes it from 0 (after
> we
> > initially set it)?
> > It's weird since it's not a problem in Chrome. If nobody changed it, it
really
> > shouldn't make a difference.

I tried android's GL tracing in systrace, and checked chrome was not doing
anything with renderbuffers. Did you want to try something else?

I'm at the chrome dev summit, so can't do anything for 2 days :/

> > 
> > On the other hand, if somebody changed it then it shouldn't make a
difference
> if
> > you are not issuing a call that is affected by the current binding.
> > So if it affects unrelated rendering to an FBO, for example, then this would
> be
> > an interesting driver bug that we have to deal with. Because otherwise a
webgl
> > app could corrupt all rendering in the presence of context virtualization.
> 
> +vmiura also FYI. See my previous comment. Looks like your change to restore
> renderbuffer bindings lazily triggered some weird bug.
> But not fully understood yet how the problem works.

[boliu, 2014-11-20 17:36:45]

Ok, here are the things I double checked:

Print out glGetIntegerv(GL_RENDERBUFFER_BINDING, &renderbuffer_binding); in
ScopedAppGLStateRestoreImpl constructor and destructor. In the app that
reproduces this corruption, binding is always 0.

Adding glBindRenderbufferEXT(GL_RENDERBUFFER, 0); to ScopedAppGLStateRestoreImpl
destructor does *not* fix the corruption.

GLES2DecoderImpl::Initialize already calls DoBindRenderbuffer(GL_RENDERBUFFER,
0); which calls glBindRenderbufferEXT(GL_RENDERBUFFER, 0);. Obviously this does
not help with the corruption issue.

[no sievers, 2014-11-20 18:24:14]

On 2014/11/20 17:36:45, boliu wrote:
> Ok, here are the things I double checked:
> 
> Print out glGetIntegerv(GL_RENDERBUFFER_BINDING, &renderbuffer_binding); in
> ScopedAppGLStateRestoreImpl constructor and destructor. In the app that
> reproduces this corruption, binding is always 0.
> 
> Adding glBindRenderbufferEXT(GL_RENDERBUFFER, 0); to
ScopedAppGLStateRestoreImpl
> destructor does *not* fix the corruption.
> 
> GLES2DecoderImpl::Initialize already calls DoBindRenderbuffer(GL_RENDERBUFFER,
> 0); which calls glBindRenderbufferEXT(GL_RENDERBUFFER, 0);. Obviously this
does
> not help with the corruption issue.

Maybe it's one of these bugs in the driver related to deferring work, and the
redundant Bind call happens to flush or so.

Either way, lgtm, not sure what else we could do, and given that the bug seems
to have disappeared after.

[boliu, 2014-11-20 18:27:33]

The CQ bit was checked by boliu@chromium.org

[commit-bot: I haz the power, 2014-11-20 18:28:20]

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

[commit-bot: I haz the power, 2014-11-20 19:09:17]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2014-11-20 19:10:03]

Patchset 1 (id:??) landed as
https://crrev.com/18abdf1afb8034be81213b23d9e05d34e92bc8d3
Cr-Commit-Position: refs/heads/master@{#305056}