Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move permutation table creation to a separate class method when using own PRNG #47

Closed
dmnsgn opened this issue Feb 25, 2022 · 3 comments

Comments

@dmnsgn
Copy link

dmnsgn commented Feb 25, 2022

Feature request

Move the following into a SimplexNoise.init() (or other name) method:

this.p = buildPermutationTable(random);
this.perm = new Uint8Array(512);
this.permMod12 = new Uint8Array(512);
for (let i = 0; i < 512; i++) {
this.perm[i] = this.p[i & 255];
this.permMod12[i] = this.perm[i] % 12;
}
}

Use case

When updating the random number generator, we are forced to recreate an instance of SimplexNoise. With this, we could create a single instance and reinit it if the PRNG changed:

// Use default Math.random (non predictable)
let simplex = new SimplexNoise(Math.random);

function setSeed(s) {
  // Update seed to make Math.random predictable
  seedMathRandom(s)
  
  // Before:
  // Recreate noise instance
  simplex = new SimplexNoise(Math.random);

  // After proposed change:
  // Reset permutation table
  simplex.init(Math.random);
}

setSeed('0')

Let me know how that sounds and if you need a PR.

@jwagner
Copy link
Owner

jwagner commented Feb 25, 2022

Hey Damien,

Thanks for the feature request. I don't quite understand your use case yet. A simplex noise instance contains nothing but the permutation table.

Why do you need to reinitialize and existing instance of simplex noise, why couldn't you just recreate it?

On a side note, I would strongly advice against 'monkey patching' the built in Math.random. Patching built-ins can disable optimizations and break third party code. Just pass in your own random function.

Cheers,
Jonas

@dmnsgn
Copy link
Author

dmnsgn commented Feb 27, 2022

A simplex noise instance contains nothing but the permutation table.
Why do you need to reinitialize and existing instance of simplex noise, why couldn't you just recreate it?

It is true, I could recreate it, the feature request is probably a very small optimisation here eg. not having to instance the SimplexNoise class when changing the seed function (which sometimes happens often in creative-coding land). My assumption is that keeping the instance would be faster than recreate it (but it might be negligible).

Just pass in your own random function.

Yep 100% right, I am trying to change that too in our lib.

@jwagner
Copy link
Owner

jwagner commented Jul 23, 2022

I'm closing this issue since there aren't any more classes in 4.0.

@jwagner jwagner closed this as completed Jul 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants