Added support for Time Elapsed queries through the command buffer.

Added support for Time Elapsed queries through the command buffer.

This CL adds support for Time Elapsed queries through the command
buffer.

Since it was easy to support, I have also added support
for getting the current TimeStamp through GetInteger64(),
GetInteger()...etc. Note that this is different from getting the
GPU timestamp using QueryCounter which is asynchronous.

BUG=345227
TEST=trybots

Committed: https://crrev.com/1b6a2940fcb25df247b9f8b795e84e1ca5742904
Cr-Commit-Position: refs/heads/master@{#336743}

[David Yen, 2015-06-17 23:28:28]

dyen@chromium.org changed reviewers:
+ sievers@chromium.org

[David Yen, 2015-06-17 23:28:59]

dyen@chromium.org changed reviewers:
+ vmiura@chromium.org
- sievers@chromium.org

[David Yen, 2015-06-17 23:32:30]

dyen@chromium.org changed reviewers:
+ bbudge@chromium.org, sievers@chromium.org

[David Yen, 2015-06-17 23:32:30]

Sorry for all the big changes to review, thanks for all your reviews so far!
This CL adds support for Time Elapsed queries through the command buffer, as
well as TimeStamp queries using GetInteger64() (and other variants that are not
useful for GL_TIME_STAMP_EXT but defined in the spec).

+vmiura to review command buffer changes
+bbudge to review ppapi changes
+sievers to review ui/gl changes

[David Yen, 2015-06-17 23:38:31]

https://codereview.chromium.org/1188013004/diff/1/gpu/command_buffer/build_gl...
File gpu/command_buffer/build_gles2_cmd_buffer.py (right):

https://codereview.chromium.org/1188013004/diff/1/gpu/command_buffer/build_gl...
gpu/command_buffer/build_gles2_cmd_buffer.py:3756: 'extension':
"disjoint_timer_query_EXT",
Does anyone know what this extension string does? I wasn't sure if I should make
it "true" or "disjoint_timer_query_EXT". Technically the other
GetQueryObjectuivEXT are not solely for occlusion_query_EXT also, does that
field support lists? This function is special because it is supported by
disjoint_timer_query_EXT but not occlusion_query_EXT.

[bbudge, 2015-06-17 23:45:03]

ppapi lgtm

[David Yen, 2015-06-18 17:08:19]

On 2015/06/17 23:45:03, bbudge wrote:
> ppapi lgtm

Strange, getting a compile error for the PPAPI bits that were auto-generated so
I removed the PPAPI bits. What do I need to do to get this working? does PPAPI
even need glGetQueryObjectui64v()?

Here is the error I am getting:

../../ppapi/shared_impl/ppb_opengles2_shared.cc:1788:30: error: excess elements
in struct initializer
      &GetQueryObjectuivEXT, &GetQueryObjectui64vEXT};
                             ^~~~~~~~~~~~~~~~~~~~~~~

[David Yen, 2015-06-23 17:24:53]

ping

[no sievers, 2015-06-23 21:44:52]

sievers@chromium.org changed reviewers:
+ bajones@chromium.org, kbr@chromium.org

[no sievers, 2015-06-23 21:45:28]

So sounds like you want to implement this for webgl. +kbr, bajones.

https://codereview.chromium.org/1188013004/diff/20001/gpu/command_buffer/clie...
File gpu/command_buffer/client/gles2_implementation.cc (right):

https://codereview.chromium.org/1188013004/diff/20001/gpu/command_buffer/clie...
gpu/command_buffer/client/gles2_implementation.cc:706: *params =
static_cast<GLint>(
I think this should be a saturated cast because of GL_EXT_disjoint_timer_query:

"If the value is so
    large in magnitude that it cannot be represented with the requested type,
    then the nearest value representable using the requested type is
    returned"

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

https://codereview.chromium.org/1188013004/diff/20001/gpu/command_buffer/serv...
gpu/command_buffer/service/gles2_cmd_decoder.cc:11594: case GL_SAMPLES_PASSED:
Good catch. Also, GL_SAMPLES_PASSED is actually not from
occlusion_query_boolean.

https://codereview.chromium.org/1188013004/diff/20001/gpu/command_buffer/serv...
gpu/command_buffer/service/gles2_cmd_decoder.cc:11596: case
GL_ANY_SAMPLES_PASSED_CONSERVATIVE:
@bajones: Do we support GL_TRANSFORM_FEEDBACK_PRIMITIVES_WRITTEN yet w/the
experimental webgl2 stuff?
If we do looks like that would somehow oddly slip through the
AllSamplesPassedQuery in query_manager.cc.

[bajones, 2015-06-23 21:53:29]

On 2015/06/23 21:45:28, sievers wrote:
> @bajones: Do we support GL_TRANSFORM_FEEDBACK_PRIMITIVES_WRITTEN yet w/the
> experimental webgl2 stuff?
> If we do looks like that would somehow oddly slip through the
> AllSamplesPassedQuery in query_manager.cc.

There's partial plumbing in place for it, but I don't believe it actually works
yet. I'm working on improving it right now.

[David Yen, 2015-06-23 23:35:33]

https://codereview.chromium.org/1188013004/diff/20001/gpu/command_buffer/clie...
File gpu/command_buffer/client/gles2_implementation.cc (right):

https://codereview.chromium.org/1188013004/diff/20001/gpu/command_buffer/clie...
gpu/command_buffer/client/gles2_implementation.cc:706: *params =
static_cast<GLint>(
On 2015/06/23 21:45:27, sievers wrote:
> I think this should be a saturated cast because of
GL_EXT_disjoint_timer_query:
> 
> "If the value is so
>     large in magnitude that it cannot be represented with the requested type,
>     then the nearest value representable using the requested type is
>     returned"

Done.

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

https://codereview.chromium.org/1188013004/diff/20001/gpu/command_buffer/serv...
gpu/command_buffer/service/gles2_cmd_decoder.cc:11594: case GL_SAMPLES_PASSED:
On 2015/06/23 21:45:27, sievers wrote:
> Good catch. Also, GL_SAMPLES_PASSED is actually not from
> occlusion_query_boolean.

We worked this out together, this should be okay.

https://codereview.chromium.org/1188013004/diff/20001/gpu/command_buffer/serv...
gpu/command_buffer/service/gles2_cmd_decoder.cc:11596: case
GL_ANY_SAMPLES_PASSED_CONSERVATIVE:
On 2015/06/23 21:45:27, sievers wrote:
> @bajones: Do we support GL_TRANSFORM_FEEDBACK_PRIMITIVES_WRITTEN yet w/the
> experimental webgl2 stuff?
> If we do looks like that would somehow oddly slip through the
> AllSamplesPassedQuery in query_manager.cc.

I talked this over with bajones, we do not support that query target.

[bajones, 2015-06-23 23:41:31]

On 2015/06/23 23:35:33, David Yen wrote:
> On 2015/06/23 21:45:27, sievers wrote:
> > @bajones: Do we support GL_TRANSFORM_FEEDBACK_PRIMITIVES_WRITTEN yet w/the
> > experimental webgl2 stuff?
> > If we do looks like that would somehow oddly slip through the
> > AllSamplesPassedQuery in query_manager.cc.
> 
> I talked this over with bajones, we do not support that query target.

I vaguely remember that conversation, but if I said that I might have been
wrong. GL_TRANSFORM_FEEDBACK_PRIMITIVES_WRITTEN is needed for WebGL2.

[David Yen, 2015-06-24 17:28:16]

On 2015/06/23 23:41:31, bajones wrote:
> On 2015/06/23 23:35:33, David Yen wrote:
> > On 2015/06/23 21:45:27, sievers wrote:
> > > @bajones: Do we support GL_TRANSFORM_FEEDBACK_PRIMITIVES_WRITTEN yet w/the
> > > experimental webgl2 stuff?
> > > If we do looks like that would somehow oddly slip through the
> > > AllSamplesPassedQuery in query_manager.cc.
> > 
> > I talked this over with bajones, we do not support that query target.
> 
> I vaguely remember that conversation, but if I said that I might have been
> wrong. GL_TRANSFORM_FEEDBACK_PRIMITIVES_WRITTEN is needed for WebGL2.

That is fine, once that feature is added we can just add that target to the
switch statement.

[no sievers, 2015-06-24 23:40:42]

lgtm. but please double-check with kbr@ or bajones@ that this will work for
webgl, and also regarding transform feedback query.

https://codereview.chromium.org/1188013004/diff/80001/gpu/command_buffer/clie...
File gpu/command_buffer/client/gles2_implementation.cc (right):

https://codereview.chromium.org/1188013004/diff/80001/gpu/command_buffer/clie...
gpu/command_buffer/client/gles2_implementation.cc:4944: *params = 64;
nit: Can you put a comment that this is because of the above where we use the
CPU time?

[David Yen, 2015-06-25 18:38:13]

ping kbr@ and bajones@, any comments regarding webgl?

https://codereview.chromium.org/1188013004/diff/80001/gpu/command_buffer/clie...
File gpu/command_buffer/client/gles2_implementation.cc (right):

https://codereview.chromium.org/1188013004/diff/80001/gpu/command_buffer/clie...
gpu/command_buffer/client/gles2_implementation.cc:4944: *params = 64;
On 2015/06/24 23:40:41, sievers wrote:
> nit: Can you put a comment that this is because of the above where we use the
> CPU time?

Done.

[Ken Russell (switch to Gerrit), 2015-06-26 16:30:18]

Thank you for pushing this through and sorry for the delay reviewing.

This LGTM overall, but we'll need to write tests to make sure the client side
APIs are working as expected.

Are timestamp counter queries supported client-side?

[David Yen, 2015-06-26 16:36:09]

On 2015/06/26 16:30:18, Ken Russell wrote:
> Thank you for pushing this through and sorry for the delay reviewing.
> 
> This LGTM overall, but we'll need to write tests to make sure the client side
> APIs are working as expected.
> 
> Are timestamp counter queries supported client-side?

I have another CL for Time stamp queries once this one lands. They do not
piggyback on glBeginQuery/glEndQuery so they are a bit more involved.

[David Yen, 2015-06-26 16:54:01]

ping vmiura for empty mojo changes.

[David Yen, 2015-06-26 22:24:02]

dyen@chromium.org changed reviewers:
+ zmo@chromium.org
- vmiura@chromium.org

[David Yen, 2015-06-26 22:24:49]

On 2015/06/26 16:54:01, David Yen wrote:
> ping vmiura for empty mojo changes.

Forgot that vmiura is on vacation, adding zmo instead for empty mojo changes.

[Zhenyao Mo, 2015-06-30 01:16:00]

David, I am not a mojo/ owner.

[Zhenyao Mo, 2015-06-30 01:23:46]

Actually I am.  LGTM then.

[David Yen, 2015-06-30 06:45:39]

The CQ bit was checked by dyen@chromium.org

[David Yen, 2015-06-30 06:45:40]

The patchset sent to the CQ was uploaded after l-g-t-m from bbudge@chromium.org,
sievers@chromium.org, kbr@chromium.org
Link to the patchset: https://codereview.chromium.org/1188013004/#ps180001
(title: "Removed extra line")

[commit-bot: I haz the power, 2015-06-30 06:45:45]

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

[commit-bot: I haz the power, 2015-06-30 08:50:08]

Committed patchset #10 (id:180001)

[commit-bot: I haz the power, 2015-06-30 08:51:11]

Patchset 10 (id:??) landed as
https://crrev.com/1b6a2940fcb25df247b9f8b795e84e1ca5742904
Cr-Commit-Position: refs/heads/master@{#336743}

[michaeln, 2015-07-01 20:00:42]

michaeln@chromium.org changed reviewers:
+ michaeln@chromium.org

[michaeln, 2015-07-01 20:00:43]

https://codereview.chromium.org/1188013004/diff/180001/gpu/command_buffer/cli...
File gpu/command_buffer/client/gles2_implementation.cc (right):

https://codereview.chromium.org/1188013004/diff/180001/gpu/command_buffer/cli...
gpu/command_buffer/client/gles2_implementation.cc:1009: *
base::Time::kNanosecondsPerMicrosecond;
Hello, we're seeing crashes on this line in the canary
https://code.google.com/p/chromium/issues/detail?id=506120

[amineer, 2015-07-01 20:28:40]

A revert of this CL (patchset #10 id:180001) has been created in
https://codereview.chromium.org/1222563003/ by amineer@chromium.org.

The reason for reverting is: May be causing crashes
BUG=506120.