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

Disable anti-aliasing in Ahem #13010

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

gsnedders
Copy link
Member

Previously, we had three gaspRange entries in the gasp table:

  • rangeMaxPPEM=8, rangeGaspBehavior=GASP_DOGRAY
  • rangeMaxPPEM=16, rangeGaspBehavior=GASP_GRIDFIT
  • rangeMaxPPEM=65535, rangeGaspBehavior=GASP_GRIDFIT|GASP_DOGRAY

We now have a single one:

  • rangeMaxPPEM=65535, rangeGaspBehavior=GASP_GRIDFIT

See also: https://bugzilla.mozilla.org/show_bug.cgi?id=1258916#c118 and https://bugzilla.mozilla.org/show_bug.cgi?id=1490969

Previously, we had three gaspRange entries in the gasp table:

 * rangeMaxPPEM=8, rangeGaspBehavior=GASP_DOGRAY
 * rangeMaxPPEM=16, rangeGaspBehavior=GASP_GRIDFIT
 * rangeMaxPPEM=65535, rangeGaspBehavior=GASP_GRIDFIT|GASP_DOGRAY

We now have a single one:

 * rangeMaxPPEM=65535, rangeGaspBehavior=GASP_GRIDFIT
@gsnedders
Copy link
Member Author

From a brief look, css/CSS2/box-display/display-change-001.xht still fails with this. :(

@Hexcles
Copy link
Member

Hexcles commented Sep 14, 2018

On a side note, looks like the check for slow tests is working as intended. The two tests are indeed slow because of the use of trickle; they should be given a long timeout.

@jfkthame
Copy link
Contributor

From a brief look, css/CSS2/box-display/display-change-001.xht still fails with this. :(

Does the rendering (antialiasing) change in any way as a result of tweaking the 'gasp' table, or is it completely ignored?

@svgeesus
Copy link
Contributor

As @jfkthame points out, whether gasp is honored depends on which rasterizer is used and with what settings. For Windows, it also matters whether sub-pixel anti-aliasing is enabled, whether hinting is enabled and whether the glyphs are truetype or CFF/type 1.

@jgraham
Copy link
Contributor

jgraham commented Sep 18, 2018

So is the conclusion that there's nothing that can be done in the font that will guarantee (on extant systems) that it's rendered without antialiasing and so we need either:

  • reftests to be valid even in the presence of antialiased Ahem
    or
  • browser-level support for disabling antialiased rendering for at least Ahem
    ?

@gsnedders
Copy link
Member Author

@jfkthame No, identical rendering.

Looking again, we don't actually have any hinting program in the font, which I guess makes grid-fitting do nothing anyway? Ultimately when we have glyphs that have straight sides I wouldn't expect it to change much?

@jfkthame
Copy link
Contributor

Looking again, we don't actually have any hinting program in the font, which I guess makes grid-fitting do nothing anyway? Ultimately when we have glyphs that have straight sides I wouldn't expect it to change much?

Depends on the rasterizer, I guess. FreeType may be using its own "auto-hinting" rather than using instructions from the font anyhow. And whether it pays attention to the 'gasp' table in that situation, I don't know offhand...

Even with simple straight-sided glyphs, grid-fitting could be important to adjust the edges to exact pixel boundaries for a given font size/resolution; if this is done, there shouldn't be any partially-transparent antialiasing at the edges.

In practice, I suspect we're going to have trouble precisely controlling the glyph rendering for all platforms and environments, and might be better off putting our efforts into crafting tests that are less dependent on this. In particular, I've seen tests that use Ahem glyphs in a testcase and then compare them with PNG images of the "expected" result. and fail because of antialiasing issues at the glyph edges. In many cases, the reference file could be rewritten so that it also uses Ahem glyphs (with the layout controlled in a different way than the testcase), which would probably avoid the issue.

@jgraham
Copy link
Contributor

jgraham commented Sep 19, 2018

I'm not confident of our ability to get people to build tests that are robust against antialiasing. There seem to be lots of tests that are "build a pattern out of Ahem blocks and compare it to the same pattern made from divs", and replacing those would be a lot of effort. Also, Blink are running with antialiasing disabled in their CI, so it seems highly likely that they will continue to submit tests which exhibit different behaviour with AA enabled.

@gsnedders
Copy link
Member Author

I'm in general dubious of our means to get test authors to do things if we can't programmatically check they've done it; we have too many contributors from too many different browser vendors to actually reasonably manage to get them to do anything that the tests for their own engine don't require.

In particular, I've seen tests that use Ahem glyphs in a testcase and then compare them with PNG images of the "expected" result. and fail because of antialiasing issues at the glyph edges. In many cases, the reference file could be rewritten so that it also uses Ahem glyphs (with the layout controlled in a different way than the testcase), which would probably avoid the issue.

They also often break under HiDPI (given bitmaps and vectors are scaled differently), which is also an issue I'd like to see fixed.

As @jgraham says though, we also have plenty that use divs and backgrounds to achieve the same affect, which shouldn't fall foul under HiDPI, and I'm much more dubious about it being worthwhile putting in the work to rewrite them.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants