Skip to content
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

api: Add API to query available font families and styles #4523

Merged

Conversation

peterhorvath111
Copy link
Contributor

Description

Fixes #4503.

Add an API to query available fonts based on family and style name. The API can be used to build a UI where users can select a font from a list.

  • font_families(): Return the list of available font families in alphabetical order.
  • font_styles(family): Return the list of available font styles of a given font family.
  • font_filename(family, style): Return the font filename that defines the font with the given family and style.

Checklist:

  • I have read the contribution guidelines.
  • I have updated the documentation, if applicable. (Check if there is no
    need to update the documentation, for example if this is a bug fix that
    doesn't change the API.)
  • I have ensured that the change is tested somewhere in the testsuite
    (adding new test cases if necessary).
  • If I added or modified a C++ API call, I have also amended the
    corresponding Python bindings (and if altering ImageBufAlgo functions, also
    exposed the new functionality as oiiotool options).
  • My code follows the prevailing code style of this project. If I haven't
    already run clang-format before submitting, I definitely will look at the CI
    test that runs clang-format and fix anything that it highlights as being
    nonconforming.

Comment on lines 1630 to 1632
/// [ 0 1 0 ]
/// [ 1 -4 1 ]
/// [ 0 1 0 ]
///
ImageBuf OIIO_API laplacian (const ImageBuf &src, ROI roi={}, int nthreads=0);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Whoa, what's up with this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oops, my bad, I don't know what happened there. Fixed now.

@lgritz
Copy link
Collaborator

lgritz commented Nov 4, 2024

Are you aware of the existing OIIO::getattribute() queries related to fonts?

/// - `string font_list`
/// - `string font_file_list`
/// - `string font_dir_list`
///
///   A semicolon-separated list of, respectively, all the fonts that
///   OpenImageIO can find, all the font files that OpenImageIO can find (with
///   full paths), and all the directories that OpenImageIO will search for
///   fonts.  (Added in OpenImageIO 2.5)

Historically, we've tended to avoid adding new single-use API calls for individual queries, instead preferring to just add new tokens that getattribute recognizes to cover as many cases as possible.

Your font_families() and font_styles() are trivially amenable to this approach, since they just return a list of strings. The font_filename() is a little trickier, but we could probably do it by accepting special query names composed of "font_filename:FAMILY:STYLE" and parsing out the family and style name. Internally, they would still be implemented by more or less the functions you implemented but they would be turned into something in the pvt namespace and hidden from public headers.

I'm not saying I'm necessarily set against doing it as functions as you have... but I wanted to give you the chance to consider sticking with the idiom we've been using, which I think has been successful in keeping ABI churn to a minimum. For functionality that's never really in a performance critical loop, it's really easy to add, remove, backport, or change the name or meaning of these tokens without any hard compatibility breaks, compared to the approach where a different set of exposed API functions is required.

Signed-off-by: peter.horvath <[email protected]>
@peterhorvath111
Copy link
Contributor Author

I don't mind switching to getattribute() queries, if that's the preferred approach.

@peterhorvath111
Copy link
Contributor Author

API functions are converted to the following getattribute() queries:

  • font_family_list: Return a semicolon-separated list of all available font families.
  • font_style_list:family: Return a semicolon-separated list of all available font styles of a given font family.
  • font_filename:family:style: Return the font filename that defines the font of the given family and style.

Signed-off-by: peter.horvath <[email protected]>
Copy link
Collaborator

@lgritz lgritz left a comment

Choose a reason for hiding this comment

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

I like! Thanks.

@lgritz
Copy link
Collaborator

lgritz commented Nov 6, 2024

Because 3.0.0 is already marked as a release candidate, my goal is only to merge critical bug fixes for the final non-rc tag on Friday. So I'll merge this into main today, then backport it into the dev-3.0 branch after tagging on Friday, so technically this will be released as part of the next (3.0.1) patch release.

If it's really important to you to get this in Friday's first 3.0.0 final release, then I can do it at essentially no real risk (since as far as public API, all this does is add some new getattribute query tokens and changes no other behavior, which was the whole point!). But if you can wait for 3.0.1 or operate from main for now, then I think it's better release policy to not add anything unnecessary once an RC is declared, no matter how harmless it appears.

@ThiagoIze
Copy link
Collaborator

Waiting for 3.0.1 is perfectly fine for us.

@lgritz lgritz merged commit d1f198c into AcademySoftwareFoundation:main Nov 6, 2024
29 checks passed
@lgritz lgritz added enhancement Improvement of existing/working features. feature request labels Nov 6, 2024
@peterhorvath111 peterhorvath111 deleted the font_family_api branch November 7, 2024 08:53
lgritz pushed a commit to lgritz/OpenImageIO that referenced this pull request Nov 10, 2024
AcademySoftwareFoundation#4523)

Add an API to query available fonts based on family and style name. The
API can be used to build a UI where users can select a font from a list.
They consist of new queries for `OIIO::getattribute()`:

- `string font_family_list`

  A semicolon-separated list of all the font family names that
  OpenImageIO can find. 

- `string font_style_list:family`

  A semicolon-separated list of all the font style names that
  belong to the given font family. 

- `string font_filename:family:style`

  The font file (with full path) that defines the given font
  family and style. 

Fixes AcademySoftwareFoundation#4503.


---------

Signed-off-by: peter.horvath <[email protected]>
lgritz pushed a commit to lgritz/OpenImageIO that referenced this pull request Nov 10, 2024
AcademySoftwareFoundation#4523)

Add an API to query available fonts based on family and style name. The
API can be used to build a UI where users can select a font from a list.
They consist of new queries for `OIIO::getattribute()`:

- `string font_family_list`

  A semicolon-separated list of all the font family names that
  OpenImageIO can find. 

- `string font_style_list:family`

  A semicolon-separated list of all the font style names that
  belong to the given font family. 

- `string font_filename:family:style`

  The font file (with full path) that defines the given font
  family and style. 

Fixes AcademySoftwareFoundation#4503.


---------

Signed-off-by: peter.horvath <[email protected]>
lgritz pushed a commit to lgritz/OpenImageIO that referenced this pull request Nov 13, 2024
AcademySoftwareFoundation#4523)

Add an API to query available fonts based on family and style name. The
API can be used to build a UI where users can select a font from a list.
They consist of new queries for `OIIO::getattribute()`:

- `string font_family_list`

  A semicolon-separated list of all the font family names that
  OpenImageIO can find. 

- `string font_style_list:family`

  A semicolon-separated list of all the font style names that
  belong to the given font family. 

- `string font_filename:family:style`

  The font file (with full path) that defines the given font
  family and style. 

Fixes AcademySoftwareFoundation#4503.


---------

Signed-off-by: peter.horvath <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Improvement of existing/working features. feature request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[FEATURE REQUEST] API to query available font families and styles
3 participants