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

erase() and noErase() support in framebuffers #6523

Closed
acamposuribe opened this issue Nov 3, 2023 · 12 comments · Fixed by #6524
Closed

erase() and noErase() support in framebuffers #6523

acamposuribe opened this issue Nov 3, 2023 · 12 comments · Fixed by #6524

Comments

@acamposuribe
Copy link
Contributor

acamposuribe commented Nov 3, 2023

Topic

I'm trying to execute erase() and noErase() within the framebuffer.begin() and framebuffer.end() functions, but they seem to be ignored. Bug or feature missing?
Thanks in advance!

Copy link

welcome bot commented Nov 3, 2023

Welcome! 👋 Thanks for opening your first issue here! And to ensure the community is able to respond to your issue, please make sure to fill out the inputs in the issue forms. Thank you!

@davepagurek
Copy link
Contributor

Sounds like a bug! Are you able to make a minimal example in a p5 editor sketch to debug with?

@acamposuribe
Copy link
Contributor Author

Sounds like a bug! Are you able to make a minimal example in a p5 editor sketch to debug with?

Yes! There you have it:
https://editor.p5js.org/acamposuribe/sketches/gaVBVGKu-

The erase() function is erasing the main canvas, rather than the framebuffer.

@davepagurek
Copy link
Contributor

If I comment out the last line with the image() call, then the black background is still preserved. I think the bug isn't that it's erasing the main canvas, but that erase is actually replacing with fully opaque white. Which is definitely still a bug!

@acamposuribe
Copy link
Contributor Author

If I comment out the last line with the image() call, then the black background is still preserved. I think the bug isn't that it's erasing the main canvas, but that erase is actually replacing with fully opaque white. Which is definitely still a bug!

Yes, you're definitely right!

@davepagurek
Copy link
Contributor

Looks like the bug is here:

this._applyBlendMode(constants.REMOVE);

_applyBlendMode just updates the WebGL blending to match what p5 has set it to, and doesn't actually take any arguments. I think this should be just blendMode(REMOVE), which seems to work:
https://editor.p5js.org/davepagurek/sketches/DvUOkmTwM

@acamposuribe
Copy link
Contributor Author

acamposuribe commented Nov 3, 2023

This makes the whole framebuffer white, though (except for the erased part)?

@davepagurek
Copy link
Contributor

You're right! Just updated the sketch again, looks like we still have to call _applyBlendMode() after setting the blend mode.

@acamposuribe
Copy link
Contributor Author

acamposuribe commented Nov 3, 2023

You're right! Just updated the sketch again, looks like we still have to call _applyBlendMode() after setting the blend mode.

Yes! Although noErase() was not working, because blendMode was not reverting back to the original. We need to store the current blendmode and pass it at the end. I made a bad fix:

https://editor.p5js.org/acamposuribe/sketches/rBOTxXyvu

@davepagurek
Copy link
Contributor

Thanks! Are you interested in combining the changes in that sketch into a PR?

@acamposuribe
Copy link
Contributor Author

Thanks! Are you interested in combining the changes in that sketch into a PR?

I'm pretty new to OpenSource. Currently creating a p5 library as a first contribution to the project.
Never made a pull request before....

@davepagurek
Copy link
Contributor

If youre interested, I can help walk you through it! We also have a p5 discord where you can ask questions faster. If you'd rather not though, that's also totally fine!

davepagurek added a commit that referenced this issue Nov 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants