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

[ot-tag] Add HB_SCRIPT_MATH (Zmth) and map it to OT ‘math’ tag #3417

Merged
merged 1 commit into from Feb 8, 2022

Conversation

lexi-lambda
Copy link
Contributor

The ISO 15924 code for mathematical notation is Zmth, but the corresponding OpenType script tag is math. HarfBuzz needs to know about the mapping, or else OpenType features that depend on the math script won’t be applied. This PR adds HB_SCRIPT_MATH to hb_script_t (for convenience) and changes hb_ot_tags_from_script_and_language and hb_ot_tag_to_script to do the necessary conversion.

src/hb-common.h Outdated Show resolved Hide resolved
@behdad
Copy link
Member

behdad commented Feb 6, 2022

Looks good except for what Khaled said. Thanks.

src/hb-common.h Outdated Show resolved Hide resolved
@dscorbett
Copy link
Collaborator

As I understand it, 'math' is unlike other script tags in that it does not correspond to any specific characters. Instead, it is used to identify features relevant to the MATH table. Is 'math' ever used for non-MATH shaping? If not, is the existing constant HB_OT_MATH_SCRIPT sufficient?

@behdad
Copy link
Member

behdad commented Feb 6, 2022

I guess specifying the HB_OT_MATH_SCRIPT does find the right OT script tag after all, but if hb_shape() is involved, I don't mind adding what is proposed here.

@lexi-lambda
Copy link
Contributor Author

lexi-lambda commented Feb 6, 2022

@dscorbett I had missed the existence of HB_OT_MATH_SCRIPT, so thank you for pointing me to it—I’ve updated the definition of hb_ot_old_tag_to_script to use that constant. And yes, you are correct that Zmth is not a “real” Unicode script and is only relevant for OpenType shaping (though it is a real ISO 15924 code).

However, I’d still like this change. In Pango, there’s no way to arrange for an arbitrary hb_script_t to be passed to hb_buffer_set_script, so there’s no way to use HB_OT_MATH_SCRIPT via Pango’s interface. Instead, Pango uses glib’s GUnicodeScript enumeration, which contains a fixed set of values that are mapped to ISO 15924 codes via g_unicode_script_to_iso15924. Since HB_OT_MATH_SCRIPT is not a valid ISO 15924 code, it would not make sense for an entry of GUnicodeScript to ever produce it.

However, one potential alternative approach to this PR would be to make the change to hb_script_from_iso15924_tag and hb_script_to_iso15924_tag instead of hb_ot_old_tag_from_script and hb_ot_old_tag_to_script, in which case the internal hb_script_t representation of the ISO 15924 Zmth code could be HB_OT_MATH_SCRIPT. That seemed less preferable to me, since in general I got the sense that hb_script_t values adhere more closely to ISO 15924 codes than OpenType script tags, but if you think that way would be better, I’d be happy to do it, instead.

@dscorbett
Copy link
Collaborator

That sounds like a good reason to merge this. The reason does not apply to 'byzm' and 'musc', the other tags for non-Unicode scripts; I was wondering if we should add constants for them too, but that does not seem as necessary.

Did you mean to squash all those commits with “(squash)” in the message?

src/hb-ot-tag.cc Outdated Show resolved Hide resolved
@dscorbett
Copy link
Collaborator

The documentation of HB_OT_MATH_SCRIPT will be wrong once this is merged.

* HB_OT_MATH_SCRIPT:
*
* OpenType script tag for math shaping, for use with
* Use with hb_buffer_set_script().

Users should pass HB_SCRIPT_MATH to hb_buffer_set_script instead of HB_OT_MATH_SCRIPT. The latter might still work, but I agree it is better to use the value that matches ISO 15924 like all the other hb_script_t values. @behdad, should HB_OT_MATH_SCRIPT therefore be deprecated?

@behdad
Copy link
Member

behdad commented Feb 6, 2022

. @behdad, should HB_OT_MATH_SCRIPT therefore be deprecated?

It feels to me that way, yes.

@lexi-lambda
Copy link
Contributor Author

Did you mean to squash all those commits with “(squash)” in the message?

No, I decided to leave them unsquashed until merging to make it easier for reviewers to see what changed. But based on your comment, it seems like maybe that was more confusing than helpful, so I’ve just gone and squashed them now. :)

@lexi-lambda
Copy link
Contributor Author

should HB_OT_MATH_SCRIPT therefore be deprecated?

I’m not sure that it needs to be deprecated given HB_OT_TAG_DEFAULT_SCRIPT exists and is non-deprecated, even though it isn’t a valid hb_script_t. The name is perhaps a little unfortunate (it would be better to have been named HB_OT_TAG_MATH_SCRIPT for consistency), but I don’t think that’s a big deal.

I agree that the documentation should be updated, though, so I’ll do that.

@lexi-lambda lexi-lambda force-pushed the ot-tag-math-script branch 2 times, most recently from f9e4a38 to ac4de6b Compare February 6, 2022 23:34
@lexi-lambda
Copy link
Contributor Author

Alright, I’ve updated the documentation to the following:

/**
* HB_OT_MATH_SCRIPT:
*
* OpenType script tag, `math`, for math shaping.
*
* <note>Previous versions of this documentation recommended passing
* #HB_OT_MATH_SCRIPT directly to hb_buffer_set_script() to enable math shaping,
* but this usage is deprecated as of version REPLACEME and may not continue to
* work in future versions of HarfBuzz. New code should use #HB_SCRIPT_MATH to
* represent the math script wherever an #hb_script_t value is expected.</note>
*
* Since: 1.3.3
*/
#define HB_OT_MATH_SCRIPT HB_TAG('m','a','t','h')

@behdad
Copy link
Member

behdad commented Feb 7, 2022

Please move HB_OT_MATH_SCRIPT to hb-ot-deprecated.h and add a Deprecated: REPLACEME tag. The wording of "Previous versions of this doccumentation" is good and can remain but can be made more concise. Thanks.

The ISO 15924 code for mathematical notation is ‘Zmth’, but the
OpenType script is ‘math’.
@lexi-lambda
Copy link
Contributor Author

I think deprecating HB_OT_MATH_SCRIPT and suggesting unconditionally replacing it with HB_SCRIPT_MATH is unhelpful, as it is still useful in situations where a raw OpenType script tag is expected, such as in the scripts argument to hb_ot_layout_collect_features. Therefore, I’ve pushed a revision that makes two changes:

  1. I’ve added a new constant to hb-ot-math.h, HB_OT_TAG_MATH_SCRIPT, which has the old value of HB_OT_MATH_SCRIPT. The documentation notes that HB_SCRIPT_MATH should be used where an hb_script_t is expected.

  2. I’ve moved HB_OT_MATH_SCRIPT to hb-ot-deprecated.h and made it an alias for HB_OT_TAG_MATH_SCRIPT, and I’ve updated its documentation to recommend HB_SCRIPT_MATH or HB_OT_TAG_MATH_SCRIPT, instead. I’ve also pared down the historical <note> to be more succinct.

Let me know if you think this is reasonable. I suspect, in practice, it would be no great loss if HB_OT_TAG_MATH_SCRIPT were not included, but it provides nice parallelism with both HB_OT_TAG_DEFAULT_SCRIPT and HB_OT_TAG_MATH, so I think it makes sense to include it.

@behdad behdad merged commit 1bc4bad into harfbuzz:main Feb 8, 2022
@lexi-lambda lexi-lambda deleted the ot-tag-math-script branch February 8, 2022 18:53
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.

None yet

4 participants