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

Safe-to-break API #224

Closed
behdad opened this issue Feb 10, 2016 · 16 comments
Closed

Safe-to-break API #224

behdad opened this issue Feb 10, 2016 · 16 comments
Assignees
Labels
Chrome Chrome/Chromium project related issues and requests enhancement Priority-High

Comments

@behdad
Copy link
Member

behdad commented Feb 10, 2016

Moving from:
https://bugs.freedesktop.org/show_bug.cgi?id=71443

We like to add bits to the buffer to return "safe-to-break-here", which means, if you break text here and shape the two sides separately, you get the same result as shaping together. This is useful for performant line-breaking.

Starting sketch here:
https://gist.github.com/behdad/149ae8947c11afddc560

behdad added a commit that referenced this issue May 5, 2016
Not all shapers code is updated to set this properly.  GSUB and Arabic shaper are updated.
GPOS and other shapers are NOT.

Fixes #224
@behdad
Copy link
Member Author

behdad commented May 5, 2016

Work-in-progress branch here: https://github.com/behdad/harfbuzz/commits/wip-unsafe-to-break

@behdad
Copy link
Member Author

behdad commented May 5, 2016

Sample run:

$ ./hb-shape Lobster.ttf Lobster  --show-flags
[L=0+776|o=1+899|b_s=2+1636#2|t=4+547#2|e=5+786#2|r=6+770#2]

@behdad
Copy link
Member Author

behdad commented May 5, 2016

cc @khaledhosny @simoncozens

@khaledhosny
Copy link
Collaborator

Thanks Behdad, I’ll have a look. I’m just wondering, does this help with breaking inside ligatures anyhow? I assume it will be a unsafe-to-break place, but then I’ll just need to reshape, no other shortcut? I’ve been trying to address breaking inside ligatures in Scribus (HOST-Oman/scribus#144) and all we did so far is to just disallow line breaking there.

@behdad
Copy link
Member Author

behdad commented May 5, 2016

Yeah, breaking inside a cluster needs reshape. Breaking inside a Unicode grapheme also needs reshape. This API identifies additional places that need a reshape.

@tayloraswift
Copy link
Contributor

any updates on this? if this doesn't work what is everyone using for linebreaking currently then

@eaenet
Copy link
Contributor

eaenet commented Feb 8, 2017

Hi Behdad, do you expend to get back to working at this in the near term? Anything I can do to help? We'd like to start using this for line breaking in Blink.
Thanks!

@behdad
Copy link
Member Author

behdad commented Feb 8, 2017

Hi Behdad, do you expend to get back to working at this in the near term? Anything I can do to help? We'd like to start using this for line breaking in Blink.

Yes, I like to. Wasn't high-priority. Will try to have something testable this quarter.

@behdad behdad self-assigned this Mar 2, 2017
@behdad behdad added Chrome Chrome/Chromium project related issues and requests Priority-High and removed Priority-Medium labels Apr 8, 2017
@behdad behdad closed this as completed in 40bd7e9 Aug 10, 2017
@behdad behdad reopened this Aug 10, 2017
@behdad
Copy link
Member Author

behdad commented Aug 10, 2017

Reopening, to fix GPOS, and non-Arabic complex shapers.

@behdad
Copy link
Member Author

behdad commented Aug 11, 2017

Fixed all:
[unsafe-to-break] Flag in Thai PUA shaping
[unsafe-to-break] Flag in Hangul shaper
[unsafe-to-break] Flag during mark attachment
[unsafe-to-break] Flag during cursive positioning
[unsafe-to-break] Flag during kerning
[unsafe-to-break] Flag during fallback positioning
[unsafe-to-break] Be careful with flag propagation when merging clusters
[unsafe-to-break] Mark Indic-like clusters as unsafe-to-break

Not tested. Adding a verification step to test suite and possibly hb-shape to verify safe-to-break points.

@khaledhosny
Copy link
Collaborator

I’m wondering, can this flag be used for letter-spacing as well as it seems to handle cursive and mark attachment, or do one still need to check Unicode grapheme clusters boundaries?

@behdad
Copy link
Member Author

behdad commented Aug 11, 2017

I’m wondering, can this flag be used for letter-spacing as well as it seems to handle cursive and mark attachment, or do one still need to check Unicode grapheme clusters boundaries?

I have another message in my drafts folder for exposing Arabic and cursive joining. This flag is much more broad. A long chain-context match marks the entire match range as unsafe-to-break.

@eaenet
Copy link
Contributor

eaenet commented Aug 15, 2017

Thanks so much Behdad, looks great and works like a charm!

One small suggestion:
Having the hb_glyph_info_t mask field hold the flags post-shape is a little weird from an API perspective but makes sense on the implementation side.
How about changing it to be a union, thereby making the API more explicit?

union {
hb_mask_t mask;
hb_glyph_flags_t flags
}

@behdad
Copy link
Member Author

behdad commented Aug 15, 2017

Thanks so much Behdad, looks great and works like a charm!

Great to hear!

One small suggestion:
Having the hb_glyph_info_t mask field hold the flags post-shape is a little weird from an API perspective but makes sense on the implementation side.

Previously mask was unused publicly. The fact that hb_mask_t was even exposed is a mistake in retrospect. So I'm fine trying to repurpose it to the extent possible.

How about changing it to be a union, thereby making the API more explicit?

union {
hb_mask_t mask;
hb_glyph_flags_t flags
}

Humm. The question would be whether all compilers we care about allow unnamed unions... Another hesitation I have is, we might not want to allocate all 32 bits of the mask to glyph flags at this time... Anyway. I'll think about it.

One alternative would be to add accessor function with a duplicate macro implementation, like hb_glyph_info_get_glyph_flags() that does the access and cast.

@eaenet
Copy link
Contributor

eaenet commented Aug 16, 2017

An assessor might be the easiest, either way it's not a big deal. Love the functionality!

@behdad
Copy link
Member Author

behdad commented Sep 21, 2017

This is all fixed now. :)

@behdad behdad closed this as completed Sep 21, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Chrome Chrome/Chromium project related issues and requests enhancement Priority-High
Projects
None yet
Development

No branches or pull requests

4 participants