-
Notifications
You must be signed in to change notification settings - Fork 475
[otlLib] Design goals and API #468
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
Comments
So, one thing I think we can all agree on is, we don't need a new set of types... So, let's agree that otlLib API converts a set of Python structs (tuple, set, dict) to otData-defined objects and back. Also, would be easier if the data that goes into each object, is fully defined by one item. Ie, buildSingleSubst() etc are already like that: they take one parameter (mapping) that contains all the data for this data structure. buildAnchors() on the other hand, is currently not like that, as it has this API:
I think it's fine to continue using named arguments, but perhaps extend it to also have a one-argument mode. Eg, change it to:
where pos is an iterable. If it has two items, those will be x,y. If it has three, the third will be point and point argument must be None. If it has four, the third and fourth will be deviceX/deviceY. Also, for simple types like Anchors, Values, Device, ..., the rest of the API will accept ot objects for this as well as raw data. Ie, in a buildCursivePos(), you can pass results of buildAnchor(), or pass the one-item input to buildAnchor() directly to buildCursivePos() and it will take care of it. Ie, we'll convert a tree of data items all with one call. We then add "unbuild" API to reverse this and return the one-item data for each object type. This allows us to go from binary to a representation that is, for lack of a better word, lisp-like. There are many benefits to this: this representation is for the most part, immutable, can be directly compared and printed, and has no noise in there. It only encodes the semantics of the objects involved, no extra attributes, or other object stuff that needs to be filtered out. It lends itself very well to optimizations, merging, diffing, and other such routines. |
Another discrepancy between feaLib and mtiLib is worth addressing in otlLib before we put much more code into it:
So, this is what I like to suggest:
We'll then add buildLookup() that takes a tuple that has lookup-type ("singleSubst", "singlePos", etc), lookup flags + markfilteringset, and tuple / list of lookup subtable data items. Comments? |
Are you sure that mtiLib is correct? If the ValueRecords have different value formats, doesn’t mtiLib need to emit multiple subtables as well? (In SinglePos format 2, the format of all ValueRecords needs to be the same). To make this more concrete, I’ve implemented SinglePos according to what we had discussed last week. |
Yes, you can just extend valuerecords with missing items set to 0/None. In fact, ot compiler layer does that. So, you just bitwise-or their value formats together. It works. |
So what you implemented is what I called the optimizing one. Indeed, this whole issue that we need to separate the two was brought to my attention by you showing me your SinglePos code. |
Needless to say, this is not necessarily the optimal packing... But that's the way mti files are supposed to work. The author takes care of sorting similar adjustments into the same lookup / subtable. |
Should otlLib._buildSinglePosFormat1/2 be public? |
Yes. But I prefer that it chooses format automatically. If you see otTables.py, that's what SingleSubst, etc do, just in a different layer. You might have an optional Format that is set to None by default, but I don't see a need for that. Just encode the data in whatever one subtable format that is more compact. |
That's what mtiLib does BTW. Please check parseSinglePos in mtiLib.init.py |
Confused. If mtiLib just passed its mapping into otlLib.buildSinglePos(), won’t you get exactly what you want? If not, it might be best to hash this out in person... |
We hashed it out over a VC. Will submit a series of changes. |
Decided to name the version that creates one subtable, well, buildSinglePosSubtable(). And the one that returns a list of subtables, buildSinglePos(). |
Should a function like |
I agree returning None is useful. Maybe have an argument allowEmpty=False? Or let's wait until someone requests. Also, buildAttachPoint() can be public as well. Functions to build any OT type are acceptable API. |
This simplifies call sites when building GDEF tables. Also, publicly expose the buildAttachPoint() function. #468 (comment)
Starting this thread, to discuss what we want otlLib to be, while it's still tiny and easy to change. I'll put in my thoughts as additional comments.
@brawer @anthrotype @moyogo
The text was updated successfully, but these errors were encountered: