Port the Life example to Pepper 2.

Port the Life example to Pepper 2.


Committed: http://code.google.com/p/nativeclient/source/detail?r=160

[David Springer, 2011-02-02 18:48:19]

I totally re-wrote the way Life works: the user event handling is all done in
the browser, using the Dragger object form tumbler.  Note that there is some
<div> magical hackery to work around a Chrome/Pepper bug where events are not
bubbled up from a NaCl module that returns false from HandleInputEvent().

[dmichael(do not use this one), 2011-02-02 23:03:59]

Some copy/paste errors, some other suggestions, and some off-by-one errors in
the untouched code.

I like the approach of passing in input events from JS.

http://codereview.chromium.org/6286025/diff/3007/examples/graphics/life/dragg...
File examples/graphics/life/dragger.js (right):

http://codereview.chromium.org/6286025/diff/3007/examples/graphics/life/dragg...
examples/graphics/life/dragger.js:8: * until it gets a mousup event.  It
manufactures three drag events: the
three +kinds of+ drag events?
I haven't read all the way through, but I expect DRAG can happen more than once.

http://codereview.chromium.org/6286025/diff/3007/examples/graphics/life/dragg...
examples/graphics/life/dragger.js:17: * |opt_target|.  If |opt_target| is null
or undefined, then this object observes
nit:  >80 characters

http://codereview.chromium.org/6286025/diff/3007/examples/graphics/life/dragg...
examples/graphics/life/dragger.js:33: * this array get sent a handleStartDrag(),
handleDrag() and handleEndDrag()
get->gets (or is)

http://codereview.chromium.org/6286025/diff/3007/examples/graphics/life/dragg...
examples/graphics/life/dragger.js:72: * Add a drag listener.  Each listener
should respond to thhree methods:
s/thhree/three

http://codereview.chromium.org/6286025/diff/3007/examples/graphics/life/dragg...
examples/graphics/life/dragger.js:94: clientY: event.offsetY };
It might be good to document what this event var contains on uikit.Dragger or
addDragListener.

http://codereview.chromium.org/6286025/diff/3007/examples/graphics/life/dragg...
examples/graphics/life/dragger.js:97:
this.listeners_[i].handleStartDrag(this.target_, dragStartEvent);
You could abstract this loop out if you wanted;  it should be pretty similar for
all 3 events.

And is the index style more common than 'for listener in this.listeners_'?  I
usually prefer a 'for each' kind of loop in languages where I have it, but I
don't know what's most common in JavaScript.

http://codereview.chromium.org/6286025/diff/3007/examples/graphics/life/life.cc
File examples/graphics/life/life.cc (right):

http://codereview.chromium.org/6286025/diff/3007/examples/graphics/life/life....
examples/graphics/life/life.cc:19: const uint32_t kInitialBrandSeed =
0xC0DE533D;
I think 'Brand' is kind of confusing;  brand is a name locally.  RandSeed (or at
least BRandSeed) would be clearer to me.

http://codereview.chromium.org/6286025/diff/3007/examples/graphics/life/life....
examples/graphics/life/life.cc:126: cell_in_[i] = brand();
We're throwing away probably 30 bits of randomness here.  We could use them if
we wanted;  rand_r is a lot slower than your average shift instruction ;-)  But
probably not worth the code complexity.

http://codereview.chromium.org/6286025/diff/3007/examples/graphics/life/life....
examples/graphics/life/life.cc:131: cell_in_[i * width + (width - 2)] = brand();
I would've expected it to be width - 1...  probably works okay this way, but I
don't think it's randomizing the last column (seems like the 2nd to last).

http://codereview.chromium.org/6286025/diff/3007/examples/graphics/life/life....
examples/graphics/life/life.cc:155: MakeRGBA(0x00, 0xFF, 0x00, 0xff),
If we care about performance, we may want to avoid function-local statics.  GCC
(and presumably nacl-gcc) makes them thread-safe by default.  We could instead
make colors a class member or a file-local constant (in an unnamed namespace). 
But it's probably not that big of a gain <shrug/>

I guess if we're only updating on simulation ticks, which aren't that frequent,
then it doesn't matter.

http://codereview.chromium.org/6286025/diff/3007/examples/graphics/life/life....
examples/graphics/life/life.cc:158: static uint8_t replace[18] = {
Why 18?  17 should suffice, as written.

http://codereview.chromium.org/6286025/diff/3007/examples/graphics/life/life....
examples/graphics/life/life.cc:160: 0, 0, 1, 1, 0, 0, 0, 0,  // row for center
cell alive
The first 0 in this row could be dead center, all 8 neighbors alive...  OR live
center with 8 dead neighbors.  This is also missing initialization for a live
center with 8 live neighbors.  (range is 0..16, for 17 total array indices)

http://codereview.chromium.org/6286025/diff/3007/examples/graphics/life/life....
examples/graphics/life/life.cc:179: src2[-1] +     src2[0] +        src2[1];
Seems like dead center with all neighbors alive is indistinguishable from live
center with all dead neighbors.  The result is the same, so I guess it's okay,
but it's not documented.  It also makes the 'replace' array a bit confusing.

It's tempting to say 'just use a conditional' instead of trying to do everything
with arithmetic and table lookup.  Or multiply the center by 9 so the replace
array can make more sense.

http://codereview.chromium.org/6286025/diff/3007/examples/graphics/life/life.css
File examples/graphics/life/life.css (right):

http://codereview.chromium.org/6286025/diff/3007/examples/graphics/life/life....
examples/graphics/life/life.css:34: * contains the NaCl module.
Maybe reference the bug...  you shouldn't have to do all this to get the events
to JS.

On a side note...  maybe it should be valid to provide a NULL function pointer
for HandleInputEvent, which tells the browser 'I don't handle events'.  For NaCl
modules, it would be nice if you're not doing anything with input events on the
native side to not have to go through the proxy for every mouse move.

http://codereview.chromium.org/6286025/diff/3007/examples/graphics/life/life.h
File examples/graphics/life/life.h (right):

http://codereview.chromium.org/6286025/diff/3007/examples/graphics/life/life....
examples/graphics/life/life.h:29: // Start up the ComputePi() thread.
Bad comment

http://codereview.chromium.org/6286025/diff/3007/examples/graphics/life/life....
examples/graphics/life/life.h:32: // Update the graphcs context to the new size,
and regnerate |pixel_buffer_|
s/regnerate/regenerate

http://codereview.chromium.org/6286025/diff/3007/examples/graphics/life/life....
examples/graphics/life/life.h:33: // to fit the new size as well.
also cell_in_ and cell_out_

http://codereview.chromium.org/6286025/diff/3007/examples/graphics/life/life....
examples/graphics/life/life.h:36: // The pp::Var takes over ownership of the
returned script object.
We can probably say more...  e.g. Return a pp::Var that represents the interface
we expose to JavaScript.  The pp::Var takes over ownership...

http://codereview.chromium.org/6286025/diff/3007/examples/graphics/life/life....
examples/graphics/life/life.h:45: void UnlockPixels() const;
These appear to be neither defined nor used, and there doesn't seem to be any
multithreading here.  Copy/paste error?

http://codereview.chromium.org/6286025/diff/3007/examples/graphics/life/life....
examples/graphics/life/life.h:98: // functions in this example take arguments,
so this vector is always empty.
Not true...  AddCellAtPoint takes arguments.

http://codereview.chromium.org/6286025/diff/3007/examples/graphics/life/life....
examples/graphics/life/life.h:110: // Add in some random noise to the cells,
this is part of a simulation tick.
Maybe mention that the randomness is added to the outline of the simulation
area.  I also think based on the code that the outline is never drawn.

http://codereview.chromium.org/6286025/diff/3007/examples/graphics/life/life....
examples/graphics/life/life.h:122: }
I think this should go back in the body...  calling rand_r isn't 'trivial' in my
mind.  I think bit_rand might be a clearer name.

http://codereview.chromium.org/6286025/diff/3007/examples/graphics/life/life....
examples/graphics/life/life.h:137: mutable pthread_mutex_t pixel_buffer_mutex_;
Unused.

http://codereview.chromium.org/6286025/diff/3007/examples/graphics/life/life....
File examples/graphics/life/life.html (right):

http://codereview.chromium.org/6286025/diff/3007/examples/graphics/life/life....
examples/graphics/life/life.html:22: </script>
2-space indent

http://codereview.chromium.org/6286025/diff/3007/examples/graphics/life/life.js
File examples/graphics/life/life.js (right):

http://codereview.chromium.org/6286025/diff/3007/examples/graphics/life/life....
examples/graphics/life/life.js:107: };
Not clear to me why there's a semicolon here...

http://codereview.chromium.org/6286025/diff/3007/examples/graphics/life/life....
examples/graphics/life/life.js:118: };
spurious semicolon?

http://codereview.chromium.org/6286025/diff/3007/examples/graphics/life/life....
examples/graphics/life/life.js:128: };
spurious semicolon?

[David Springer, 2011-02-03 18:12:26]

http://codereview.chromium.org/6286025/diff/3007/examples/graphics/life/dragg...
File examples/graphics/life/dragger.js (right):

http://codereview.chromium.org/6286025/diff/3007/examples/graphics/life/dragg...
examples/graphics/life/dragger.js:8: * until it gets a mousup event.  It
manufactures three drag events: the
On 2011/02/02 23:03:59, dmichael wrote:
> three +kinds of+ drag events?
> I haven't read all the way through, but I expect DRAG can happen more than
once.

Done.

http://codereview.chromium.org/6286025/diff/3007/examples/graphics/life/dragg...
examples/graphics/life/dragger.js:17: * |opt_target|.  If |opt_target| is null
or undefined, then this object observes
On 2011/02/02 23:03:59, dmichael wrote:
> nit:  >80 characters

Done.

http://codereview.chromium.org/6286025/diff/3007/examples/graphics/life/dragg...
examples/graphics/life/dragger.js:33: * this array get sent a handleStartDrag(),
handleDrag() and handleEndDrag()
On 2011/02/02 23:03:59, dmichael wrote:
> get->gets (or is)

Done.

http://codereview.chromium.org/6286025/diff/3007/examples/graphics/life/dragg...
examples/graphics/life/dragger.js:72: * Add a drag listener.  Each listener
should respond to thhree methods:
On 2011/02/02 23:03:59, dmichael wrote:
> s/thhree/three

Done.

http://codereview.chromium.org/6286025/diff/3007/examples/graphics/life/dragg...
examples/graphics/life/dragger.js:94: clientY: event.offsetY };
On 2011/02/02 23:03:59, dmichael wrote:
> It might be good to document what this event var contains on uikit.Dragger or
> addDragListener.

Good point.  It is a DOM Event object, which is documented here:
http://www.w3.org/TR/DOM-Level-3-Events/#events-mouseevents. I notice that
offsetX/Y is actually not in the W3 DOM spec... I'll look into a way to use
clientX/Y instead.

http://codereview.chromium.org/6286025/diff/3007/examples/graphics/life/dragg...
examples/graphics/life/dragger.js:97:
this.listeners_[i].handleStartDrag(this.target_, dragStartEvent);
On 2011/02/02 23:03:59, dmichael wrote:
> You could abstract this loop out if you wanted;  it should be pretty similar
for
> all 3 events.
> 
> And is the index style more common than 'for listener in this.listeners_'?  I
> usually prefer a 'for each' kind of loop in languages where I have it, but I
> don't know what's most common in JavaScript.

for...in in JS can cause problems because of the dynamic nature of property
bindings.  It's safer to use the for() {} style.

As for abstracting: I could write a small function that takes name of the
handler function to call, but this is getting into some very peculiar JS.

http://codereview.chromium.org/6286025/diff/3007/examples/graphics/life/life.cc
File examples/graphics/life/life.cc (right):

http://codereview.chromium.org/6286025/diff/3007/examples/graphics/life/life....
examples/graphics/life/life.cc:19: const uint32_t kInitialBrandSeed =
0xC0DE533D;
On 2011/02/02 23:03:59, dmichael wrote:
> I think 'Brand' is kind of confusing;  brand is a name locally.  RandSeed (or
at
> least BRandSeed) would be clearer to me.

Done.

http://codereview.chromium.org/6286025/diff/3007/examples/graphics/life/life....
examples/graphics/life/life.cc:126: cell_in_[i] = brand();
On 2011/02/02 23:03:59, dmichael wrote:
> We're throwing away probably 30 bits of randomness here.  We could use them if
> we wanted;  rand_r is a lot slower than your average shift instruction ;-) 
But
> probably not worth the code complexity.

OK.  How about we revisit this in another CL?  Want to do it?

http://codereview.chromium.org/6286025/diff/3007/examples/graphics/life/life....
examples/graphics/life/life.cc:131: cell_in_[i * width + (width - 2)] = brand();
On 2011/02/02 23:03:59, dmichael wrote:
> I would've expected it to be width - 1...  probably works okay this way, but I
> don't think it's randomizing the last column (seems like the 2nd to last).

The right-most cells have to start 1 pixel in from the edge, otherwise they
don't change (mutate?).

http://codereview.chromium.org/6286025/diff/3007/examples/graphics/life/life....
examples/graphics/life/life.cc:155: MakeRGBA(0x00, 0xFF, 0x00, 0xff),
On 2011/02/02 23:03:59, dmichael wrote:
> If we care about performance, we may want to avoid function-local statics. 
GCC
> (and presumably nacl-gcc) makes them thread-safe by default.  We could instead
> make colors a class member or a file-local constant (in an unnamed namespace).

> But it's probably not that big of a gain <shrug/>
> 
> I guess if we're only updating on simulation ticks, which aren't that
frequent,
> then it doesn't matter.

Done.

http://codereview.chromium.org/6286025/diff/3007/examples/graphics/life/life....
examples/graphics/life/life.cc:158: static uint8_t replace[18] = {
On 2011/02/02 23:03:59, dmichael wrote:
> Why 18?  17 should suffice, as written.
It turns out that |count| (below) has range [0..17].  This init just left the
last two elements of |replace| initialized to 0.  I made the initialization
explicit.

http://codereview.chromium.org/6286025/diff/3007/examples/graphics/life/life....
examples/graphics/life/life.cc:160: 0, 0, 1, 1, 0, 0, 0, 0,  // row for center
cell alive
On 2011/02/02 23:03:59, dmichael wrote:
> The first 0 in this row could be dead center, all 8 neighbors alive...  OR
live
> center with 8 dead neighbors.  This is also missing initialization for a live
> center with 8 live neighbors.  (range is 0..16, for 17 total array indices)

Good catch.  I ported this without trying to understand the algorithm. 
Suggestion: how about we fix this up to run properly in another CL?  Maybe the
same one where you improve the random number usage?
Done.

http://codereview.chromium.org/6286025/diff/3007/examples/graphics/life/life....
examples/graphics/life/life.cc:179: src2[-1] +     src2[0] +        src2[1];
On 2011/02/02 23:03:59, dmichael wrote:
> Seems like dead center with all neighbors alive is indistinguishable from live
> center with all dead neighbors.  The result is the same, so I guess it's okay,
> but it's not documented.  It also makes the 'replace' array a bit confusing.
> 
> It's tempting to say 'just use a conditional' instead of trying to do
everything
> with arithmetic and table lookup.  Or multiply the center by 9 so the replace
> array can make more sense.

Interesting observation.  Comment added.

http://codereview.chromium.org/6286025/diff/3007/examples/graphics/life/life.css
File examples/graphics/life/life.css (right):

http://codereview.chromium.org/6286025/diff/3007/examples/graphics/life/life....
examples/graphics/life/life.css:34: * contains the NaCl module.
On 2011/02/02 23:03:59, dmichael wrote:
> Maybe reference the bug...  you shouldn't have to do all this to get the
events
> to JS.
> 
> On a side note...  maybe it should be valid to provide a NULL function pointer
> for HandleInputEvent, which tells the browser 'I don't handle events'.  For
NaCl
> modules, it would be nice if you're not doing anything with input events on
the
> native side to not have to go through the proxy for every mouse move.

I haven't written up the bug yet.  I want to make a small example to go with it
so that we can test.

I factored this a little to use CSS classes instead of ID names, and applied the
class to the <embed> element directly instead of wrapping it in a div.  This
seems to work fine, and gets rid of one of DOM elements.

http://codereview.chromium.org/6286025/diff/3007/examples/graphics/life/life.h
File examples/graphics/life/life.h (right):

http://codereview.chromium.org/6286025/diff/3007/examples/graphics/life/life....
examples/graphics/life/life.h:29: // Start up the ComputePi() thread.
On 2011/02/02 23:03:59, dmichael wrote:
> Bad comment

Done.

http://codereview.chromium.org/6286025/diff/3007/examples/graphics/life/life....
examples/graphics/life/life.h:32: // Update the graphcs context to the new size,
and regnerate |pixel_buffer_|
On 2011/02/02 23:03:59, dmichael wrote:
> s/regnerate/regenerate

Done.

http://codereview.chromium.org/6286025/diff/3007/examples/graphics/life/life....
examples/graphics/life/life.h:33: // to fit the new size as well.
On 2011/02/02 23:03:59, dmichael wrote:
> also cell_in_ and cell_out_

Done.

http://codereview.chromium.org/6286025/diff/3007/examples/graphics/life/life....
examples/graphics/life/life.h:36: // The pp::Var takes over ownership of the
returned script object.
On 2011/02/02 23:03:59, dmichael wrote:
> We can probably say more...  e.g. Return a pp::Var that represents the
interface
> we expose to JavaScript.  The pp::Var takes over ownership...

Done.

http://codereview.chromium.org/6286025/diff/3007/examples/graphics/life/life....
examples/graphics/life/life.h:45: void UnlockPixels() const;
On 2011/02/02 23:03:59, dmichael wrote:
> These appear to be neither defined nor used, and there doesn't seem to be any
> multithreading here.  Copy/paste error?

Done.

http://codereview.chromium.org/6286025/diff/3007/examples/graphics/life/life....
examples/graphics/life/life.h:98: // functions in this example take arguments,
so this vector is always empty.
On 2011/02/02 23:03:59, dmichael wrote:
> Not true...  AddCellAtPoint takes arguments.

Done.

http://codereview.chromium.org/6286025/diff/3007/examples/graphics/life/life....
examples/graphics/life/life.h:110: // Add in some random noise to the cells,
this is part of a simulation tick.
On 2011/02/02 23:03:59, dmichael wrote:
> Maybe mention that the randomness is added to the outline of the simulation
> area.  I also think based on the code that the outline is never drawn.

Done.

http://codereview.chromium.org/6286025/diff/3007/examples/graphics/life/life....
examples/graphics/life/life.h:122: }
On 2011/02/02 23:03:59, dmichael wrote:
> I think this should go back in the body...  calling rand_r isn't 'trivial' in
my
> mind.  I think bit_rand might be a clearer name.

Done.

Actually, I factored the random bit generation into its own class.  This should
make it easier later to change the rand machine.

http://codereview.chromium.org/6286025/diff/3007/examples/graphics/life/life....
examples/graphics/life/life.h:137: mutable pthread_mutex_t pixel_buffer_mutex_;
On 2011/02/02 23:03:59, dmichael wrote:
> Unused.

Done.

http://codereview.chromium.org/6286025/diff/3007/examples/graphics/life/life....
File examples/graphics/life/life.html (right):

http://codereview.chromium.org/6286025/diff/3007/examples/graphics/life/life....
examples/graphics/life/life.html:22: </script>
On 2011/02/02 23:03:59, dmichael wrote:
> 2-space indent

Done.

http://codereview.chromium.org/6286025/diff/3007/examples/graphics/life/life.js
File examples/graphics/life/life.js (right):

http://codereview.chromium.org/6286025/diff/3007/examples/graphics/life/life....
examples/graphics/life/life.js:107: };
On 2011/02/02 23:03:59, dmichael wrote:
> Not clear to me why there's a semicolon here...

Done.

http://codereview.chromium.org/6286025/diff/3007/examples/graphics/life/life....
examples/graphics/life/life.js:118: };
On 2011/02/02 23:03:59, dmichael wrote:
> spurious semicolon?

Done.

http://codereview.chromium.org/6286025/diff/3007/examples/graphics/life/life....
examples/graphics/life/life.js:128: };
On 2011/02/02 23:03:59, dmichael wrote:
> spurious semicolon?

Done.

[dmichael(do not use this one), 2011-02-03 20:08:43]

Down to mostly nits

http://codereview.chromium.org/6286025/diff/3007/examples/graphics/life/dragg...
File examples/graphics/life/dragger.js (right):

http://codereview.chromium.org/6286025/diff/3007/examples/graphics/life/dragg...
examples/graphics/life/dragger.js:97:
this.listeners_[i].handleStartDrag(this.target_, dragStartEvent);
On 2011/02/03 18:12:27, David Springer wrote:
> On 2011/02/02 23:03:59, dmichael wrote:
> > You could abstract this loop out if you wanted;  it should be pretty similar
> for
> > all 3 events.
> > 
> > And is the index style more common than 'for listener in this.listeners_'? 
I
> > usually prefer a 'for each' kind of loop in languages where I have it, but I
> > don't know what's most common in JavaScript.
> 
> for...in in JS can cause problems because of the dynamic nature of property
> bindings.  It's safer to use the for() {} style.
> 
> As for abstracting: I could write a small function that takes name of the
> handler function to call, but this is getting into some very peculiar JS.
Nevermind; was neglecting that the function call is different.  I think you
could just pass the function itself, but that would be less clear, if anything. 
Forget I said it.

http://codereview.chromium.org/6286025/diff/3007/examples/graphics/life/life.cc
File examples/graphics/life/life.cc (right):

http://codereview.chromium.org/6286025/diff/3007/examples/graphics/life/life....
examples/graphics/life/life.cc:131: cell_in_[i * width + (width - 2)] = brand();
On 2011/02/03 18:12:27, David Springer wrote:
> On 2011/02/02 23:03:59, dmichael wrote:
> > I would've expected it to be width - 1...  probably works okay this way, but
I
> > don't think it's randomizing the last column (seems like the 2nd to last).
> 
> The right-most cells have to start 1 pixel in from the edge, otherwise they
> don't change (mutate?).
I don't understand why the last column should be treated differently than
left-most column, top row, and bottom row.  Can you explain more?  It looks to
me like the intent of the original code was that the bounding cells would _not_
be mutated, but instead just affect the cells directly inside of them, to make
the simulation more interesting.  The outer pixels aren't even drawn.  I haven't
run this in a while, but I would expect to see a weird little artifact of a
column of random pixels on the right, which I don't think is meant to be seen.

http://codereview.chromium.org/6286025/diff/3008/examples/graphics/life/life....
examples/graphics/life/life.cc:51: LifeScriptObject* script_object = new
LifeScriptObject(this);
This is still weird...  the top row shows 0-7, the 2nd row shows 8-15.  For a
dead center, there are 9 possible values (0, 1, 2, 3, 4, 5, 6, 7, 8) for the
count of living neighbors.  Similarly for a live center.  So one of these rows
(technically both, except that index 8 is ambiguous) should have 9 items.

http://codereview.chromium.org/6286025/diff/3008/examples/graphics/life/life....
examples/graphics/life/life.cc:52: return pp::Var(this, script_object);
You should only need 1 of these (total of 17 possible values in [0,16]).

http://codereview.chromium.org/6286025/diff/3008/examples/graphics/life/life.h
File examples/graphics/life/life.h (right):

http://codereview.chromium.org/6286025/diff/3008/examples/graphics/life/life....
examples/graphics/life/life.h:108: static const uint32_t kInitialRandSeed;
I'd rather remove the static and make random_bit_seed_ get initialized in the
constructor.  You can use a constant for the initial seed and have Life's
constructor initialize it using that constant.

http://codereview.chromium.org/6286025/diff/3008/examples/graphics/life/life.js
File examples/graphics/life/life.js (right):

http://codereview.chromium.org/6286025/diff/3008/examples/graphics/life/life....
examples/graphics/life/life.js:128: };
Missed a spot ;-)

[David Springer, 2011-02-03 21:09:46]

http://codereview.chromium.org/6286025/diff/3008/examples/graphics/life/life.cc
File examples/graphics/life/life.cc (right):

http://codereview.chromium.org/6286025/diff/3008/examples/graphics/life/life....
examples/graphics/life/life.cc:51: 0, 0, 1, 1, 0, 0, 0, 0,   // row for center
cell alive
On 2011/02/03 20:08:44, dmichael wrote:
> This is still weird...  the top row shows 0-7, the 2nd row shows 8-15.  For a
> dead center, there are 9 possible values (0, 1, 2, 3, 4, 5, 6, 7, 8) for the
> count of living neighbors.  Similarly for a live center.  So one of these rows
> (technically both, except that index 8 is ambiguous) should have 9 items.

I called out the middle value as special.

http://codereview.chromium.org/6286025/diff/3008/examples/graphics/life/life....
examples/graphics/life/life.cc:52: 0, 0
On 2011/02/03 20:08:44, dmichael wrote:
> You should only need 1 of these (total of 17 possible values in [0,16]).

Done.

http://codereview.chromium.org/6286025/diff/3008/examples/graphics/life/life.h
File examples/graphics/life/life.h (right):

http://codereview.chromium.org/6286025/diff/3008/examples/graphics/life/life....
examples/graphics/life/life.h:108: static const uint32_t kInitialRandSeed;
On 2011/02/03 20:08:44, dmichael wrote:
> I'd rather remove the static and make random_bit_seed_ get initialized in the
> constructor.  You can use a constant for the initial seed and have Life's
> constructor initialize it using that constant.

Done.

http://codereview.chromium.org/6286025/diff/3008/examples/graphics/life/life.js
File examples/graphics/life/life.js (right):

http://codereview.chromium.org/6286025/diff/3008/examples/graphics/life/life....
examples/graphics/life/life.js:128: };
On 2011/02/03 20:08:44, dmichael wrote:
> Missed a spot ;-)

Done. :D

[nfullagar, 2011-02-03 21:18:58]

http://codereview.chromium.org/6286025/diff/3008/examples/graphics/life/life.cc
File examples/graphics/life/life.cc (right):

http://codereview.chromium.org/6286025/diff/3008/examples/graphics/life/life....
examples/graphics/life/life.cc:157: cell_in_[i * width + (width - 2)] =
random_bits_.value();
should above be i * width + (width - 1) ?

http://codereview.chromium.org/6286025/diff/3008/examples/graphics/life/life....
examples/graphics/life/life.cc:175: for (int x = 1; x < (width - 1); ++x) {
should ++src0, ++src, ++src2; be moved up here (or add one to where they are
initialized above.)

Otherwise, src0[-1], src1[-1], src2[-1] will wrap around on the first iteration.

[dmichael(do not use this one), 2011-02-03 21:22:59]

Found a couple of nits, but o/w LGTM.  Thanks!

http://codereview.chromium.org/6286025/diff/2008/examples/graphics/life/life.cc
File examples/graphics/life/life.cc (right):

http://codereview.chromium.org/6286025/diff/2008/examples/graphics/life/life....
examples/graphics/life/life.cc:51: 0, 0, 0, 1, 0, 0, 0, 0, 0, // Values if the
center cell is dead.
Lint is complaining that you should have >=2 spaces before the comment.

Otherwise, thanks for the change;  much clearer now.

http://codereview.chromium.org/6286025/diff/2008/examples/graphics/life/life....
examples/graphics/life/life.cc:158: cell_in_[i * width + (width - 2)] =
random_bits_.value();
Still a little baffled by this, but let's stick with what works for now.

http://codereview.chromium.org/6286025/diff/2008/examples/graphics/life/life....
examples/graphics/life/life.cc:179: // alive neighbors.
the 'Note that' part is no longer true with this latest fix.

http://codereview.chromium.org/6286025/diff/2008/examples/graphics/life/life.css
File examples/graphics/life/life.css (right):

http://codereview.chromium.org/6286025/diff/2008/examples/graphics/life/life....
examples/graphics/life/life.css:41: height: 400;
Love it, looks much cooler this way.

[David Springer, 2011-02-03 22:04:55]

http://codereview.chromium.org/6286025/diff/3008/examples/graphics/life/life.cc
File examples/graphics/life/life.cc (right):

http://codereview.chromium.org/6286025/diff/3008/examples/graphics/life/life....
examples/graphics/life/life.cc:157: cell_in_[i * width + (width - 2)] =
random_bits_.value();
On 2011/02/03 21:18:58, nfullagar wrote:
> should above be i * width + (width - 1) ?

Done.

http://codereview.chromium.org/6286025/diff/3008/examples/graphics/life/life....
examples/graphics/life/life.cc:175: for (int x = 1; x < (width - 1); ++x) {
On 2011/02/03 21:18:58, nfullagar wrote:
> should ++src0, ++src, ++src2; be moved up here (or add one to where they are
> initialized above.)
> 
> Otherwise, src0[-1], src1[-1], src2[-1] will wrap around on the first
iteration.

Fixed by adding 1 to src0 above and starting the whole thing off on the right
pixel.

http://codereview.chromium.org/6286025/diff/2008/examples/graphics/life/life....
examples/graphics/life/life.cc:51: 0, 0, 1, 1, 0, 0, 0, 0,   // row for center
cell alive
On 2011/02/03 21:22:59, dmichael wrote:
> Lint is complaining that you should have >=2 spaces before the comment.
> 
> Otherwise, thanks for the change;  much clearer now.

Done.

http://codereview.chromium.org/6286025/diff/2008/examples/graphics/life/life....
examples/graphics/life/life.cc:158: }
On 2011/02/03 21:22:59, dmichael wrote:
> Still a little baffled by this, but let's stick with what works for now.

Done.

http://codereview.chromium.org/6286025/diff/2008/examples/graphics/life/life....
examples/graphics/life/life.cc:179: count = src0[-1] +     src0[0] +       
src0[1] +
On 2011/02/03 21:22:59, dmichael wrote:
> the 'Note that' part is no longer true with this latest fix.

Done.

http://codereview.chromium.org/6286025/diff/2008/examples/graphics/life/life.css
File examples/graphics/life/life.css (right):

http://codereview.chromium.org/6286025/diff/2008/examples/graphics/life/life....
examples/graphics/life/life.css:41: height: 400;
On 2011/02/03 21:22:59, dmichael wrote:
> Love it, looks much cooler this way.

Done.

[dmichael(do not use this one), 2011-02-03 22:40:01]

LGMT

[David Springer, 2011-02-03 22:51:14]

Thanks for all the great reviews!

Committed as rev 160.