Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[t2CharStringPen] add "roundTolerance" option #804

Merged
merged 7 commits into from
Jan 14, 2017

Conversation

anthrotype
Copy link
Member

I went for the easy route, and simply added a roundTolerance argument to the T2CharStringPen as @behdad was discussing in #769.

I personally think that doing more clever things would be a bit beyond the scope of fontTools.
But I'm happy to be convinced otherwise ;)

PTAL, thanks

Cosimo Lupo added 2 commits January 12, 2017 19:48
…argument

Rounding of coordinates is now disabled by default.

The pen now accepts an optional 'roundTolerance' float, with values between
0 and 1 (default is 0).

Values >= 0.5 mean round all coordinates to integers.

Values between 0 and 0.5 mean round only when the absolute difference
between the original float and the rounded integer is within the tolerance

Fixes fonttools#769
@justvanrossum
Copy link
Collaborator

I think the default should remain rounding behavior and that the new behavior should be explicitly turned on.

@anthrotype
Copy link
Member Author

Yeah, initially I also thought it'd better to keep the current default, but then got carried away :)
I'll restore the default rounding.

Shall I assert the tolerance is comprised between 0 and 0.5? Values between 0.5 and 1.0 don't make much sense..

@anthrotype anthrotype changed the title [t2CharStringPen] disable default rounding; add "roundTolerance" option [t2CharStringPen] add "roundTolerance" option Jan 12, 2017
Cosimo Lupo added 3 commits January 12, 2017 22:38
…Tolerance

hmtx only can have integer values, so it would be weird to have widths as float in CFF...
abs ends up calling fabs for floats anyway, and is a bit faster if 'number' is an int
@behdad
Copy link
Member

behdad commented Jan 13, 2017

No, please don't assert tolerance. I like to be able to say "UPEM * .005" for example and that should work. Assertions are for other things: for when the input is wrong. Which is not the case here.

@anthrotype
Copy link
Member Author

anthrotype commented Jan 13, 2017

I'm confused. I'm using the built in round() to convert float to int. The maximum tolerance (which I define as the max absolute difference between the rounded and unrounded values) is 0.5, and couldn't be greater.. Am I'm missing something?

@@ -77,7 +76,7 @@ def _round(number):
# return rounded integer if the tolerance is 0.5, or if the absolute
# difference between the original float and the rounded integer is
# within the tolerance
if tolerance == .5 or fabs(rounded - number) <= tolerance:
if tolerance == .5 or abs(rounded - number) <= tolerance:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just make this "tolerance >= .5"

@behdad
Copy link
Member

behdad commented Jan 13, 2017

the max absolute difference between the rounded and unrounded values) is 0.5

Correct. Which means rounding is an acceptable solution for any user-specified tolerance >= .5. A tolerance of 1.5 is NOT by any means an error.

@anthrotype
Copy link
Member Author

Ok, I got it. Thanks

@anthrotype anthrotype merged commit 5479090 into fonttools:master Jan 14, 2017
@anthrotype anthrotype deleted the t2pen-round branch January 14, 2017 11:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants