servo / font-kit

A cross-platform font loading library written in Rust
Apache License 2.0
693 stars 104 forks source link

Fix DirectWrite raster_bounds arithmetic #92

Closed idursun closed 5 years ago

idursun commented 5 years ago

Fixes #85

Offsetting origin calculation that I am referring to is:

https://github.com/pcwalton/font-kit/blob/6801dce15cbc75448d99420eeebbe8c867585f36/examples/render-glyph.rs#L133-L136

This change sets the origin of the raster_bounds rectangle so that the origin offset arithmetic actually holds for directwrite.

c:\repositories\rust\font-kit>cargo run --example render-glyph ArialMT { 32
    Finished dev [unoptimized + debuginfo] target(s) in 0.09s
     Running `target\debug\examples\render-glyph.exe ArialMT { 32`
glyph 94:
            ▓▓██████
          ██████████
        ▒▒████▒▒
        ▓▓██▓▓
        ▓▓██▓▓
        ▓▓██▓▓
        ▓▓██▓▓
        ▓▓██▓▓
        ▓▓██▓▓
        ████▓▓
        ████▒▒
      ▒▒████▒▒
    ▒▒████▓▓
░░████████
░░████▒▒
░░████████
    ▒▒████▓▓
      ▒▒████▒▒
      ░░████▒▒
        ████▓▓
        ▓▓██▓▓
        ▓▓██▓▓
        ▓▓██▓▓
        ▓▓██▓▓
        ▓▓██▓▓
        ▓▓██▓▓
        ▒▒████▒▒
          ██████████
            ▓▓██████
RazrFalcon commented 5 years ago

I've tested it with silicon and looks like everything is fine. Could you add some tests, especially for the { case?

idursun commented 5 years ago

Sure 👍

Similar to the rasterization tests (i.e. rasterize_glyph_with_grayscale_aa, rasterize_glyph_bilevel) that are asserted with check_L_shape?

RazrFalcon commented 5 years ago

Yes, I think this will work well.

idursun commented 5 years ago

I have added a "best-effort" test for verifying the rasterization of any top-down symmetrical letters.

Currently, it's only testing { letter and running against windows and mac. Although, I believe it should kinda work against all loaders.

RazrFalcon commented 5 years ago

I think this is good enough. Thanks again!

idursun commented 5 years ago

Changed to target only Windows. Apparently, macos glyph is not as symmetrical.

pcwalton commented 5 years ago

Thanks!