trunk: "imprecise" viewBox height affects node snapping to grid (rev >= 12554/77)

Bug #1235279 reported by su_v
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Inkscape
Fix Released
Medium
Alvin Penner

Bug Description

Steps to reproduce:
1) launch trunk (default prefs, default new doc)
2) open 'File > Document Properties' and change
   - page width: 600
   - page height: 400
    (unit unchanged: px)
3) Add a new layer (Shift+Ctrl+N) to be sure to work without transforms on parent container.
4) enable 'View > Grid'
5) switch to the pen tool and draw the diagonals (on the new layer), using node snapping for start and end nodes of each line.
6) open XML Editor and check the path data

Expected result:
The path data uses integer px (SVG user unit) values.

Actual results:
There are rounding errors (?) in the coordinates of both start and end nodes.

7) in the XML Editor, set the height value in the viewBox attribute to the correct integer value for the height (400)
8) activate 'Snap to cusp nodes' (snap controls bar)
9) select each of the four nodes and snap them again to the grid lines in each page corner

Result:
After correcting the viewBox height value snapping works with the expected precision.

Reproduced with r12653 on Ubuntu 13.04 (inkscape-trunk PPA) and r12654 on OS X 10.7.5

Based on tests with archived builds on OS X:
- works as expected with rev <= 12553
- setting custom page size broken between 12554-12576,
- imprecise viewBox height for custom page sizes with rev 12557 and later,
the regression seems related to the merges of the GSoC 2013 unit improvement branch.
<http://bazaar.launchpad.net/~inkscape.dev/inkscape/trunk/changes/12577>

Attached: sample SVG file created based on steps 1-5 above, revision 12654.

Revision history for this message
su_v (suv-lp) wrote :
description: updated
su_v (suv-lp)
description: updated
Revision history for this message
Johan Engelen (johanengelen) wrote :

note that the OP has a 1x1px grid

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

There are no units in play here (at least from a users perspective): the goal was to work only with SVG user units aka 'px' (exactly for the reason to avoid unit conversions and achieve higher precision): the problem AFAICT is the height of the viewBox attribute added by Inkscape based on the custom page size:

<…>
   width="600"
   height="400"
   id="svg2"
   version="1.1"
   inkscape:version="0.48+devel r12654"
   viewBox="0 0 600 400.00001"
<…>

Revision history for this message
Johan Engelen (johanengelen) wrote :

To fix this bug, hints:

document.cpp, SPDocument::setWidth, line 589: " root->viewBox.setMax(Geom::Point(root->viewBox.left() + (root->width.computed / old_computed) * root->viewBox.width(), root->viewBox.bottom()));"
Similarly for SPDocument::setHeight.
That strikes me as way too much calculation. It should read something like root->viewBox.left() + new_width. Why is this calculation there at all? (taking the old value into account doesn't make sense to me)

default.svg contains
      width="744.09448819"
      height="1052.3622047">"
A lot of digits! Then in the XML reader it says:
     width = 744.09448
     height = 1052.3622
  (also for viewbox attribute)
But the Document Properties dialog says:
   width = 744.09448 (same, hence no rounding errors in updating the document width?)'
  height = 1052.36218 (note the difference!)

su_v (suv-lp)
Changed in inkscape:
assignee: nobody → Matthew Petroff (matthewpetroff)
Revision history for this message
Matthew Petroff (matthewpetroff) wrote :

The calculation needs to be there for viewBoxes that aren't equal to the document dimensions, else the scale of items will change.

I wrote a fix that assumes the viewBox matches the document dimensions when there is a difference of less than 0.00001 between the viewBox and the document dimensions and uses your suggestion then, else using the existing code. I'm not sure if it's a good idea to make such an assumption, but I know of no good way to round floating point numbers to a certain number of decimal places.

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

Here are different STR which cause unexpected results with rev >= 12557 ("Use viewBox for new documents"):

1) launch current trunk, default prefs, default new doc (locale en_US))
2) import a bitmap image (linked)
3) open 'Image Properties' from the context menu
4) position the image at 0,0 (SVG origin, upper left corner of page)
5) move the image with 'Shift'+ arrow key
6) move it back, by using 'Shift'+ arrow key in opposite direction

Expected result:
The image is moved back to the SVG origin (0,0)

Actual result:
The image is slightly offset by a small fraction (-3.3035803e-07), even though the default 'steps' units are a integer 'px' value (20px with 'Shift'), using the same unit like elsewhere in the current document ('px', aka SVG user units).

su_v (suv-lp)
Changed in inkscape:
importance: Undecided → Medium
status: New → Confirmed
Changed in inkscape:
status: Confirmed → Fix Released
Revision history for this message
Matthew Petroff (matthewpetroff) wrote :

I had to revert my fix since it sometimes broke changing document units. Now I can't change the bug status back to confirmed.

su_v (suv-lp)
Changed in inkscape:
status: Fix Released → Triaged
Revision history for this message
Johan Engelen (johanengelen) wrote :

unassigning after inactivity for a while

Changed in inkscape:
assignee: Matthew Petroff (matthewpetroff) → nobody
Revision history for this message
Alvin Penner (apenner) wrote :

I believe this has been fixed in rev 13581, which was originally intended to deal with Bug 1374614

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

Closing - STR as originally reported no longer reproduced with default new document in current trunk lp:inkscape r13632 and stable release branch lp:inkscape/0.91.x r13628.

Changed in inkscape:
assignee: nobody → Alvin Penner (apenner)
status: Triaged → Fix Committed
Bryce Harrington (bryce)
Changed in inkscape:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

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