[skrifa] tthint: value stack/basic instructions #767
Conversation
Adds a type for managing the interpreter value stack along with instructions to manipulate the stack as well as basic arithmetic and logical instructions. Covers implementations for 47 opcodes. More progress on #620
| /// | ||
| /// See <https://learn.microsoft.com/en-us/typography/opentype/spec/tt_instructions#pushing-data-onto-the-interpreter-stack> | ||
| /// and <https://gitlab.freedesktop.org/freetype/freetype/-/blob/57617782464411201ce7bbc93b086c1b4d7d84a5/src/truetype/ttinterp.c#L3858> | ||
| pub(super) fn op_push(&mut self, args: &Args) -> OpResult { |
There was a problem hiding this comment.
this + args is the only thing so far that raises my eyebrows at all, it seems like a vaguely annoying API? but i'm also not sure where the input comes from, what it looks like in practice etc.
I also of course don't have a better idea; the one alternative that occurs to me is separate Bytes/Words or ByteOps/WordOps and then a trait they both implement, but this would only be useful if we could easily construct these types wherever we're getting the ops from, which is probably not so simple.
Basically this is a comment to demonstrate that I am actually reading this PR, and you can probably ignore it 💁♂️
There was a problem hiding this comment.
This is actually a good call out because I was considering leaving this out of the PR due to the lack of context around the type, but decided to include it to avoid breaking up the ValueStack impl.
Hopefully this will make more sense when the remainder of the code lands. Constructing this is actually very simple and is handled by the instruction decoder. The code looks like this:
if arg_count > 0 {
args.bytes = self
.bytecode
.get(arg_start..arg_start + arg_count)
.ok_or(HintErrorKind::UnexpectedEndOfBytecode)?;
args.is_words = (PUSHW000..=PUSHW111).contains(&opcode)
|| opcode == NPUSHW;
}where arg_start and arg_count are derived from the current program counter and a table indexed by opcode. NPUSHB and NPUSHW read the arg count as the next byte in the instruction stream.
The dispatch code is even simpler, capturing all 18 opcodes in a single match arm and calling out to op_push as defined in this PR:
PUSHB000..=PUSHW111 | NPUSHB | NPUSHW => {
self.op_push(&ins.args)?;
}Thanks for the quick review!
More progress on #620
Adds a type for managing the interpreter value stack along with instructions to manipulate the stack as well as basic arithmetic and logical instructions.
Covers implementations for 47 opcodes.
This appears big, but the bulk is really comments, references and tests. Most of the actual code is fairly simple.