Benchmark tool for GPU-accelerated video rendering

media/tools/shader_bench/gpu_painter.h

+// Copyright (c) 2010 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.
+
+#ifndef MEDIA_TOOLS_BENCHMARK_GPU_PAINTER_H_

[scherkus (not reviewing), 2010/11/30 23:36:10]

should be MEDIA_TOOLS_SHADER_BENCH_GPU_PAINTER_H_

[vrk (LEFT CHROMIUM), 2010/12/01 02:46:11]

Good catch, thanks! Done.

+#define MEDIA_TOOLS_BENCHMARK_GPU_PAINTER_H_
+
+#include "app/gfx/gl/gl_bindings.h"
+#include "app/gfx/gl/gl_context.h"
+#include "app/gfx/gl/gl_implementation.h"
+#include "media/tools/shader_bench/painter.h"
+
+// Class that renders video frames to a window via GPU.
+class GPUPainter : public Painter {
+ public:
+  GPUPainter();
+
+  // Sets context for subsequent gl calls in this painter.
+  virtual void SetGLContext(gfx::GLContext* context);
+
+ protected:

[scherkus (not reviewing), 2010/11/30 23:36:10]

try to avoid protected

I'd simply make the methods public and/or provide public context() getter

[vrk (LEFT CHROMIUM), 2010/12/01 02:46:11]

Done.

+  // Loads shader into given context, from the source code of the
+  // shader. type refers to the shader type, either GL_VERTEX_SHADER or
+  // GL_FRAGMENT_SHADER. Returns id of shader.
+  static GLuint LoadShader(gfx::GLContext* context, unsigned type,

[scherkus (not reviewing), 2010/11/30 23:36:10]

might be wrong but looks like this is only used by CreateShaderProgram(), in
which case this doesn't need to be in .h file

[vrk (LEFT CHROMIUM), 2010/12/01 02:46:11]

Made into private member function instead.

+                           const char* shader_source);
+
+  // Creates shader program into given context, from the vertex and fragment
+  // shader source code. Returns the id of the shader program.
+  static GLuint CreateShaderProgram(gfx::GLContext* context,

[scherkus (not reviewing), 2010/11/30 23:36:10]

does it makes sense to have this be a member function instead of static seeing
as it has access to context_?

just curious

[vrk (LEFT CHROMIUM), 2010/12/01 02:46:11]

Yeah, makes sense! I was just unnecessarily generalizing. Modified the class to
make things simpler.

+                           const char* vertex_shader_source,
+                           const char* fragment_shader_source);
+
+  // Reference to the gl context.
+  gfx::GLContext* context_;
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(GPUPainter);
+};
+
+#endif  // MEDIA_TOOLS_BENCHMARK_GPU_PAINTER_H_