[p5.js 2.0 Bug Report]: build*Shader methods don't work yet in instance mode

[davepagurek]

Most appropriate sub-area of p5.js?

• Accessibility
• Color
• Core/Environment/Rendering
• Data
• DOM
• Events
• Image
• IO
• Math
• Typography
• Utilities
• WebGL
• Build process
• Unit testing
• Internationalization
• Friendly errors
• Other (specify if possible)

p5.js version

2.2.1

Web browser and version

All

Operating system

All

Steps to reproduce this

Steps:

To use .modify() in instance mode, you have to call it like this:

baseMaterialShader.modify(({ p5 }) => {
  p5.getWorldInputs((inputs) => {
    inputs.position.x += 5
    return inputs
  })
}, { p5 })

Currently, the new build*Shader methods don't pass along the second context object:

p5.js/src/webgl/material.js

Lines 708 to 710 in 276a96a

	fn.buildFilterShader = function (callback) {
	return this.baseFilterShader().modify(callback);
	};

It needs to do so to be able to support that syntax, e.g.:

buildMaterialShader(({ p5 }) => {
  p5.getWorldInputs((inputs) => {
    inputs.position.x += 5
    return inputs
  })
}, { p5 })

[abhiraj75]

Hey @davepagurek , do you have any approach in mind?

[nbogie]

Hi @abhiraj75 , I can't speak for Dave, but here's what I suggest, as there are quite a few pieces involved here!

1. get comfortable with using p5.strands and buildFilterShader in global mode
2. check you are comfortable with p5 in instance mode
3. reproduce the described problem locally, writing a minimal complete sketch that uses the desired interface for one of the methods
4. create failing unit tests encompassing the previous step (but for all of the involved methods) - this will let you know when you're done!
5. check that there is unit test coverage for the methods when they're used normally in global mode, adding tests if necessary - this will help confirm that your eventual changes don't break what is currently working!
6. read the source for the mentioned functions, to build an understanding of why #3 fails
7. if required, ask questions (here or on discord) about how those functions work
8. propose how you might solve the problem (again, here or on discord)

The actual fix might be tiny, but that's of course just one piece of the whole. The other pieces, including building an understanding of the context, testing before and after, writing appropriate unit tests, and updating documentation, will likely form the bulk of the work on this one.

[aashu2006]

Hi @davepagurek @nbogie , I looked into this and it seems the issue is that the build*Shader methods call .modify(callback) but don’t forward the optional scope parameter. Since .modify(callback, scope) already supports passing something like { p5 } (which is needed in instance mode), the context is not reaching the callback when using instance mode.

A clean fix i think would be to add an optional scope parameter to all the build*Shader methods and simply forward it to .modify(callback, scope). It is a minimal and fully backwards compatible change, and it keeps the API flexible instead of hardcoding any context internally.

If this sounds good, I can go ahead and implement it along with proper instance-mode tests.
Thanks 🙂

[davepagurek]

That sounds good @aashu2006, go ahead!

[abhiraj75]

Thanks @nbogie !