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

[varLib.mutator] Improve CharString rounding #3481

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

khaledhosny
Copy link
Collaborator

Since CharString point coordinates are relative, rounding each point independently can accumulate rounding errors causing to the last point to not match the first one, which leads to rendering artifacts.

This is a hack to round the absolute point coordinates, but using pens to convert the relative coordinates to absolute ones and back.

This is a hack since it drops hinting operators, and it can’t preserve the original operators, in addition to being slower.

There is probably a more clever way to doing this.

Because CharString commands are relative, rounding point coordinates can
result in accumulating round errors causing the last point in the
contour to not match the first point, leaning to rendering artifacts.

This test demonstrates this.
Since CharString point coordinates are relative, rounding each point
independently can accumulate rounding errors causing to the last point
to not match the first one, which leads to rendering artifacts.

This is a hack to round the absolute point coordinates, but using pens
to convert the relative coordinates to absolute ones and back.

This is a hack since it drops hinting operators, and it can’t preserve
the original operators, in addition to being slower.

There is probably a more clever way to doing this.
@khaledhosny
Copy link
Collaborator Author

This is the kind of artifacts caused by the round errors:
image
(it further leads to tiny little paths after overlap removal)

Interestingly, harfbuzz instancing produces the same artifacts, but AFDKO’s tx does not.

@justvanrossum
Copy link
Collaborator

Smells like a variant of this: #2838

khaledhosny added a commit to TiroTypeworks/TiroTools that referenced this pull request Apr 13, 2024
Use AFDKO’s tx for instancing CFF2 table, to work around issues with
FontTools instancer (fonttools/fonttools#3481).
@khaledhosny
Copy link
Collaborator Author

Smells like a variant of this: #2838

I think so (here at the instancing not merging side, though).

Not rounding at all works as well, but it gives larger file size and can expose bugs in implementations not prepared to handle fractional CFF coordinates.

@behdad
Copy link
Member

behdad commented Apr 13, 2024

Interestingly, harfbuzz instancing produces the same artifacts, but AFDKO’s tx does not.

@qxliu76

@anthrotype
Copy link
Member

thanks for spotting this Khaled

There is probably a more clever way to doing this.
harfbuzz instancing produces the same artifacts, but AFDKO’s tx does not.

would be nice to find out how tx manages to do this while, if I understand correctly, also preserving hinting and other operators. Maybe @skef could help undestand this?

@skef
Copy link
Contributor

skef commented Apr 16, 2024

tx translates the various PostScript-like CharString formats into callbacks of this form:

struct abfGlyphCallbacks_ {
    void *direct_ctx;
    void *indirect_ctx;
    abfGlyphInfo *info;
    int (*beg)(abfGlyphCallbacks *cb, abfGlyphInfo *info);
    void (*width)(abfGlyphCallbacks *cb, float hAdv);
    void (*move)(abfGlyphCallbacks *cb, float x0, float y0);
    void (*line)(abfGlyphCallbacks *cb, float x1, float y1);
    void (*curve)(abfGlyphCallbacks *cb,
                  float x1, float y1,
                  float x2, float y2,
                  float x3, float y3);
    void (*stem)(abfGlyphCallbacks *cb,
                 int flags, float edge0, float edge1);
    void (*flex)(abfGlyphCallbacks *cb, float depth,
                 float x1, float y1,
                 float x2, float y2,
                 float x3, float y3,
                 float x4, float y4,
                 float x5, float y5,
                 float x6, float y6);
    void (*genop)(abfGlyphCallbacks *cb, int cnt, float *args, int op);
    void (*seac)(abfGlyphCallbacks *cb,
                 float adx, float ady, int bchar, int achar);
    void (*end)(abfGlyphCallbacks *cb);
    void (*moveVF)(abfGlyphCallbacks *cb, abfBlendArg *x0, abfBlendArg *y0);
    void (*lineVF)(abfGlyphCallbacks *cb, abfBlendArg *x1, abfBlendArg *y1);
    void (*curveVF)(abfGlyphCallbacks *cb,
                    abfBlendArg *x1, abfBlendArg *y1,
                    abfBlendArg *x2, abfBlendArg *y2,
                    abfBlendArg *x3, abfBlendArg *y3);
    void (*stemVF)(abfGlyphCallbacks *cb,
                   int flags, abfBlendArg *edge0, abfBlendArg *edge1);
};

Note that the hints are represented with absolute positions and are (currently) in "hint substitution" style rather than "hintmask" style (for CFF variants they're translated back into hintmask style after processing).

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

5 participants