Add additional random number generator

Add additional random number generator.

This adds SkMWCRandom, which generates random numbers using George Marsaglia's multiply-with-carry method. The current linear congruent generator method fails all three of the tuftests suite (http://www.cs.hku.hk/cisc/projects/va/index.htm#download), whereas this one passes all three. Because this stores four values instead of one, it's not possible to return a single seed for getSeed(), so that functionality has been changed to getState() and setState(). The methods nextF() has also been changed to use a faster method of generating numbers in the interval [0,1). Otherwise, the remaining methods are identical.

[reed1, 2013-01-29 16:29:35]

https://codereview.appspot.com/7235056/diff/1/include/utils/SkRandom.h
File include/utils/SkRandom.h (right):

https://codereview.appspot.com/7235056/diff/1/include/utils/SkRandom.h#newcod...
include/utils/SkRandom.h:164: struct State {
can we make these private (to everyone but SkMWCRandom?

https://codereview.appspot.com/7235056/diff/1/include/utils/SkRandom.h#newcod...
include/utils/SkRandom.h:169: SkMWCRandom(uint32_t seed) { init(seed); }
constructor that takes a State?

https://codereview.appspot.com/7235056/diff/1/include/utils/SkRandom.h#newcod...
include/utils/SkRandom.h:287: */
Is memcpy needed? Can't you just say fState = *state;

https://codereview.appspot.com/7235056/diff/1/include/utils/SkRandom.h#newcod...
include/utils/SkRandom.h:306: }
Should this guy be a static function?

[JimVV, 2013-01-29 19:07:24]

In addition to the changes below, I also fixed up the init() method. This random
number generator is sensitive to having its seeds set to 0, so I added a check
for that. The alternative is to store six 
values, which is a step in the wrong direction on the time/size scale.

As a side note, PictureTest's test_peephole() stores a copy of the seed every
iteration but does nothing with it. I'm not sure what the correct behavior is
there.

And I just realized that an alternative to storing/restoring the State variable
is to add a copy constructor and operator= to SkMWCRandom. I'd probably do that
instead, but I'm not sure which is more Skia-like.

On 2013/01/29 16:29:35, reed1 wrote:
> https://codereview.appspot.com/7235056/diff/1/include/utils/SkRandom.h
> File include/utils/SkRandom.h (right):
> 
>
https://codereview.appspot.com/7235056/diff/1/include/utils/SkRandom.h#newcod...
> include/utils/SkRandom.h:164: struct State {
> can we make these private (to everyone but SkMWCRandom?

Done.
 
>
https://codereview.appspot.com/7235056/diff/1/include/utils/SkRandom.h#newcod...
> include/utils/SkRandom.h:169: SkMWCRandom(uint32_t seed) { init(seed); }
> constructor that takes a State?

Done.
 
>
https://codereview.appspot.com/7235056/diff/1/include/utils/SkRandom.h#newcod...
> include/utils/SkRandom.h:287: */
> Is memcpy needed? Can't you just say fState = *state;

Done.
 
>
https://codereview.appspot.com/7235056/diff/1/include/utils/SkRandom.h#newcod...
> include/utils/SkRandom.h:306: }
> Should this guy be a static function?

Done.

[JimVV, 2013-01-29 19:31:51]

Updated to change getState/setState to copy constructor and assignment operator.

On 2013/01/29 19:07:24, JimVV wrote:
> In addition to the changes below, I also fixed up the init() method. This
random
> number generator is sensitive to having its seeds set to 0, so I added a check
> for that. The alternative is to store six 
> values, which is a step in the wrong direction on the time/size scale.
> 
> As a side note, PictureTest's test_peephole() stores a copy of the seed every
> iteration but does nothing with it. I'm not sure what the correct behavior is
> there.
> 
> And I just realized that an alternative to storing/restoring the State
variable
> is to add a copy constructor and operator= to SkMWCRandom. I'd probably do
that
> instead, but I'm not sure which is more Skia-like.
> 
> On 2013/01/29 16:29:35, reed1 wrote:
> > https://codereview.appspot.com/7235056/diff/1/include/utils/SkRandom.h
> > File include/utils/SkRandom.h (right):
> > 
> >
>
https://codereview.appspot.com/7235056/diff/1/include/utils/SkRandom.h#newcod...
> > include/utils/SkRandom.h:164: struct State {
> > can we make these private (to everyone but SkMWCRandom?
> 
> Done.
>  
> >
>
https://codereview.appspot.com/7235056/diff/1/include/utils/SkRandom.h#newcod...
> > include/utils/SkRandom.h:169: SkMWCRandom(uint32_t seed) { init(seed); }
> > constructor that takes a State?
> 
> Done.
>  
> >
>
https://codereview.appspot.com/7235056/diff/1/include/utils/SkRandom.h#newcod...
> > include/utils/SkRandom.h:287: */
> > Is memcpy needed? Can't you just say fState = *state;
> 
> Done.
>  
> >
>
https://codereview.appspot.com/7235056/diff/1/include/utils/SkRandom.h#newcod...
> > include/utils/SkRandom.h:306: }
> > Should this guy be a static function?
> 
> Done.

[reed1, 2013-01-30 13:03:46]

lgtm w/ Capitalization nit

https://codereview.appspot.com/7235056/diff/8001/include/utils/SkRandom.h
File include/utils/SkRandom.h (right):

https://codereview.appspot.com/7235056/diff/8001/include/utils/SkRandom.h#new...
include/utils/SkRandom.h:312: }
Capitalize Static Functions

[JimVV, 2013-01-30 14:19:27]

One last pass.... I figured out last night how to simply the generator down to
two variables. This is actually a variation on the most basic Mother-of-All
generator that I'm surprised Marsaglia didn't try, as it does a better job of
mixing the lower bits. Perhaps I'm missing something, but in any case it passes
the tuftests suite.

On 2013/01/30 13:03:46, reed1 wrote:
> lgtm w/ Capitalization nit
> 
> https://codereview.appspot.com/7235056/diff/8001/include/utils/SkRandom.h
> File include/utils/SkRandom.h (right):
> 
>
https://codereview.appspot.com/7235056/diff/8001/include/utils/SkRandom.h#new...
> include/utils/SkRandom.h:312: }
> Capitalize Static Functions

[reed1, 2013-01-30 14:57:17]

Do we need to rename it to include your initials as well?

lgtm