Check gl version before querying GL_MAX_SAMPLES

It requires at minumun GLES 3.0 or GL 3.0 or availabilty of
at least one of these extensions:
- GL_ANGLE_framebuffer_multisample
- GL_APPLE_framebuffer_multisample
- GL_EXT_framebuffer_multisample
- GL_EXT_multisampled_render_to_texture
- GL_NV_framebuffer_multisample

Otherwise it generates GL_INVALID_ENUM.

It does not lead to any problem except that gl error is set
by this time. At least it avoids getting an error message
when Mesa logging is enabled.

BUG=564066
R=bajones@chromium.org, kbr@chromium.org, zmo@chromium.org

TEST= export MESA_DEBUG

Review URL: https://codereview.chromium.org/1481523003

Committed: https://crrev.com/246d1be5b308907e8e7c1cbc73b904f25049576d
Cr-Commit-Position: refs/heads/master@{#363072}

[Zhenyao Mo, 2015-11-25 18:09:25]

https://codereview.chromium.org/1481523003/diff/1/gpu/config/gpu_info_collect...
File gpu/config/gpu_info_collector.cc (right):

https://codereview.chromium.org/1481523003/diff/1/gpu/config/gpu_info_collect...
gpu/config/gpu_info_collector.cc:111: if (gl_info.IsAtLeastGL(3, 0) ||
gl_info.IsAtLeastGLES(3, 0))
It's more complicated.  You also need to consider the extension support, say,
EXT_multisampled_render_to_texture.

[Julien Isorce Samsung, 2015-11-26 11:42:26]

Description was changed from

==========
Check gl version before querying GL_MAX_SAMPLES

It requires at minimum GLES 3.0 or GL 3.0. Otherwise it
generates GL_INVALID_ENUM.

I does not lead to any problem except gl error is set
by this time. At least it avoids getting an error message
when Mesa logging is enabled.

BUG=

R=bajones@chromium.org, kbr@chromium.org, zmo@chromium.org

TEST= set MESA_DEBUG=1
==========

to

==========
It requires at minumun GLES 3.0 or GL 3.0 or availabilty of
GL_EXT_multisampled_render_to_texture.

Otherwise it generates GL_INVALID_ENUM.

It does not lead to any problem except that gl error is set
by this time. At least it avoids getting an error message
when Mesa logging is enabled.

BUG=

R=bajones@chromium.org, kbr@chromium.org, zmo@chromium.org

TEST= set MESA_DEBUG=1

Review URL: https://codereview.chromium.org/1481523003
==========

[Julien Isorce Samsung, 2015-11-26 11:42:57]

On 2015/11/25 18:09:25, Zhenyao Mo wrote:
>
https://codereview.chromium.org/1481523003/diff/1/gpu/config/gpu_info_collect...
> gpu/config/gpu_info_collector.cc:111: if (gl_info.IsAtLeastGL(3, 0) ||
> gl_info.IsAtLeastGLES(3, 0))
> It's more complicated.  You also need to consider the extension support, say,
> EXT_multisampled_render_to_texture.

Thx. Done in "Patch Set 2". I also updated CL description.

[Zhenyao Mo, 2015-11-30 17:57:49]

Sorry not to give complete suggestion in the first place, but if you look at
usr/include/GLES2/gl2ext.h, and search for MAX_SAMPLES, you will find a couple
more extensions that may enable this enum.

[Ken Russell (switch to Gerrit), 2015-12-01 00:26:28]

Please also file a bug about this and reference it from this CL description.
Please describe the motivation for this bug fix there.

[Julien Isorce Samsung, 2015-12-01 16:58:01]

Description was changed from

==========
It requires at minumun GLES 3.0 or GL 3.0 or availabilty of
GL_EXT_multisampled_render_to_texture.

Otherwise it generates GL_INVALID_ENUM.

It does not lead to any problem except that gl error is set
by this time. At least it avoids getting an error message
when Mesa logging is enabled.

BUG=

R=bajones@chromium.org, kbr@chromium.org, zmo@chromium.org

TEST= set MESA_DEBUG=1

Review URL: https://codereview.chromium.org/1481523003
==========

to

==========
It requires at minumun GLES 3.0 or GL 3.0 or availabilty of
at least one of these extensions:
- GL_ANGLE_framebuffer_multisample
- GL_APPLE_framebuffer_multisample
- GL_EXT_framebuffer_multisample
- GL_EXT_multisampled_render_to_texture
- GL_NV_framebuffer_multisample

Otherwise it generates GL_INVALID_ENUM.

It does not lead to any problem except that gl error is set
by this time. At least it avoids getting an error message
when Mesa logging is enabled.

BUG=564066

R=bajones@chromium.org, kbr@chromium.org, zmo@chromium.org

TEST= export MESA_DEBUG

Review URL: https://codereview.chromium.org/1481523003
==========

[Julien Isorce Samsung, 2015-12-01 17:04:16]

On 2015/11/30 17:57:49, Zhenyao Mo wrote:
> Sorry not to give complete suggestion in the first place, but if you look at
> usr/include/GLES2/gl2ext.h, and search for MAX_SAMPLES, you will find a couple
> more extensions that may enable this enum.

No problem I should have checked :) Which I did now by running this command on
latest Mesa:

grep -r --include \*.h -B 9 -e "^#define GL_MAX_SAMPLES.*[[:space:]]0x8D57"
mesa/include/* | grep -e "#define .* 1$"

It outputs:

include/GL/glext.h-#define GL_EXT_framebuffer_multisample 1
include/GLES2/gl2ext.h-#define GL_ANGLE_framebuffer_multisample 1
include/GLES2/gl2ext.h-#define GL_APPLE_framebuffer_multisample 1
include/GLES2/gl2ext.h-#define GL_EXT_multisampled_render_to_texture 1
include/GLES2/gl2ext.h-#define GL_NV_framebuffer_multisample 1

The "9" is a but of heuristic but the idea in this command is to find extension
name associated with occurrence of "#define GL_MAX_SAMPLES_XXX 0x8D57"

On 2015/12/01 00:26:28, Ken Russell wrote:
> Please also file a bug about this and reference it from this CL description.
> Please describe the motivation for this bug fix there.
Done https://code.google.com/p/chromium/issues/detail?id=564066

[Zhenyao Mo, 2015-12-02 00:53:07]

LGTM

https://codereview.chromium.org/1481523003/diff/40001/gpu/config/gpu_info_col...
File gpu/config/gpu_info_collector.cc (right):

https://codereview.chromium.org/1481523003/diff/40001/gpu/config/gpu_info_col...
gpu/config/gpu_info_collector.cc:122: glGetIntegerv(GL_MAX_SAMPLES,
&max_samples);
nit: with multi-line condition, add { } around the body.

[Julien Isorce Samsung, 2015-12-02 12:57:11]

On 2015/12/02 00:53:07, Zhenyao Mo wrote:
>
https://codereview.chromium.org/1481523003/diff/40001/gpu/config/gpu_info_col...
> gpu/config/gpu_info_collector.cc:122: glGetIntegerv(GL_MAX_SAMPLES,
> &max_samples);
> nit: with multi-line condition, add { } around the body.

Ack, I added it in "Patch Set 4". Thx

[Julien Isorce Samsung, 2015-12-02 12:57:30]

The CQ bit was checked by j.isorce@samsung.com

[Julien Isorce Samsung, 2015-12-02 12:57:30]

The patchset sent to the CQ was uploaded after l-g-t-m from zmo@chromium.org
Link to the patchset: https://codereview.chromium.org/1481523003/#ps60001
(title: " ")

[commit-bot: I haz the power, 2015-12-02 12:57:35]

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

[commit-bot: I haz the power, 2015-12-02 13:18:11]

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

[commit-bot: I haz the power, 2015-12-02 13:18:12]

Try jobs failed on following builders:
  cast_shell_linux on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)

[Julien Isorce Samsung, 2015-12-02 14:20:58]

On 2015/12/02 13:18:12, commit-bot: I haz the power wrote:
> Try jobs failed on following builders:
>   cast_shell_linux on tryserver.chromium.linux (JOB_FAILED,
>
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)

failures:
GPUConfig/GPUInfoCollectorTest.CollectGraphicsInfoGL/0
GPUConfig/GPUInfoCollectorTest.CollectGraphicsInfoGL/2

I uploaded new patch set 5, which fix these unit tests. I ran: gpu_unittests
--gtest_filter=*GPUConfig* --single-process-tests
I left extensions not alpha sorted to make the diff easier.

Sorry I should have run these tests first.

[Julien Isorce Samsung, 2015-12-02 14:21:07]

The CQ bit was checked by j.isorce@samsung.com to run a CQ dry run

[commit-bot: I haz the power, 2015-12-02 14:21:37]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1481523003/80001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1481523003/80001

[commit-bot: I haz the power, 2015-12-02 16:06:32]

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

[commit-bot: I haz the power, 2015-12-02 16:06:33]

Dry run: This issue passed the CQ dry run.

[Julien Isorce Samsung, 2015-12-02 21:59:15]

Hi Zhenyao, are you ok with delta(s) between Patch Set 4 and 5 ? (Only 2 of the
4 changes in gpu/config/gpu_info_collector_unittest.cc are really necessary,
those for version < 3.0 since it will hit gl_info.IsAtLeastGL(3, 0) ||
gl_info.IsAtLeastGLES(3, 0) from gpu/config/gpu_info_collector.cc diff) . See
also my previous comment.

[Ken Russell (switch to Gerrit), 2015-12-03 02:33:47]

On 2015/12/02 21:59:15, j.isorce wrote:
> Hi Zhenyao, are you ok with delta(s) between Patch Set 4 and 5 ? (Only 2 of
the
> 4 changes in gpu/config/gpu_info_collector_unittest.cc are really necessary,
> those for version < 3.0 since it will hit gl_info.IsAtLeastGL(3, 0) ||
> gl_info.IsAtLeastGLES(3, 0) from gpu/config/gpu_info_collector.cc diff) . See
> also my previous comment.

I think you should make the minimal changes needed to the test; in other words,
remove the addition of the extension from the mocked data for Linux and Windows.

[Julien Isorce Samsung, 2015-12-03 14:51:36]

The CQ bit was checked by j.isorce@samsung.com

[Julien Isorce Samsung, 2015-12-03 14:51:36]

The patchset sent to the CQ was uploaded after l-g-t-m from zmo@chromium.org
Link to the patchset: https://codereview.chromium.org/1481523003/#ps100001
(title: " ")

[commit-bot: I haz the power, 2015-12-03 14:52:07]

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

[commit-bot: I haz the power, 2015-12-03 17:18:12]

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

[commit-bot: I haz the power, 2015-12-03 17:18:13]

Try jobs failed on following builders:
  linux_android_rel_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_android_r...)

[Julien Isorce Samsung, 2015-12-03 20:30:47]

On 2015/12/03 17:18:13, commit-bot: I haz the power wrote:
> Try jobs failed on following builders:
>   linux_android_rel_ng on tryserver.chromium.linux (JOB_FAILED,
>
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_android_r...)

Failures are:

C  176.510s Main  [==========] 2247 tests ran.
C  176.510s Main  [  PASSED  ] 2236 tests.
C  176.510s Main  [  FAILED  ] 11 tests, listed below:
C  176.510s Main  [  FAILED  ]
AccountReconcilorMaybeEnabled/AccountReconcilorTest.StartReconcileOnlyOnce/1
(UNKNOWN)
C  176.510s Main  [  FAILED  ] CloudPolicyInvalidatorTest.Disconnect (UNKNOWN)
C  176.510s Main  [  FAILED  ] DnsProbeRunnerTest.Probe_FAIL (UNKNOWN)
C  176.510s Main  [  FAILED  ]
IOThreadTestWithIOThreadObject.UpdateServerWhitelist (UNKNOWN)
C  176.510s Main  [  FAILED  ] JsonSchemaCompilerErrorTest.TypeIsRequired
(UNKNOWN)
C  176.510s Main  [  FAILED  ] ManifestIconSelectorTest.NoIcons (UNKNOWN)
C  176.510s Main  [  FAILED  ]
PermissionRequestCreatorApiaryTest.AccessTokenError (UNKNOWN)
C  176.510s Main  [  FAILED  ] ProfileAttributesStorageTest.ProfileNotFound
(UNKNOWN)
C  176.510s Main  [  FAILED  ] ProfileSyncServiceStartupTest.StartDownloadFailed
(UNKNOWN)
C  176.510s Main  [  FAILED  ] ResourcePrefetchPredictorTest.GetCorrectPLT
(UNKNOWN)
C  176.510s Main  [  FAILED  ]
SavePasswordInfoBarDelegateTest.CancelTestCredentialSourcePasswordManager
(UNKNOWN)
C  176.510s Main  
C  176.510s Main  11 FAILED TESTS

[FATAL:io_thread.cc(621)] Check failed: !globals_. 

Well since this CL was passing the CQ dry run yesterday and since then the
change has been minor (and only in a unit test), I'll give another try on the
commit queue.

[Julien Isorce Samsung, 2015-12-03 20:30:54]

The CQ bit was checked by j.isorce@samsung.com

[commit-bot: I haz the power, 2015-12-03 20:34:03]

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

[commit-bot: I haz the power, 2015-12-03 21:53:44]

Description was changed from

==========
It requires at minumun GLES 3.0 or GL 3.0 or availabilty of
at least one of these extensions:
- GL_ANGLE_framebuffer_multisample
- GL_APPLE_framebuffer_multisample
- GL_EXT_framebuffer_multisample
- GL_EXT_multisampled_render_to_texture
- GL_NV_framebuffer_multisample

Otherwise it generates GL_INVALID_ENUM.

It does not lead to any problem except that gl error is set
by this time. At least it avoids getting an error message
when Mesa logging is enabled.

BUG=564066

R=bajones@chromium.org, kbr@chromium.org, zmo@chromium.org

TEST= export MESA_DEBUG

Review URL: https://codereview.chromium.org/1481523003
==========

to

==========
It requires at minumun GLES 3.0 or GL 3.0 or availabilty of
at least one of these extensions:
- GL_ANGLE_framebuffer_multisample
- GL_APPLE_framebuffer_multisample
- GL_EXT_framebuffer_multisample
- GL_EXT_multisampled_render_to_texture
- GL_NV_framebuffer_multisample

Otherwise it generates GL_INVALID_ENUM.

It does not lead to any problem except that gl error is set
by this time. At least it avoids getting an error message
when Mesa logging is enabled.

BUG=564066

R=bajones@chromium.org, kbr@chromium.org, zmo@chromium.org

TEST= export MESA_DEBUG

Review URL: https://codereview.chromium.org/1481523003
==========

[commit-bot: I haz the power, 2015-12-03 21:53:45]

Committed patchset #6 (id:100001)

[commit-bot: I haz the power, 2015-12-03 21:54:37]

Description was changed from

==========
It requires at minumun GLES 3.0 or GL 3.0 or availabilty of
at least one of these extensions:
- GL_ANGLE_framebuffer_multisample
- GL_APPLE_framebuffer_multisample
- GL_EXT_framebuffer_multisample
- GL_EXT_multisampled_render_to_texture
- GL_NV_framebuffer_multisample

Otherwise it generates GL_INVALID_ENUM.

It does not lead to any problem except that gl error is set
by this time. At least it avoids getting an error message
when Mesa logging is enabled.

BUG=564066

R=bajones@chromium.org, kbr@chromium.org, zmo@chromium.org

TEST= export MESA_DEBUG

Review URL: https://codereview.chromium.org/1481523003
==========

to

==========
It requires at minumun GLES 3.0 or GL 3.0 or availabilty of
at least one of these extensions:
- GL_ANGLE_framebuffer_multisample
- GL_APPLE_framebuffer_multisample
- GL_EXT_framebuffer_multisample
- GL_EXT_multisampled_render_to_texture
- GL_NV_framebuffer_multisample

Otherwise it generates GL_INVALID_ENUM.

It does not lead to any problem except that gl error is set
by this time. At least it avoids getting an error message
when Mesa logging is enabled.

BUG=564066

R=bajones@chromium.org, kbr@chromium.org, zmo@chromium.org

TEST= export MESA_DEBUG

Review URL: https://codereview.chromium.org/1481523003

Committed: https://crrev.com/246d1be5b308907e8e7c1cbc73b904f25049576d
Cr-Commit-Position: refs/heads/master@{#363072}
==========

[commit-bot: I haz the power, 2015-12-03 21:54:38]

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