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

make fields of lookup subtables public #100

Closed
wants to merge 1 commit into from

Conversation

tizee
Copy link

@tizee tizee commented Jun 29, 2022

A small fix on lookup subtables as the visibility of fields of a public struct are default to private, which means they cannot be accessed from the outside of the module.

    if let Some(table) = face.tables().gsub {
        for lookup in table.lookups {
            for subtable in lookup.subtables {
                // process subtable
                println!("{}", subtable.kind);
            }
     }
 }

Use case:

  • Some applications may need to manipulate these lookup tables manually so I need access to its fields. (Recently I was developing an text rendering application based on ttf-parser's LookupSubtables)

BTW, this project does not have a convention of code style .i.e a rustfmt file. So if code style matters, I am willing to revert these unrelated changes (The formatting is performed by rustfmt with its default config).

@tizee
Copy link
Author

tizee commented Jun 29, 2022

I personally don't like code formatters and therefore don't use them. At the same time I'm basically following the default rustfmt logic with some small exceptions. So for a new file you could use auto-formatting, but leave other files as is.

Now I see that.

@RazrFalcon
Copy link
Owner

The main goal of ttf-parser is to hide all the internal data. Meaning that the public API must not expose &[u8]. This is intentional.

What you have to do in this particular case it to use LookupSubtables::get.
Can you explain what you're trying to do? Maybe we would be able to provide a necessary API for your use case.

@tizee
Copy link
Author

tizee commented Jun 30, 2022

I'd like t filter out all specific subtables via kind before parsing them.

I have to iterate over all subtables to parse them into corresponding types.

            let tables: Vec<_> = lookup
                .subtables
                .into_iter()
                .collect();

Then I could filter ones I need with patterns.

As the code is intentionally designed for hiding all internal data, there is no need for using kind. Instead of that, we should use corresponding types with patterns for this.

Use LookupSubtables::get(offset) to parse a subtable is not-straightforward.

    pub fn get<T: LookupSubtable<'a>>(&self, index: u16) -> Option<T> {
        let offset = self.offsets.get(index)?.to_usize();
        let data = self.data.get(offset..)?;
        T::parse(data, self.kind)
}

The indices of these subtables are unknown beforehand so it discourages the use of this function. But it's not the concern of this PR.

Thanks for answering. I'll close this PR.

@tizee tizee closed this Jun 30, 2022
@RazrFalcon
Copy link
Owner

So do you found a solution or still locking for it? I know that OpenType tables API is very complex, but this is because those table are complex.

The reason kind is also hidden, is because it's value depends on which table you're parsing. GPOS and GSUB has their own types.

Are sure that parsing a GPOS/GSUB subtable is that expensive?

I this is absolutely needed, I can add LookupSubtables::get_kind that will return a corresponding enum.
As the comment of the get() method mentions, LookupSubtables should be generic by itself. But it would require a significant rewrite. Will see.

#101

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

2 participants