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

Supporting existing canvas element for createCanvas() and createGraphics() #6229

Merged
merged 5 commits into from Jun 30, 2023

Conversation

ffd8
Copy link
Contributor

@ffd8 ffd8 commented Jun 21, 2023

Resolves #4564

Changes: createCanvas() and createGraphics() (including p5.Graphics), to both accept passing a 3rd or 4th param (depending on if renderer is given) with an existing html canvas element. Within createGraphics(), it was necessary to both replace the defaultCanvas0 (created with setup) and tell p5 that the _defaultGraphicsCreated is false to re-initiate the renderer. With createGraphics(), in only required intercepting the creation of a canvas element which would have been applied to the body (or node).

createCanvas() changes
createGraphics() changes
p5.Graphics changes

PR Checklist

@davepagurek davepagurek self-requested a review June 22, 2023 13:04
@davepagurek
Copy link
Contributor

I think this looks good! I made an example showing the issue with setAttributes here: https://editor.p5js.org/davepagurek/sketches/XooCglkti The existing canvas has a style="background: blue" attribute that results in the background appearing blue at first. If you call setAttributes(), it recreates the canvas and that attribute is gone. This is a mostly contrived example, but I think the bigger issue is that whatever other library you were using with the other canvas now will no longer have a reference to a canvas in the DOM.

I think that's fine, because if your intention was to use this to inject p5 into another library's canvas, then we intentionally don't want to be messing with the attributes they've set up. So maybe we can just add a note in the docs about this? e.g.:

Optionally, an existing canvas can be passed using a selector, ie. document.getElementById(''). Avoid using setAttributes() afterwards, as this will remove and recreate the existing canvas.

Forgot to remove the alt-text of an example that was never added.
src/core/rendering.js Outdated Show resolved Hide resolved
Co-authored-by: Dave Pagurek <dave@davepagurek.com>
@Qianqianye Qianqianye merged commit 637e0cc into processing:main Jun 30, 2023
5 checks passed
@Qianqianye
Copy link
Contributor

Thanks @ffd8 for working on it and @davepagurek for reviewing!

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.

Supporting existing canvas elements for createGraphic
3 participants