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

Use float instead of double for altitudeAngle and azimuthAngle #485

Closed
wants to merge 1 commit into from

Conversation

jrandolf
Copy link

@jrandolf jrandolf commented Oct 5, 2023

Altitude angle and azimuth angle are computed always computed with 32-bit floating points. For example, on macOS, https://developer.apple.com/documentation/appkit/nsevent/1534226-tilt. A priori, using a double doesn't offer more precision.

[EDIT: On macOS, the NSPoint struct uses double when the OS is x64, so this implies double accuracy is correct]


Preview | Diff

@mustaqahmed
Copy link
Member

While I agree that the precision of a double may not be required here, it doesn't hurt either, right? I am worried about the non-zero compat risk that some code in the wild could break because of some assumption around precision.

More on that compat point: we picked double here from the TouchEvent spec (where it has been like this for years). What if there is a comparison between altitude angles of a TouchEvent and the corresponding PointerEvent that assumes an 8-digit precision (i.e. "finer" than float)? This may sound impossible but consider this closely related experience: we failed to even add precision to click coordinates because the integer coordinates of a MouseEvent was being compared with the coordinates of a click in a popular mapping website!

Without a clear benefit, we should avoid such a risk IMHO.

@jrandolf jrandolf closed this Oct 9, 2023
@jrandolf jrandolf deleted the jrandolf/angle branch October 9, 2023 01:04
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.

None yet

2 participants