Skip to content

Enlarge bounding box by the largest stroke width. #179

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

Merged
merged 2 commits into from
Oct 6, 2017

Conversation

paulkaplan
Copy link
Contributor

Resolves

What Github issue does this resolve (please include link)?

Fixes #175

Proposed Changes

Describe what this Pull Request does

Enlarge the bounding box that is read from the DOM method getBBox() which doesn't take into account the strokes by the largest found stroke width. As mentioned in the code comment, this may have false positives, as in it may enlarge an SVG viewbox that didn't need it (if, for example, the largest shape was a fill, not a stroke). But it will always make the viewbox large enough to include the strokes, which seems like an ok trade-off.

Reason for Changes

Explain why these changes should be made

Clipping of SVG's, which is impacting project rendering and also messes with the upcoming speech bubble rendering.

Test Coverage

Please show how you have added tests to cover your changes

None.

/cc @tmickel since it looks like a lot of this code was yours.

@fsih
Copy link
Contributor

fsih commented Oct 5, 2017

Should it be sufficient to enlarge by 1/2 the largest stroke width?

@paulkaplan
Copy link
Contributor Author

@fsih good catch! done in
ab4046b

Copy link
Contributor

@cwillisf cwillisf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm OK with this for now but I expect we'll want to refine it later. I can imagine situations where this could go really wrong -- for example, if there's a thin border around some content which uses a very large stroke width.

@paulkaplan paulkaplan merged commit 0b3e7c3 into scratchfoundation:develop Oct 6, 2017
@paulkaplan paulkaplan deleted the fix-bbox-strokes branch October 6, 2017 20:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Clipping of SVG strokes
3 participants