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

MathUtils: Make UUID RFC compliant. #23679

Merged
merged 2 commits into from
Mar 9, 2022
Merged

MathUtils: Make UUID RFC compliant. #23679

merged 2 commits into from
Mar 9, 2022

Conversation

Mugen87
Copy link
Collaborator

@Mugen87 Mugen87 commented Mar 8, 2022

Fixed #23677.

Description

Makes the UUID RFC compliant by not using uppercase letters.

@mrdoob
Copy link
Owner

mrdoob commented Mar 8, 2022

@takahirox I remember you looked into the memory implications of this.
Would you suggest we use toLowerCase() instead?

@mrdoob mrdoob added this to the r139 milestone Mar 8, 2022
@takahirox
Copy link
Collaborator

@takahirox I remember you looked into the memory implications of this.
Would you suggest we use toLowerCase() instead?

Perhaps, yes. But it was long ago when I looked into it. There is a chance that browsers optimize concatenated strings heap memory space now. I think we need to test again to decide whether we should use toLoweCase() or not.

@mrdoob
Copy link
Owner

mrdoob commented Mar 9, 2022

Should we just use toLowerCase() to be safe?

@takahirox
Copy link
Collaborator

Yeah, using it for now sounds good to me. We can test and remove it if needed later.

@mrdoob mrdoob merged commit f35120c into mrdoob:dev Mar 9, 2022
@mrdoob
Copy link
Owner

mrdoob commented Mar 9, 2022

Thanks!

@aardgoose
Copy link
Contributor

I have just checked and Chrome 99 still has the excess memory use problem if the to(Upper/Lower)Case is removed.

@mrdoob
Copy link
Owner

mrdoob commented Mar 14, 2022

@aardgoose thank you! 🙏

abernier pushed a commit to abernier/three.js that referenced this pull request Sep 16, 2022
* MathUtils: Make UUID RFC compliant.

* Update MathUtils.js

Co-authored-by: mrdoob <[email protected]>
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.

MathUtils.generateUUID() produces upper-case characters
4 participants