Please check r13544 or later for problems at unit change
One of the changes I introduced at revision 13544 was a modification of
scaleChildItemsRec()
so that it no longer recurses into groups to apply scaling changes associated with a change of units. This was breaking EMF import of clipped objects. That happened because during the recursion the coordinates of the clipped objects were changed directly, not by a transform. Since the clipping object (in defs) was not similarly scaled this broke the clip. See for instance:
https://bugs.launchpad.net/inkscape/+bug/1348417
In the changed version the scaling change is applied as a transform at the group level, and all the coordinates within that group are not directly modified, so the clipping object and clipped objects are still at the same scale and they continue to work together. Since EMF import of clipped objects always works like (pseudocode):
<g (clip reference)> clipped objects </g>
applying the transform at the group level fixes the problem in that context.
I suspect that scaleChildItemsRec() is still going to cause problems for any SVG object other than a group that looks something like (pseudocode):
<(not a group) (a clip or mask ref) x=100 y=100 (other stuff)
when the document units are changed because it will change the 100's to something else, and not modify the referenced clipping object. I have not confirmed this though.
In any case, would the people who worked on document units please see if my changes at revision 13544 break anything when the document units are changed? I did some preliminary testing and it looked OK, but I don't really know what test cases to use to explore all the edge cases.
Thanks,
David Mathog mathog@...1176... Manager, Sequence Analysis Facility, Biology Division, Caltech
On 2014-09-05 19:29 , mathog wrote:
One of the changes I introduced at revision 13544 was a modification of
scaleChildItemsRec()so that it no longer recurses into groups to apply scaling changes associated with a change of units. This was breaking EMF import of clipped objects. That happened because during the recursion the coordinates of the clipped objects were changed directly, not by a transform. Since the clipping object (in defs) was not similarly scaled this broke the clip. See for instance:
https://bugs.launchpad.net/inkscape/+bug/1348417
In the changed version the scaling change is applied as a transform at the group level, and all the coordinates within that group are not directly modified, so the clipping object and clipped objects are still at the same scale and they continue to work together. Since EMF import of clipped objects always works like (pseudocode):
<g (clip reference)> clipped objects
</g>
applying the transform at the group level fixes the problem in that context.
I suspect that scaleChildItemsRec() is still going to cause problems for any SVG object other than a group that looks something like (pseudocode):
<(not a group) (a clip or mask ref) x=100 y=100 (other stuff)
when the document units are changed because it will change the 100's to something else, and not modify the referenced clipping object. I have not confirmed this though.
In any case, would the people who worked on document units please see if my changes at revision 13544 break anything when the document units are changed? I did some preliminary testing and it looked OK, but I don't really know what test cases to use to explore all the edge cases.
The changes in rev 13544 seem to also fix the issue tracked in
- Bug #1287288 "trunk: clip-path of clipped group incorrectly scaled when changing default units (rev >= 12554)" https://bugs.launchpad.net/inkscape/+bug/1287288
at least based on the two test cases attached there (one with clipped groups, one with masked groups). I plan to close the report once we have one more confirmation (maybe with real-life test case).
Regards, V
On 2014-09-05 22:03 , su_v wrote:
On 2014-09-05 19:29 , mathog wrote:
One of the changes I introduced at revision 13544 was a modification of
scaleChildItemsRec()so that it no longer recurses into groups to apply scaling changes associated with a change of units. This was breaking EMF import of clipped objects. That happened because during the recursion the coordinates of the clipped objects were changed directly, not by a transform. Since the clipping object (in defs) was not similarly scaled this broke the clip. See for instance:
https://bugs.launchpad.net/inkscape/+bug/1348417
In the changed version the scaling change is applied as a transform at the group level, and all the coordinates within that group are not directly modified, so the clipping object and clipped objects are still at the same scale and they continue to work together. Since EMF import of clipped objects always works like (pseudocode):
<g (clip reference)> clipped objects
</g>
applying the transform at the group level fixes the problem in that context.
I suspect that scaleChildItemsRec() is still going to cause problems for any SVG object other than a group that looks something like (pseudocode):
<(not a group) (a clip or mask ref) x=100 y=100 (other stuff)
when the document units are changed because it will change the 100's to something else, and not modify the referenced clipping object. I have not confirmed this though.
In any case, would the people who worked on document units please see if my changes at revision 13544 break anything when the document units are changed? I did some preliminary testing and it looked OK, but I don't really know what test cases to use to explore all the edge cases.
The changes in rev 13544 seem to also fix the issue tracked in
- Bug #1287288 "trunk: clip-path of clipped group incorrectly scaled when changing default units (rev >= 12554)" https://bugs.launchpad.net/inkscape/+bug/1287288
at least based on the two test cases attached there (one with clipped groups, one with masked groups). I plan to close the report once we have one more confirmation (maybe with real-life test case).
New regression with r13544:
Steps to reproduce: 1) launch inkscape (default (new) prefs, default template (locale: en_US)) 2) draw a rect 100px x 100px (size in rect tool controls bar) 3) deselect the rectangle (<esc>) 4) open Document properties and change default units to 'mm' 5) draw a rectangle 100mm x 100mm (size in rect tool controls bar)
Expected result: The two rectangles differ in size on canvas.
Actual result: - The two rectangles have the same size on-canvas - Select tool and rectangle tool are out of sync wrt size and units
The underlying issue seems that now (after r13544) changing document units applies a preserved 'scale()' transformation to all top-level layer groups, which did not happen with earlier revisions.
Regards, V
New regression with r13544:
regression confirmed on rev 13546. When following the steps for drawing the rectangles, we find the Select tool F1 reports the size as 28.5 mm while the Rectangle tool F4 reports the size as 100 mm even though they are actually the same size. This is apparently related to the presence of the preserved transform. I think there is a pre-existing bug here, one which existed before rev 13544. However, that's not the point. The point is that it is clear that the original intent of the unit change code was that the unit change conversions would be done directly on the actual coordinates of the objects and not be preserved in a group transform. If this procedure of applying unit changes directly to actual coordinates of objects is causing problems with clip objects then I think the best appraoch would be to also recalculate the clip coordinates in the defs section in exactly the same way as they were recalculated in the main section of the svg file, rather than encapsulating them in a group transform. Personally I would suggest reverting rev 13544 until a solution can be found that does not require Preserved transforms.
Cheers, Alvin
-- View this message in context: http://inkscape.13.x6.nabble.com/Please-check-r13544-or-later-for-problems-a... Sent from the Inkscape - Dev mailing list archive at Nabble.com.
two comments: the pre-existing bug has been reported at Bug 307656 and Bug 1366434. It appears to be related specifically to the rectangle tool F4. However there are two new regressions which are not related to F4.
First one: rev 13544 has broken one of the fixes for Bug 1240455, comment 24: https://bugs.launchpad.net/inkscape/+bug/1240455/comments/24
using Extensions->Render->Grids->Grid and using the default spacing of 10 px. - case 1 : open Inkscape with default units of px, draw default Grid - case 2 : open Inkscape, change units to mm, draw default Grid
the expectation is that these two grids will look the same relative to the page size. Rev 13534 shows the expected behavior, rev 13546 does not.
Second one: - open Inkscape, draw two objects, a rectangle and a star, or ellipse - select both of them - change document units - the expectation is that the shapes should not change size - Rev 13534 shows the expected behavior, rev 13546 does not.
Cheers, Alvin
-- View this message in context: http://inkscape.13.x6.nabble.com/Please-check-r13544-or-later-for-problems-a... Sent from the Inkscape - Dev mailing list archive at Nabble.com.
On 06-Sep-2014 16:49, alvinpenner wrote:
However, that's not the point. The point is that it is clear that the original intent of the unit change code was that the unit change conversions would be done directly on the actual coordinates of the objects and not be preserved in a group transform.
Yes, but is that a rational thing to do?
We already know it breaks clipping, it probably breaks masking, and who knows what else it breaks. Moreover it is exceedingly complex. In theory the simplest way to rescale a drawing is to just put a single group with a transform on top and put everything within it. If that level already exists, then just change the existing transform.
Is that not much (MUCH!) simpler than adjusting every value in the drawing?
And what is the advantage of changing all the underlying values? These are often already scaled by other transforms. So those numerical values may or may not correspond to the units.
Also, in terms of numerical stability. Mucking around with the units is going to introduce rounding errors into every single value in the document, and these are going to run off in random directions. Do it the other way, at the top level, and repeatedly rescaling the document might introduce rounding errors in the top transform, but it won't do anything to the values underneath, all of which will stay "sharp", as it were.
Regards,
David Mathog mathog@...1176... Manager, Sequence Analysis Facility, Biology Division, Caltech
On 8-9-2014 19:27, mathog wrote:
On 06-Sep-2014 16:49, alvinpenner wrote:
However, that's not the point. The point is that it is clear that the original intent of the unit change code was that the unit change conversions would be done directly on the actual coordinates of the objects and not be preserved in a group transform.
Yes, but is that a rational thing to do?
Yes it is.
We already know it breaks clipping, it probably breaks masking, and who knows what else it breaks. Moreover it is exceedingly complex.
-.-
Also, in terms of numerical stability. Mucking around with the units is going to introduce rounding errors into every single value in the document, and these are going to run off in random directions. Do it the other way, at the top level, and repeatedly rescaling the document might introduce rounding errors in the top transform, but it won't do anything to the values underneath, all of which will stay "sharp", as it were.
Your change does exactly the opposite of this. The user does not change units at the end of creating a document. (and even if he does, than he also wants exactly what we are doing right now) If document-units says "mm", then that is what that is: stuff is expressed in mm. Not in some other unit with a top-level transform mucking around with the values.
-Johan
On 08-Sep-2014 11:00, Johan Engelen wrote:
If document-units says "mm", then that is what that is: stuff is expressed in mm.
Only in the simplest possible case.
1. new document, set to mm 2. draw a 10mm x 10mm rectangle. 3. duplicate it. 4. group the two rectangles. 5. scale the group upwards a bit.
The resulting SVG looks like this (only one rectangle shown, style omitted)
<g id="g3337"
transform="matrix(2.3274107,0,0,2.3475402,-21.407125,-174.42731)"> <rect y="117.18413" x="16.12698" height="10" width="10" id="rect3320" />
The object has height "10", but it isn't 10mm, it is 23.475mm. My point being that the only time x,y,height,width in even this simple example are all in the units specified for the drawing is if no transforms are applied above it. The general case is that the final value must incorporate all transforms. It seems like a total waste of effort (to me) to use anything other than pixels for the "internal" representation, with a final scale transform at the top to set the document units. Moreover, doing it that way doesn't break anything (that isn't already broken because it fails to check all transforms.) For instance, it doesn't break clipping. The current method does break things and fixing it would require even more complexity, by converting the defs as well.
Regards,
David Mathog mathog@...1176... Manager, Sequence Analysis Facility, Biology Division, Caltech
Attached are some aesthetic comments on the difference between the original code and the modified code which preserves the transforms when changing document units. There are two problems that can occur when preserving unit transforms in a layer definition. First of all the operation of changing document units does not commute with the operation of creating a new layer, which is unexpected:
- open Inkscape with default units px - draw rectangle - change units to mm - create new Layer 2 - draw same-sized rectangle - save as rect_13546.svg
Compare the two objects in the XML editor and note that the second rectangle 'height' and 'width' are expressed in mm as expected. The first rectangle has its dimensions expressed in pixels which have subsequently been transformed to mm. The net result is that the 'height' and 'width' properties are not comparable when viewing them in the XML editor, because the history of how they were produced has been remembered.
Peforming the same operations in rev 13534 we get the file rect_13534.svg. In this file all the properties are expressed in mm, as expected.
Secondly, the impact of the Preference setting for Optimized/Preserved transforms is modified significantly. The above operations were done with the Preference setting "Behavior->Transforms->Optimized". Now we perform the same two experiments with "Behavior->Transforms->Preserved", to produce the files rect_13546_preserved.svg and rect_13534_preserved.svg.
Inspecting the file rect_13546_preserved.svg, we see that for both rectangles, the original coordinates are expressed in pixels, as expected, and in both layers the conversion to mm occurs as a result of a transform, but the first rectangle has the transform in the layer <group> element while the second rectangle has the transform in the <rect> element, so the location of the transform element is not consistent.
Inspecting the file rect_13534_preserved.svg, we see that for both rectangles, the original coordinates are expressed in pixels, as expected, and in both cases the transform to mm occurs in a <rect> element, so the two objects are directly comparable, even in the XML editor.
The conclusion is that the original code in rev 13534 produces a more consistent representation of the two layers.
rect_13534.svg http://inkscape.13.x6.nabble.com/file/n4971483/rect_13534.svg rect_13534_preserved.svg http://inkscape.13.x6.nabble.com/file/n4971483/rect_13534_preserved.svg rect_13546.svg http://inkscape.13.x6.nabble.com/file/n4971483/rect_13546.svg rect_13546_preserved.svg http://inkscape.13.x6.nabble.com/file/n4971483/rect_13546_preserved.svg
hth, Alvin
-- View this message in context: http://inkscape.13.x6.nabble.com/Please-check-r13544-or-later-for-problems-a... Sent from the Inkscape - Dev mailing list archive at Nabble.com.
I do not have time to study this issue carefully at them moment and to follow who is arguing what... but:
Units should never be used inside an SVG file other than to set the width and height on the SVG root element (and even then it is questionable).
The original intent for including units inside an SVG was so that one could set the size of an object on a display regardless of the display's DPI. This did not catch on. Very few displays returned the accurate DPI information that was needed to make this work. The CSS working group after many long arguments fixed one inch to 96 pixels regardless of the size of the display. This is now written in stone.
I argued in the SVG WG that one should be able to set the number of pixels per inch with a property (like we have for exporting a PNG). This was a non-starter.
If one really wants to use a unit of "mm" then set the width and height of the SVG root in "mm" and provide a 'viewBox' that matches. For example:
<svg width="100mm" height="100mm" viewBox="0 0 100 100">
will give a scale of 1 'user unit' (px) to 1mm.
Tav
On Mon, 2014-09-08 at 12:01 -0700, alvinpenner wrote:
Attached are some aesthetic comments on the difference between the original code and the modified code which preserves the transforms when changing document units. There are two problems that can occur when preserving unit transforms in a layer definition. First of all the operation of changing document units does not commute with the operation of creating a new layer, which is unexpected:
- open Inkscape with default units px
- draw rectangle
- change units to mm
- create new Layer 2
- draw same-sized rectangle
- save as rect_13546.svg
Compare the two objects in the XML editor and note that the second rectangle 'height' and 'width' are expressed in mm as expected. The first rectangle has its dimensions expressed in pixels which have subsequently been transformed to mm. The net result is that the 'height' and 'width' properties are not comparable when viewing them in the XML editor, because the history of how they were produced has been remembered.
Peforming the same operations in rev 13534 we get the file rect_13534.svg. In this file all the properties are expressed in mm, as expected.
Secondly, the impact of the Preference setting for Optimized/Preserved transforms is modified significantly. The above operations were done with the Preference setting "Behavior->Transforms->Optimized". Now we perform the same two experiments with "Behavior->Transforms->Preserved", to produce the files rect_13546_preserved.svg and rect_13534_preserved.svg.
Inspecting the file rect_13546_preserved.svg, we see that for both rectangles, the original coordinates are expressed in pixels, as expected, and in both layers the conversion to mm occurs as a result of a transform, but the first rectangle has the transform in the layer <group> element while the second rectangle has the transform in the <rect> element, so the location of the transform element is not consistent.
Inspecting the file rect_13534_preserved.svg, we see that for both rectangles, the original coordinates are expressed in pixels, as expected, and in both cases the transform to mm occurs in a <rect> element, so the two objects are directly comparable, even in the XML editor.
The conclusion is that the original code in rev 13534 produces a more consistent representation of the two layers.
rect_13534.svg http://inkscape.13.x6.nabble.com/file/n4971483/rect_13534.svg rect_13534_preserved.svg http://inkscape.13.x6.nabble.com/file/n4971483/rect_13534_preserved.svg rect_13546.svg http://inkscape.13.x6.nabble.com/file/n4971483/rect_13546.svg rect_13546_preserved.svg http://inkscape.13.x6.nabble.com/file/n4971483/rect_13546_preserved.svg
hth, Alvin
-- View this message in context: http://inkscape.13.x6.nabble.com/Please-check-r13544-or-later-for-problems-a... Sent from the Inkscape - Dev mailing list archive at Nabble.com.
Want excitement? Manually upgrade your production database. When you want reliability, choose Perforce Perforce version control. Predictably reliable. http://pubads.g.doubleclick.net/gampad/clk?id=157508191&iu=/4140/ostg.cl... _______________________________________________ Inkscape-devel mailing list Inkscape-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/inkscape-devel
Can somebody please explain why it is important that the internal coordinates and lengths of the SVG file be at every location in some particular set of units? What specific problem(s) require this constraint?
As Tav indicated in his message in this thread, and as I pointed out earlier, the only number that has any meaning for a coordinate is the final value after all transforms are applied. Consequently the only way that all coordinates can be expressed in units is if only unitary transforms are allowed, and that is a needlessly restrictive condition for most drawings.
Regards,
David Mathog mathog@...1176... Manager, Sequence Analysis Facility, Biology Division, Caltech
It is a question of aesthetics, and it is a question of internal consistency of representation, and it is a question of commutativity of operations, as discussed above.
Alvin
-- View this message in context: http://inkscape.13.x6.nabble.com/Please-check-r13544-or-later-for-problems-a... Sent from the Inkscape - Dev mailing list archive at Nabble.com.
David, If you still do not understand why your "fix" was bad, then please stop working on this part of Inkscape.
Thanks, Johan
On 8-9-2014 22:05, mathog wrote:
Can somebody please explain why it is important that the internal coordinates and lengths of the SVG file be at every location in some particular set of units? What specific problem(s) require this constraint?
As Tav indicated in his message in this thread, and as I pointed out earlier, the only number that has any meaning for a coordinate is the final value after all transforms are applied. Consequently the only way that all coordinates can be expressed in units is if only unitary transforms are allowed, and that is a needlessly restrictive condition for most drawings.
Regards,
David Mathog mathog@...1176... Manager, Sequence Analysis Facility, Biology Division, Caltech
Want excitement? Manually upgrade your production database. When you want reliability, choose Perforce Perforce version control. Predictably reliable. http://pubads.g.doubleclick.net/gampad/clk?id=157508191&iu=/4140/ostg.cl... _______________________________________________ Inkscape-devel mailing list Inkscape-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/inkscape-devel
Thanks for your mail Tav. What you are describing is exactly what Inkscape is doing.
-Johan
On 8-9-2014 21:36, Tavmjong Bah wrote:
I do not have time to study this issue carefully at them moment and to follow who is arguing what... but:
Units should never be used inside an SVG file other than to set the width and height on the SVG root element (and even then it is questionable).
The original intent for including units inside an SVG was so that one could set the size of an object on a display regardless of the display's DPI. This did not catch on. Very few displays returned the accurate DPI information that was needed to make this work. The CSS working group after many long arguments fixed one inch to 96 pixels regardless of the size of the display. This is now written in stone.
I argued in the SVG WG that one should be able to set the number of pixels per inch with a property (like we have for exporting a PNG). This was a non-starter.
If one really wants to use a unit of "mm" then set the width and height of the SVG root in "mm" and provide a 'viewBox' that matches. For example:
<svg width="100mm" height="100mm" viewBox="0 0 100 100">
will give a scale of 1 'user unit' (px) to 1mm.
Tav
On Mon, 2014-09-08 at 12:01 -0700, alvinpenner wrote:
Attached are some aesthetic comments on the difference between the original code and the modified code which preserves the transforms when changing document units. There are two problems that can occur when preserving unit transforms in a layer definition. First of all the operation of changing document units does not commute with the operation of creating a new layer, which is unexpected:
- open Inkscape with default units px
- draw rectangle
- change units to mm
- create new Layer 2
- draw same-sized rectangle
- save as rect_13546.svg
Compare the two objects in the XML editor and note that the second rectangle 'height' and 'width' are expressed in mm as expected. The first rectangle has its dimensions expressed in pixels which have subsequently been transformed to mm. The net result is that the 'height' and 'width' properties are not comparable when viewing them in the XML editor, because the history of how they were produced has been remembered.
Peforming the same operations in rev 13534 we get the file rect_13534.svg. In this file all the properties are expressed in mm, as expected.
Secondly, the impact of the Preference setting for Optimized/Preserved transforms is modified significantly. The above operations were done with the Preference setting "Behavior->Transforms->Optimized". Now we perform the same two experiments with "Behavior->Transforms->Preserved", to produce the files rect_13546_preserved.svg and rect_13534_preserved.svg.
Inspecting the file rect_13546_preserved.svg, we see that for both rectangles, the original coordinates are expressed in pixels, as expected, and in both layers the conversion to mm occurs as a result of a transform, but the first rectangle has the transform in the layer <group> element while the second rectangle has the transform in the <rect> element, so the location of the transform element is not consistent.
Inspecting the file rect_13534_preserved.svg, we see that for both rectangles, the original coordinates are expressed in pixels, as expected, and in both cases the transform to mm occurs in a <rect> element, so the two objects are directly comparable, even in the XML editor.
The conclusion is that the original code in rev 13534 produces a more consistent representation of the two layers.
rect_13534.svg http://inkscape.13.x6.nabble.com/file/n4971483/rect_13534.svg rect_13534_preserved.svg http://inkscape.13.x6.nabble.com/file/n4971483/rect_13534_preserved.svg rect_13546.svg http://inkscape.13.x6.nabble.com/file/n4971483/rect_13546.svg rect_13546_preserved.svg http://inkscape.13.x6.nabble.com/file/n4971483/rect_13546_preserved.svg
hth, Alvin
-- View this message in context: http://inkscape.13.x6.nabble.com/Please-check-r13544-or-later-for-problems-a... Sent from the Inkscape - Dev mailing list archive at Nabble.com.
Want excitement? Manually upgrade your production database. When you want reliability, choose Perforce Perforce version control. Predictably reliable. http://pubads.g.doubleclick.net/gampad/clk?id=157508191&iu=/4140/ostg.cl... _______________________________________________ Inkscape-devel mailing list Inkscape-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/inkscape-devel
Want excitement? Manually upgrade your production database. When you want reliability, choose Perforce Perforce version control. Predictably reliable. http://pubads.g.doubleclick.net/gampad/clk?id=157508191&iu=/4140/ostg.cl... _______________________________________________ Inkscape-devel mailing list Inkscape-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/inkscape-devel
participants (5)
-
alvinpenner -
Johan Engelen -
mathog -
su_v -
Tavmjong Bah