-
-
Notifications
You must be signed in to change notification settings - Fork 8k
Make PyFT2Font a subclass of FT2Font #30324
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
Conversation
The goal here is to avoid having to this whole |
b3d0b19
to
d725cf9
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Modulo clarifying a comment and moving two parts of the codes closer together again.
This makes it easier to do later refactors.
d725cf9
to
b1e1375
Compare
Modulo clarifying the possibly duplicate close #30324 (review) |
b1e1375
to
db17baf
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@QuLogic can self-merge when CI passes again.
PR summary
This avoids a level of indirection, and also means that a
FT2Font
is aPyFT2Font
(since there are no other subclasses), which helps with some future work. The only downside is that the constructor/destructor order is fixed and slightly different, so we need to split the open/close stages for theFT_Face
and explicitly call them inPyFT2Font
.There are currently still two copies of the fallback list, one on each level. The one on
FT2Font
is what's actually used, but the one onPyFT2Font
is the actual owner (because pybind11 owns the objects, so we need a Python object to own them). I think it may be possible to drop that extra copy by switching tostd::shared_ptr
, but that will likely require the smart holders that are new in pybind11 v3, and I haven't fully tested that out yet.This PR is based on/waiting for #30322.PR checklist