Fixed issue where disjoint timer offsets were incorrectly calculated.

gpu/command_buffer/service/gpu_tracer.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "gpu/command_buffer/service/gpu_tracer.h"
 
 #include <deque>
 
 #include "base/bind.h"
 #include "base/debug/trace_event.h"
@@ skipping 84 matching lines @@
 }
 
 void GPUTrace::Start() {
   TRACE_EVENT_COPY_ASYNC_BEGIN0(
       TRACE_DISABLED_BY_DEFAULT("gpu.service"), name().c_str(), this);
 
   switch (tracer_type_) {
     case kTracerTypeInvalid:
       break;
 
+    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:
-    case kTracerTypeDisjointTimer:
       // GL_TIMESTAMP and GL_TIMESTAMP_EXT both have the same value.
       glQueryCounter(queries_[0], GL_TIMESTAMP);
       break;
   }
 }
 
 void GPUTrace::End() {
   end_requested_ = true;
   switch (tracer_type_) {
     case kTracerTypeInvalid:
@@ skipping 218 matching lines @@
   }
 
   // Clear pending traces if there were are any errors
   GLenum err = glGetError();
   if (err != GL_NO_ERROR)
     traces_.clear();
 }
 
 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_)
       return;
 
     TRACE_EVENT0("gpu", "GPUTracer::CalculateTimerOffset");
 
     // NOTE(vmiura): It would be better to use glGetInteger64v, however
     // 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;
     timer_offset_ = system_now.ToInternalValue() - gl_now;
     gpu_timing_synced_ = true;
   }
 }
 
 void GPUTracer::IssueProcessTask() {
   if (traces_.empty() || process_posted_)
     return;
 
   process_posted_ = true;
   base::MessageLoop::current()->PostDelayedTask(
       FROM_HERE,
       base::Bind(&GPUTracer::Process, base::AsWeakPtr(this)),
       base::TimeDelta::FromMilliseconds(kProcessInterval));
 }
 
 }  // namespace gles2
 }  // namespace gpu