Skip to content

Code improvements of the Perlin noise generator. #875

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

Merged
merged 1 commit into from
Aug 28, 2015
Merged

Code improvements of the Perlin noise generator. #875

merged 1 commit into from
Aug 28, 2015

Conversation

mgold
Copy link
Contributor

@mgold mgold commented Aug 28, 2015

When the top of the file lists a chain of ports and adaptations three projects deep, there's bound to be some cruft that accumulates.

  • Replace the cosine lookup table with Math.cos. This is simpler and does not affect the output or performance.
  • Rename noise_fsc to scaled_cosine, and move it out of noise so it's not constantly redefined.
  • Remove comments referencing the toxi bug tracker, from which this code was derived
  • Remove a few comments asking if something was ok - I either fixed it or agree it's ok

Regarding the removal of the cosine lookup table, this seems to have been ported from toxi without much thought. Lookup tables aren't necessarily an optimization in JS, especially because of the memory overhead (SO thread). It also had a half-dozen constants, some of them poorly named. I was able to replace all of this without affecting either to output of the generator or performance on this test sketch:

sideLen = 400;
noiseScale = 0.02;

function setup() {
  createCanvas(sideLen, sideLen)
  noFill();
  noLoop();
  noiseSeed(42);
}


function draw() {
  for (i = 0; i < sideLen; i++){
    for (j = 0; j < sideLen; j++){
      stroke(noise(i*noiseScale, j*noiseScale)*255);
      point(i,j);
    }
  }
}

* Replace the cosine lookup table with Math.cos. This is simpler and does not affect the output or performance.
* Rename `noise_fsc` to `scaled_cosine`, and move it out of `noise` so it's not constantly redefined.
* Remove comments referencing the toxi bug tracker, from which this code was derived
* Remove a few comments asking if something was ok - I either fixed it or agree it's ok
@shiffman
Copy link
Member

Thanks @mgold for this contribution! I did the original porting and you are absolutely right that my goal was to just quickly get things in p5.js and working. Now's a great time to revisit for improvements and optimizations. @lmccart, this pull request looks good to me, I'll wait for you to merge in case you have any further comments / feedback.

(As an aside to this, these threads are relevant in terms of whether we want to break from the original Processing implementation with more "current" Perlin noise implementations.)

processing/processing#2549
processing/processing-docs#51

lmccart pushed a commit that referenced this pull request Aug 28, 2015
Code improvements of the Perlin noise generator.
@lmccart lmccart merged commit 0b81d6d into processing:master Aug 28, 2015
@lmccart
Copy link
Member

lmccart commented Aug 28, 2015

beautiful, thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants