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

Need better blend state management in RendererGL #4562

Closed
stalgiag opened this issue May 19, 2020 · 2 comments
Closed

Need better blend state management in RendererGL #4562

stalgiag opened this issue May 19, 2020 · 2 comments

Comments

@stalgiag
Copy link
Contributor

There is problematic state management of _cachedBlendMode and curBlendMode in RendererGL. This was introduced by an optimization to RendererGL._applyBlendMode that tries to avoid unnecessary GL calls by caching the last applied blend mode. The problem is that erase() and noErase() also change _cachedBlendMode.

_cachedBlendMode is being used here to do two conceptually different things.

  1. only do a call to gl.blendFunc() when the new GL blend mode is different than the applied blend mode. This makes it so that if someone calls blendMode() every frame, it won't do unnecessary GL calls.
  2. It is used to store the blend mode before calls to erase() so that the previous blend mode can be applied after noErase().

To further complicate things, these two uses mutate _cachedBlendMode() asynchronously from one another because _applyBlendMode is only called upon rendering.

Another property that is something likepreEraseBlend could be added to RendererGL but then the _cachedBlendMode use between Renderer2D and RendererGL would diverge because Renderer2D only uses _cachedBlendMode to track pre/post eraser blends since it doesn't need the GL optimizations.

This is all to say that the blend state management in RendererGL is very tricky currently and probably should be restructured.

@Garima3110
Copy link
Member

I am trying to work on this issue.
Thankyou!

Garima3110 added a commit to Garima3110/p5.js that referenced this issue Jan 5, 2024
@Garima3110 Garima3110 mentioned this issue Jan 5, 2024
3 tasks
davepagurek added a commit that referenced this issue Jan 14, 2024
@davepagurek
Copy link
Contributor

I'm going to close this now that #6699 is merged in. Feel free to open more issues with further refactors!

@davepagurek davepagurek moved this from System Level Changes to DONE! 🎉 in p5.js WebGL Projects Jul 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: DONE! 🎉
Development

No branches or pull requests

3 participants