Add p5.strands support for randomGaussian()#8800
Conversation
Continuous ReleaseCDN linkPublished PackagesCommit hash: 055554d Previous deploymentsThis is an automated message. |
| augmentFn(fn, p5, 'randomGaussian', function(...args){ | ||
| if(!strandsContext.active){ | ||
| return originalRandomGaussian.apply(this, args); | ||
| } | ||
| const mean = args.length >= 1 ? args[0] : 0; | ||
| const stdDev = args.length>=2 ? args[1] : 1; | ||
|
|
||
| const u1 = this.random(); | ||
| const u2 = this.random(); | ||
| const z = this.sqrt(this.log(u1).mult(-2)).mult(this.cos(u2.mult(2*Math.PI))); | ||
|
|
||
| return z.mult(p5.strandsNode(stdDev)).add(p5.strandsNode(mean)); | ||
| }); |
There was a problem hiding this comment.
Hi, thanks for the work @harshiltewari2004 , just one concern I have: are you defining randomGaussian inside the random function intentionally? Do you think it should be outside, otherwise randomGaussian won't exist until the user calls random() at least once, and it'll get redefined on every random() call.
Also, can you ad tests for this implementation?
There was a problem hiding this comment.
Good catch, thanks @perminder-17 — fixed in the latest commit. The randomGaussian augmentation is now a sibling of random, not nested inside it.
|
Happy to add tests @perminder-17 — I noticed there's no test/unit/strands/ directory yet, so I held off. Should I establish that pattern here, or is there an existing test approach for strands I missed? |
2 participants
Resolves #8775
Changes
Adds
randomGaussian()support for p5.strands, following @davepagurek's guidance in #8775. The implementation is a JS-side wrapper over the existingrandom()strands function, applying a Box-Muller transform to two uniform samples to produce a normal-distributed sample.src/strands/strands_api.jsnext to the existingrandom()registration.randomGaussian()when called outside a strands shader context.this.random()calls + chained node math.Implementation note
Uses
this.x()rather thanfn.x()per @davepagurek's note in the issue thread.Testing
No unit tests included — there's no existing
test/unit/strands/directory and no clear precedent for unit-testing strands functions (which produce IR nodes rather than directly-testable values). Happy to add tests in whichever direction the maintainers prefer.Local
npm testshows the same 4 flaky visual regression test failures (test/unit/visual/cases/typography.js) that I observed on dev-2.0 before my changes — unrelated to this PR.PR Checklist
npm run lintpasses