Fixed issue where disjoint timer offsets were incorrectly calculated.

gpu/command_buffer/service/gpu_tracer.cc

Index: gpu/command_buffer/service/gpu_tracer.cc
diff --git a/gpu/command_buffer/service/gpu_tracer.cc b/gpu/command_buffer/service/gpu_tracer.cc
index 93c54341dfe201fa5b5525bb7808a76fe95b865e..9eb20fe4bf8d51dcf21c5ec6c4361ec3aa07a5d0 100644
--- a/gpu/command_buffer/service/gpu_tracer.cc
+++ b/gpu/command_buffer/service/gpu_tracer.cc
@@ -102,8 +102,19 @@ void GPUTrace::Start() {
     case kTracerTypeInvalid:
       break;
 
-    case kTracerTypeARBTimer:
     case kTracerTypeDisjointTimer:
+      // For the disjoint timer, GPU idle team does not seem to increment the

[vmiura, 2014/09/19 22:55:09]

nit: team -> time?

[David Yen, 2014/09/19 23:01:18]

Done.

+      // internal counter. We must calculate the offset before any query. The
+      // good news is any device that supports disjoint timer will also support
+      // glGetInteger64v, so we can query it directly unlike the ARBTimer case.
+      if (offset_ == 0) {
+        GLint64 gl_now = 0;
+        glGetInteger64v(GL_TIMESTAMP, &gl_now);
+        offset_ = base::TimeTicks::NowFromSystemTraceTime().ToInternalValue() -
+                  gl_now / base::Time::kNanosecondsPerMicrosecond;
+      }
+

[vmiura, 2014/09/19 22:55:09]

nit: Could you add a comment that this intentionally falls through to case
kTracerTypeARBTimer?

[David Yen, 2014/09/19 23:01:18]

Done.

+    case kTracerTypeARBTimer:
       // GL_TIMESTAMP and GL_TIMESTAMP_EXT both have the same value.
       glQueryCounter(queries_[0], GL_TIMESTAMP);
       break;
@@ -342,10 +353,14 @@ void GPUTracer::ProcessTraces() {
 
 void GPUTracer::CalculateTimerOffset() {
   if (tracer_type_ != kTracerTypeInvalid) {
-    // If GPU device category is off, invalidate timing sync.
     if (*gpu_trace_dev_category == '\0') {
+      // If GPU device category is off, invalidate timing sync.
       gpu_timing_synced_ = false;
       return;
+    } else if (tracer_type_ == kTracerTypeDisjointTimer) {
+      // Disjoint timers offsets should be calculated before every query.
+      gpu_timing_synced_ = true;
+      timer_offset_ = 0;
     }
 
     if (gpu_timing_synced_)
@@ -357,27 +372,16 @@ void GPUTracer::CalculateTimerOffset() {
     // it's not available everywhere.
     GLuint64 gl_now = 0;
     GLuint query;
-    GLint disjoint_value = 0;
 
-    if (tracer_type_ == kTracerTypeDisjointTimer) {
-      // Clear the disjoint bit before we do any queries.
-      glGetIntegerv(GL_GPU_DISJOINT_EXT, &disjoint_value);
-    }

[vmiura, 2014/09/19 22:55:09]

Should we clear existing GL_GPU_DISJOINT_EXT values by calling
"glGetIntegerv(GL_GPU_DISJOINT_EXT, &disjoint_value)" somewhere after
*gpu_trace_dev_category is toggled to enabled?

[David Yen, 2014/09/19 23:01:18]

It gets cleared under BeginDecoding() as well, this clear used to be for the
TIMESTAMP query specifically.

+    glGenQueriesARB(1, &query);
 
     glFinish();
-    glGenQueriesARB(1, &query);
     glQueryCounter(query, GL_TIMESTAMP);
     glFinish();
 
     glGetQueryObjectui64v(query, GL_QUERY_RESULT, &gl_now);
     glDeleteQueriesARB(1, &query);
 
-    if (tracer_type_ == kTracerTypeDisjointTimer) {
-      glGetIntegerv(GL_GPU_DISJOINT_EXT, &disjoint_value);
-      if (disjoint_value)
-        return;
-    }
-
     base::TimeTicks system_now = base::TimeTicks::NowFromSystemTraceTime();
 
     gl_now /= base::Time::kNanosecondsPerMicrosecond;