Issue #174, Issue #69 : Added automatic detection of Inkscape DPI settings#420
Open
jarney wants to merge 2 commits into
Open
Issue #174, Issue #69 : Added automatic detection of Inkscape DPI settings#420jarney wants to merge 2 commits into
jarney wants to merge 2 commits into
Conversation
…cape DPI settings from before 0.92 and after 0.92 so documents preserve 'real-world' dimensions. Also supported viewbox which isn't located at the origin by shifting the location of the document according to what's specified in the viewbox. Also added some unit-tests to confirm the scale behavior and make sure that various different unit settings result in the same physical cut paths and unit conversions accurately represent the SVG intent.
frmdstryr
reviewed
Jun 16, 2026
frmdstryr
left a comment
Collaborator
There was a problem hiding this comment.
Overall looks fine. Can you please make address the two comments then it's good to merge.
| # Not enough version information to make a correct decision, | ||
| # so we must return None. | ||
| return None | ||
| except: |
Collaborator
There was a problem hiding this comment.
Please use except ValueError: instead of a bare except
Comment on lines
+2
to
+10
| Copyright (c) 2018, Jairus Martin. | ||
|
|
||
| Distributed under the terms of the GPL v3 License. | ||
|
|
||
| The full license is in the file LICENSE, distributed with this software. | ||
|
|
||
| Created on Mar 14, 2018 | ||
|
|
||
| @author: jrm |
Collaborator
There was a problem hiding this comment.
Please use 2026 with your name or "The Inkcut Team" and either remove/correct the Created on or author lines.
…be specific to ValueError instead of bare except as requested in review.
Author
|
Thank you for the review and for authoring the package to begin with. Apologies for blindly cut-n-paste the copyright notice. I've made the changes you requested. Cheers! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This is intended to address issue #174, #179, and issue #69
Here, we introduce a "Default DPI" setting wherein the user can specify the DPI to assume for documents that don't have any unit information in the width,height of the tag. In addition, we allow detection of Inkscape documents by looking at the version information and assuming 96DPI for recent versions of Inkscape and 90DPI for versions prior to 0.92.
Implementation should be consistent with Inkscape handling of units.
In addition, we translate the document by the (unit-scaled) amount specified in the "viewbox" attribute of the root tag.
Unit tests were also added to confirm that the same rectangle expressed in various unit schemes all result in the correct final dimensions in terms of Inkcut internal units to ensure that the commands sent to the device would result in the same physical cuts/movements.