Making use of bindless variants mailbox produce/consume on remainders.

Making use of bindless variants mailbox produce/consume on remainders.

Increase performance of RenderPass drawing in compositor and Video-WebGL copy.

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

[dshwang, 2014-07-04 13:29:24]

could you review?

It's follow-up of https://codereview.chromium.org/310623004

[danakj, 2014-07-07 16:47:07]

https://codereview.chromium.org/371463009/diff/1/cc/output/gl_renderer.cc
File cc/output/gl_renderer.cc (right):

https://codereview.chromium.org/371463009/diff/1/cc/output/gl_renderer.cc#new...
cc/output/gl_renderer.cc:2390: GetFramebufferTexture(texture_id, RGBA_8888,
window_rect);
I liked that this call wasn't duplicated, only code that was actually different
was inside the if/else blocks before. If you maintained that, couldn't this diff
also be a lot smaller?

[dshwang, 2014-07-08 13:08:15]

On 2014/07/07 16:47:07, danakj wrote:
> https://codereview.chromium.org/371463009/diff/1/cc/output/gl_renderer.cc
> File cc/output/gl_renderer.cc (right):
> 
>
https://codereview.chromium.org/371463009/diff/1/cc/output/gl_renderer.cc#new...
> cc/output/gl_renderer.cc:2390: GetFramebufferTexture(texture_id, RGBA_8888,
> window_rect);
> I liked that this call wasn't duplicated, only code that was actually
different
> was inside the if/else blocks before. If you maintained that, couldn't this
diff
> also be a lot smaller?

Thank you for review. I minimized the diff in gl_renderer. Could you review
again?

[danakj, 2014-07-08 15:51:30]

Thanks, cc LGTM

https://codereview.chromium.org/371463009/diff/20001/cc/output/gl_renderer.cc
File cc/output/gl_renderer.cc (right):

https://codereview.chromium.org/371463009/diff/20001/cc/output/gl_renderer.cc...
cc/output/gl_renderer.cc:2405: GLC(gl_, gl_->BindTexture(GL_TEXTURE_2D, 0));
FWIW GetFramebufferTexture already binds/unbinds, so I'm not sure what to do
with this. It's fine here, or it could move below the produce, or it could go
away, idk, just pointing it out.

[dshwang, 2014-07-08 16:36:16]

Thank you for review!

https://codereview.chromium.org/371463009/diff/20001/cc/output/gl_renderer.cc
File cc/output/gl_renderer.cc (right):

https://codereview.chromium.org/371463009/diff/20001/cc/output/gl_renderer.cc...
cc/output/gl_renderer.cc:2405: GLC(gl_, gl_->BindTexture(GL_TEXTURE_2D, 0));
GetFramebufferTexture actually call BindTexture(GL_TEXTURE_2D, 0) at the end of
fuction. So it's not really needed now.
So we can remove this line, but someone might be wonder why we don't unbind the
texture against line 2372.

If we move this line above GetFramebufferTexture, it hits perf adversely,
because GetFramebufferTexture bind the texture again.

gles2_helper skipped redundant texture binding, so I remains this line for
readability.

Do you want to remove this line?

[dshwang, 2014-07-08 16:38:52]

jamesr@, could you review content/renderer?
sievers@, could you review content/common/gpu?

[danakj, 2014-07-08 16:40:25]

https://codereview.chromium.org/371463009/diff/20001/cc/output/gl_renderer.cc
File cc/output/gl_renderer.cc (right):

https://codereview.chromium.org/371463009/diff/20001/cc/output/gl_renderer.cc...
cc/output/gl_renderer.cc:2405: GLC(gl_, gl_->BindTexture(GL_TEXTURE_2D, 0));
On 2014/07/08 16:36:16, dshwang wrote:
> GetFramebufferTexture actually call BindTexture(GL_TEXTURE_2D, 0) at the end
of
> fuction. So it's not really needed now.
> So we can remove this line, but someone might be wonder why we don't unbind
the
> texture against line 2372.
> 
> If we move this line above GetFramebufferTexture, it hits perf adversely,
> because GetFramebufferTexture bind the texture again.
> 
> gles2_helper skipped redundant texture binding, so I remains this line for
> readability.
> 
> Do you want to remove this line?

I like having something here to show it's unbound since we did a bind above, so
let's keep it, i was just thinking aloud.

[danakj, 2014-07-08 16:40:26]

https://codereview.chromium.org/371463009/diff/20001/cc/output/gl_renderer.cc
File cc/output/gl_renderer.cc (right):

https://codereview.chromium.org/371463009/diff/20001/cc/output/gl_renderer.cc...
cc/output/gl_renderer.cc:2405: GLC(gl_, gl_->BindTexture(GL_TEXTURE_2D, 0));
On 2014/07/08 16:36:16, dshwang wrote:
> GetFramebufferTexture actually call BindTexture(GL_TEXTURE_2D, 0) at the end
of
> fuction. So it's not really needed now.
> So we can remove this line, but someone might be wonder why we don't unbind
the
> texture against line 2372.
> 
> If we move this line above GetFramebufferTexture, it hits perf adversely,
> because GetFramebufferTexture bind the texture again.
> 
> gles2_helper skipped redundant texture binding, so I remains this line for
> readability.
> 
> Do you want to remove this line?

I like having something here to show it's unbound since we did a bind above, so
let's keep it, i was just thinking aloud.

[dshwang, 2014-07-10 15:34:15]

Hi, could you have a chance to review?

On 2014/07/08 16:38:52, dshwang wrote:
> jamesr@, could you review content/renderer?
> sievers@, could you review content/common/gpu?

[dshwang, 2014-07-14 13:57:25]

jamesr@, could you review content/renderer?
sievers@, could you review content/common/gpu?

[no sievers, 2014-07-14 18:57:46]

On 2014/07/14 13:57:25, dshwang wrote:
> sievers@, could you review content/common/gpu?

lgtm

[dshwang, 2014-07-15 15:05:17]

On 2014/07/14 18:57:46, sievers wrote:
> On 2014/07/14 13:57:25, dshwang wrote:
> > sievers@, could you review content/common/gpu?
> 
> lgtm

Thank you for reviewing.

jamesr@, could you review content/renderer? It's follow-up cl which you
reviewed; https://codereview.chromium.org/310623004

[jamesr, 2014-07-15 17:52:21]

lgtm

[dshwang, 2014-07-15 18:06:24]

On 2014/07/15 17:52:21, jamesr wrote:
> lgtm

Thank you for review!

[dshwang, 2014-07-15 18:06:34]

The CQ bit was checked by dongseong.hwang@intel.com

[commit-bot: I haz the power, 2014-07-15 18:10:08]

CQ is trying da patch. Follow status at

https://chromium-status.appspot.com/cq/dongseong.hwang@intel.com/371463009/20001

[commit-bot: I haz the power, 2014-07-16 04:25:14]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  android_dbg_triggered_tests on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_dbg_triggered...)
  linux_chromium_chromeos_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_chrome...)
  linux_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_rel/bu...)
  mac_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/mac_chromium_rel/buil...)
  win_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_rel/buil...)
  win_chromium_x64_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_x64_rel/...)

[commit-bot: I haz the power, 2014-07-16 04:58:26]

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

[commit-bot: I haz the power, 2014-07-16 04:58:27]

Try jobs failed on following builders:
  android_dbg_triggered_tests on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_dbg_triggered...)

[dshwang, 2014-07-18 14:43:44]

danakj@, could you review cc/test? TestWebGraphicsContext3D needs new
ProduceTextureDirectCHROMIUM and CreateAndConsumeTextureCHROMIUM methods
implementation to use them in gl_renderer.

[dshwang, 2014-07-22 08:20:02]

The CQ bit was checked by dongseong.hwang@intel.com

[commit-bot: I haz the power, 2014-07-22 08:21:15]

CQ is trying da patch. Follow status at

https://chromium-status.appspot.com/cq/dongseong.hwang@intel.com/371463009/40001

[commit-bot: I haz the power, 2014-07-22 09:04:43]

Change committed as 284652

[danakj, 2014-07-27 13:07:17]

On 2014/07/18 14:43:44, dshwang wrote:
> danakj@, could you review cc/test? TestWebGraphicsContext3D needs new
> ProduceTextureDirectCHROMIUM and CreateAndConsumeTextureCHROMIUM methods
> implementation to use them in gl_renderer.

Lgtm