[Android WebView] More App GL state save/restore

[Android WebView] More App GL state save/restore

Restores state that matches ContextState::RestoreGlobalState
with some exceptions.

Also remove some unneeded states from MODE_DRAW.

BUG=
NOTRY=true

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=213273

[boliu, 2013-07-23 05:24:28]

Haven't done much testing yet, but I think this is super slow :(

[joth, 2013-07-23 05:52:57]

https://codereview.chromium.org/19787007/diff/1/android_webview/browser/scope...
File android_webview/browser/scoped_app_gl_state_restore.cc (right):

https://codereview.chromium.org/19787007/diff/1/android_webview/browser/scope...
android_webview/browser/scoped_app_gl_state_restore.cc:98:
glGetIntegerv(GL_STENCIL_CLEAR_VALUE, &stencil_clear_value_);
you got this one twice.

https://codereview.chromium.org/19787007/diff/1/android_webview/browser/scope...
android_webview/browser/scoped_app_gl_state_restore.cc:122:
glGetIntegerv(GL_STENCIL_BACK_PASS_DEPTH_PASS, &stencil_back_z_pass_op_);
Lets chat this through with aelias. Stencil shouldn't be used by the
implementation: we need to respect the value already set to deal with rotate.

Remove stencil will make this list more manageable :)

[no sievers, 2013-07-23 18:39:53]

Maybe it makes sense to start sharing the code from gpu/ for state save/restore.

Is it possible to add Save*() functions to ContextState and do the necessary
tweaks to use this here instead?

[no sievers, 2013-07-23 18:41:28]

On 2013/07/23 18:39:53, Daniel Sievers wrote:
> Maybe it makes sense to start sharing the code from gpu/ for state
save/restore.
> 
> Is it possible to add Save*() functions to ContextState and do the necessary
> tweaks to use this here instead?

Hmm, I just realized that it does use a lot of refcounted classes from
command_buffer/service :/

[boliu, 2013-07-23 20:41:24]

So my original worry about low framerate was greatly exaggerated. Tested on n7,
n4, n10, gn, all of them finished this save and restore (PS2 with stencil
removed) under 0.1ms (way less on some). The low framerate is due to some other
regression (or from things I had patched in last night), I'll look into that a
bit more.

On 2013/07/23 18:39:53, Daniel Sievers wrote:
> Maybe it makes sense to start sharing the code from gpu/ for state
save/restore.
> 
> Is it possible to add Save*() functions to ContextState and do the necessary
> tweaks to use this here instead?

One step at a time? I could see eventually moving this to say
ContextState::SaveCurrentState(). But that's a bigger change, which probably
should use the python script to generate these things. Also like you said, need
to be able to tweak it to only save/restore certain things.

https://codereview.chromium.org/19787007/diff/1/android_webview/browser/scope...
File android_webview/browser/scoped_app_gl_state_restore.cc (right):

https://codereview.chromium.org/19787007/diff/1/android_webview/browser/scope...
android_webview/browser/scoped_app_gl_state_restore.cc:98:
glGetIntegerv(GL_STENCIL_CLEAR_VALUE, &stencil_clear_value_);
On 2013/07/23 05:52:57, joth wrote:
> you got this one twice. 

Done.

https://codereview.chromium.org/19787007/diff/1/android_webview/browser/scope...
android_webview/browser/scoped_app_gl_state_restore.cc:122:
glGetIntegerv(GL_STENCIL_BACK_PASS_DEPTH_PASS, &stencil_back_z_pass_op_);
On 2013/07/23 05:52:57, joth wrote:
> Lets chat this through with aelias. Stencil shouldn't be used by the
> implementation: we need to respect the value already set to deal with rotate.
> 
> Remove stencil will make this list more manageable :)

Let's just not restore stencil here for now then. No need to get this all
correct on first try as long as we are not regressing.

If we are not restoring stencil, then we should make chromium gl restore not
clobber stencil, which is unrelated to this change.

[joth, 2013-07-23 22:02:54]

lgtm

https://codereview.chromium.org/19787007/diff/10001/android_webview/browser/s...
File android_webview/browser/scoped_app_gl_state_restore.cc (right):

https://codereview.chromium.org/19787007/diff/10001/android_webview/browser/s...
android_webview/browser/scoped_app_gl_state_restore.cc:186:
glDisable(GL_DITHER);
Maybe extract a helper for all these:  

 glEnableOrDisable(GL_DITHER, enable_dither_);

https://codereview.chromium.org/19787007/diff/10001/android_webview/browser/s...
File android_webview/browser/scoped_app_gl_state_restore.h (right):

https://codereview.chromium.org/19787007/diff/10001/android_webview/browser/s...
android_webview/browser/scoped_app_gl_state_restore.h:39: const CallMode mode_;
nit: make it the first member?

[boliu, 2013-07-23 22:16:20]

Both done, will wait for android bots compile :)

[joth, 2013-07-23 22:24:59]

lgtm

[commit-bot: I haz the power, 2013-07-23 22:41:09]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/boliu@chromium.org/19787007/15001

[commit-bot: I haz the power, 2013-07-23 23:40:09]

Change committed as 213273