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

Image uniforms get reset after each draw using a shader #7030

Open
1 of 17 tasks
nakednous opened this issue May 9, 2024 · 10 comments
Open
1 of 17 tasks

Image uniforms get reset after each draw using a shader #7030

nakednous opened this issue May 9, 2024 · 10 comments

Comments

@nakednous
Copy link
Contributor

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

1.6.0 onwards

Web browser and version

No response

Operating system

No response

Steps to reproduce this

I've implemented shadow mapping in p5.js v1.5.0 using p5.Graphics and the p5.treegl library. While this works seamlessly in v1.5.0, I'm facing challenges with versions from p5.js v1.6.0 onwards. I am planning to transition to using p5.Framebuffer once this issue is resolved.

Steps to Reproduce:

  1. Run the shadow mapping implementation in p5.js v1.5.0 here.
  2. Attempt the same using p5.js v1.9.3 here.

I've verified that the p5.treegl commands function identically in both versions. Notably, p5.js v1.6.0 introduced significant updates to shader-related features. I'm unsure if this issue qualifies as a bug or if I should report it or ask for help at the p5.js forum.

@davepagurek, could you possibly take a look and provide some guidance?

@davepagurek
Copy link
Contributor

Thanks for catching this! After #5923, textures are unbound when the shader is unbound to prevent some other bugs (keeping textures bound when not using them causes a lot of other downstream bugs). However, we unbind the shader immediately after each draw, so only the first shape has the depth texture bound, and then subsequent shapes have an empty texture. Right now, it means I guess you'll have to re-apply the uniform before each shape you draw.

To fix this in p5, maybe we need to keep track of what textures shader had bound the last time it was used so that we can re-bind those values when you use the shader again. That way textures will work like other uniforms.

@nakednous
Copy link
Contributor Author

Thanks for the suggestion! I can confirm that reapplying the depth uniform before rendering each shape serves as a workaround.

@davepagurek davepagurek changed the title shadow mapping Image uniforms get reset after each draw using a shader May 21, 2024
@davepagurek
Copy link
Contributor

If anyone is interested in taking this on, I think what we would need to do is adjust setUniform to store the last set texture on the shader:

case gl.SAMPLER_2D:
gl.activeTexture(gl.TEXTURE0 + uniform.samplerIndex);
uniform.texture =
data instanceof p5.Texture ? data : this._renderer.getTexture(data);

...and then re-apply the last used texture value, if it exists, when you bind the shader again in _setFillUniforms and _setStrokeUniforms:

_setFillUniforms(fillShader) {
fillShader.bindShader();

One complication: since this gets called after each draw finishes, this call below would end up storing an empty image when we really just want it to unbind the current image:

unbindTextures() {
for (const uniform of this.samplers) {
this.setUniform(uniform.name, this._renderer._getEmptyTexture());
}
}

So maybe unbindTextures should just do this bit instead of calling setUniform?

uniform.texture =
data instanceof p5.Texture ? data : this._renderer.getTexture(data);

@JordanSucher
Copy link
Contributor

I tried to recreate #5923 so I could understand the issues that arise if we don't unbind a texture, but I found that when I commented out the unbindTextures() call, it didn't seem to cause any problems - tried the sketch from that issue, fill renders as expected.

Is it possible we don't need to have that call anymore?

red.cube.mov

@davepagurek
Copy link
Contributor

I made a new sketch on the most recent p5 here: https://editor.p5js.org/davepagurek/sketches/pwIAXyF0X

With the implementation of unbindTextures empty, the fills still stop working for me and I get the same error about framebuffer feedback in the console.

@JordanSucher
Copy link
Contributor

very strange - that sketch works fine for me across chrome, safari, arc

@nakednous
Copy link
Contributor Author

I tested this on Arch Linux.
It fills the texture in both Chromium and Falkon, but in Firefox it only does it the first frame (mousePress to reveal it).

@davepagurek
Copy link
Contributor

It's a weird situation because it's left up to the driver developers to determine what's considered feedback or not, so on some platforms it works. Unfortunately we mostly have to program to the lowest common denominator, and for now that means cleaning up our textures when not using them.

@nakednous
Copy link
Contributor Author

weirdly enough leaving unbindTextures empty, solves the shadow mapping issue here even in firefox.

@davepagurek
Copy link
Contributor

That makes sense, there isn't any framebuffer feedback going on in that sketch so in that case there isn't a need to unbind anything. Unbinding is only required for a few cases where there would be a cycle between the bound texture and the texture being written to, which basically only happens if a framebuffer is involved.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Bugs with Solutions (No PR or test)
Development

No branches or pull requests

3 participants