[graphite] Fix shaping with varying font sizes

[khaledhosny]

See https://bugs.documentfoundation.org/show_bug.cgi?id=103403#c7

This is similar to commit 061105e for Core Text.

[khaledhosny]

This patch look big than it is actually it, because I couldn’t find a way to query grfont for its ppm so i had to store the x and y scale in the font user data.
cc @mhosken

[behdad]

I suggest we create grfont at a fixed font size and scale the output. That's what we do for CoreText. Your current patch uses the font size at the time the grfont is created first. Right?

[behdad]

I should actually fix harfbuzz to recreate font data when font scale changes...

[khaledhosny]

I suggest we create grfont at a fixed font size and scale the output. That's what we do for CoreText. Your current patch uses the font size at the time the grfont is created first. Right?

Right. I was thinking what about size-specific adjustments (not sure Graphite has any), but then again I’ll be getting arbitrary ones with the current code.

Thinking about it again, may be keep the code in HarfBuzz the way it is and just set the font scale always to EM size at LibreOffice end and scale the output.

[behdad]

It's still an improvement to fix the code to shape at a fixed size and scale.

[khaledhosny]

I’m just concerned if someone is using this and is expecting size-specific adjustments or metrics, though it might not even be a valid concern. @mhosken what do you think?

[behdad]

Yeah I don't think graphite does size-specific shaping.

[mhosken]

Firstly, Graphite doesn't do size related shaping. I think that the only tool in the LIbreOffice suite that would be at all interested in size related shaping is Impress, where screen rendering leads the way. In all other tools, printer rendering leads the way and that means that ideally shaping should use printer metrics rather than screen metrics. I know it renders everything twice (screen and printer), but I think I'm saying we don't need to worry about this.

[khaledhosny]

I now shape at UPEM, then scale the output afterwords. But I’m getting very large glyph advances now, and if I drop the scaling part everything is fine! No idea what is going on.

[khaledhosny]

OK, I think the bad advances I’m getting because hb_graphite2_get_advance() will use the font scale not the UPEM, not sure how to fix this.

[behdad]

Right... We should fix this properly then. I'll try to find time to do that.

[mhosken]

If you want to switch hb over to working in design space at upem and then scaling the results, that makes life a tad easier in graphite since you can run without a gr_font. Passing NULL to gr_make_segment returns positions in design space.

[behdad]

If you want to switch hb over to working in design space at upem and then scaling the results, that makes life a tad easier in graphite since you can run without a gr_font. Passing NULL to gr_make_segment returns positions in design space.

Ok, that would work nicely when I add hb_face_get_canonical_font() [0].

[0] https://lists.freedesktop.org/archives/harfbuzz/2015-October/005174.html

[khaledhosny]

Thanks @mhosken for the suggestion, this seems to work now.

The font data is unused now, except for hb_graphite2_font_get_gr_font(), but I think this function makes no much sense now. Should I mark it deprecated and always return NULL?

[mhosken]

Yes this looks fine. It's shame there is still all that boilerplate kicking around. If neither coretext nor graphite now have a font object, is it still needed? Just an idea.

[khaledhosny]

@behdad and @mhosken, should I merge this?

[behdad]

Is this ready to merge?

[khaledhosny]

Yes.