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 integers when calling canvas draw methods #62

Closed
wants to merge 1 commit into from

Conversation

taneliang
Copy link
Member

Part of #50.

Not sure if this actually does anything useful as our performance metrics aren't precise enough, but using integers instead of floating points is recommended by quite a few online sources including https://developer.mozilla.org/en-US/docs/Web/API/Canvas_API/Tutorial/Optimizing_canvas#Avoid_floating-point_coordinates_and_use_integers_instead

Pushing this PR separately from more major work as I think I'm out of tiny optimizations. The next few PRs are likely going to be large ones.

Poked app, everything still works.

@taneliang taneliang requested a review from kartikcho July 9, 2020 04:29
@kartikcho
Copy link
Member

Wouldn't this make things slightly inaccurate or smaller?

@taneliang
Copy link
Member Author

Didn't really think about that, but I think subpixel rendering would appear to be more inaccurate to users?

@kartikcho
Copy link
Member

You're rounding all values and their computed values, I think you should definitely take a look if this is causing some things to look misaligned. You would have to zoom in a lot to find the misalignments though.

@taneliang
Copy link
Member Author

You're right, some things do appear to be misaligned. This isn't worth the perf improvements then, if there are any.

https://scheduling-profiler-prototype-fp8vn112i.vercel.app/

@taneliang taneliang closed this Jul 9, 2020
@taneliang taneliang deleted the eliang/canvas-optimize-1 branch July 9, 2020 06:23
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