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

Question regarding discrepancies when using hinted drawing mode #936

Closed
LaurenzV opened this issue Jun 5, 2024 · 8 comments · Fixed by #1033
Closed

Question regarding discrepancies when using hinted drawing mode #936

LaurenzV opened this issue Jun 5, 2024 · 8 comments · Fixed by #1033

Comments

@LaurenzV
Copy link
Contributor

LaurenzV commented Jun 5, 2024

Hello,

I'm trying to use skrifa as a fuzzing tool to verify that my subsetter is working correctly. Everything works fine when I use the unhinted drawing mode, however, I'm facing some problems when using the hinted drawing mode. I'm facing the same problem when using subsets produced by fonttools instead of my own subsetter, so I don't think the issue is on my side.

In particular, the problem is that there seem to be slight variations in the drawn outlines depending on which glyphs are added to the subset (even though the glyphs shouldn't be related to each other at all! I verified this by comparing the ttx output of fonttools). So for example, I have a font where I'm drawing the glyph 1 using hinted mode. If I create a subset with fonttools using the glyphs 1-5 and I redraw glyph 1 in hinted mode, I get the same result. However, if the subset includes the glyphs 1-10 I suddenly get slightly different results for the same glyph!

So I wanted to ask whether this is expected behavior, i.e. whether it's possible that the number of glyphs somehow affects the result of hinting. If not, I can try to get a small example to working to demonstrate the issue.

@dfrg
Copy link
Member

dfrg commented Jun 6, 2024

I wouldn’t expect subsetting to have any effect on hinting so this behavior definitely seems weird to me. If you send me the font and subset where the two produce different hinted outlines, I’d be happy to take a look.

@LaurenzV
Copy link
Contributor Author

LaurenzV commented Jun 6, 2024

So here is the code I'm using:

use std::fs;
use skrifa::instance::{LocationRef, Size};
use skrifa::MetadataProvider;
use skrifa::outline::{DrawSettings, HintingInstance, HintingMode, OutlinePen};

fn main() {
   let target_glyph = skrifa::GlyphId::new(1);

   let old_sink = {
      let mut sink = Sink(vec![]);
      let font = fs::read("JacquardaBastarda9Charted-Regular.ttf").unwrap();
      let face = skrifa::FontRef::new(&font).unwrap();
      let hinting_instance = HintingInstance::new(
         &face.outline_glyphs(),
         Size::new(150.0),
         LocationRef::default(),
         HintingMode::Smooth { lcd_subpixel: None, preserve_linear_metrics: false },
      ).unwrap();
      let settings = DrawSettings::hinted(&hinting_instance, false);
      let glyph = face.outline_glyphs().get(target_glyph).unwrap();
      glyph.draw(settings, &mut sink).unwrap();
      sink
   };

   let new_sink = {
      let mut sink = Sink(vec![]);
      let font = fs::read("out_ft.otf").unwrap();
      let face = skrifa::FontRef::new(&font).unwrap();
      let hinting_instance = HintingInstance::new(
         &face.outline_glyphs(),
         Size::new(150.0),
         LocationRef::default(),
         HintingMode::Smooth { lcd_subpixel: None, preserve_linear_metrics: false },
      ).unwrap();
      let settings = DrawSettings::hinted(&hinting_instance, false);
      let glyph = face.outline_glyphs().get(target_glyph).unwrap();
      glyph.draw(settings, &mut sink).unwrap();
      sink
   };

   assert_eq!(old_sink, new_sink);

}

#[derive(Debug, Default, PartialEq)]
struct Sink(Vec<Inst>);


#[derive(Debug, PartialEq)]
enum Inst {
   MoveTo(f32, f32),
   LineTo(f32, f32),
   QuadTo(f32, f32, f32, f32),
   CurveTo(f32, f32, f32, f32, f32, f32),
   Close,
}

impl OutlinePen for Sink {
   fn move_to(&mut self, x: f32, y: f32) {
      self.0.push(Inst::MoveTo(x, y));
   }

   fn line_to(&mut self, x: f32, y: f32) {
      self.0.push(Inst::LineTo(x, y));
   }

   fn quad_to(&mut self, x1: f32, y1: f32, x: f32, y: f32) {
      self.0.push(Inst::QuadTo(x1, y1, x, y));
   }

   fn curve_to(&mut self, x1: f32, y1: f32, x2: f32, y2: f32, x: f32, y: f32) {
      self.0.push(Inst::CurveTo(x1, y1, x2, y2, x, y));
   }

   fn close(&mut self) {
      self.0.push(Inst::Close);
   }
}

You can try the following steps to (hopefully) reproduce my issue:

  1. Download https://fonts.google.com/specimen/Jacquarda+Bastarda+9+Charted
  2. Run fonttools subset JacquardaBastarda9Charted-Regular.ttf --gids=0-12 --output-file=out_ft.otf (I'm using v4.50, but I don't think the version should be relevant)
  3. Run the program. The assertion should work and the program should finish successfully.
  4. Now run the same command as above, but with --gids=0-11.
  5. Run the program. The assertion does not hold anymore, there seem to be small differences:
image

I hope I'm not missing something obvious. 😅 but since I'm only drawing the glyph 1, I don't think it should make a difference whether the 12th glyph is included or not. And I am seeing the exact same behavior with hb-subset as well as my own subsetter, so I don't think it's a subsetting issue.

@dfrg
Copy link
Member

dfrg commented Jun 7, 2024

Thanks for the detailed description! I was able to reproduce this easily.

Turns out that this is caused by the execution budget for the hinter which takes the number of glyphs into account when computing limits for backwards jumps (loops) in the bytecode. When you subset this below a certain number of glyphs (13, apparently), the budget is no longer sufficient to complete the hinting program and it gives up. Note that if you set the second parameter (is_pedantic) of DrawSettings::hinted to true, then we'll actually return an error and this code will panic on the unwrap.

In this case, we match FreeType which fails in the same way and produces the same discrepancy in "non pedantic" mode.

The good news is that this isn't a bug in your subsetter :)

The bad news is that changing this heuristic will break our compatibility with FreeType. Even so, using glyph count as a parameter for an execution budget seems to be a poor choice if we consider subsetted fonts.

@LaurenzV
Copy link
Contributor Author

LaurenzV commented Jun 7, 2024

Ah, I didn't consider trying the pedantic mode... Thanks a lot for looking into it! I guess in this case I will just stick to the unhinted drawer.

@LaurenzV LaurenzV closed this as not planned Won't fix, can't repro, duplicate, stale Jun 7, 2024
@rsheeter
Copy link
Collaborator

rsheeter commented Jun 7, 2024

takes the number of glyphs into account when computing limits for backwards jumps

As in a font with fewer glyphs gets a lower budget?!

using glyph count as a parameter for an execution budget seems to be a poor choice if we consider subsetted fonts.

Let's deviate from FreeType here. The other direction - large budget for a lot of glyphs - is also undesirable.

Do we have an issue for budgets where we might note the insights from this thread?

@dfrg
Copy link
Member

dfrg commented Jun 7, 2024

As in a font with fewer glyphs gets a lower budget?!

Yes, here is the relevant code:

limit = limit.min(100 * outlines.glyph_count());

Let's deviate from FreeType here

Agreed. I suggest we just remove the line linked above.

Do we have an issue for budgets where we might note the insights from this thread?

We don’t but this issue seems useful for tracking. Reopening.

@dfrg
Copy link
Member

dfrg commented Jul 25, 2024

@LaurenzV sorry for the delay but this should be fixed now and a release is imminent :)

@LaurenzV
Copy link
Contributor Author

Cool, thanks!

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 a pull request may close this issue.

3 participants