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

[shape] Add dedicated OpenType and AAT shapers #4269

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft

Conversation

khaledhosny
Copy link
Collaborator

Fixes #4108

@khaledhosny
Copy link
Collaborator Author

@behdad is this the direction to take for #4108?

This is the main HarfBuzz shaper and it does OpenType or AAT shaping
using based on the available font tables, and it never fails.

We will introduce “ot” and “aat” shaper that do only OpenType or AAT
shaping and fail otherwise.
They currently fail if relevant tables are missing, but don’t actually
enforce the relevant shaping model.
@behdad
Copy link
Member

behdad commented Jun 7, 2023

This is a good start. But I think you need to tell the shaper which mode it's operating under. If a font has morx+GPOS, right now the ot shaper still picks up both of those.

@behdad
Copy link
Member

behdad commented Jun 7, 2023

Also, I think both those shapers should NOT fail if their layout tables are missing. We just need to communicate to the main shaper what layout tables to prefer / what not to pick up. Shaping without layout tables is fine I think, no?

@behdad
Copy link
Member

behdad commented Jun 7, 2023

Maybe lets sketch out what exactly the two should do first.

@khaledhosny
Copy link
Collaborator Author

This is a good start. But I think you need to tell the shaper which mode it's operating under. If a font has morx+GPOS, right now the ot shaper still picks up both of those.

Yes, this is just a sketch, and I wanted to get your input before going further.

Maybe lets sketch out what exactly the two should do first.

So I suppose there are a few situations to handle:

  1. A font with only OpenType layout tables
  2. A font with only AAT layout tables
  3. A font with no layout tables at all
  4. A font with both OpenType and AAT layout tables.

For the first 3 cases I don’t want anything to change, shaping should continue as it does not. For the last case I want OpenType layout tables to take precedence.

My idea is to order the shapers as following: ot, hb, fallback. So ot shaper should do only OpenType layout and it wouldn’t fail for cases 1 and 4, it would fail for 2 and 3 so that hb shaper takes over and does what it does now (fallback shaper is here just in case, as usual).

In my use case I wouldn’t use aat shaper at all, but it might be used by e.g. LuaTeX (luaotfload) where there user has an option to select AAT shaping (which I think no longer works since aat_coretext shaper was dropped).

I can imagine ot and aat shaper being also used for testing purposes, where not doing fallback shaping is desirable.

@khaledhosny
Copy link
Collaborator Author

@behdad does any of what I wrote above make sense?

@behdad
Copy link
Member

behdad commented Jun 19, 2023

@behdad does any of what I wrote above make sense?

I've been travelling. Will respond soon.

@behdad
Copy link
Member

behdad commented Jun 19, 2023

Okay the main thing I'm uncomfortable with is suddenly making "ot" shaper fail at all. This wasn't the case before. I'm fine if it starts ignoring "aat" tables, but failing might be bad. I know there are a client or more that explicitly pass "ot"...

@behdad
Copy link
Member

behdad commented Jun 19, 2023

The way I see it is rather:

  • "ot" would never apply morx/kerx,
  • "aat" would never apply GSUB,
  • "hb" is as is today.

shaper order will have "hb" at the top.

@behdad
Copy link
Member

behdad commented Jun 19, 2023

My reasoning is that "no layout table" is valid OpenType / AAT shaping. So we shouldn't fail.

@khaledhosny
Copy link
Collaborator Author

But if the shapers don’t fail, then if I set ot first then a font with AAT tables will never use them, and vice versa, so I can’t address my current issue: some fonts have both AAT and OT tables and I want to use OT for them (because that is what happens in Windows, and at least in one case the AAT tables were unusable tables generated by FontForge and no one noticed they are there), but if the font has only AAT tables I still want to use that.

@behdad
Copy link
Member

behdad commented Jun 19, 2023

I can see two options for "ot", does any of these satisfy your requirements:

  • Ignore AAT tables,
  • Prefer OpenType tables.

@khaledhosny
Copy link
Collaborator Author

OK, so another idea. Make the new ot and aat shapers use only the respective layout tables but not fail, and instead of depending on them failing and HarfBuzz falling back to the next shaper, the client code would do something like:

bool has_aat = hb_aat_layout_has_substitution(face) || hb_aat_layout_has_positioning(face);
bool has_ot = hb_ot_layout_has_substitution(face) || hb_ot_layout_has_positioning(face);

if (has_aat && has_ot)
  // hybrid font, we want OT
  shapers = {"ot", "fallback"};
else
  // Keep HarfBuzz defaults
  shapers = default_shapers;

@khaledhosny khaledhosny mentioned this pull request Jun 23, 2023
@behdad
Copy link
Member

behdad commented Jun 23, 2023

if (has_aat && has_ot)
  // hybrid font, we want OT
  shapers = {"ot", "fallback"};

But if "ot" always tries ot before aat, then you won't even need the conditional, right?

@behdad
Copy link
Member

behdad commented Jun 23, 2023

if (has_aat && has_ot)
  // hybrid font, we want OT
  shapers = {"ot", "fallback"};

But if "ot" always tries ot before aat, then you won't even need the conditional, right?

Or is that what you want to avoid?

@behdad
Copy link
Member

behdad commented Jun 23, 2023

@jfkthame can we get some direction here?

@khaledhosny
Copy link
Collaborator Author

if (has_aat && has_ot)
  // hybrid font, we want OT
  shapers = {"ot", "fallback"};

But if "ot" always tries ot before aat, then you won't even need the conditional, right?

Or is that what you want to avoid?

Right, I wouldn’t need it indeed. Not for my use case here.

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 this pull request may close these issues.

A way to explicitly prefer OpenType shaping over AAT
2 participants