Check for some extensions before calling potentially NULL GL entry points.

Check for some extensions before calling potentially NULL GL entry points.

Some extensions are exposed by the command buffer unconditionally, add
some tracking of which extensions are natively supported and call the
correct entry point.

BUG=602737
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel

Review-Url: https://codereview.chromium.org/2689203002
Cr-Commit-Position: refs/heads/master@{#450816}
Committed: https://chromium.googlesource.com/chromium/src/+/0526716fe414eef61b0c151affd57b6f3bb6c96f

[Geoff Lang, 2017-02-13 15:56:17]

Description was changed from

==========
Check for some extensions before calling potentially NULL GL entry points.

Some extensions are exposed by the command buffer unconditionally, add
some tracking of which extensions are natively supported and call the
correct entry point.

BUG=602737
==========

to

==========
Check for some extensions before calling potentially NULL GL entry points.

Some extensions are exposed by the command buffer unconditionally, add
some tracking of which extensions are natively supported and call the
correct entry point.

BUG=602737
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
==========

[Geoff Lang, 2017-02-13 15:56:36]

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

[commit-bot: I haz the power, 2017-02-13 15:57:06]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Geoff Lang, 2017-02-13 15:58:44]

geofflang@chromium.org changed reviewers:
+ zmo@chromium.org

[Geoff Lang, 2017-02-13 15:58:45]

PTAL

[commit-bot: I haz the power, 2017-02-13 17:06:47]

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

[commit-bot: I haz the power, 2017-02-13 17:06:48]

Dry run: This issue passed the CQ dry run.

[Zhenyao Mo, 2017-02-13 18:12:08]

https://codereview.chromium.org/2689203002/diff/1/gpu/command_buffer/service/...
File gpu/command_buffer/service/gles2_cmd_decoder_passthrough.h (right):

https://codereview.chromium.org/2689203002/diff/1/gpu/command_buffer/service/...
gpu/command_buffer/service/gles2_cmd_decoder_passthrough.h:388: bool
has_chromium_copy_compressed_texture_;
Can we consolidate these to FeatureInfo? It seems to be a better place for these
caps.

[Geoff Lang, 2017-02-13 19:53:06]

https://codereview.chromium.org/2689203002/diff/1/gpu/command_buffer/service/...
File gpu/command_buffer/service/gles2_cmd_decoder_passthrough.h (right):

https://codereview.chromium.org/2689203002/diff/1/gpu/command_buffer/service/...
gpu/command_buffer/service/gles2_cmd_decoder_passthrough.h:388: bool
has_chromium_copy_compressed_texture_;
On 2017/02/13 18:12:08, Zhenyao Mo wrote:
> Can we consolidate these to FeatureInfo? It seems to be a better place for
these
> caps.

I tried but these features are enabled sometimes when the extension doesn't
exist.  For example, the FeatureInfo says that ext_discard_framebuffer is
present when the native extension exists or it's on es3 because it knows it can
be emulated.  I need to do a little extra tracking to know which native function
to call.

[Zhenyao Mo, 2017-02-13 19:58:15]

On 2017/02/13 19:53:06, Geoff Lang wrote:
>
https://codereview.chromium.org/2689203002/diff/1/gpu/command_buffer/service/...
> File gpu/command_buffer/service/gles2_cmd_decoder_passthrough.h (right):
> 
>
https://codereview.chromium.org/2689203002/diff/1/gpu/command_buffer/service/...
> gpu/command_buffer/service/gles2_cmd_decoder_passthrough.h:388: bool
> has_chromium_copy_compressed_texture_;
> On 2017/02/13 18:12:08, Zhenyao Mo wrote:
> > Can we consolidate these to FeatureInfo? It seems to be a better place for
> these
> > caps.
> 
> I tried but these features are enabled sometimes when the extension doesn't
> exist.  For example, the FeatureInfo says that ext_discard_framebuffer is
> present when the native extension exists or it's on es3 because it knows it
can
> be emulated.  I need to do a little extra tracking to know which native
function
> to call.

I see.  Then I think we should fix the messiness in FeatureInfo to move forward.

For example, FeatureInfo.ext_discard_framebuffer should only be true of the
native extension exists, and the command buffer should check
FeatureInfo.ext_discard_framebuffer or es3.

Sorry to put this burden on you.

[Geoff Lang, 2017-02-15 17:56:05]

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

[commit-bot: I haz the power, 2017-02-15 17:56:51]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Geoff Lang, 2017-02-15 18:05:34]

On 2017/02/13 19:58:15, Zhenyao Mo wrote:
> On 2017/02/13 19:53:06, Geoff Lang wrote:
> >
>
https://codereview.chromium.org/2689203002/diff/1/gpu/command_buffer/service/...
> > File gpu/command_buffer/service/gles2_cmd_decoder_passthrough.h (right):
> > 
> >
>
https://codereview.chromium.org/2689203002/diff/1/gpu/command_buffer/service/...
> > gpu/command_buffer/service/gles2_cmd_decoder_passthrough.h:388: bool
> > has_chromium_copy_compressed_texture_;
> > On 2017/02/13 18:12:08, Zhenyao Mo wrote:
> > > Can we consolidate these to FeatureInfo? It seems to be a better place for
> > these
> > > caps.
> > 
> > I tried but these features are enabled sometimes when the extension doesn't
> > exist.  For example, the FeatureInfo says that ext_discard_framebuffer is
> > present when the native extension exists or it's on es3 because it knows it
> can
> > be emulated.  I need to do a little extra tracking to know which native
> function
> > to call.
> 
> I see.  Then I think we should fix the messiness in FeatureInfo to move
forward.
> 
> For example, FeatureInfo.ext_discard_framebuffer should only be true of the
> native extension exists, and the command buffer should check
> FeatureInfo.ext_discard_framebuffer or es3.
> 
> Sorry to put this burden on you.

Refactored a bit to use feature_info.  Only functional change is that
GL_EXT_discard_framebuffer is no longer exposed unless there is native support. 
Is that Ok?  For ANGLE, it is always supported.

[commit-bot: I haz the power, 2017-02-15 18:10:26]

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

[commit-bot: I haz the power, 2017-02-15 18:10:27]

Dry run: Try jobs failed on following builders:
  cast_shell_android on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/cast_shell_a...)

[Geoff Lang, 2017-02-15 18:11:11]

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

[commit-bot: I haz the power, 2017-02-15 18:12:00]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-15 18:47:28]

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

[commit-bot: I haz the power, 2017-02-15 18:47:29]

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

[Geoff Lang, 2017-02-15 19:05:32]

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

[Geoff Lang, 2017-02-15 19:06:00]

On 2017/02/15 18:05:34, Geoff Lang wrote:
> On 2017/02/13 19:58:15, Zhenyao Mo wrote:
> > On 2017/02/13 19:53:06, Geoff Lang wrote:
> > >
> >
>
https://codereview.chromium.org/2689203002/diff/1/gpu/command_buffer/service/...
> > > File gpu/command_buffer/service/gles2_cmd_decoder_passthrough.h (right):
> > > 
> > >
> >
>
https://codereview.chromium.org/2689203002/diff/1/gpu/command_buffer/service/...
> > > gpu/command_buffer/service/gles2_cmd_decoder_passthrough.h:388: bool
> > > has_chromium_copy_compressed_texture_;
> > > On 2017/02/13 18:12:08, Zhenyao Mo wrote:
> > > > Can we consolidate these to FeatureInfo? It seems to be a better place
for
> > > these
> > > > caps.
> > > 
> > > I tried but these features are enabled sometimes when the extension
doesn't
> > > exist.  For example, the FeatureInfo says that ext_discard_framebuffer is
> > > present when the native extension exists or it's on es3 because it knows
it
> > can
> > > be emulated.  I need to do a little extra tracking to know which native
> > function
> > > to call.
> > 
> > I see.  Then I think we should fix the messiness in FeatureInfo to move
> forward.
> > 
> > For example, FeatureInfo.ext_discard_framebuffer should only be true of the
> > native extension exists, and the command buffer should check
> > FeatureInfo.ext_discard_framebuffer or es3.
> > 
> > Sorry to put this burden on you.
> 
> Refactored a bit to use feature_info.  Only functional change is that
> GL_EXT_discard_framebuffer is no longer exposed unless there is native
support. 
> Is that Ok?  For ANGLE, it is always supported.

Reverted the functional change to GL_EXT_discard_framebuffer, a bunch of tests
had expectations around it.  Should be no functional changes now.

[commit-bot: I haz the power, 2017-02-15 19:07:14]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Zhenyao Mo, 2017-02-15 19:46:35]

lgtm

[commit-bot: I haz the power, 2017-02-15 20:59:46]

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

[commit-bot: I haz the power, 2017-02-15 20:59:47]

Dry run: This issue passed the CQ dry run.

[Geoff Lang, 2017-02-15 21:23:05]

The CQ bit was checked by geofflang@chromium.org

[commit-bot: I haz the power, 2017-02-15 21:24:53]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-15 22:23:48]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1487193785381630,
"parent_rev": "32e074e42207bf161510aba4c4b5539d0818d2f1", "commit_rev":
"0526716fe414eef61b0c151affd57b6f3bb6c96f"}

[commit-bot: I haz the power, 2017-02-15 22:27:34]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1487193785381630,
"parent_rev": "32e074e42207bf161510aba4c4b5539d0818d2f1", "commit_rev":
"0526716fe414eef61b0c151affd57b6f3bb6c96f"}

[commit-bot: I haz the power, 2017-02-15 22:28:10]

Description was changed from

==========
Check for some extensions before calling potentially NULL GL entry points.

Some extensions are exposed by the command buffer unconditionally, add
some tracking of which extensions are natively supported and call the
correct entry point.

BUG=602737
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
==========

to

==========
Check for some extensions before calling potentially NULL GL entry points.

Some extensions are exposed by the command buffer unconditionally, add
some tracking of which extensions are natively supported and call the
correct entry point.

BUG=602737
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel

Review-Url: https://codereview.chromium.org/2689203002
Cr-Commit-Position: refs/heads/master@{#450816}
Committed:
https://chromium.googlesource.com/chromium/src/+/0526716fe414eef61b0c151affd5...
==========

[commit-bot: I haz the power, 2017-02-15 22:28:11]

Committed patchset #4 (id:60001) as
https://chromium.googlesource.com/chromium/src/+/0526716fe414eef61b0c151affd5...