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

Fix sincos implementation to cope with Emscripten #969

Merged
merged 1 commit into from
Nov 10, 2023

Conversation

serge-sans-paille
Copy link
Contributor

No description provided.

@serge-sans-paille serge-sans-paille changed the title Fix sincos implementation to cope with Emscripten [WIP] Fix sincos implementation to cope with Emscripten Nov 8, 2023
@serge-sans-paille
Copy link
Contributor Author

@JohanMabille / @anutosh491 just disabling the #ifdef seems to work, can you explain that?

@JohanMabille
Copy link
Member

JohanMabille commented Nov 9, 2023

Looks like the versions of Firefox and the SDKs that we use for testing have changed since we introduced this workaround, maybe the intrinsics have been fixed? Or maybe we are not testing what we think,meaning we don't build with WASM intrinsics, but rather with SSE2 that are then used to generate WASM? This latter seems highly improbable to me, but that's a possibility.

@serge-sans-paille
Copy link
Contributor Author

Anyway I double checked the sincos implementation compared to calling sin and cos, and it's exactly the same.

@JohanMabille
Copy link
Member

x-files-paranormal

@JohanMabille
Copy link
Member

(I have the feeling that I'm posting this GIF way too often, not a good sign)

I've double-checked and there is absolutely no difference between
calling sin then cos or just sincos. They perform the exact same
operation.
@serge-sans-paille serge-sans-paille changed the title [WIP] Fix sincos implementation to cope with Emscripten Fix sincos implementation to cope with Emscripten Nov 9, 2023
@serge-sans-paille
Copy link
Contributor Author

@JohanMabille I think it's ok to merge this patch, but I ultimately leave it to you.

@JohanMabille JohanMabille merged commit e9bd979 into master Nov 10, 2023
98 checks passed
@JohanMabille JohanMabille deleted the feature/accurate-sincos branch November 10, 2023 03:09
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.

2 participants