Initial checkin of the compositor_model_bench tool, which simulates the GPU demands of Chromium's...

Initial checkin of the compositor_model_bench tool, which simulates the GPU demands of Chromium's GPU-accelerated compositor. In the current version, we simulate the performance of the existing forward rendering model; coming changes will prototype a new model.

Patch 3: fix the number of spaces per indentation (hopefully everywhere)
Patch 4: move most of the code in compositor_model_bench.cc into a class so that I don't have as many globals scattered around
Patch 5: take out all standard streams, use LOG streams instead
Patch 6: fixed whitespace in class declarations
Patch 7: made suggested changes to render_model_utils
Patch 8: when I took out the streams for reading in a file and replaced them with a more complicated C-style implementation, said implementation was broken. At least the fix was quick!
Patch 9: superficial improvement to tools.gyp
Patch 10: made suggested changes to shaders
Patch 11: made suggested changes to render_tree
Patch 12: forgot to include what I use (DCHECK) and also there was a typo. Whoops!
Patch 12: made suggested changes to render_models, forward_render_model
Patch 13: one save didn't make the last patch; got it now
Patch 14: I think this is the one I described as patch 13 above. I lost count...
Patch 15: made suggested changes to forward_render_model
Patch 16: this (should) be the last of the trivial style issues
Patch 17: spoke a bit too soon -- some capitalization stuff
Patch 18: made suggested changes for measuring simulation times
Patch 19: made suggested changes for reading command line arguments, using our path/file libraries
Patch 20: a couple lint issues I've been missing on the server-side. Also, this was set up in a separate working copy than I was in before (the other one is stuck working on a change that's been more complicated than was expected) -- so I'm not sure if gcl will correctly apply this patch to the issue from before (#7686009)
Patch 21: switching to VBO's instead of immediate mode drawing -- also, the first build that runs on a Cr-48
Patch 3 (would've been patch 22 in the original issue): Fixing some lint errors
Patch 4: handling resize. Resizing the window of an ongoing session doesn't actually work on my dev machine, but at least now the viewport doesn't get cropped on the Cr-48.
Patch 5: fixing (unwarned) undefined behavior resulting from a smart pointer to a forward-declared type
Patch 6: new suggested changes to compositor_model_bench.cc
Patch 7: new suggested changes to render_model_utils
Patch 8: new suggested changes to render_models.cc
Patch 9: sweeping changes to get the access restrictions suggested for render_tree.h
Patch 10: stylistic changes suggested in render_tree.cc
Patch 11: suggested stylistic changes in shaders.cc -- only the simple ones; still have to run some tests with changing attribute mappings
Patch 12: the rest of the changes for shaders.cc (named parameters, and using 0 & 1 instead of 10 & 11 -- which I swear didn't work the first time I tried it?)
Patch 13: apparently the undefined behavior I was worrying about (in patch 5) isn't actually a problem, so I switched back to using a scoped_ptr

[piman, 2011-08-26 02:24:16]

http://codereview.chromium.org/7718020/diff/20001/gpu/tools/compositor_model_...
File gpu/tools/compositor_model_bench/compositor_model_bench.cc (right):

http://codereview.chromium.org/7718020/diff/20001/gpu/tools/compositor_model_...
gpu/tools/compositor_model_bench/compositor_model_bench.cc:139:
Resize(e.xconfigure.width, e.xconfigure.height);
If you intend to fall-through, you need to be explicit about it (add a //
fall-through after the next case label).
In this case, just break here (safer if someone decides to add another case and
missed that the case above was falling through)

http://codereview.chromium.org/7718020/diff/20001/gpu/tools/compositor_model_...
gpu/tools/compositor_model_bench/compositor_model_bench.cc:150: private:
nit: blank line before private:

http://codereview.chromium.org/7718020/diff/20001/gpu/tools/compositor_model_...
gpu/tools/compositor_model_bench/compositor_model_bench.cc:166: root_window,
nit: indentation (align with display_ above).

http://codereview.chromium.org/7718020/diff/20001/gpu/tools/compositor_model_...
gpu/tools/compositor_model_bench/compositor_model_bench.cc:203: True /* Direct
rendering */);
nit: indentation

http://codereview.chromium.org/7718020/diff/20001/gpu/tools/compositor_model_...
gpu/tools/compositor_model_bench/compositor_model_bench.cc:223: "(" <<
ModelToString(spec.model_under_test) << ")";
nit: indentation (continuations are 4 spaces).

http://codereview.chromium.org/7718020/diff/20001/gpu/tools/compositor_model_...
gpu/tools/compositor_model_bench/compositor_model_bench.cc:229:
path.LossyDisplayName();
nit: indentation (4 spaces)

http://codereview.chromium.org/7718020/diff/20001/gpu/tools/compositor_model_...
gpu/tools/compositor_model_bench/compositor_model_bench.cc:234: root,
nit: intdentation (arguments should align)

http://codereview.chromium.org/7718020/diff/20001/gpu/tools/compositor_model_...
gpu/tools/compositor_model_bench/compositor_model_bench.cc:354: void
_process_events(Simulator* sim) {
This wrapper is not necessary, you can use NewRunnableMethod to do this
implicitly. Same below.

http://codereview.chromium.org/7718020/diff/20001/gpu/tools/compositor_model_...
File gpu/tools/compositor_model_bench/forward_render_model.cc (right):

http://codereview.chromium.org/7718020/diff/20001/gpu/tools/compositor_model_...
gpu/tools/compositor_model_bench/forward_render_model.cc:61: delete visitor_;
use scoped_ptr instead?

http://codereview.chromium.org/7718020/diff/20001/gpu/tools/compositor_model_...
gpu/tools/compositor_model_bench/forward_render_model.cc:69: BeginFrame(10, 11);
I was initially confused about the magic "10" and "11". I think they don't have
to leak out of shaders.cc (see comments there).

http://codereview.chromium.org/7718020/diff/20001/gpu/tools/compositor_model_...
gpu/tools/compositor_model_bench/forward_render_model.cc:70: if (visitor_)
visitor_ can't be NULL, right? You should be able to skip this test

http://codereview.chromium.org/7718020/diff/20001/gpu/tools/compositor_model_...
File gpu/tools/compositor_model_bench/render_model_utils.cc (right):

http://codereview.chromium.org/7718020/diff/20001/gpu/tools/compositor_model_...
gpu/tools/compositor_model_bench/render_model_utils.cc:39: node->tile_width,
nit: indentation (align arguments)

http://codereview.chromium.org/7718020/diff/20001/gpu/tools/compositor_model_...
gpu/tools/compositor_model_bench/render_model_utils.cc:83: i !=
discovered_ids_.end();
nit: indentation (align all 3 statements)

http://codereview.chromium.org/7718020/diff/20001/gpu/tools/compositor_model_...
gpu/tools/compositor_model_bench/render_model_utils.cc:111: int width,
nit:indentation (align parameters)

http://codereview.chromium.org/7718020/diff/20001/gpu/tools/compositor_model_...
gpu/tools/compositor_model_bench/render_model_utils.cc:114: if (*texID == -1)
return;    // -1 means it's not a real texture.
nit: return on a separate line

http://codereview.chromium.org/7718020/diff/20001/gpu/tools/compositor_model_...
gpu/tools/compositor_model_bench/render_model_utils.cc:134: int width,
nit: indentation

http://codereview.chromium.org/7718020/diff/20001/gpu/tools/compositor_model_...
gpu/tools/compositor_model_bench/render_model_utils.cc:167: 0,
nit: align arguments

http://codereview.chromium.org/7718020/diff/20001/gpu/tools/compositor_model_...
File gpu/tools/compositor_model_bench/render_model_utils.h (right):

http://codereview.chromium.org/7718020/diff/20001/gpu/tools/compositor_model_...
gpu/tools/compositor_model_bench/render_model_utils.h:31: // off to
handle_texture (which behaves appropriately depending
handle_texture->HandleTexture

http://codereview.chromium.org/7718020/diff/20001/gpu/tools/compositor_model_...
gpu/tools/compositor_model_bench/render_model_utils.h:51: int width,
nit:indentation (parameters should align)

http://codereview.chromium.org/7718020/diff/20001/gpu/tools/compositor_model_...
File gpu/tools/compositor_model_bench/render_models.cc (right):

http://codereview.chromium.org/7718020/diff/20001/gpu/tools/compositor_model_...
gpu/tools/compositor_model_bench/render_models.cc:33: window_width,
nit: align arguments

http://codereview.chromium.org/7718020/diff/20001/gpu/tools/compositor_model_...
File gpu/tools/compositor_model_bench/render_tree.cc (right):

http://codereview.chromium.org/7718020/diff/20001/gpu/tools/compositor_model_...
gpu/tools/compositor_model_bench/render_tree.cc:225: ) {
nit: the ')' (and the '{' with it) should be on the previous line.

http://codereview.chromium.org/7718020/diff/20001/gpu/tools/compositor_model_...
gpu/tools/compositor_model_bench/render_tree.cc:246: ) {
nit: the ')' (and the '{' with it) should be on the previous line.

http://codereview.chromium.org/7718020/diff/20001/gpu/tools/compositor_model_...
gpu/tools/compositor_model_bench/render_tree.cc:290: ) {
nit: the ')' (and the '{' with it) should be on the previous line.

http://codereview.chromium.org/7718020/diff/20001/gpu/tools/compositor_model_...
gpu/tools/compositor_model_bench/render_tree.cc:389: "-----------------------\n"
<<
nit: indent (4 spaces)

http://codereview.chromium.org/7718020/diff/20001/gpu/tools/compositor_model_...
gpu/tools/compositor_model_bench/render_tree.cc:411: "\n(" << error_message <<
")";
nit: 4 spaces

http://codereview.chromium.org/7718020/diff/20001/gpu/tools/compositor_model_...
gpu/tools/compositor_model_bench/render_tree.cc:421: " doesn not encode a JSON
dictionary.";
nit: 4 spaces

http://codereview.chromium.org/7718020/diff/20001/gpu/tools/compositor_model_...
File gpu/tools/compositor_model_bench/render_tree.h (right):

http://codereview.chromium.org/7718020/diff/20001/gpu/tools/compositor_model_...
gpu/tools/compositor_model_bench/render_tree.h:47: int layerID;
according to our style, classes are not supposed to have public members. You can
make them private (and fix the namimg, e.g. layer_id_) and add accessors as
appropriate.
Same with other classes below.

http://codereview.chromium.org/7718020/diff/20001/gpu/tools/compositor_model_...
File gpu/tools/compositor_model_bench/shaders.cc (right):

http://codereview.chromium.org/7718020/diff/20001/gpu/tools/compositor_model_...
gpu/tools/compositor_model_bench/shaders.cc:43: static float ProjMat[] = {
ProjMat -> g_projection_matrix

http://codereview.chromium.org/7718020/diff/20001/gpu/tools/compositor_model_...
gpu/tools/compositor_model_bench/shaders.cc:47: -1.0, 1.0, 0.0, 1.0 };
'};' should be on a separate line

http://codereview.chromium.org/7718020/diff/20001/gpu/tools/compositor_model_...
gpu/tools/compositor_model_bench/shaders.cc:72: static void
Translate_InPlace(float* m, float tx, float ty, float tz) {
Translate_InPlace -> TranslateInPlace

http://codereview.chromium.org/7718020/diff/20001/gpu/tools/compositor_model_...
gpu/tools/compositor_model_bench/shaders.cc:234: GLchar* log = new
GLchar[length+1];
you can make this a scoped_array and skip the delete below.

http://codereview.chromium.org/7718020/diff/20001/gpu/tools/compositor_model_...
gpu/tools/compositor_model_bench/shaders.cc:257:
glBindAttribLocationARB(g_program_objects[program_index], 11, "a_texCoord");
Why 10 and 11, instead of say, 0 and 1 ?

You probably need the attribute 0 to be bound and enabled anyway (I'm not sure
why it works without it) - historical/legacy reasons.

It would be good to have those be named constants (static const int
kPositionLocation = 0), and I think that at least for now, they don't have to
leak out of this file (the way they are passed to BeginFrame).

http://codereview.chromium.org/7718020/diff/20001/gpu/tools/compositor_model_...
gpu/tools/compositor_model_bench/shaders.cc:271: n->fragment_shader,
nit: indentation (align arguments)

http://codereview.chromium.org/7718020/diff/20001/gpu/tools/compositor_model_...
gpu/tools/compositor_model_bench/shaders.cc:324: FRAGMENT_SHADER_RGBA_TEX_ALPHA,
nit: indentation

http://codereview.chromium.org/7718020/diff/20001/gpu/tools/compositor_model_...
gpu/tools/compositor_model_bench/shaders.cc:360: sizeof(vertices),
nit: indentation

http://codereview.chromium.org/7718020/diff/20001/gpu/tools/compositor_model_...
gpu/tools/compositor_model_bench/shaders.cc:365: sizeof(indices),
nit: indentation

http://codereview.chromium.org/7718020/diff/20001/gpu/tools/compositor_model_...
gpu/tools/compositor_model_bench/shaders.cc:397: "matrix");
nit: indentation (or fold on previous line if it fits)

http://codereview.chromium.org/7718020/diff/20001/gpu/tools/compositor_model_...
gpu/tools/compositor_model_bench/shaders.cc:398: glUniformMatrix4fvARB(mat, 1,
true, proj_transform);
true->GL_TRUE

http://codereview.chromium.org/7718020/diff/20001/gpu/tools/compositor_model_...
gpu/tools/compositor_model_bench/shaders.cc:438: glUniformMatrix4fvARB(mat, 1,
true, proj_transform);
true -> GL_TRUE

[joshtrask, 2011-08-26 04:12:35]

http://codereview.chromium.org/7718020/diff/20001/gpu/tools/compositor_model_...
File gpu/tools/compositor_model_bench/forward_render_model.cc (right):

http://codereview.chromium.org/7718020/diff/20001/gpu/tools/compositor_model_...
gpu/tools/compositor_model_bench/forward_render_model.cc:61: delete visitor_;
On 2011/08/26 02:24:16, piman wrote:
> use scoped_ptr instead?

I had been doing so at one point, but it occurred to me today that putting
visitor_ in the instance variables means I was using a forward declaration to
specialize the scoped_ptr template, and so I believe scoped_ptr's deletion
behavior is undefined. I could separate the ForwardRenderNodeVisitor into a
different file so that it's available in ForwardRenderModel.h, but I'm not too
keen on separating the visitor from the rest of the render model simulator. What
do you think?

[piman, 2011-08-26 04:15:05]

On Thu, Aug 25, 2011 at 9:12 PM, <joshtrask@google.com> wrote:

> Reviewers: piman,
>
>
>
> http://codereview.chromium.**org/7718020/diff/20001/gpu/**
>
tools/compositor_model_bench/**forward_render_model.cc<http://codereview.chromium.org/7718020/diff/20001/gpu/tools/compositor_model_bench/forward_render_model.cc>
> File gpu/tools/compositor_model_**bench/forward_render_model.cc (right):
>
> http://codereview.chromium.**org/7718020/diff/20001/gpu/**
>
tools/compositor_model_bench/**forward_render_model.cc#**newcode61<http://codereview.chromium.org/7718020/diff/20001/gpu/tools/compositor_model_bench/forward_render_model.cc#newcode61>
> gpu/tools/compositor_model_**bench/forward_render_model.cc:**61: delete
> visitor_;
> On 2011/08/26 02:24:16, piman wrote:
>
>> use scoped_ptr instead?
>>
>
> I had been doing so at one point, but it occurred to me today that
> putting visitor_ in the instance variables means I was using a forward
> declaration to specialize the scoped_ptr template, and so I believe
> scoped_ptr's deletion behavior is undefined.


It's actually OK if the constructor and destructor of the outer class (and
any functions that need to access the scoped_ptr) are defined in the .cc
file. It's a common pattern in the Chrome tree.

Antoine



> I could separate the
> ForwardRenderNodeVisitor into a different file so that it's available in
> ForwardRenderModel.h, but I'm not too keen on separating the visitor
> from the rest of the render model simulator. What do you think?
>
> Description:
> Initial checkin of the compositor_model_bench tool, which simulates the GPU
> demands of Chromium's GPU-accelerated compositor. In the current version,
> we
> simulate the performance of the existing forward rendering model; coming
> changes
> will prototype a new model.
>
> Patch 3: fix the number of spaces per indentation (hopefully everywhere)
> Patch 4: move most of the code in compositor_model_bench.cc into a class so
> that
> I don't have as many globals scattered around
> Patch 5: take out all standard streams, use LOG streams instead
> Patch 6: fixed whitespace in class declarations
> Patch 7: made suggested changes to render_model_utils
> Patch 8: when I took out the streams for reading in a file and replaced
> them
> with a more complicated C-style implementation, said implementation was
> broken.
> At least the fix was quick!
> Patch 9: superficial improvement to tools.gyp
> Patch 10: made suggested changes to shaders
> Patch 11: made suggested changes to render_tree
> Patch 12: forgot to include what I use (DCHECK) and also there was a typo.
> Whoops!
> Patch 12: made suggested changes to render_models, forward_render_model
> Patch 13: one save didn't make the last patch; got it now
> Patch 14: I think this is the one I described as patch 13 above. I lost
> count...
> Patch 15: made suggested changes to forward_render_model
> Patch 16: this (should) be the last of the trivial style issues
> Patch 17: spoke a bit too soon -- some capitalization stuff
> Patch 18: made suggested changes for measuring simulation times
> Patch 19: made suggested changes for reading command line arguments, using
> our
> path/file libraries
> Patch 20: a couple lint issues I've been missing on the server-side. Also,
> this
> was set up in a separate working copy than I was in before (the other one
> is
> stuck working on a change that's been more complicated than was expected)
> -- so
> I'm not sure if gcl will correctly apply this patch to the issue from
> before
> (#7686009)
> Patch 21: switching to VBO's instead of immediate mode drawing -- also, the
> first build that runs on a Cr-48
> Patch 3 (would've been patch 22 in the original issue): Fixing some lint
> errors
> Patch 4: handling resize. Resizing the window of an ongoing session doesn't
> actually work on my dev machine, but at least now the viewport doesn't get
> cropped on the Cr-48.
> Patch 5: fixing (unwarned) undefined behavior resulting from a smart
> pointer to
> a forward-declared type
> Patch 6: new suggested changes to compositor_model_bench.cc
> Patch 7: new suggested changes to render_model_utils
> Patch 8: new suggested changes to render_models.cc
> Patch 9: sweeping changes to get the access restrictions suggested for
> render_tree.h
> Patch 10: stylistic changes suggested in render_tree.cc
> Patch 11: suggested stylistic changes in shaders.cc -- only the simple
> ones;
> still have to run some tests with changing attribute mappings
>
> Please review this at
http://codereview.chromium.**org/7718020/<http://codereview.chromium.org/7718...
>
> SVN Base:
http://src.chromium.org/svn/**trunk/src/<http://src.chromium.org/svn/trunk/src/>
>
> Affected files:
>  build/all.gyp
>
>  gpu/tools/compositor_model_**bench/compositor_model_bench.**cc
>  gpu/tools/compositor_model_**bench/forward_render_model.h
>
>  gpu/tools/compositor_model_**bench/forward_render_model.cc
>  A     gpu/tools/compositor_model_**bench/measuring_confs/YT0.txt.**json
>  A     gpu/tools/compositor_model_**bench/measuring_confs/YT1.txt.**json
>  A     gpu/tools/compositor_model_**bench/measuring_confs/YT10.**txt.json
>  A     gpu/tools/compositor_model_**bench/measuring_confs/YT11.**txt.json
>  A     gpu/tools/compositor_model_**bench/measuring_confs/YT12.**txt.json
>  A     gpu/tools/compositor_model_**bench/measuring_confs/YT13.**txt.json
>  A     gpu/tools/compositor_model_**bench/measuring_confs/YT2.txt.**json
>  A     gpu/tools/compositor_model_**bench/measuring_confs/YT3.txt.**json
>  A     gpu/tools/compositor_model_**bench/measuring_confs/YT4.txt.**json
>  A     gpu/tools/compositor_model_**bench/measuring_confs/YT5.txt.**json
>  A     gpu/tools/compositor_model_**bench/measuring_confs/YT6.txt.**json
>  A     gpu/tools/compositor_model_**bench/measuring_confs/YT7.txt.**json
>  A     gpu/tools/compositor_model_**bench/measuring_confs/YT8.txt.**json
>  A     gpu/tools/compositor_model_**bench/measuring_confs/YT9.txt.**json
>  A     gpu/tools/compositor_model_**bench/measuring_confs/YT_**
> special0.txt.json
>  A     gpu/tools/compositor_model_**bench/measuring_confs/YT_**
> special1.txt.json
>  A     gpu/tools/compositor_model_**bench/measuring_confs/YT_**
> special10.txt.json
>  A     gpu/tools/compositor_model_**bench/measuring_confs/YT_**
> special11.txt.json
>  A     gpu/tools/compositor_model_**bench/measuring_confs/YT_**
> special12.txt.json
>  A     gpu/tools/compositor_model_**bench/measuring_confs/YT_**
> special2.txt.json
>  A     gpu/tools/compositor_model_**bench/measuring_confs/YT_**
> special3.txt.json
>  A     gpu/tools/compositor_model_**bench/measuring_confs/YT_**
> special4.txt.json
>  A     gpu/tools/compositor_model_**bench/measuring_confs/YT_**
> special5.txt.json
>  A     gpu/tools/compositor_model_**bench/measuring_confs/YT_**
> special6.txt.json
>  A     gpu/tools/compositor_model_**bench/measuring_confs/YT_**
> special7.txt.json
>  A     gpu/tools/compositor_model_**bench/measuring_confs/YT_**
> special8.txt.json
>  A     gpu/tools/compositor_model_**bench/measuring_confs/YT_**
> special9.txt.json
>  A     gpu/tools/compositor_model_**bench/measuring_confs/abra0.**txt.json
>  A     gpu/tools/compositor_model_**bench/measuring_confs/abra1.**txt.json
>  A     gpu/tools/compositor_model_**bench/measuring_confs/abra2.**txt.json
>  A     gpu/tools/compositor_model_**bench/measuring_confs/abra3.**txt.json
>  A     gpu/tools/compositor_model_**bench/measuring_confs/abra4.**txt.json
>  A     gpu/tools/compositor_model_**bench/measuring_confs/abra5.**txt.json
>  A     gpu/tools/compositor_model_**bench/measuring_confs/test00_**
> 0.txt.json
>  A     gpu/tools/compositor_model_**bench/measuring_confs/test00_**
> 1.txt.json
>  A     gpu/tools/compositor_model_**bench/measuring_confs/test00_**
> 2.txt.json
>  A     gpu/tools/compositor_model_**bench/measuring_confs/test00_**
> 3.txt.json
>  A     gpu/tools/compositor_model_**bench/measuring_confs/test01_**
> 0.txt.json
>  A     gpu/tools/compositor_model_**bench/measuring_confs/test01_**
> 1.txt.json
>  A     gpu/tools/compositor_model_**bench/measuring_confs/test01_**
> 2.txt.json
>  A     gpu/tools/compositor_model_**bench/measuring_confs/test01_**
> 3.txt.json
>  A     gpu/tools/compositor_model_**bench/measuring_confs/test02_**
> 0.txt.json
>  A     gpu/tools/compositor_model_**bench/measuring_confs/test02_**
> 1.txt.json
>  A     gpu/tools/compositor_model_**bench/measuring_confs/test02_**
> 2.txt.json
>  A     gpu/tools/compositor_model_**bench/measuring_confs/test02_**
> 3.txt.json
>  A     gpu/tools/compositor_model_**bench/measuring_confs/test03_**
> 0.txt.json
>  A     gpu/tools/compositor_model_**bench/measuring_confs/test03_**
> 1.txt.json
>  A     gpu/tools/compositor_model_**bench/measuring_confs/test03_**
> 2.txt.json
>  A     gpu/tools/compositor_model_**bench/measuring_confs/test03_**
> 3.txt.json
>  A     gpu/tools/compositor_model_**bench/measuring_confs/test04_**
> 0.txt.json
>  A     gpu/tools/compositor_model_**bench/measuring_confs/test04_**
> 1.txt.json
>  A     gpu/tools/compositor_model_**bench/measuring_confs/test04_**
> 2.txt.json
>  A     gpu/tools/compositor_model_**bench/measuring_confs/test04_**
> 3.txt.json
>  A     gpu/tools/compositor_model_**bench/render_model_utils.h
>  A     gpu/tools/compositor_model_**bench/render_model_utils.cc
>  A     gpu/tools/compositor_model_**bench/render_models.h
>  A     gpu/tools/compositor_model_**bench/render_models.cc
>  A     gpu/tools/compositor_model_**bench/render_tree.h
>  A     gpu/tools/compositor_model_**bench/render_tree.cc
>  A     gpu/tools/compositor_model_**bench/shaders.h
>  A     gpu/tools/compositor_model_**bench/shaders.cc
>  A     gpu/tools/tools.gyp
>
>
>

[joshtrask, 2011-08-26 04:29:04]

http://codereview.chromium.org/7718020/diff/20001/gpu/tools/compositor_model_...
File gpu/tools/compositor_model_bench/forward_render_model.cc (right):

http://codereview.chromium.org/7718020/diff/20001/gpu/tools/compositor_model_...
gpu/tools/compositor_model_bench/forward_render_model.cc:70: if (visitor_)
Is that true? What about std::bad_alloc?

On 2011/08/26 02:24:16, piman wrote:
> visitor_ can't be NULL, right? You should be able to skip this test

[joshtrask, 2011-08-26 04:30:07]

http://codereview.chromium.org/7718020/diff/20001/gpu/tools/compositor_model_...
File gpu/tools/compositor_model_bench/forward_render_model.cc (right):

http://codereview.chromium.org/7718020/diff/20001/gpu/tools/compositor_model_...
gpu/tools/compositor_model_bench/forward_render_model.cc:70: if (visitor_)
Well, I suppose if that was going to give me a problem, it would've been before
I got this far...

On 2011/08/26 04:29:04, joshtrask wrote:
> Is that true? What about std::bad_alloc?
> 
> On 2011/08/26 02:24:16, piman wrote:
> > visitor_ can't be NULL, right? You should be able to skip this test
>

[piman, 2011-08-27 17:49:36]

LGTM