Skip to content

Conversation

garretrieger
Copy link
Collaborator

No description provided.

Limit the max number of operations (set additions) that can be performed.
@garretrieger garretrieger requested a review from behdad October 26, 2018 00:10
Skip processing of any that have already been visited.
@behdad
Copy link
Member

behdad commented Oct 26, 2018

While legit fonts don't use the same bytes for a script and a langsys, your code works. But I dislike making that assumption. I think we should have separate sets for script and langsys. wdyt?

Also, taking offset from start of table is misleading, in that we might also receive the nul object, which is outside of the table. I suggest you handle that by first checking if number of children in object is zero and return in that case, then check in the set (the way you have it with offsets is fine).

Nit, I'd rename "get_gsubgpos_table" to "_get_gsubgpos".

Thanks.

table_tag,
script_index,
&script_index)
&& context.visit (&context.gsubgpos.get_script (script_index)))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't like that we are once calling get_script to check, and then call a function with script_index. Please use the function chain to take the script object and next the langsys object, instead of indices, so we look them up once. And I would move the "visit" check into the next function instead of before calling it.

Also, "visit" name is misleading since it suggests it does the actual visitation. "may_visit" or something is better. Or invert it as "visited"...

@behdad
Copy link
Member

behdad commented Oct 26, 2018

I'm working on this.

behdad added a commit that referenced this pull request Oct 26, 2018
behdad added a commit that referenced this pull request Oct 26, 2018
@behdad behdad closed this in eb44bfc Oct 26, 2018
ebraminio pushed a commit to ebraminio/harfbuzz that referenced this pull request Nov 24, 2018
ebraminio pushed a commit to ebraminio/harfbuzz that referenced this pull request Nov 24, 2018
ebraminio pushed a commit to ebraminio/harfbuzz that referenced this pull request Nov 24, 2018
ebraminio pushed a commit to ebraminio/harfbuzz that referenced this pull request Nov 24, 2018
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.

2 participants