gpu: Record GL state to trace

gpu/command_buffer/service/gpu_state_tracer.cc

Index: gpu/command_buffer/service/gpu_state_tracer.cc
diff --git a/gpu/command_buffer/service/gpu_state_tracer.cc b/gpu/command_buffer/service/gpu_state_tracer.cc
new file mode 100644
index 0000000000000000000000000000000000000000..4721551781ef7211297c88c311012ce7baf029a1
--- /dev/null
+++ b/gpu/command_buffer/service/gpu_state_tracer.cc
@@ -0,0 +1,129 @@
+// Copyright (c) 2013 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_state_tracer.h"
+
+#include <sstream>
+
+#include "base/base64.h"
+#include "base/debug/trace_event.h"
+#include "context_state.h"
+#include "ui/gfx/codec/png_codec.h"
+#include "ui/gl/gl_bindings.h"
+
+namespace gpu {
+namespace gles2 {
+namespace {
+
+class Snapshot : public base::debug::ConvertableToTraceFormat {
+ public:
+  static scoped_ptr<Snapshot> Create(const ContextState* state);
+
+  bool SaveScreenshot();
+
+  // base::debug::ConvertableToTraceFormat implementation.
+  virtual void AppendAsTraceFormat(std::string* out) const OVERRIDE;
+
+ private:
+  explicit Snapshot(const ContextState* state);
+
+  const ContextState* state_;
+  std::stringstream snapshot_;
+  DISALLOW_COPY_AND_ASSIGN(Snapshot);
+};
+
+}  // namespace
+
+Snapshot::Snapshot(const ContextState* state) : state_(state) {}
+
+scoped_ptr<Snapshot> Snapshot::Create(const ContextState* state) {
+  return scoped_ptr<Snapshot>(new Snapshot(state));
+}
+
+bool Snapshot::SaveScreenshot() {
+  const int kBytesPerPixel = 4;
+  gfx::Size viewport(state_->viewport_width, state_->viewport_height);
+  std::vector<unsigned char> pixels(viewport.width() * viewport.height() *
+                                    kBytesPerPixel);
+
+  glPixelStorei(GL_PACK_ALIGNMENT, kBytesPerPixel);
+  glReadPixels(state_->viewport_x,
+               state_->viewport_y,
+               viewport.width(),
+               viewport.height(),
+               GL_RGBA,
+               GL_UNSIGNED_BYTE,
+               &pixels[0]);
+  glPixelStorei(GL_PACK_ALIGNMENT, state_->pack_alignment);
+
+  // Flip the screenshot vertically.
+  int bytes_per_row = viewport.width() * kBytesPerPixel;
+  for (int y = 0; y < viewport.height() / 2; y++) {
+    for (int x = 0; x < bytes_per_row; x++) {
+      std::swap(pixels[y * bytes_per_row + x],
+                pixels[(viewport.height() - y - 1) * bytes_per_row + x]);
+    }
+  }
+
+  std::vector<unsigned char> png_data;
+  if (!gfx::PNGCodec::Encode(&pixels[0],
+                             gfx::PNGCodec::FORMAT_RGBA,
+                             viewport,
+                             bytes_per_row,
+                             false,
+                             std::vector<gfx::PNGCodec::Comment>(),
+                             &png_data))
+    return false;

[dsinclair, 2013/09/09 14:25:08]

Is this something that happens commonly? Should we DCHECK here?

[Sami, 2013/09/09 16:32:15]

It shouldn't really happen unless we're really out of memory, so a DCHECK is
probably sufficient.

+
+  base::StringPiece base64_input(reinterpret_cast<const char*>(&png_data[0]),

[dsinclair, 2013/09/09 14:25:08]

Should the conversion of the png_data happen here or in AppendAsTraceFormat? As
currently written you'll incur the cost of converting to base64 immediately. If
you moved it into the AppendAsTraceFormat you'd incur the cost when tracing is
finished.

[Sami, 2013/09/09 16:32:15]

I first did it this way to avoid keeping too many snapshots in memory. However I
just did a couple of tests with deferred encoding and it seems to work fine --
and the framerate is much better. So thanks for the idea :)

+                                 png_data.size());
+  std::string base64_output;
+  Base64Encode(base64_input, &base64_output);
+
+  snapshot_ << "\"screenshot\":\"" << base64_output << "\"";
+  return true;
+}
+
+void Snapshot::AppendAsTraceFormat(std::string* out) const {
+  *out += "{";
+  *out += snapshot_.str();
+  *out += "}";
+}
+
+scoped_ptr<GPUStateTracer> GPUStateTracer::Create(const ContextState* state) {
+  return scoped_ptr<GPUStateTracer>(new GPUStateTracer(state));
+}
+
+GPUStateTracer::GPUStateTracer(const ContextState* state) : state_(state) {
+  TRACE_EVENT_OBJECT_CREATED_WITH_ID(
+      TRACE_DISABLED_BY_DEFAULT("gpu.debug"), "gpu::State", this);

[dsinclair, 2013/09/09 14:25:08]

Why 'this' and not 'state'?

[Sami, 2013/09/09 16:32:15]

Done. Had to change base/debug/trace_event.h to accept a const pointer here
though.

+}
+
+GPUStateTracer::~GPUStateTracer() {
+  TRACE_EVENT_OBJECT_DELETED_WITH_ID(
+      TRACE_DISABLED_BY_DEFAULT("gpu.debug"), "gpu::State", this);
+}
+
+void GPUStateTracer::TakeSnapshot() {
+  bool is_tracing;

[dsinclair, 2013/09/09 14:25:08]

Would it make sense to move the CATEGORY_GROUP_ENABLED check out to the
callsite? Then, we would not incur the cost of calling into the TakeSnapshot
method.

[Sami, 2013/09/09 16:32:15]

Yeah, I can see calls to TakeSnapshot() getting added in various places that
can't tolerate the extra cost.

+  TRACE_EVENT_CATEGORY_GROUP_ENABLED(TRACE_DISABLED_BY_DEFAULT("gpu.debug"),
+                                     &is_tracing);
+  if (!is_tracing)
+    return;
+
+  scoped_ptr<Snapshot> snapshot(Snapshot::Create(state_));
+
+  // Only save a screenshot for now.
+  if (!snapshot->SaveScreenshot())
+    return;
+
+  TRACE_EVENT_OBJECT_SNAPSHOT_WITH_ID(
+      TRACE_DISABLED_BY_DEFAULT("gpu.debug"),
+      "gpu::State",
+      this,
+      snapshot.PassAs<base::debug::ConvertableToTraceFormat>());
+}
+
+}  // namespace gles2
+}  // namespace gpu