[p5.js 2.0 Bug Report]: p5.js overrides uSampler uniform for user shaders

[davepagurek]

Most appropriate sub-area of p5.js?

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

p5.js version

2.0.5 (2.0+)

Web browser and version

All

Operating system

All

Steps to reproduce this

Steps:

1. Create a shader using a uniform called uSampler
2. Set a value for it on the shader
3. Draw something using the shader Instead of using the provided sampler value, it gets reset to be empty.

Snippet:

In 1.11.0 this draws a red rectangle. In 2.0.5 this is an empty canvas:

function setup() {
  createCanvas(400, 400, WEBGL);
  let myShader = createFilterShader(`precision highp float;
uniform sampler2D uSampler;
varying vec2 vTexCoord;

void main() {
  gl_FragColor = texture2D(uSampler, vTexCoord);
}
  `)

  let fbo = createFramebuffer()
  fbo.draw(() => background('red'))
  
  shader(myShader)
  myShader.setUniform('uSampler', fbo)
  noStroke()
  plane(width, height)
}

https://editor.p5js.org/davepagurek/sketches/kF84pt-Am

That's because this code gets run after the user's setUniform, overriding it: https://github.com/processing/p5.js/blob/f78009a93d896daa670e7199c9737063a0c6bedb/src/webgl/p5.RendererGL.js#L2378-L2383

It's being set back to an empty texture for good reason, as mentioned in the comments in the code, but we shouldn't do that if it's a user shader and has already had a value set.

[lokesh-singhal]

Hey @davepagurek , I like to work on this issue. Can you please assign it to me? I plan to implement the fix using a flag-based solution.

[davepagurek]

Thanks @lokesh-singhal! Are you thinking of tracking when a user calls shader.setUniform('uSampler', something)?

[lokesh-singhal]

Hi @davepagurek , I’ve implemented the fix to prevent uSampler from being overwritten by using a flag that detects when the user has manually set the texture. However, I’m encountering screenshot mismatches in the visual tests after running npm test.

Additionally, I wanted to confirm whether internalOverdrive should be handled or tested as part of this change, since it may affect the texture pipeline.

Could you please guide me on the next steps—should I regenerate the screenshots or adjust the implementation?

Thanks!

[davepagurek]

Which screenshot tests are failing? There are two that are known to be sorta flaky because they involve loading fonts from Google Fonts -- those are probably not caused by your changes, and can be ignored. If other ones are failing, it's likely that it was caused by a code change (you can also try switching your code back to the dev-2.0 branch to see if the same tests fail for you without any changes as a way of being sure you didn't cause them.)

Additionally, I wanted to confirm whether internalOverdrive should be handled or tested as part of this change, since it may affect the texture pipeline.

What do you mean by internalOverride?

[lokesh-singhal]

Hi @davepagurek , I’d like to request to be unassigned from this issue. I’ve been working on the fix but I’m currently unable to make further progress, especially with the framebuffer and texture binding parts in p5.RendererGL.

I think it would be best to let someone else take this forward. Thank you for the opportunity and for your guidance so far!

[davepagurek]

No problem @lokesh-singhal, thanks for your help so far!

[Harsh16gupta]

Hi @davepagurek, I’d like to take this on. I’ve gone through the discussion and looked into the code path where uSampler gets reset, and I’m confident I can put together a clean fix. Just let me know if it’s alright for me to move forward with it.