inkex.py: support arbitrary (but uniform) document scale (0.92)

Bug #1508400 reported by su_v
18
This bug affects 3 people
Affects Status Importance Assigned to Milestone
Inkscape
Invalid
Medium
Unassigned

Bug Description

Follow-up to:
Bug #1474188 inkex.py: avoid ZeroDivisionError in getDocumentUnit()

Steps to reproduce:
1) create a drawing with Inkscape 0.91 (default template of 0.91)
   (e.g. prepare a simple path, and a quadrilateral for perspective, save)
2) verify that 'Extensions > Modify Path > Perspective' works as expected
--> ok
3) open drawing created with 0.91 in current trunk
3) verify that 'Extensions > Modify Path > Perspective' produces the same result
--> fails (drawing scale is wrong)

Todo:
1) support arbitrary (but uniform) document scale;
   don't fall back to CSS px in self.getDocumentUnit()
2) verify support in extensions which currently assume a fixed scale of "self.unittouu('1px')"

Revision history for this message
su_v (suv-lp) wrote :
Revision history for this message
su_v (suv-lp) wrote :

A possible fix could be similar to the attached diff (just a quick test without checking for any unintended side-effects). I'll work in a branch and update the report later.

Revision history for this message
su_v (suv-lp) wrote :

List of possibly affected extensions (i.e. extensions which might benefit from or support using a new self.getDocumentScale() method).

Revision history for this message
su_v (suv-lp) wrote :

Alternate solution (does not require to change extensions themselves, just inkex.py)

Revision history for this message
su_v (suv-lp) wrote :

Alternate solution v2 (supports 'preserveAspectRatio' for uniform scaling).

Revision history for this message
Piers Titus van der Torren (ptt) wrote :

Looks like a good enhancement.

Would cleanup some extentions that now calculate the document scale from the viewbox themself:
measure.py
jessyInk.js
scour.py
hpgl_encoder.py

And would help to fix other extentions that now handle document units incorrectly:
dxf_outlines.py (bug #1525418)
probably all the ones listed in 1508400-possibly-affected-extensions.txt

Any reason to hold it back?

Revision history for this message
su_v (suv-lp) wrote :

Linked branch submitted as merge proposal. The branch also includes a new extension to test the proposed changes ('Extensions > Export > Node coordinates') - it can be omitted safely from the merge (deleted from the branch before merging).

Minor note on the late refactoring commit: Initially, I attempted to keep the diff for inkex.py as minimal as possible, but finally decided to shift the focus on the proposed change (e.g. also refactoring unittouu() ).

Revision history for this message
su_v (suv-lp) wrote :

[ Earlier diffs deleted to prevent further outside linking to obsolete earlier versions which no longer apply. ]

Revision history for this message
jazzynico (jazzynico) wrote :

Confirmed on Windows 7, Inkscape trunk rev. 14859.

Changed in inkscape:
importance: Undecided → Medium
status: New → Triaged
Revision history for this message
jazzynico (jazzynico) wrote :

@~suv - I'm about to work on the some of your patches, and I know you're not completely satisfied with some of them. So please tell me if there are things you don't want to be committed, or on the contrary if you think are really urgent for 0.92.
Thanks!

Revision history for this message
su_v (suv-lp) wrote :

@JazzyNico

1) This issue of this one (bug #1508400) I would personally consider a blocker for 0.92 unless solved otherwise (e.g. by enforcing a conversion of the document's SVG user unit size based on CSS Pixel size of 1/90 inch to one based on 1/96 inch the first time it is loaded in Inkscape 0.92 - note that this would mean rescaling the content itself, not just adjusting the viewbox, and possible require to introduce some kind of versioning scheme for Inkscape SVG files).

I had chosen a more generic summary line for this report because at the time I felt it more accurately reflected the actual limitation of unit conversions in current inkex.py - its relevancy for 0.92 is high because documents created with <= 0.91 use one such "arbitrary" document scale when opened/viewed in recent trunk / 0.92. The underlying issue relates in general to what was/is discussed in bug #1387864 (tracker bug for 0.91) and #1389723 (tracker bug for 0.92).

I'm aware that the branch proposed for merging has conflicts now (IIRC due to the (partial) PEP8 updates applied in trunk since the original merge proposal was created). I'm also aware that the proposed change is not "elegant" (code-wise) nor really "pythonic", and that - if minimal changes to inkex.py are preferred by the project - someone might be able to work out a simpler solution which just (and only) adds support for documents based on older Inkscape versions (i.e. with SVG user units defaulting to the CSS Pixel size of 1/90 inch).

It's been quite a while since I last looked at the changes proposed for merging, sorry - I kept copies of the same changes in use for some of my custom extensions, but even with those my local testing was rather reduced in the past few weeks or even months.

2) I personally would not consider bug #1474188 a blocker for 0.92 (it would be nice to have that one fixed, too, of course - whatever the chosen solution ends up to be). Bug #1474188 is not such a generic wide-spread issue as this one (bug #1508400) - even if mostly triggered by a bug (already fixed) in 0.91 (which could cause to generate attribute values for the top-level <svg> element not handled well everywhere in current inkex.py IIRC).

Revision history for this message
jazzynico (jazzynico) wrote :

So if I understand correctly, applying the branch should fix both Bug #1474188 and bug #1508400. I'm not very familiar with units, but if I understand your code correctly, the changes should not introduce any drawback and thus there's not risk in using it in 0.92.

I'm going to take some time in the next few days to test it a bit more extensively.

Changed in inkscape:
assignee: nobody → jazzynico (jazzynico)
status: Triaged → In Progress
Revision history for this message
jazzynico (jazzynico) wrote :
Revision history for this message
jazzynico (jazzynico) wrote :

Well, I'm a bit lost with that bug. I'm not even sure I can reproduce it properly. And the fact that the whole document has a different size when open with the trunk doesn't help.
I guess writing a python test file to cover the unit conversion would help a lot, rather than relying on manual tests.

Revision history for this message
su_v (suv-lp) wrote :

@JazzyNico - I'm closing this report (the linked merge proposal is already deleted) and retract the patch originally proposed.

Bug #1525418 can be used instead to further track specific issues related to unit conversions in documents using a custom scale when processed by python-based script extensions (see also comment #8 here in bug #1508400). Piers' analysis in bug #1525418 might help to develop a simpler solution (or at least to fix individual extensions known to be affected).

Changed in inkscape:
status: In Progress → Invalid
jazzynico (jazzynico)
Changed in inkscape:
assignee: jazzynico (jazzynico) → nobody
milestone: 0.92 → none
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Duplicates of this bug

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.