Rounded rect Rx and Ry fields wrong

Not reproduced with r12687 on OS X 10.7.5 using default (new) prefs, a new document based on the default template (for locale 'en_US'): independent of whether the default units are changed to 'mm' (in 'File > Document Properties > Page') or only the units on the controls bar of the Select tool and the Rectangle tool: the rounded corner radius displayed on the controls bar of the rectangle tool reflects the value correctly in mm.

Test object drawn after changing the units: 50 x 50 mm, corner radius Ry 10mm

Could you attach a sample SVG file which exposes the reported issue, or provide detailed steps how to reproduce such a situation from scratch?

It happens for me when starting from scratch:
 - new document (with my default which is in mm);
 - draw a rectangle;
 - go into Ry field and type 2;
 - drag a vertical guide to the top right corner(boundary);
 - open the guide dialog, set "Relative change" and type -2 mm in the X field;
 - the guide is where the corner should be.
Doing the steps I found also that:
 - close Inkscape;
 - relaunch Inkscape for a new document;
 - draw a rectangle: for me it's already rounded (las used style saved) but the corners are reported to be 25,110 mm while the last typed value (2,0 mm) should have been remembered;
 - measuring the rounded corner it turns out to be 7,087 mm (the value that must be typed in to get 2,0 mm).

There seems to be a factor of ~3,54 (or ~0,282) between what you type and what you get.

Attached is my default.

LucaDC wrote:
> new document (with my default which is in mm)

Do you use a custom or modified template (please attach), or use inkscape with a locale other than en_US?

Nevermind, it's already attached. Sorry for the noise!

Reproduced with LucaDC's custom template file which does not set a viewBox, nor define the units with the top-level svg width/height attributes, but sets the default units to 'mm'.

In my tests the width/height values on the controls bar of the rectangle tool are equally wrong (comparing the size of a 100x100 mm rectangle to the size of the A4 page, or to the 'mm' values shown in the ruler), not just the values for the corner radii.

Likely related: bug #1236257 (problem caused by predefined settings in certain localized templates).

Based on tests with archived builds (on OS X):
- not reproduced with rev <= 12552
- reproduced with rev >= 12554
this incompatibility with older custom templates was introduced with the merge of the GSoC unit-improvement branch in revision 12554:
<http://bazaar.launchpad.net/~inkscape.dev/inkscape/trunk/revision/12554>

> (...) this incompatibility with older custom templates (...)
So, if I understand correctly, my template is outdated. I'd better update it, to be safe with other possible incompatibilities.
I'm going to regenerate it starting from the current default.svg and applying the changes I need: mm and some preferences for snapping. Or is there a better way?

Another weird effect:
 - open a new document with my template (see previous attachment);
 - draw a shape (e.g. a rectangle);
 - open "Document properties" dialog: the shape becomes bigger.
Probably Inkscape suddenly realizes that there's a mismatch between units but updates only part of the document.
The same thing happened to me with an old document I had to modify and change the page to portrait. I actually managed to do so after undoing the change and making some modification that fixed the mismatch (don't know what, exactly), then opening the properties dialog didn't trigger the problem anymore.

I've opened a new Bug #1241363 for the problem in the last comment.

With revision >= 12718, the template file now works correctly if used as template (via 'File > New').

If opened directly as regular file (via 'File > Open'), the numbers displayed e.g. in the rectangle tool do not match the units --> likely this is part of the problem tracked in bug #1241363.

On 2013-10-25 15:36 , ~suv wrote:
> If opened directly as regular file (via 'File > Open'), the numbers
> displayed e.g. in the rectangle tool do not match the units

Still reproducible with r13562.

@Alvin - it seems to me that when opening LucaDC's custom template file directly, the symptoms (unit mismatch in the size controls of the select and rectangle tool) are the same as tracked in
- Bug #1366434 "a preserved transform in a group can cause a discrepancy between the size reported by Select tool versus Rectangle tool.”
- Bug #307656 “rect toolbar width/height numbers do not consider parent transform.”

Could you confirm this?

yes, it does look quite similar, even though there is no explicit transform showing up anywhere. I sort of suspect that maybe a one-time transform using a viewbox has been used to get the original rendering correct. But unfortunately the view box or the trnasform has not been explicitly included in the svg file anywhere, so that subsequent operations fail because they are not aware of what was done on startup.

while playing with this, I re-confirmed the unusual behavior mentioned in comment 7, not sure if this is reported elsewhere.

- open Inkscape
- open Document Properties
- open file default.svg from Comment 2
- draw any object
- change units to px
- size of object changes unexpectedly

partial fix committed to rev 13938.

If the file is opened with File->Open, and if the viewbox does not exist, then it will now be set the same as it was during a File->New operation. This resolves the problems reported in comment 7, 9, and 11.

I believe the remaining issues are duplicates of Bug #1366434 and Bug #307656, as previosuly suggested in comment 10. I would propose that this report either be closed or marked as a duplicate.

<opinion>
Revision 13938 will break a lot of old files created with Inkscape 0.48 - whenever the user chose a different display (aka default) unit than px in Inkscape 0.48, the content of the file when opened in trunk >= rev 13938 will be scaled wrongly and no longer match the page size. Basing the auto-added viewBox on _display_ units seems an odd choice to me (display units do not and never did in a stable release affect the drawing scale).
</opinion>

well, that puts us in a rather difficult position, because it is inconceivable that we would allow the file open operation to behave differently than the file new operation, when they are both operating on the same file. The new code for the file open operation has been copied directly from the file new code to ensure that they both behave the same. This simultaneously solves two rather nasty bugs that had been observed previously during studying this bug.

I don't understand how open and new could operate on the same file; actually I don't understand how new could operate on a file at all.

<opinion>
Anyway, to me this sound as a technicality that won't comfort so much the user staring at his corrupted file.

Again: I think that avoiding breaking existing files should be a +inf priority.
Any argument against this rule is going to sound so feeble to me.
I think that every possible effort to avoid existing documents' corruption is simply a duty, no matter how hard it's going to be. Letting out a version that fails on this would be a really huge mistake.

Or am I wrong because versions >0.91 are intended for new users only? Should every old user keep two inkscape versions installed?
</opinion>

<opinion>
In my understanding, the argument made in comment 14 is flawed - it would be quite safe to assume that any Inkscape SVG without a viewBox attribute has a 'px'-based drawing scale (based on 90dpi) - the display setting for the GUI never changed the drawing scale in a stable release version. r13938 appears to deliberately introduce a breaking change which
- prevents further editing of many old documents which were last saved in Inkscape <= 0.48 with "Default units" other than 'px' for the GUI
- prevents correct adaptation of such files to the internal resolution change (90dpi -> 96dpi) later on
  (once a solution for that is implemented - the lack of such a procedure is a release blocker for 0.92)
- prevents latest trunk being used for bug triage of reports with Inkscape SVG files from Inkscape <= 0.48 as test cases attached

I do hope that this change will be reverted soon (before it starts hurting users of development snapshot builds - files modified due to the changes in r13938 will be broken for further editing in Inkscape 0.48 and 0.91 too, once they had been opened and resaved with a devel build).

Tips for anyone affected by r13938:
* preventing incorrect scaling of old documents without viewBox attribute in trunk rev >= 13938: before opening such files in inkscape trunk rev >= 13938, open them in a plain-text editor instead and make sure that the attribute 'inkscape:document-units' is set to 'px'.
* fixing files resaved with a such modified drawing scale: open in a plain-text editor, delete the added viewBox attribute, change the attribute 'inkscape:document-units' to 'px' and hope that the enforced rescaling didn't already trigger other changes which can't be that easily undone.
</opinion>

well in that case we do indeed have a problem. There are a number of reasons why it would create a problem if rev 13938 were reverted. First of all, it is a duplicate of rev 12575 which solved a number of bugs associated with importing files which originated from other software, not Inkscape. Rev 12575 unfortunately modified only the file_new routine and neglected to modify the file_open routine. Rev 13938 corrects that oversight. The point is that you cannot allow different behavior between a file open command and a file new command. This would mean that someone could create a template file using file_open and then try to use it as a template, which uses file_new, and they would discover that it no longer worked.
 There are two cases that I have seen so far where there is a potential problem. The first case is the one mentioned above where you have an Inkscape file created with Inscape 0.48 or earlier and where the document-units are not px. I am attaching an example here, ellipse_48_mm.svg. If you load this in rev13938 or newer, the ellipse in it will be too large. The reason it is too large is because the original Inkscape program stored the data in units of px despite the fact that the document-units were called mm. This was an error caused by an incomplete implementation of the concept of document units, not related to the viewbox in any way. The way to fix it is to edit the file and remove the line that says 'inkscape:document-units="mm"'. Then the file will load correctly. What we need is an automatic routine that the user could optionally use at their own discretion to do just that, similar to what we will need in order to resolve any discrepancies between 90 dpi and 96 dpi. The user needs to get involved at this point and make the decision as to whether to convert or not convert.

 Attached is a second example of a potential problem, called ellipse_91_mm.svg. This file loads correctly in rev 13938 or newer, but it does not load correctly in rev 13937, the ellipse is too small. This is the inverse of the problem encountered above. In this case the document data was originally stored in units of mm, since it was made with a recent version of Inkscape. However the program is attempting to interpret the data as though they were in units of px, due to the fact that there is no viewbox, and due to the fact that the line which specifies the document units has been ignored in this particlar rev of Inkscape, which means that it will default to px, which is a mistake. This is the type of situation that you would encounter if you were importing an svg file from some other software program, where that software program had decided to implement the concept of document units, but had not implemented the concept of viewbox. What needs to be remembered here is that the implementaion of a viewbox has nothing at all to do with the implementation of different document units. It is entirely conceivable that a piece of software could implement one concept and deliberately refuse to implement the other one, because they are not related in any way. It is extremely unfortunate that Inkscape attempted to implement both at the same time, because it has led to a great deal of confusion.
 The point I would like to make is that both of these scenarios are equally important and they both need to addressed as equals, which in my opinion means that some user interaction will be required in order to make a decision as to whether to convert or not. All that is needed is a decision whether to use the declared units in the file, or whether to default to px.
 The second point I would like to make is that rev 13937 simply postpones the inevitable problem, without actually solving anything. In rev 13937 what is happening is a refusal to define a viewbox at all. If you look at the file after loading and saving, there is still no viewbox, which means it is still not compatible with current Inkscape and never will be. The only way to make it compatible with current Inkscape is to delete the document-units specification and load it into rev 13938 which will successfully convert it into the new format. There is also another potential problem associated with not immediately converting into the new format. In the bug report there were a number of references to unexpected changes in the size, when accidentally opening or using the document units dialog, see comments 7 and 11. These unexpected changes were occurring because the program was attempting to declare a viewbox for the very first time, and was failing because it was using the wrong units. It is true that this particular problem no longer occurs in current Inkscape, because the relevant code has been temporarily disabled, but sooner or later this code will need to be re-enabled again, and then we will have a major problem, because these inconsistencies will show up again. All we have done so far is to sweep the problem under the rug in the hope that it will go away, but it will not go away.
 It is my belief ...

actually I have done some further testing and thinking, and finally realized that none of the existing routines will be satisfactory, neither in the sp_file_open routine, nor in the sp_file_new routine, neither before rev 13937 nor after rev 13938. The reason being that they all use the call to doc->getDisplayUnit() in the case where there is no viewbox. External svg viewers such as IE do not have access to this variable, therefore we are certain to be incompatible with them if we use this variable. IE, as far as I can determine, defaults back to the use of px, and defaults also to a value of 96 dpi as well, if no viewbox exists, and therefore so should we, assuming no viewbox exists. At the same time, it is unacceptable to not define a viewbox at all, because that is just postponing the inevitable. At the same time it is also unacceptable to have file open behave differently than file new, so I would propose the following solution:

replace line 147 in sp_file_new and line 294 in sp_file_open in the file file.cpp:

        doc->setViewBox(Geom::Rect::from_xywh(0, 0, doc->getWidth().value(doc->getDisplayUnit()), doc->getHeight().value(doc->getDisplayUnit())));

with the new line:

        doc->setViewBox(Geom::Rect::from_xywh(0, 0, doc->getWidth().value("px"), doc->getHeight().value("px")));

I believe this should satisfy everyone, including the original problem reported here in comment 2. I will do some further testing tomorrow, but if it works as expected and if there are no strong objections in the next 24 hours, then I will commit this proposed change.

Alvin, thank you for the deep analysis you're doing here and for the generous explanations you're giving.
I completely agree with you that this problem has been "swept under the rug" for too long and that it will not go away alone.

To comment your considerations, I still think that the attempt to find a single general procedure to deal with this problem is not going to be appropriate. The different situations that you have depicted, ask for targeted approaches. You should not try to address Inkscape documents and files form other programs in the same way because they have different histories. Also, files from different Inkscape versions may need different approaches too.
This is why I stressed a bit before 0.91 to insert something to be able to recognize which convention was being used in a particular file (because it was going to be changed), to be able to know how to correctly convert it. Then I lost focus on what have been done as I sticked with Rev13640 because it's the last one that doesn't corrupt my documents (0.91 corrupts guides). I really hope that something has been done!

Now, I think that distinguishing between Inkscape's and external programs' documents is pretty straightforward, so the "document-fix" procedure should take advantage of this in the first place.
Then it comes to deal with Inkscape internal fuzzinesses: my understanding is that the vast majority of documents in the world today is in px @ 90 DPI (apart from the "reported" document's unit). Hence the "document-fix" procedure should apply this kind of correction if it can't understand from which other "particular" revision of Inkscape the document comes from.
Here I've already expressed my opinion: either the "document-fix" procedure is absolutely sure on what it's working on (e.g. from the Inkscape version in the document) or it ought to ask the user for what to do.
I think that with a carefully written "document-fix" procedure that parses the document to understand the conventions used, almost all situations could be correctly faced (of course not in a manually edited file with its own convention).

The case of a template that no longer works is a very minor case: I suppose that most users will accept to be asked to modify their custom made templates to satisfy a new program version. It's an operation you'd have to do only once and templates documents are usually very simple and small. The only important thing here is giving correct information!
In any case this aspect should not get too much in the way while looking for the correct solution for the _real_ problem, that is _opening_ old (big and complex carefully-carved-by-its-creator) documents.

All this to say that I'm really much skeptical that your proposed one-line and one-for-all solution will really fix everything; but, at the same time, I don't have enough knowledge to say that it will not work for sure.
So, let's try it and as soon as it will be on trunk I'll take a look with my documents: if it works for them you won't hear me complaining again. :)
Thank you.

thank you for the feedback,
an improved fix has been committed to rev 13955

with this change, if the viewbox does not exist, then the document units will be set to be px

I just tried your last fix on a couple of files of mine and it worked but for the 90/96 DPI change. I hope to find time to make deeper and wider tests.
I think that if there's no viewbox it makes sense supposing that the file comes from the historical standard of 90 DPI. Hence a new viewbox with the 96/90 ratio could be defined to fix objects' and page's dimensions while leaving untouched all numbers in the file.

>> I think that if there's no viewbox it makes sense supposing that the file comes from the historical standard of 90 DPI.

unfortunately, if we were to do that, we would be breaking compatibility with external viewers which may not use a viewbox and may not be using 90 dpi as a standard. For example attached is a demo file. This was created in Inkscape 0.91 using a standard letter sized page, 8.5x11 in. Then I saved it as plain svg and deleted the viewbox to make it appear that it was created by a third-party software. This file loads correctly into recent Inkscape trunk, meaning that the ellipse fits the page border exactly. It also loads correctly into Internet Explorer 8, which is actually using the Adobe SVG Viewer 3.0. It also loads correctly into IE9 which has a native svg viewer. But it does not load correctly into Inkscape 0.91, due to the difference in resolution.
    I believe the 90 dpi issue will need to be dealt with separately, possibly with a Python extension like DPISwitcher from Bug 1389723

forgot the file

I can't see how we could break compatibility if the situation before all the modifications was to store everything in an arbitrary chosen 90 DPI scale without a viewbox: was it compatible with something?

Also, manually modified files may not be compatible with anything. If the DPI is not written inside the file, then IMHO it's correct to assume any possible rate; the recommendation is to use 96 DPI so that's ok.

But, in old Inskscape file we indeed know which was the DPI standard, also if it's not written in the file, because we all know it was 90 DPI. So we have all the information to perform the correct conversion, provided we correctly recognize that the file was created with inkscape and this is not a problem, isn't it?

About the file you posted: the page size is in inches and the ellipse in user units but the user unit is not specified. I suppose that a viewer could display whatever it likes (apart from considering the 96 DPI recommendation, of course).
I don't think that anyone could complain if such a file is not displayed as intended.

perhaps that was not the best way of expressing it. What I meant to say is that we cannot arbitrarily and automatically create a viewbox on the assumption that the original file actually had a dpi of 90. Creating such a viewbox might be appropriate if the file actually originated in an older version of Inkscape, but it would be wrong if the file did not originate in Inkscape.

Well, of course! That's why it's important to distinguish Inkscape-made files and act differently on them.
It would be weird if Inkscape was compliant with all the world's files and not with its own... :)

Whatever the current status and outcome of this report and its discussion in the comments is - there is unlikely a major change pending wrt to viewBox and document drawing scale to be committed to a minor bug-fix release (0.91.1): moving milestone to 0.92 for now.

Whether or not, and how exactly Inkscape 0.92 should add a (missing) viewBox attribute (and thus set an absolute drawing scale) unconditionally when loading a file, and how this will affect or interact with any migration strategies of the internal resolution change for files created with older releases should be discussed on the mailing list.

Feel free to add a comment and request to revert the milestone change if you disagree and think the change was done in error.

Issue is resolved in 1.2-alpha (and earlier versions. Looks correct in 0.92.5 and below also apply to 0.92.5).

The 90-96 dpi switch has been dealt with as best as we can do so (if we detect an old inkscape file, we try to offer to change the DPI, and we have an extension that can convert manually)

The Display units are now separate from storage of length values (barring any bugs), and are only used to display the values. I don't think I can give a succinct summary that won't confuse, but it works. For the most part, we also allow you to change the units provided in the <svg> width/height (and with the default mm document, use mm units in the svg width/height values), but will generally use those values directly (instead of playing tricks with the display units)

Let's close here and open new issues for any problems that come up.

Closing as part of migration to Gitlab, new issues can be opened at inkscape.org/report (link should redirect to current bug tracker)