Enable TexStorage on Desktop GL lower than 4.2

Enable TexStorage on Desktop GL lower than 4.2

On MacOSX core profile, the highest GL supported is 4.1. However, TexStorage functions are only included in 4.2 or higher.

However, it seems that most (if not all) mac drivers expose ARB_texture_storage extension, so we simply expose the GL binding when the extension exists.

This CL also consolidate similar code in handling TexStorage2D and 3D.

BUG=601513
TEST=gpu_unittests
R=piman@chromium.org,kbr@chromium.org
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel;tryserver.chromium.win:win_optional_gpu_tests_rel

Committed: https://crrev.com/8edf606b24fe8f69becdbf0afe8ae69c16375482
Cr-Commit-Position: refs/heads/master@{#386586}

[Zhenyao Mo, 2016-04-08 00:19:20]

Description was changed from

==========
Emulate TexStorage calls on Desktop GL lower than 4.2

BUG=601513
TEST=gpu_unittests
R=piman@chromium.org,kbr@chromium.org
==========

to

==========
Emulate TexStorage calls on Desktop GL lower than 4.2

BUG=601513
TEST=gpu_unittests
R=piman@chromium.org,kbr@chromium.org
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel;tryserver.chromium.win:win_optional_gpu_tests_rel
==========

[Zhenyao Mo, 2016-04-08 00:19:31]

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

[commit-bot: I haz the power, 2016-04-08 00:19:55]

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

[commit-bot: I haz the power, 2016-04-08 03:37:49]

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

[commit-bot: I haz the power, 2016-04-08 03:37:49]

Dry run: Try jobs failed on following builders:
  mac_optional_gpu_tests_rel on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_optional_gpu_...)

[Zhenyao Mo, 2016-04-11 18:08:50]

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

[commit-bot: I haz the power, 2016-04-11 18:09:14]

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

[Zhenyao Mo, 2016-04-11 18:18:00]

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

[commit-bot: I haz the power, 2016-04-11 18:18:53]

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

[commit-bot: I haz the power, 2016-04-11 20:04:49]

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

[commit-bot: I haz the power, 2016-04-11 20:04:49]

Dry run: This issue passed the CQ dry run.

[Zhenyao Mo, 2016-04-11 21:37:53]

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

[Zhenyao Mo, 2016-04-11 21:38:14]

piman: PTAL

[commit-bot: I haz the power, 2016-04-11 21:38:30]

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

[piman, 2016-04-11 22:59:32]

https://codereview.chromium.org/1872663002/diff/60001/gpu/command_buffer/serv...
File gpu/command_buffer/service/feature_info.cc (right):

https://codereview.chromium.org/1872663002/diff/60001/gpu/command_buffer/serv...
gpu/command_buffer/service/feature_info.cc:1037:
gl_version_info_->IsAtLeastGL(4, 2))) {
I'm a bit confused... Is the goal to expose GL_EXT_texture_storage on the client
even when GL < 4.2? If so, it sounds like you would want to remove this check.
If not, I'm not sure how the client is supposed to know that calling TexStorage*
is allowed.

https://codereview.chromium.org/1872663002/diff/60001/gpu/command_buffer/serv...
File gpu/command_buffer/service/gles2_cmd_decoder.cc (right):

https://codereview.chromium.org/1872663002/diff/60001/gpu/command_buffer/serv...
gpu/command_buffer/service/gles2_cmd_decoder.cc:8834:
!features().ext_texture_storage) {
I'm not sure when that condition would be true. GL_TEXTURE_IMMUTABLE_FORMAT is
only added to the validator when ext_texture_storage is set to true.

[Zhenyao Mo, 2016-04-11 23:17:19]

Please take another look.

https://codereview.chromium.org/1872663002/diff/60001/gpu/command_buffer/serv...
File gpu/command_buffer/service/feature_info.cc (right):

https://codereview.chromium.org/1872663002/diff/60001/gpu/command_buffer/serv...
gpu/command_buffer/service/feature_info.cc:1037:
gl_version_info_->IsAtLeastGL(4, 2))) {
On 2016/04/11 22:59:31, piman wrote:
> I'm a bit confused... Is the goal to expose GL_EXT_texture_storage on the
client
> even when GL < 4.2? If so, it sounds like you would want to remove this check.
> If not, I'm not sure how the client is supposed to know that calling
TexStorage*
> is allowed.

My bad.  I got my two different options mixed.

Option 1: implement the workaround.
Option 2: after I check various Mac devices, it seems ARB_texture_storage is
widely supported, so the workaround may not be necessary at all. We just need to
fix the gl bindings to make it work.

Let's go down option 2 for now, unless situations arise where a Mac driver
doesn't have the extension.

https://codereview.chromium.org/1872663002/diff/60001/gpu/command_buffer/serv...
File gpu/command_buffer/service/gles2_cmd_decoder.cc (right):

https://codereview.chromium.org/1872663002/diff/60001/gpu/command_buffer/serv...
gpu/command_buffer/service/gles2_cmd_decoder.cc:8834:
!features().ext_texture_storage) {
On 2016/04/11 22:59:31, piman wrote:
> I'm not sure when that condition would be true. GL_TEXTURE_IMMUTABLE_FORMAT is
> only added to the validator when ext_texture_storage is set to true.

This will be removed for option 2.

[Zhenyao Mo, 2016-04-11 23:17:27]

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

[commit-bot: I haz the power, 2016-04-11 23:18:12]

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

[Zhenyao Mo, 2016-04-11 23:22:30]

If you agree with going down this path, I'll update the CL subject and
description accordingly.

[piman, 2016-04-11 23:37:19]

On Mon, Apr 11, 2016 at 4:22 PM, <zmo@chromium.org> wrote:

> If you agree with going down this path, I'll update the CL subject and
> description accordingly.
>

SG for going down option 2.



>
> https://codereview.chromium.org/1872663002/
>

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[Zhenyao Mo, 2016-04-11 23:41:48]

Description was changed from

==========
Emulate TexStorage calls on Desktop GL lower than 4.2

BUG=601513
TEST=gpu_unittests
R=piman@chromium.org,kbr@chromium.org
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel;tryserver.chromium.win:win_optional_gpu_tests_rel
==========

to

==========
Enable TexStorage on Desktop GL lower than 4.2

On MacOSX core profile, the highest GL supported is 4.1. However, TexStorage
functions are only included in 4.2 or higher.

However, it seems that most (if not all) mac drivers expose ARB_texture_storage
extension, so we simply expose the GL binding when the extension exists.

This CL also consolidate similar code in handling TexStorage2D and 3D.

BUG=601513
TEST=gpu_unittests
R=piman@chromium.org,kbr@chromium.org
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel;tryserver.chromium.win:win_optional_gpu_tests_rel
==========

[Zhenyao Mo, 2016-04-11 23:42:05]

Updated. PTAL.

[piman, 2016-04-12 01:48:59]

lgtm

[commit-bot: I haz the power, 2016-04-12 02:38:24]

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

[commit-bot: I haz the power, 2016-04-12 02:38:25]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[Zhenyao Mo, 2016-04-12 03:30:43]

The CQ bit was checked by zmo@chromium.org

[commit-bot: I haz the power, 2016-04-12 03:31:07]

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

[commit-bot: I haz the power, 2016-04-12 04:33:23]

Description was changed from

==========
Enable TexStorage on Desktop GL lower than 4.2

On MacOSX core profile, the highest GL supported is 4.1. However, TexStorage
functions are only included in 4.2 or higher.

However, it seems that most (if not all) mac drivers expose ARB_texture_storage
extension, so we simply expose the GL binding when the extension exists.

This CL also consolidate similar code in handling TexStorage2D and 3D.

BUG=601513
TEST=gpu_unittests
R=piman@chromium.org,kbr@chromium.org
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel;tryserver.chromium.win:win_optional_gpu_tests_rel
==========

to

==========
Enable TexStorage on Desktop GL lower than 4.2

On MacOSX core profile, the highest GL supported is 4.1. However, TexStorage
functions are only included in 4.2 or higher.

However, it seems that most (if not all) mac drivers expose ARB_texture_storage
extension, so we simply expose the GL binding when the extension exists.

This CL also consolidate similar code in handling TexStorage2D and 3D.

BUG=601513
TEST=gpu_unittests
R=piman@chromium.org,kbr@chromium.org
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel;tryserver.chromium.win:win_optional_gpu_tests_rel
==========

[commit-bot: I haz the power, 2016-04-12 04:33:24]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2016-04-12 04:35:06]

Description was changed from

==========
Enable TexStorage on Desktop GL lower than 4.2

On MacOSX core profile, the highest GL supported is 4.1. However, TexStorage
functions are only included in 4.2 or higher.

However, it seems that most (if not all) mac drivers expose ARB_texture_storage
extension, so we simply expose the GL binding when the extension exists.

This CL also consolidate similar code in handling TexStorage2D and 3D.

BUG=601513
TEST=gpu_unittests
R=piman@chromium.org,kbr@chromium.org
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel;tryserver.chromium.win:win_optional_gpu_tests_rel
==========

to

==========
Enable TexStorage on Desktop GL lower than 4.2

On MacOSX core profile, the highest GL supported is 4.1. However, TexStorage
functions are only included in 4.2 or higher.

However, it seems that most (if not all) mac drivers expose ARB_texture_storage
extension, so we simply expose the GL binding when the extension exists.

This CL also consolidate similar code in handling TexStorage2D and 3D.

BUG=601513
TEST=gpu_unittests
R=piman@chromium.org,kbr@chromium.org
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel;tryserver.chromium.win:win_optional_gpu_tests_rel

Committed: https://crrev.com/8edf606b24fe8f69becdbf0afe8ae69c16375482
Cr-Commit-Position: refs/heads/master@{#386586}
==========

[commit-bot: I haz the power, 2016-04-12 04:35:07]

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