Skip to content

Add three.js benchmark #75

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

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

Conversation

iainireland
Copy link

No description provided.

Copy link

netlify bot commented May 27, 2025

Deploy Preview for webkit-jetstream-preview ready!

Name Link
🔨 Latest commit c8b5466
🔍 Latest deploy log https://app.netlify.com/projects/webkit-jetstream-preview/deploys/688a88e4029f71000858c5f3
😎 Deploy Preview https://deploy-preview-75--webkit-jetstream-preview.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@Constellation Constellation self-requested a review June 5, 2025 21:54

// Initialize particles
const particles = [];
for(let i = 0; i < NUM_PARTICLES; i++) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we put this in the init() method?
This might be nice in the future for getting some extra coverage if we add timers.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also wouldn't be opposed to having this be part of the first iteration since this seems like the setup a real three.js app would do.

@camillobruni
Copy link
Contributor

Generally LGTM.

Would it make sense to bump to the latest threejs version as well?
This would help comparing the profiles against the actual GPU rendered page a bit easier.

Copy link
Contributor

@kmiller68 kmiller68 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall looks good to me too but it needs to have a tag that runs by default. Maybe we should add a "Frameworks" tag? I'm ok with doing that in a follow up but I assume wasn't what you intended.


// Initialize particles
const particles = [];
for(let i = 0; i < NUM_PARTICLES; i++) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also wouldn't be opposed to having this be part of the first iteration since this seems like the setup a real three.js app would do.

@iainireland
Copy link
Author

I added the "Default" tag, updated to the latest version of three.js, and called the initialization as part of the first iteration.

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.

3 participants