Skip to content

Use DOMContentLoaded instead of onload for initializing p5 #3347

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

Closed
wants to merge 2 commits into from
Closed

Use DOMContentLoaded instead of onload for initializing p5 #3347

wants to merge 2 commits into from

Conversation

calebegg
Copy link
Contributor

Documentation:
https://developer.mozilla.org/en-US/docs/Web/Events/DOMContentLoaded

MDN says:

It is an incredibly common mistake to use load where DOMContentLoaded would be much more appropriate

DOMContentLoaded is supported in all modern browsers (IE9+), and can make a
significant difference in page load performance when a page has many
external assets, such as images and iframes.

/cc @jesse-harding

Documentation:
https://developer.mozilla.org/en-US/docs/Web/Events/DOMContentLoaded

Quoth MDN: “A very different event load should be used only to detect a
fully-loaded page. It is an incredibly common mistake to use load where
DOMContentLoaded would be much more appropriate, so be cautious.”

DOMContentLoaded is supported in all modern browsers, and can make a
significant difference in page load performance when a page has many
external assets, such as images and iframes.
@limzykenneth
Copy link
Member

Are we sure we don't need any images or stylesheets that is requested in the DOM before p5 starts? I'm not entirely sure myself but we need to make sure that is the case first.

@outofambit
Copy link
Contributor

Are we sure we don't need any images or stylesheets that is requested in the DOM before p5 starts? I'm not entirely sure myself but we need to make sure that is the case first.

@calebegg any thoughts on this? i'm doing a little research to see if this makes sense for p5.

@calebegg
Copy link
Contributor Author

It's up to you. I wrote this patch for a friend and he's used it successfully to significantly speed up his site, so I decided to offer it to you as a courtesy. I've included a link to MDN's explanation of the differences.

I cannot imagine a scenario where this change would break a p5 sketch, but I've experienced breakages with even the "safest" of changes in large projects, as I'm sure have y'all.

@Spongman
Copy link
Contributor

my only concern would be drawing text in setup() using css-loaded fonts. although, i'm not even sure if this works right now...

@meiamsome
Copy link
Member

@Spongman wouldn't this kick off the draw loop too, even with unloaded assets, which might be problematic.

This could also be a problem for people who are perhaps loading data from image or video elements that are included in the page, right?

Perhaps we could have a loadPage() preload method that forces p5 to wait for the load event on the page as an opt-in to ensure all such assets exist.

@outofambit
Copy link
Contributor

thanks @calebegg!

This could also be a problem for people who are perhaps loading data from image or video elements that are included in the page, right?

this is my main concern, too @meiamsome. 🤔 i'll put together a test case soon

@Spongman
Copy link
Contributor

Spongman commented Feb 24, 2019

i believe HTMLVideoElement only delays the load event if its delaying-the-load-event attribute is true.

@stalgiag
Copy link
Contributor

Hm I feel like this doesn't technically speed anything up, it just changes the order for p5 to start. This test seems like a good explanation of the difference. Should p5 wait for external assets to load before initializing? I kind of feel that it should. I think it would be rare for someone to need a p5 sketch to load first on a page with a ton of external assets. Whereas I can imagine it being more common for someone to try to grab pixel data from a DOM img in setup. I think it is more beginner friendly to handle the latter case.

@jesse-harding
Copy link

jesse-harding commented Feb 25, 2019 via email

@stalgiag
Copy link
Contributor

Ah gotchya thanks for the example. I agree that a preload method would be cool. I guess the question then is which should be the default behavior. Load first as in this PR or wait for all external assets in the page to load as in current behavior? There isn't an obvious favorite to me.

@jesse-harding
Copy link

jesse-harding commented Feb 25, 2019 via email

@stalgiag
Copy link
Contributor

Thanks for the PR and discussion starter @calebegg. I am closing this for now as I opened an issue that points to the feature request discussed in this PR.

@stalgiag stalgiag closed this Feb 25, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants