On 5-10-2013 0:22, Krzysztof Kosiński wrote:

2013/10/4 Johan Engelen <jbc.engelen@...2592...>:

...

On 4-10-2013 18:30, Krzysztof Kosiński wrote:

...

The concept of using a viewBox that makes user units equivalent to inches or other large units is severely broken, because currently we use only 6 digits of precision for all coordinates. That was sufficient for pixels, but is definitely not sufficient for a lossless conversion between pixels and inches.

Assuming 90dpi, you're saying that 4 digits of precision in terms of pixels is not enough in an inch-document-unit document? Can you provide an example of where it goes wrong?

See the 2nd bug: https://bugs.launchpad.net/inkscape/+bug/1235279

Maybe I'm not understanding this correctly, but insufficient precision seems to be a likely cause.

See my comment #4 in that bug report. There is a funny calculation there that should not be there I feel. And I think it all happens because the page height spinbox actually has a different value than the one in XML. Then with that funny calculation --> error. If you change the funny calculation to something more logical, it all works fine. (not quite, because of default units/etc, but as a proof of bug ;)

I don't know quite why that funny calculation is there, so I hope Matthew can have a look at it.

Cheers, Johan

On Oct 4, 2013, at 8:35 PM, Krzysztof Kosiński wrote:

I rewrote some of the unit code to fix the performance regression, and it seems it suffers not only from poor performance, but also design problems.

1. There is no way to convert a length such as "45%" or "12em" into

user units without additional information: the x-height, font size and the current viewbox. The new code is in fact worse in this respect than SVGLength, because the latter does have an update() method which allows one to provide this information. Calling value("px") on a Quantity defined in em can give the correct result only by accident.

2. The same class is used to represent radial and linear values, and

warnings are emitted at runtime when those two are mixed. This kind of checking should definitely be done at compile time, by using Geom::Angle for radial values.

3. Many functions take strings instead of integer constants, causing

performance problems. SVGLength uses an integer constant for the unit and caches the value after conversion user units (the only thing actually used when rendering), which makes it a lot faster.

I know that I had some concerns in these areas as the code was being worked out. I've been heavily distracted by day-job stuff, but that's settling down now. So giving things a once-over is my immediate goal.

The second point highlighted here is an architectural issue. I'm not certain on the particulars of which course we should take here, but Krzysztof is definitely correct in that we want to try to constrain things to compile-time checks.

The first point is one that might take a bit of looking at in order to come up wit the better solution. Calling out these use cases is *extremely* useful and will let us move forward quickly on resolution.

And finally the third point highlighted the immediate must-fix. We probably want to move to have string use kept to a minimum, and to avoid solutions with excessive run-time memory use. Round-trip stability is another thing we'll need to keep an eye on, as with various precision issues that is easy to lose accidentally.

What would be good would be if we can get any of these issues covered in unit tests. I can get some started, but getting contributions of more tests is very helpful.