gl_unittests: on a core profile, use VAOs

gl_unittests: on a core profile, use VAOs

BUG=598902
Committed: https://crrev.com/c2e6816e43bcb083eb62ec0d263a2d7fc4ad65ec
Cr-Commit-Position: refs/heads/master@{#401732}

[Corentin Wallez, 2016-06-23 19:58:08]

cwallez@chromium.org changed reviewers:
+ piman@chromium.org, zmo@chromium.org

[Corentin Wallez, 2016-06-23 19:58:09]

PTAL, this should be the last patch before I try relanding the GLContextGLX
changes.

[Corentin Wallez, 2016-06-23 20:00:04]

https://codereview.chromium.org/2094823002/diff/1/ui/gl/gl_helper.cc
File ui/gl/gl_helper.cc (right):

https://codereview.chromium.org/2094823002/diff/1/ui/gl/gl_helper.cc#newcode100
ui/gl/gl_helper.cc:100: bool GLHelper::ShouldTestsUseVAOs() {
AFAIK, after the GLContextGLX change this will always be true because
gl_unittests run on OSMesa. That plus OSMesa's 3.0 context having VAOs could
lead to a simplification where we just use VAOs and don't bother with this
helper function. What do you think?

[piman, 2016-06-23 20:09:02]

https://codereview.chromium.org/2094823002/diff/1/ui/gl/gl_helper.cc
File ui/gl/gl_helper.cc (right):

https://codereview.chromium.org/2094823002/diff/1/ui/gl/gl_helper.cc#newcode100
ui/gl/gl_helper.cc:100: bool GLHelper::ShouldTestsUseVAOs() {
On 2016/06/23 20:00:04, Corentin Wallez wrote:
> AFAIK, after the GLContextGLX change this will always be true because
> gl_unittests run on OSMesa. That plus OSMesa's 3.0 context having VAOs could
> lead to a simplification where we just use VAOs and don't bother with this
> helper function. What do you think?

I think we still run the tests on GLES 2.0 (e.g. Android, Chrome OS), so let's
leave this for now.

https://codereview.chromium.org/2094823002/diff/1/ui/gl/gl_helper.cc#newcode105
ui/gl/gl_helper.cc:105: return GLVersionInfo(version, renderer,
extensions.c_str())
nit: you should be able to use GLContext::GetCurrent()->GetVersionInfo() instead
of reconstructing it.

[Corentin Wallez, 2016-06-23 20:42:23]

On 2016/06/23 at 20:09:02, piman wrote:
> https://codereview.chromium.org/2094823002/diff/1/ui/gl/gl_helper.cc
> File ui/gl/gl_helper.cc (right):
> 
>
https://codereview.chromium.org/2094823002/diff/1/ui/gl/gl_helper.cc#newcode100
> ui/gl/gl_helper.cc:100: bool GLHelper::ShouldTestsUseVAOs() {
> On 2016/06/23 20:00:04, Corentin Wallez wrote:
> > AFAIK, after the GLContextGLX change this will always be true because
> > gl_unittests run on OSMesa. That plus OSMesa's 3.0 context having VAOs could
> > lead to a simplification where we just use VAOs and don't bother with this
> > helper function. What do you think?
> 
> I think we still run the tests on GLES 2.0 (e.g. Android, Chrome OS), so let's
leave this for now.
> 
Thanks for the clarification.

>
https://codereview.chromium.org/2094823002/diff/1/ui/gl/gl_helper.cc#newcode105
> ui/gl/gl_helper.cc:105: return GLVersionInfo(version, renderer,
extensions.c_str())
> nit: you should be able to use GLContext::GetCurrent()->GetVersionInfo()
instead of reconstructing it.

Done

[Corentin Wallez, 2016-06-23 20:42:32]

The CQ bit was checked by cwallez@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-06-23 20:42:51]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2094823002/20001

[piman, 2016-06-23 21:12:55]

lgtm

[commit-bot: I haz the power, 2016-06-23 21:35:50]

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

[commit-bot: I haz the power, 2016-06-23 21:35:51]

Dry run: This issue passed the CQ dry run.

[Corentin Wallez, 2016-06-23 21:36:30]

The CQ bit was checked by cwallez@chromium.org

[commit-bot: I haz the power, 2016-06-23 21:36:46]

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

[commit-bot: I haz the power, 2016-06-23 21:43:38]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-06-23 21:45:22]

Description was changed from

==========
gl_unittests: on a core profile, use VAOs

BUG=598902
==========

to

==========
gl_unittests: on a core profile, use VAOs

BUG=598902

Committed: https://crrev.com/c2e6816e43bcb083eb62ec0d263a2d7fc4ad65ec
Cr-Commit-Position: refs/heads/master@{#401732}
==========

[commit-bot: I haz the power, 2016-06-23 21:45:24]

Patchset 2 (id:??) landed as
https://crrev.com/c2e6816e43bcb083eb62ec0d263a2d7fc4ad65ec
Cr-Commit-Position: refs/heads/master@{#401732}