Consider replacing seeds with rngs

[Ralith]

32-bit seeds aren't great, and pervasive use of mostly-0 seeds may be causing odd results already. I propose that impl Seedable for T be replaced with impl Distribution<T> for Standard per rand 0.5 convention, with Default impls and new using an XorShiftRng with a hardcoded seed for reproducibility and convenience.

This would save users and implementers the effort of having to come up with a good seed (particularly when unpredictable results are desired) and improves the quality of randomness used at no cost. I'm interested in implementing this if people are on board with the idea. The changes involved might look like this:

diff --git a/src/noise_fns/generators/super_simplex.rs b/src/noise_fns/generators/super_simplex.rs
index 34c52fc..fdba524 100644
--- a/src/noise_fns/generators/super_simplex.rs
+++ b/src/noise_fns/generators/super_simplex.rs
@@ -1,6 +1,7 @@
+use rand::{distributions::{Distribution, Standard}, Rng};
 use {gradient, math};
 use math::{Point2, Point3};
-use noise_fns::{NoiseFn, Seedable};
+use noise_fns::{NoiseFn, default_rng};
 use permutationtable::PermutationTable;
 use std::ops::Add;

@@ -89,17 +90,14 @@ const LATTICE_LOOKUP_3D: [[i8; 3]; 4 * 16] =
 /// Noise function that outputs 2/3-dimensional Super Simplex noise.
 #[derive(Clone, Copy, Debug)]
 pub struct SuperSimplex {
-    seed: u32,
     perm_table: PermutationTable,
 }

 impl SuperSimplex {
-    pub const DEFAULT_SEED: u32 = 0;
-
     pub fn new() -> Self {
+        let mut rng = default_rng();
         SuperSimplex {
-            seed: Self::DEFAULT_SEED,
-            perm_table: PermutationTable::new(Self::DEFAULT_SEED),
+            perm_table: rng.gen(),
         }
     }
 }
@@ -110,23 +108,9 @@ impl Default for SuperSimplex {
     }
 }

-impl Seedable for SuperSimplex {
-    /// Sets the seed value for Super Simplex noise
-    fn set_seed(self, seed: u32) -> Self {
-        // If the new seed is the same as the current seed, just return self.
-        if self.seed == seed {
-            return self;
-        }
-
-        // Otherwise, regenerate the permutation table based on the new seed.
-        SuperSimplex {
-            seed,
-            perm_table: PermutationTable::new(seed),
-        }
-    }
-
-    fn seed(&self) -> u32 {
-        self.seed
+impl Distribution<SuperSimplex> for Standard {
+    fn sample<R: Rng + ?Sized>(&self, rng: &mut R) -> SuperSimplex {
+        SuperSimplex { perm_table: rng.gen() }
     }
 }

diff --git a/src/noise_fns/mod.rs b/src/noise_fns/mod.rs
index 82d5924..503b88c 100644
--- a/src/noise_fns/mod.rs
+++ b/src/noise_fns/mod.rs
@@ -1,3 +1,5 @@
+use rand::{SeedableRng, XorShiftRng};
+
 pub use self::cache::*;
 pub use self::combiners::*;
 pub use self::generators::*;
@@ -44,3 +46,7 @@ pub trait Seedable {
     /// Getter to retrieve the seed from the function
     fn seed(&self) -> u32;
 }
+
+const DEFAULT_SEED: [u8; 16] = [0x4f, 0x09, 0xd6, 0x9f, 0x62, 0x9b, 0x09, 0x0c, 0x0c, 0x49, 0x09, 0xfe, 0x6f, 0x1d, 0x4a, 0x38];
+
+fn default_rng() -> XorShiftRng { XorShiftRng::from_seed(DEFAULT_SEED) }

[Razaekel]

The purpose of having seeds is to provide reproducible results by using the same seed while giving users the ability to specify the seed as part of their generation process. Having a hard-coded seed would defeat the purpose of this library.

[Ralith]

Sorry, I must have been unclear. The library already contains many hard-coded seeds. This proposal contains two parts: improving the hard-coded seeds, and adjusting, not removing, the interface by which the user controls dynamic seeding. As outlined in the patch above, instead of supplying a u32, the user would provide a &mut T where T: Rng which is then used to initialize the noise function's internal state as appropriate. The user can seed an RNG in whatever manner suits them (from a constant, from storage, from entropy, ...).

[Razaekel]

Some discussion with Ralith indicated that I may have been misinterpreting the proposal, so I'm re-opening it for further discussion and expanding the details.

[Ralith]

Seeds in noise-rs are used to initialize internal RNGs, which are in turn used to construct noise function internal states (currently just permutation tables, maybe other things in the future?). The heart of this proposal is to instead allow the user to pass in a reference to a user-constructed RNG and use that directly, via the standard rand API. To construct a non-default instance of a noise function, a user would first construct some RNG, then call gen on it. For example:

let mut rng = XorShiftRng::from_seed(seed);
let noise = rng.gen::<SuperSimplex>();
noise.get(...);

This has a number of benefits:

• Randomness A 32-bit seed is small; even the low-quality XorShiftRng is designed to take 128-bit seeds. This proposal allows the user to use whatever RNG suits them, and seed it in its preferred manner. No future API changes would be necessary to support new RNGs with different seeding requirements. While quality of randomness used may not currently be the dominant factor in the quality of noise functions, the existing interface nonetheless imposes an unnecessary and easily-corrected bottleneck.
• Composability A number of noise functions are defined in terms of inner noise functions, and composing noise functions is in general a popular approach to obtaining high-quality output. Attempting to construct an composition of noise functions from a single seed using the current interface is awkward; using the same seed, or simple variations of it, for each one produces undesirable correlated output. By contrast, passing a reference to the same RNG through each component noise function's constructor preserves the full strength of the statistical independence guarantees provided by the RNG.
• Ease of use Applications which require multiple noise functions and/or additional random data can generate as much as they like based on a single seed by making repeated calls to gen. Users who want irreproducible results can use rand::thread_rng(). Users who don't care about any of this can still call Default::default() for high-quality deterministic output.

The proposed interface is not mutually exclusive with the existing Seedable trait. If desired, it could be retained, although the seed getter could not return meaningful results for functions generated via the proposed method. For the above reasons I think it should at least be marked deprecated.

[v0x0g]

What about a combined approach?

I was thinking of providing some default method such as Seedable::new_seed() that generates a high quality seed using the OsRng. This would be called by default when constructing the structs, but could be modified afterwards. The noise generators like noise::Perlin already have private fields, so this should not be a breaking change, but it should significantly increase the quality of generators since they won't have a fixed default seed all the time.

I still support @Ralith with this one though, I think adding RNGs would be a good idea and I would personally like it if it were implemented.