GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — Make PyFT2Font a subclass of FT2Font by QuLogic · Pull Request #30324 · matplotlib/matplotlib

Conversation

@QuLogic

@QuLogic QuLogic commented Jul 17, 2025 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

PR summary

This avoids a level of indirection, and also means that a FT2Font is a PyFT2Font (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 the FT_Face and explicitly call them in PyFT2Font.

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 on PyFT2Font 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 to std::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

  • [n/a] "closes #0000" is in the body of the PR description to link the related issue
  • new and changed code is tested
  • [n/a] Plotting related features are demonstrated in an example
  • [n/a] New Features and API Changes are noted with a directive and release note
  • [n/a] Documentation complies with general and docstring guidelines

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@QuLogic QuLogic added this to Font and text overhaul Jul 17, 2025
@QuLogic QuLogic added the status: waiting for other PR label Jul 17, 2025
@github-project-automation github-project-automation Bot moved this to Waiting for other PR in Font and text overhaul Jul 17, 2025
@github-actions github-actions Bot added topic: rcparams topic: text/fonts Documentation: user guide files in galleries/users_explain or doc/users labels Jul 17, 2025
@QuLogic QuLogic added this to the v3.11.0 milestone Jul 17, 2025
@QuLogic

QuLogic commented Jul 18, 2025

Copy link
Copy Markdown
Member Author

The goal here is to avoid having to this whole set_parent thing: https://github.com/matplotlib/matplotlib/pull/30000/files#diff-c8ef50e596f5bb461a98e5c60d0061276d773c260b206290ee71c63817242c08R174-R183 for which we have to map FT_Face -> FT2Font -> PyFT2Font but with this PR, the latter two are the same.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@QuLogic
QuLogic force-pushed the ft2font-subcless branch from b3d0b19 to d725cf9 Compare July 24, 2025 19:05
@github-actions github-actions Bot removed topic: rcparams Documentation: user guide files in galleries/users_explain or doc/users labels Jul 24, 2025
@QuLogic
QuLogic marked this pull request as ready for review July 24, 2025 19:06
@QuLogic QuLogic removed the status: waiting for other PR label Jul 24, 2025
@QuLogic QuLogic moved this from Waiting for other PR to Ready for Review in Font and text overhaul Jul 24, 2025
tacaswell
tacaswell reviewed Aug 1, 2025
View reviewed changes
Comment thread src/ft2font.cpp Resolved

Uh oh!

There was an error while loading. Please reload this page.
tacaswell
tacaswell reviewed Aug 1, 2025
View reviewed changes
Comment thread src/ft2font_wrapper.cpp Resolved

Uh oh!

There was an error while loading. Please reload this page.
tacaswell
tacaswell approved these changes Aug 1, 2025
View reviewed changes

@tacaswell tacaswell left a comment

Copy link
Copy Markdown
Member

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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@QuLogic
Split font opening/closing out of FT2Font constructor/destructor
fa62956 
This makes it easier to do later refactors.
@QuLogic
QuLogic force-pushed the ft2font-subcless branch from d725cf9 to b1e1375 Compare August 2, 2025 08:42
timhoffm
timhoffm approved these changes Aug 5, 2025
View reviewed changes
@timhoffm

timhoffm commented Aug 5, 2025

Copy link
Copy Markdown
Member

Modulo clarifying the possibly duplicate close #30324 (review)
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@QuLogic
Make PyFT2Font a subclass of FT2Font
db17baf
@QuLogic
QuLogic force-pushed the ft2font-subcless branch from b1e1375 to db17baf Compare August 7, 2025 19:21
tacaswell
tacaswell approved these changes Aug 7, 2025
View reviewed changes

@tacaswell tacaswell left a comment

Copy link
Copy Markdown
Member

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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@QuLogic
QuLogic merged commit db17baf into matplotlib:text-overhaul Aug 7, 2025
@github-project-automation github-project-automation Bot moved this from Ready for Review to Done in Font and text overhaul Aug 7, 2025
@QuLogic
QuLogic deleted the ft2font-subcless branch August 7, 2025 21:18
QuLogic added a commit to QuLogic/matplotlib that referenced this pull request Aug 21, 2025
@QuLogic
DOC: Fix missing references on text-overhaul branch
ad32f0d 
I thought this was because this branch was missing
d2d969e, but it was actually a change
introduced in matplotlib#30324 as a separate (but similar-looking) issue.
@QuLogic QuLogic mentioned this pull request Aug 21, 2025
DOC: Fix missing references on text-overhaul branch #30453
Merged
1 task done
wavebyrd pushed a commit to wavebyrd/matplotlib that referenced this pull request Mar 13, 2026
@QuLogic
DOC: Fix missing references on text-overhaul branch
2bd8ab3 
I thought this was because this branch was missing
115ea9f, but it was actually a change
introduced in matplotlib#30324 as a separate (but similar-looking) issue.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tacaswell tacaswell tacaswell approved these changes
@timhoffm timhoffm timhoffm approved these changes

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

topic: text/fonts

Projects

Font and text overhaul
Status: Done

Milestone

v3.11.0

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

3 participants

@QuLogic @timhoffm @tacaswell
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402520 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.