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

SVGLoader: Fix setting first path point after multiple moveTo commands #22983

Merged
merged 1 commit into from
Dec 9, 2021

Conversation

nkrkv
Copy link
Contributor

@nkrkv nkrkv commented Dec 8, 2021

There’s a bug in interpreting m and M commands in the path d attribute. Here’s an example of SVG as rendered by a browser and Inkscape:

hindi.svg.zip

firefox

Here’s how it is loaded by SVGLoader (note how the last char is inset to the center):

before

Here’s a result with the patch applied:

after

The problem is in the definition of the initial point (firstPoint in terms of three.js). The SVG standard says:

§ 9.3.4 The "closepath" (Z or z) ends the current subpath by connecting it back to its initial point. An automatic straight line is drawn from the current point to the initial point of the current subpath.

And:

§ 9.3.3 The "moveto" commands (M or m) must establish a new initial point and a new current point. The effect is as if the "pen" were lifted and moved to a new location.

That “must” from the later is basically not applied in the current implementation. This leads to glitches in paths that contain Z after multiple M commands because only the very first M sets the initial point and the path is mistakenly joined with it.

@mrdoob mrdoob added this to the r136 milestone Dec 8, 2021
@yomboprime
Copy link
Collaborator

Seems right and the solution correct. Thanks!

@yomboprime yomboprime merged commit 5726c71 into mrdoob:dev Dec 9, 2021
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