2013/8/18 mathog <mathog@...1176...>:
12471 removes all of these defines and makes substitutions throughout the code where
PX_PER_IN
is replaced by
Inkscape::Util::Quantity::convert(1, "in", "px")
My first criticism is that the new quantity is at first glance, indeterminate. Is it converting IN to PX or vice versa, what does "1" do?
Obviously, it converts one inch to pixels.
My second criticism is that eliminating the defines results in hundreds of substitutions in 12471 that are entirely unnecessary. All that was required was to change the defines in src/unit-constants.h, for instance from
#define PX_PER_IN (PT_PER_IN / PT_PER_PX)
to
#define PX_PER_IN Inkscape::Util::Quantity::convert(1, "in", "px")
after which virtually no changes elsewhere in the code are required. By doing that, the existing and relatively clear code, like:
p0[X] = (p0[X] * IN_PER_PX * dwDPI);
need not become the obfuscated (to my mind) form:
p0[X] = (p0[X] * Inkscape::Util::Quantity::convert(1, "px", "in") * dwDPI);
Indeed, this is obfuscated. It should be just:
p0[X] = Inkscape::Util::Quantity::convert(p0[X], "px", "in")
Another problem is that the name chosen for this function is extremely long. I have no idea who invented the "Util" namespace, this should all be in the Inkscape namespace preferably as a global function (gratuitous use of namespace is a plague in our code), or what does "Quantity" mean. Ideally the code should look like this:
p0[X] = Inkscape::convert_unit(p0[X], "px", "in");
I think this is more expressive that p0[X] * IN_PER_PX.
Regards, Krzysztof