search

Automattic / node-canvas

Node canvas is a Cairo backed Canvas implementation for NodeJS.
10.19k stars 1.17k forks source link

SVG Path - elliptical arc not drawn correctly #1377

Open matthewbillienyc opened 5 years ago

matthewbillienyc commented 5 years ago

Created this test app to demonstrate the issue: https://github.com/matthewbillienyc/node-canvas-svg-test/

Node Canvas SVG to PNG issue

So this repo is to show how between node-canvas versions 2.0.0-alpha.181 and 2.1.0 a bug with rendering SVG arcs was introduced.

Environment Info
Ubuntu 18.04
Nodejs 6.11.0
node-canvas 2.0.0-alpha.18

The png image that is created looks correct. Imgur

node-canvas 2.1.0

The png image that is created does not look correct. Imgur

SVG data

Here is an online SVG rendering of the SVG data below. The offending data is this: A 10 10 0 1 1 16.499995000000418 10.49000000166666. Specifically it looks like the 10.49000000166666 in the arc path is what causes it to break.

    <svg xmlns:xlink="http://www.w3.org/1999/xlink" version="1.1" style="font-family:RobotoCondensed;font-size:12px;" xmlns="http://www.w3.org/2000/svg" width="400" height="400" viewBox="0 0 400 400">
        <g class="translate" transform="translate(200,200)">
            <path fill="#6833c9" class="flag" d="M 16.5 10.5 A 10 10 0 1 1 16.499995000000418 10.49000000166666 Z M 6.5 20.5 L 6.5 29.531480378285778" stroke="#6833c9" stroke-width="1"></path>
        </g>
    </svg>
Why is it breaking? Great question!

I am not 100% sure. But we spent some time looking into by comparing all the commits between 2.0.0-alpha.18 and 2.1.0. You will see that it looks like this Pull Request (1288: Adapt to V8 7.0) is the offending change. More specifically I believe it is these lines in CanvasRenderingContext2d.cc that caused the breaking of the arc path.

KFoxder commented 5 years ago

@LinusU and @zbjornson Thanks for fixing that issue with the resolution. We ran across this breaking change as well in 2.1.0 and we're hoping one of you could help by taking a look. The repo we set up is pretty straightforward to show the breaking change.

I do think it has to do with these lines but I can't say why these lines would have broken it since it was a double before when using ->NumberValue()...

2.0.0-alpha.18: https://github.com/Automattic/node-canvas/blob/586b395afb4a7/src/CanvasRenderingContext2d.cc#L2513-L2526

2.1.0: https://github.com/Automattic/node-canvas/blob/v2.1.0/src/CanvasRenderingContext2d.cc#L2497-L2524

Hakerh400 commented 5 years ago

Tested locally using mentioned versions of prebuilt canvas and Node.js:

jls83 commented 4 years ago

@zbjornson I'm still seeing this issue on the latest versions of node-canvas & Node.js. I put together another test repo (much credit to @matthewbillienyc!) that tests against various node-canvas and Node.js versions here: https://github.com/jls83/node_canvas_svg_2

KFoxder commented 4 years ago

@Hakerh400 @zbjornson @LinusU so update here. We actually ended up resolving this issue by building from source.

If anyone stumbles on this thread and wants a resolution (although I cannot exactly say "why") if you install via npm install --build-from-source on Ubuntu that should do the trick!