Make sure FBO is correct for both kModeProcess and kModeDraw

Make sure FBO is correct for both kModeProcess and kModeDraw

This is achieved by setting the correct FBO before each kModeDraw and
hardware renderer won't reset it at the end of the draw so that
kModeProcess between kModeDraws also has the correct FBO.

This CL is to solve a bug where kModeProcess could run some GL commands
that binds the wrong FBO, and the following kModeDraw is messed up.

BUG=527113
Committed: https://crrev.com/6e71d4fae8fac13fc46399f367ab77a675027499
Cr-Commit-Position: refs/heads/master@{#348766}

[hush (inactive), 2015-09-14 18:03:04]

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

[hush (inactive), 2015-09-14 18:03:04]

Hi Bo,
PTAL

[boliu, 2015-09-14 18:06:41]

There are two problems. And let's make separate fixes.

One is we are drawing to the wrong FBO, and fix as it currently stands is
totally correct yet, when there are potentially more than one context.

The other one is triggering unnecessary kModeProcess, and fix for that one looks
ok.

Let's fix them in that order, and merge the first fix back to m46.

[hush (inactive), 2015-09-14 20:59:29]

Patchset #3 (id:40001) has been deleted

[hush (inactive), 2015-09-14 21:00:05]

I'm repurposing this CL to solving problem #1.

PTAL ps3

[jbauman, 2015-09-14 21:07:30]

https://codereview.chromium.org/1335033004/diff/60001/android_webview/browser...
File android_webview/browser/shared_renderer_state.cc (right):

https://codereview.chromium.org/1335033004/diff/60001/android_webview/browser...
android_webview/browser/shared_renderer_state.cc:284:
DeferredGpuCommandService::GetInstance()->PerformIdleWork(true);
The hardware_renderer_ won't do gl_surface_->ResetBackingFrameBufferObject(); in
this case. Could that cause problems?

[hush (inactive), 2015-09-14 21:20:18]

https://codereview.chromium.org/1335033004/diff/60001/android_webview/browser...
File android_webview/browser/shared_renderer_state.cc (right):

https://codereview.chromium.org/1335033004/diff/60001/android_webview/browser...
android_webview/browser/shared_renderer_state.cc:284:
DeferredGpuCommandService::GetInstance()->PerformIdleWork(true);
On 2015/09/14 21:07:30, jbauman wrote:
> The hardware_renderer_ won't do gl_surface_->ResetBackingFrameBufferObject();
in
> this case. Could that cause problems?

I think it won't cause problems as long as the correct FBO is already set before
GL commands are run. I can probably remove
gl_surface_->ResetBackingFrameBufferObject(); from hardware_renderer too.

[boliu, 2015-09-14 21:25:36]

https://codereview.chromium.org/1335033004/diff/60001/android_webview/browser...
File android_webview/browser/hardware_renderer.cc (right):

https://codereview.chromium.org/1335033004/diff/60001/android_webview/browser...
android_webview/browser/hardware_renderer.cc:189:
gl_surface_->ResetBackingFrameBufferObject();
Remove this too (and implementation)

https://codereview.chromium.org/1335033004/diff/60001/android_webview/browser...
File android_webview/browser/shared_renderer_state.cc (right):

https://codereview.chromium.org/1335033004/diff/60001/android_webview/browser...
android_webview/browser/shared_renderer_state.cc:266:
state_restore.framebuffer_binding_ext());
Limit this to kModeDraw only. You can probably move up the lazy create from
below

https://codereview.chromium.org/1335033004/diff/60001/android_webview/browser...
android_webview/browser/shared_renderer_state.cc:284:
DeferredGpuCommandService::GetInstance()->PerformIdleWork(true);
On 2015/09/14 21:20:17, hush wrote:
> On 2015/09/14 21:07:30, jbauman wrote:
> > The hardware_renderer_ won't do
gl_surface_->ResetBackingFrameBufferObject();
> in
> > this case. Could that cause problems?
> 
> I think it won't cause problems as long as the correct FBO is already set
before
> GL commands are run. I can probably remove
> gl_surface_->ResetBackingFrameBufferObject(); from hardware_renderer too.

Idea is FBO will always be the correct one at all times (instead of only during
draw like it is now). So whatever context this idle work is running on should
have the right FBO too.

[hush (inactive), 2015-09-14 23:00:28]

PTAL Patch set 5

https://codereview.chromium.org/1335033004/diff/60001/android_webview/browser...
File android_webview/browser/hardware_renderer.cc (right):

https://codereview.chromium.org/1335033004/diff/60001/android_webview/browser...
android_webview/browser/hardware_renderer.cc:189:
gl_surface_->ResetBackingFrameBufferObject();
On 2015/09/14 21:25:35, boliu wrote:
> Remove this too (and implementation)

Done.

https://codereview.chromium.org/1335033004/diff/60001/android_webview/browser...
File android_webview/browser/shared_renderer_state.cc (right):

https://codereview.chromium.org/1335033004/diff/60001/android_webview/browser...
android_webview/browser/shared_renderer_state.cc:266:
state_restore.framebuffer_binding_ext());
On 2015/09/14 21:25:35, boliu wrote:
> Limit this to kModeDraw only. You can probably move up the lazy create from
> below

Done.

[boliu, 2015-09-14 23:04:28]

lgtm

[hush (inactive), 2015-09-14 23:06:44]

The CQ bit was checked by hush@chromium.org

[commit-bot: I haz the power, 2015-09-14 23:08:03]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1335033004/80002
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1335033004/80002

[commit-bot: I haz the power, 2015-09-14 23:36:48]

Committed patchset #5 (id:80002)

[commit-bot: I haz the power, 2015-09-14 23:38:07]

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

[commit-bot: I haz the power, 2015-09-23 12:39:06]

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