Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Add language parameter to Text objects#29794

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Open
QuLogic wants to merge1 commit intomatplotlib:main
base:main
Choose a base branch
Loading
fromQuLogic:text-language

Conversation

QuLogic
Copy link
Member

PR summary

Along with#29695, thislanguage parameter is an additional setting that may affect text layout. As with the other PR, this is not complete, but rather something to get the API decided upon (and likely will be rebased on raqm PR after it's done.)

There are two parts to the API:

  1. thelanguage parameter attached toText, and any other API that wraps it (i.e.,Axes.text orAxes.annotate)
  2. thelanguage parameter in backend'sRenderer.draw_text

At the moment, for both, I have set these tostr | list[tuple[str, int, int]] where the latter denotes a list of (language, start, end)-tuples. However, this was before I found out aboutthe font feature machinery, and it is possible that we may wish to use that syntax instead, at least for API part 1. For part 2, it's a little nicer to have the explicit types, just for passing to the FT2Font extension, but this may not be relevant to other implementations.

PR checklist

@anntzer
Copy link
Contributor

anntzer commentedMar 22, 2025
edited
Loading

re: adding language to draw_text: the general discussion about extensibility of renderer API applies, i.e.

  • if you're going to add language as a parameter to draw_text then it should be at least keyword-only.
  • it's likely that the more maintainable API would likely to just embed language as part of fontproperties (it's not really different from e.g. font features, I'd think) and tell the backend to read that info.
  • for draw_text we have actually more or less given up on having a "clean" renderer API because we pass the whole Text instance to draw_text (perPropagate mpl.text.Text instances to the backends and fix documentation #1081) anyways so perhaps backends that care could also just read that info off the Text instance. It feels a bit dirty, but heh... ("dirty" because we as well may just have a renderer API that'srenderer.draw_text(textobj) to start with...)

@anntzer
Copy link
Contributor

Side point regarding allowinglist[tuple[str, int, int]] as language setting: this may be a problem if one usessomelanguage[idx:] to set the language from an index up to the end of the string (whose length could technically be modified later, even though it's not clear this necessarily makes much practical sense). Likely you needlist[tuple[str, int, int | None]] (and then you may possibly support None (=0) for the start as well).

@QuLogic
Copy link
MemberAuthor

In the call a couple weeks ago, we decided to drop the newlanguage parameter todraw_text, because that already had amtext parameter which contained the wholeText object. This would avoid any signature introspection.

However, one followup is thatRenderer.get_text_width_height_descent wouldalso depend on font features/language settings, and really should get these passed through somehow as well.

@@ -65,7 +65,7 @@ def layout(string, font, *, kern_mode=Kerning.DEFAULT):
"""
x = 0
prev_glyph_idx = None
char_to_font = font._get_fontmap(string)
char_to_font = font._get_fontmap(string) # TODO: Pass in language.
Copy link
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Note, this function is getting rewritten with libraqm, so this is just a note for later.

@@ -43,7 +43,7 @@ def warn_on_missing_glyph(codepoint, fontnames):
f"Matplotlib currently does not support {block} natively.")


def layout(string, font, *, kern_mode=Kerning.DEFAULT):
def layout(string, font,language,*, kern_mode=Kerning.DEFAULT):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

These params should be kwonly? (throughout most of the PR, I think)

@@ -1,3 +1,4 @@
#include <cstddef>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

why do you need this?

Copy link
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Accidental;clangd likes to add these itself and sometimes I miss it.

@QuLogicQuLogicforce-pushed thetext-language branch 2 times, most recently from3c188df to931423bCompareMay 13, 2025 03:28
@QuLogic
Copy link
MemberAuthor

I've added atext.language rcParam as discussed on the call last week.

@QuLogic
Copy link
MemberAuthor

Also added a What's new note, though as with the other PR, it won't show the difference untillibraqm is added.

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@anntzeranntzeranntzer left review comments

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Projects
None yet
Milestone
v3.11.0
Development

Successfully merging this pull request may close these issues.

2 participants
@QuLogic@anntzer

[8]ページ先頭

©2009-2025 Movatter.jp