paperjs / paper.js

The Swiss Army Knife of Vector Graphics Scripting – Scriptographer ported to JavaScript and the browser, using HTML5 Canvas. Created by @lehni & @puckey

http://paperjs.org

Other

14.47k stars 1.22k forks source link

strokeBounds is incorrect for certain paths #1824

Open adroitwhiz opened 4 years ago

adroitwhiz commented 4 years ago

Description/Steps to reproduce

In some cases, paths' strokeBounds will be incorrectly sized too small. This can easily be seen with e.g. a rotated ellipse, as in the sketch below.

Create a paper.Path.Ellipse
Rotate it
Get its stroke bounds
Notice that the bounds do not cover the actual stroke (and vary depending on the rotation)

Link to reproduction test-case

Sketch

Expected result

An item's strokeBounds should cover the entire item-- otherwise, this can lead to viewboxes of exported SVGs being incorrectly shrunken and cutting off outlines.

Additional information

This occurs across browsers/OSes

lehni commented 4 years ago

@adroitwhiz yes that's a bug indeed! Your fix looks good, thank you! I'll do some testing shortly, but it looks good to go! Out of curiosity: Github says LLK/paper.js is 91 commits ahead. Is there other work that we should bring back over?

adroitwhiz commented 4 years ago

There's a lot of stuff related to text import/export, most of which (I think) is specific to the Scratch paint editor.

There's a couple other things that may be worth merging:

https://github.com/LLK/paper.js/pull/5 - add an option to allow hit tests to hit paths with no fill (plus a follow-up and to-be-merged addition that does the same thing for paths with no stroke)
https://github.com/LLK/paper.js/pull/28 - fix style merging in SVG export; I'm working on a more complete version of this that I plan to submit here as well

@fsih Any other LLK-specific paper.js fixes that could be upstreamed here?

lehni commented 3 years ago

@adroitwhiz apologies for not taking care of this yet. Unfortunately I have had very little time to take care of Paper.js in the past year due to a difficult personal situation. May I ask why you closed the issue?

adroitwhiz commented 3 years ago

PR #1825 fixed this specific issue. I can leave it open to discuss the other LLK-specific changes if you want.

lehni commented 3 years ago

Ah good. Perhaps we should close this and create follow up issues for the other bits?

adroitwhiz commented 3 years ago

Sounds good.