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

Fixed uniform shader reset #7207

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

Forchapeatl
Copy link
Contributor

@Forchapeatl Forchapeatl commented Sep 3, 2024

Keeping track of bounded textures for later reuse thus , fixing uniform shader reset

Resolves #7030

Changes:

Screenshots of the change:

Before:

Apex_1725369970370.mp4

After:

Apex_1725371131798.mp4

PR Checklist

@Forchapeatl Forchapeatl marked this pull request as ready for review September 3, 2024 13:48
@Forchapeatl
Copy link
Contributor Author

Forchapeatl commented Sep 3, 2024

Hello @davepagurek . I am using a set data struckure to keep track of bounded textures. Please have a look at my PR. I also tested it on 5921 and it works as expected. There are some recordings of the changes in both cases.

@davepagurek
Copy link
Contributor

I think we still want to be setting the bound texture back to an empty texture in most cases (like you mentioned in #7030 (comment), this could just when a framebuffer is bound, but we can do it in all cases to be safe too.) With the current code, I made a copy of the test case from #5921 and it shows the same behaviour in Firefox on my mac where it just shows wireframes: https://editor.p5js.org/davepagurek/sketches/CMFeoUlHE

The problem is pretty platform- and driver-dependent, so it might be a harder one to test depending on your hardware. For me, Chrome displays that sketch just fine, but Firefox shows just wireframes and logs this warning:

WebGL warning: drawElementsInstanced: Texture level 0 would be read by TEXTURE_2D unit 0, but written by framebuffer attachment COLOR_ATTACHMENT0, which would be illegal feedback.

I think the challenge here will be to store two separate values on the uniform:

  • uniform.texture, the thing bound by WebGL
  • uniform.userTexture (as an example name), the thing the user last tried to store there

Some logic to do that:

  • set the texture back to an empty texture in unbindTextures (maybe only if the texture was a framebuffer, or just doing it all the time is fine too)
  • Somehow distinguish between setUniform from that unbind and setUniform called by the user, and only when the user calls it, record the bound texture
    • The thing to store is the value we would be putting in uniform.texture, which contains the texture data itself, not the texture index, which is just the slot it's being put into
    • By storing this as a property on the uniform itself, maybe as uniform.userTexture, we don't need to store the slot because the uniform itself represents that
    • One way to distinguish those two is to make a different copy of the method to be used by unbindTextures so it can have different logic
  • In bindTextures, apply uniform.userTexture if it exists, or uniform.texture otherwise

@Forchapeatl
Copy link
Contributor Author

uniform.userTexture (as an example name), the thing the user last tried to store there

Hi @davepagurek, it's been hard for me to get this value . Please for a few hints

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.

Image uniforms get reset after each draw using a shader
2 participants