trunk: extensions not compatible with recent unit/viewbox changes (rev >= 12554)

Bug #1240455 reported by su_v on 2013-10-16
8
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Inkscape
High
Alvin Penner

Bug Description

Steps to reproduce:
1) launch current trunk (default prefs, default new document (no localized template))
2) apply 'Extensions > Render > Gear > Gear…' (default settings)
3) open 'File > Document Properties > Page' and change default units from 'px' to 'mm'
4) apply 'Extensions > Render > Gear > Gear…' again with same settings
5) apply it with units changed to 'mm'

Expected result:
Since the change of the default units by itself is not supposed to change the page size, the expected result is that the gear created in step 4 matches the one created in step 2

Actual result:
The gear created after changing the default units is much larger in scale, and not inserted in the center of the current viewport (visible canvas area) which matches the center of the page if following the 'steps to reproduce' exactly. Changing units to 'mm' in the Gear settings only results in a relative change, and does not match closer to the default units of the current document than the one created with default Gear setting ('px').

This is likely to affect many of the script-based extensions (and even internal ones, e.g. 'Extensions > Render > Grid > Grid…').

AFAIU the details on how page size, default units and drawing scale (viewBox) may be combined will vary depending on
- which template was used
- which version was used to create the file (existing documents)
- which changes (and in which order) have been applied to document properties before applying the extension

Confirmed with r12693 on Ubuntu 12.10 and r12692 on OS X 10.7.5.

Based on tests with archived builds:
- STR not reproduced with rev <= 12553
- STR reproduced with rev >= 12554

su_v (suv-lp) on 2013-10-16
description: updated
su_v (suv-lp) on 2013-10-16
summary: - trunk: extensions do not compatible with recent unit/viewbox changes
- (rev >= 12554)
+ trunk: extensions not compatible with recent unit/viewbox changes (rev
+ >= 12554)
jazzynico (jazzynico) wrote :

Confirmed on Windows XP, Inkscape trunk revision 12698.

Console messages:

** (inkscape.exe:3836): WARNING **: Incompatible units
** (inkscape.exe:3836): WARNING **: Incompatible units
** (inkscape.exe:3836): CRITICAL **: Gtk::TreeModel::iterator Inkscape::UI::Widget::PageSizer::find_paper_size(Inkscape::Util::Quantity, Inkscape::Util::Quantity) const: assertion `smaller <= larger' failed

Changed in inkscape:
importance: Undecided → High
status: New → Triaged
su_v (suv-lp) wrote :

JazzyNico wrote:
> Console messages:
> (…)

The warnings are tracked in bug #1240308 (AFAICT they do not depend on changing the unit - they occur after having opened the dialog once in the current session).

Johan Engelen (johanengelen) wrote :

this should be fixed for the Gear extension. I've applied a general fix to all extensions using the same unit conversion that Gear uses. please test again.

Changed in inkscape:
status: Triaged → Incomplete
jazzynico (jazzynico) wrote :

Fix confirmed on Windows XP, Inkscape trunk revision 12742 (tested with a dozen extensions).
Thanks!

Changed in inkscape:
milestone: 0.49 → none
status: Incomplete → Fix Released
su_v (suv-lp) wrote :

Extensions which use SVG user units (aka px) and do not support choosing a custom unit still act unexpectedly.

Steps to reproduce:
1) launch trunk with default prefs, and new document based on default non-localized template
2) change document units to 'cm'
3) apply 'Extensions > Render > Alphabet Soup'

Variation:
Change the default units to 'm' or 'ft' and the the alphabet soup is more like an alphabet ocean: so huge that even at min zoom level (1%) only a small portion is shown, and the inserted object cannot really be further edited in Inkscape.

su_v (suv-lp) wrote :

Stroke width of objects inserted via extensions scales out-of-proportion.

Steps to reproduce:
1) launch trunk with default prefs, and new document based on default non-localized template
2) change document units to 'm'
3) apply 'Extensions > Render > Gear > Gear' (with default settings)

The stroke width of the gear is rendered so huge that the entire document page is black, and one has to zoom out quite a lot to view the full gear (verify with outline view mode that the geometry of the created gear has the expected size).

su_v (suv-lp) wrote :

Proposing to reopen …

jazzynico (jazzynico) on 2013-10-29
Changed in inkscape:
status: Fix Released → Triaged
milestone: none → 0.49
Johan Engelen (johanengelen) wrote :

a stroke width of "1" is interpreted according to the viewbox. if you set units to 'meters' it means 1meter stroke width.
the problem is (i think) that many extensions simply create an object with an arbitrary stroke width, not considering the document units.
So yes indeed, a bug, needs to be fixed, probably pretty easy to fix. simply convert the stroke width by calling "self.unittouu()" in the python script

tags: added: easy-fix
Luxo (wmd-o) wrote :

Bugs in ViewBox
inkScape version: 0.48+devel r12742

Steps to reproduce:
1) create simple svg-file

<?xml version="1.1" encoding="UTF-8" standalone="no"?>
<svg width="100mm" height="100mm" viewBox="0 0 100 100">
  <line y2="50" x2="50" y1="0" x1="0" stroke-width="1" stroke="#FF0000" fill="none" />
  <line y2="50" x2="50" y1="100" x1="0" stroke-width="1" stroke="#FF0000" fill="none" />
</svg>
2) open this svg-file in inkscape
3) apply 'Extensions > Render > Gear > Gear…' (default settings)

gear appears not in a center of a workarea
gear have incorrect size

4) file - document properties - default units - mm
gear transfromed to position and size as expected before
but lines were resized

Luxo (wmd-o) wrote :

No problem with such file:

<?xml version="1.1" encoding="UTF-8" standalone="no"?>
<svg width="100mm" height="100mm" viewBox="0 0 100 100"

xmlns:sodipodi="http://sodipodi.sourceforge.net/DTD/sodipodi-0.dtd"
 xmlns:inkscape="http://www.inkscape.org/namespaces/inkscape"

 >
 <sodipodi:namedview inkscape:document-units="mm"/>

  <line y2="50" x2="50" y1="0" x1="0" stroke-width="1" stroke="#FF0000" fill="none" />
  <line y2="50" x2="50" y1="100" x1="0" stroke-width="1" stroke="#FF0000" fill="none" />
</svg>

Bryce Harrington (bryce) wrote :

It looks like over the course of this bug the original issue (and subsequent issues) got fixed. Messing around with current trunk I see some quirkiness but not the described problems. Could someone elaborate on what the remaining issues are?

Otherwise, I guess we can close this as fixed?

su_v (suv-lp) wrote :

Steps to reproduce from comment #5 and #6 still reproducible as described with latest trunk (r12913).

su_v (suv-lp) wrote :

With regard to the steps to reproduce in comment #6: apparently document units 'm' in trunk affect the precision of the generated gear (negatively) - noticeable in outline view mode as well as after manually adjusting the huge stroke width.
Reproduced with r12910 on Ubuntu 13.04 (VM, 64bit) and r12913 on OS X 10.7.5.

--
(replaces hidden comment #13 to correct typo in Ubuntu version)

Alvin Penner (apenner) wrote :

attached is a modified version of render_gears.py.
This is just to deal with comment 6 above, variation in stroke width when changing units.
If this result looks reasonable, then I will proceed to change some of the other render extensions in similar ways, since there are a number of extensions with scaling problems like this.

Alvin Penner (apenner) wrote :

the files 'draw_from_triangle', 'foldablebox', 'render_alphabetsoup', 'render_gear_rack', 'render_gears', have been updated in rev 13001 to deal with scaling problems due to new document units. The approach taken is to convert input parameters into document units if they have units of length. Stroke width is also converted, using a default value of 1px.

further changes will be during the next week, focussing only on the Render submenu.

Alvin Penner (apenner) wrote :

the files grid_cartesian.py, grid_isometric.py, grid_polar.py, guides_creator.py, lindenmayer.py, rtree.py, spirograph.py, triangle.py, wireframe_sphere.py, have been updated in rev 13007 to deal with scaling problems. The approach taken was the same as the previous comment

further changes will be made during the next week, focussing only on the Render submenu.

su_v (suv-lp) on 2014-02-08
Changed in inkscape:
assignee: nobody → Alvin Penner (apenner)
status: Triaged → In Progress
su_v (suv-lp) wrote :

'make check' now fails (worked with r13003, fails with r13009):

>> Testing triangle
E
======================================================================
ERROR: test_run_without_parameters (__main__.Grid_PolarBasicTest)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "triangle.test.py", line 22, in test_run_without_parameters
    e.affect( args, False )
  File "../inkex.py", line 261, in affect
    self.effect()
  File "../triangle.py", line 133, in effect
    draw_tri_from_3_sides(s_a, s_b, s_c, offset, tri)
  File "../triangle.py", line 85, in draw_tri_from_3_sides
    draw_SVG_tri(a, b, c , offset, Grid_Polar.unittouu(e, '2px'), 'Triangle', parent)
TypeError: unbound method unittouu() must be called with Grid_Polar instance as first argument (got float instance instead)

----------------------------------------------------------------------
Ran 1 test in 0.003s

FAILED (errors=1)

(AFAIU this likely will halt trunk PPA builds because they are only uploaded if 'make check' succeeds).

Alvin Penner (apenner) wrote :

sorry about that, in rev 13010 I've reverted the changes to the files triangle.py and wireframe_sphere.py since they both will have the same problem when testing.

somewhat off topic there appear to be some missing test files. I cannot find the files:
- grid_isometric.test.py
- wireframe_sphere.test.py
- render_gear_rack.test.py

should these all be present?

Alvin Penner (apenner) wrote :

improved versions of triangle.py and wireframe_sphere.py have been committed to rev 13012, these should pass the unit tests.

su_v (suv-lp) wrote :

Confirmed on OS X 10.7.5: 'make check' succeeds with r13012 - thx a lot, Alvin :-)

Wrt missing test files: AFAIK they are 'optional' (not a hard requirement e.g. when adding new extensions) - ideally there'd be more (and more specific tests) available.

Alvin Penner (apenner) wrote :

good to hear. these unit tests seem to be performing a useful service. When I wrote the code changes in rev 13007 I suspected that I was breaking the law, but still it seemed to run okay so I thought maybe I could squeak by with no harm.

while looking at this code, I also looked at Bug 1277649 and Bug 1258473. I suspect that these bugs may actually be quite hard to fix, certainly well above my paygrade. Any attempt to patch the reference to 'self.unittouu' with some other type of class reference is almost certainly going to encounter the same problems I had with triangle.py in rev 13007.

Alvin Penner (apenner) wrote :

the files hershey.py, polyhedron_3d.py, render_barcode.py, render_barcode_datamatrix.py, render_barcode_qrcode.py, have been updated in rev 13015 to deal with scaling problems. The approach taken was to apply a transform to the group that contains the newly rendered object.

I will look at one more Render item, namely Grids->Grid, which is an internal extension.

su_v (suv-lp) on 2014-02-10
tags: removed: easy-fix
Alvin Penner (apenner) wrote :

the file grid.cpp has been updated in rev 13018.

I believe this takes care of all the items on the Render submenu, with the exception of Layout->N-up Layout, which crashes (Bug 1277649)

su_v (suv-lp) wrote :

Issue described in comment #14 still present (only tested with gears, possibly present with other extensions too):

Steps to reproduce:
1) launch current trunk (default new prefs, default template, locale en_US.UTF-8)
2) apply 'Extensions > Render > Gear > Gear…'
3) change default units to 'm'
--> the precision of the generated gear degrades
4) scroll the canvas somewhat so that the center is an empty area
5) repeat step 2 (create gear with same default options)
--> the precision of the second generated gear is even worse
6) try to node-edit the outer path of the second gear
--> inkscape crashes:

Program received signal EXC_BAD_ACCESS, Could not access memory.
Reason: KERN_PROTECTION_FAILURE at address: 0x00007fff5f3ffff8
0x0000000100283b5e in geom_cubic_bbox_wind_distance ()
(gdb) bt
#0 0x0000000100283b5e in geom_cubic_bbox_wind_distance ()
(message repeated endlessly)

su_v (suv-lp) wrote :

Re comment #25:
- Crashes are seemingly random, sometimes already when zooming in.
- Editing the now diamond-shaped circle in the center of either gear also triggers a crash - e.g. when dragging one of the arc handles.

Alvin Penner (apenner) wrote :

the files dimension.py, perspective.py, printing_marks.py, summersnight.py (envelope), have been updated in rev 13028 to deal with scaling problems. These all have in common the fact that they use the command-line visual bbox obtained with the --query function. This returns the bbox in pixels regardless of the document units, so it needs to be scaled after being used.

Alvin Penner (apenner) wrote :

the files interp.py, dots.py, voronoi2svg.py, webslicer_create_rect.py, have been updated in rev 13030 to deal with some minor scaling problems related to document units.
    I think this takes care of all the scaling issues that I am aware of. (Apart from the issues of numerical precision which have been reported above for units of m).
   There is still one more extension that needs fixing, namely measure.py, but I do not know how to fix this extension until Bug 1280684 is resolved.

Alvin Penner (apenner) wrote :

the file measure.py has been updated in rev 13035 to deal with scaling problems related to document units

Alvin Penner (apenner) wrote :

the file render_gears.py has been updated in rev 13036 to have a higher precision in the output format for the gears. The circle at the centre is not affected.

This leads to a significant improvement in the appearance of the gears that are rendered after the conversion of document units from px to m. For the gears that are initially rendered in units of px and then subsequently converted to m, it does not appear to generate any visible improvement at all. There is numerical error occurring during this conversion but it appears to be occurring in C code in Inkscape, not in the Python extension.

su_v (suv-lp) wrote :

@Alvin - thanks a lot for spending so much efforts to adapt the extensions bundled with Inkscape to the internal changes. I was wondering whether you kept notes of the process?

(The internal changes and those to inkex.py will certainly affect many custom extensions developed & maintained outside of Inkscape - it might be helpful if e.g. the release notes could link to a wiki page with more details / instructions / tips on how to address known issues which can cause extensions to fail in 0.48+devel/0.91 …)

Alvin Penner (apenner) wrote :

I'll take a second look over the next week or so, to refresh my memory and prepare a more formal list of things to do:
Here's the ones I remember immediately:
1. always use unittouu on all .inx input parameters that have units of length
2. always specify stroke-width explicitly and always use unittouu to convert it to document units
3. remember that the console command to query the bbox returns pixels regardless of the document units, so they need to converted as well
4. items like guides, which are in the namedview section, are apparently not affected by document units, while items in the main svg body are affected

I'll rethink this more carefully over the next few days or so.

Alvin Penner (apenner) wrote :

I've rewritten the rough guidelines for dealing with scaling issues, and attached it here as a text file. This is more complete than the previous comment, and has some examples of each case.

su_v (suv-lp) wrote :

Thanks a lot, Alvin. I put the text onto a new wiki page, and added links to the referenced files:
<http://wiki.inkscape.org/wiki/index.php/Notes_On_Units_Handling_in_Extensions_in_0.91>
Could you please review the current state of the page, and update it as needed? I'm not sure whether the links to specific revisions are really helpful - maybe the links to the extension scripts themselves are sufficient?

Todo:
- better title for the wiki document
- add a link to it in the release notes for 0.91
- add the page to the 'Extensions' category in the wiki
- possibly add it to 'See also' sections of other extensions-related pages

Alvin Penner (apenner) wrote :

cool!

I like the references to specific rev numbers, they highlight the changes very specifically. That's essentially what I did when reviewing the changes, except I was too lazy to write down the rev numbers.

su_v (suv-lp) wrote :

@Alvin - any chance you could help me with the script failures described in bug #1046068, comment 14-17?
(ignore the ones from UniConvertor - but if we could find a solution for dxf_outlines.py, hpgl_output.py, ink2canvas.py and synfig_output.py to fail more gracefully (or silently) with such content, that would be great :) )

Alvin Penner (apenner) wrote :

yes, I can take a look at that. How do I produce the crash that is happening here?

su_v (suv-lp) wrote :

> How do I produce the crash that is happening here?

That's the problem - AFAICT it is specific to the Quartz backend of GDK/GTK+ (the toolkit requests the clipboard data in all supported output mime-types/file formats from the application on quit). I have no clue how this could be triggered or simulated on Windows.

The SVG file which triggers the errors (copied from $TMPDIR):
<https://bugs.launchpad.net/inkscape/+bug/1046068/+attachment/4076724/+files/1046068-ink_ext_XXXXXX.svgLTB8DX>

Alvin Penner (apenner) wrote :

thanks, I'll try out that file later today.

In the meantime, just a quick shot in the dark. If you are getting errors like this one:

Traceback (most recent call last):
  File "dxf_outlines.py", line 345, in <module>
    e.affect()
  File "/Users/su_v/Applications/TST/with a space/Inkscape.app/Contents/Resources/share/inkscape/extensions/inkex.py", line 261, in affect
    self.effect()
  File "dxf_outlines.py", line 328, in effect
    h = self.unittouu(self.document.getroot().xpath('@height', namespaces=inkex.NSS)[0])
IndexError: list index out of range

then you might try Python code like this:

h = something reasonable
if self.document.getroot().xpath('@height', namespaces=inkex.NSS):
     h = self.unittouu(self.document.getroot().xpath('@height', namespaces=inkex.NSS)[0])

Alvin Penner (apenner) wrote :

jftr, I don't plan on working on this any more, I think I've fixed all the ones that I am capable of fixing.

su_v (suv-lp) wrote :

Closing as 'Fix committed' - @Alvin, many thanks for all the fixes!

Remaining regressions should be filed separately (unless already done).

Changed in inkscape:
status: In Progress → Fix Committed
Bryce Harrington (bryce) on 2015-02-21
Changed in inkscape:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers