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

Arc-related fix for NaN arguments #30

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

Conversation

ShaneSP
Copy link

@ShaneSP ShaneSP commented May 9, 2023

Purpose: This is not intended to be a functional pull request, instead we needed to make this change for our purposes to support arcs in certain conditions. We thought creating the pull request would be helpful to see for maintainers to see the changes.

Context: We're using react-konva as our canvas engine when generating SVGs.

There seem to be two issues:

  1. Context.prototype.translate
    • x and y parameters were NaN in some cases. Using x || 0 seemed to be a reasonable hot-fix for us. It may be that this is a react-konva specific issue and other applications wouldn't be passing NaN as a translate argument, but this still might be worth adding anyway since the browser throws an error with NaN.
  2. Context.prototype.arc
    • The issue was similar to Context.prototype.translate, but not as clear. We were able to address the NaN by adding blanket default fallback value of || 0 to most of the arguments.

@zenozeng
Copy link
Owner

@ShaneSP Hi, sorry for my late response. Regarding the parameter check, I suggest following the WHATWG specifications. If it is NaN or infinite, then simply return.

The translate(x, y) method, when invoked, must run these steps:

1. If either of the arguments are infinite or NaN, then return.

https://html.spec.whatwg.org/multipage/canvas.html#dom-context-2d-translate

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