Skip to content

refactor: add default_font, change font api#66

Merged
nuzzles merged 3 commits intomainfrom
text-refactoring
Jul 20, 2024
Merged

refactor: add default_font, change font api#66
nuzzles merged 3 commits intomainfrom
text-refactoring

Conversation

@nuzzles
Copy link
Copy Markdown
Member

@nuzzles nuzzles commented Jul 20, 2024 •
edited
Loading

Added

  • There is now a default_font feature that uses the same FiraMono-subset.ttf font used in the bevy/default_font feature.

Changed

  • The font API has changed significantly. Please visit examples/text for further usage. This is to prepare for additional text features such as linebreak behavior, bounded text, and text justification.
    • VelloText has been renamed to VelloTextSection.
    • VelloText.content has been renamed to VelloText.value.
    • There is now a VelloTextStyle struct and it is a required field of VelloText.
    • VelloFont has been removed from VelloTextBundle and moved into VelloTextStyle.

@sebasjham sebasjham mentioned this pull request Jul 20, 2024
Merged
Base automatically changed from refactor-naming to main July 20, 2024 18:55
@nuzzles nuzzles force-pushed the text-refactoring branch from 7e7764d to 4b4c76d Compare July 20, 2024 19:00
@nuzzles nuzzles requested a review from sebasjham July 20, 2024 19:00
@nuzzles nuzzles mentioned this pull request Jul 20, 2024
Closed
@nuzzles nuzzles enabled auto-merge July 20, 2024 19:12
@nuzzles nuzzles added this pull request to the merge queue Jul 20, 2024
Merged via the queue into main with commit 5882f36 Jul 20, 2024
@nuzzles nuzzles deleted the text-refactoring branch July 20, 2024 19:17
DJMcNab
DJMcNab reviewed Jul 22, 2024
View reviewed changes
Copy link
Copy Markdown
Member

@DJMcNab DJMcNab left a comment

Choose a reason for hiding this comment

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

We need to make sure we're handling the licensing properly.

Can we ask the Bevy project to make a crate containing just the font file, so that the usages can be properly shared?

src/text/FiraMono-subset.ttf
Copy link
Copy Markdown
Member

@DJMcNab DJMcNab Jul 22, 2024

Choose a reason for hiding this comment

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

What's the license for this file?

Copy link
Copy Markdown
Member Author

@nuzzles nuzzles Jul 22, 2024

Choose a reason for hiding this comment

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

OFL 1.1

src/text/FiraMono-subset.ttf
Copy link
Copy Markdown
Member

@DJMcNab DJMcNab Jul 22, 2024

Choose a reason for hiding this comment

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

If you are using both this crate with default_font and bevy/default_font, does your binary include both?

I do see that the file is relatively small, so it's probably fine.

Copy link
Copy Markdown
Member Author

@nuzzles nuzzles Jul 22, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

We aren't currently using bevy/default_font, not until bevyengine/bevy#14406 is merged at least.

@nuzzles
Copy link
Copy Markdown
Member Author

nuzzles commented Jul 22, 2024

We're good - see

bevyengine/bevy#14406

We recently submitted a PR to expose the font data under current licensing. Which is approved, waiting merge.

So we will switch to that as soon as we can.

@DJMcNab
Copy link
Copy Markdown
Member

DJMcNab commented Jul 22, 2024

I don't agree with @alice-i-cecile's reading of the OFL in bevyengine/bevy#8445. According to their FAQ:

The only situation in which an OFL font can be distributed without the text of the OFL (either in a separate file or in font metadata), is when a font is embedded in a document or bundled within a program. In the case of metadata included within a font, it is legally sufficient to include only a link to the text of the OFL on https://openfontlicense.org/, but we strongly recommend against this. Most modern font formats include metadata fields that will accept the full OFL text, and full inclusion increases the likelihood that users will understand and properly apply the license.

I can't find the license text within the font file (at least using fc-query), and this use case (hosted on GitHub) is not "bundled within a program" (even though it will be once the program is built).
Even if Bevy has decided it's OK for them, if we're distributing this file, I strongly want us to make it clear that it's under the OFL.

So we should either just remove this file and only use the capability from that PR, or add the OFL.

@nuzzles
Copy link
Copy Markdown
Member Author

nuzzles commented Jul 22, 2024 •
edited
Loading

I don't agree with @alice-i-cecile's reading of the OFL in bevyengine/bevy#8445. According to their FAQ:

The only situation in which an OFL font can be distributed without the text of the OFL (either in a separate file or in font metadata), is when a font is embedded in a document or bundled within a program. In the case of metadata included within a font, it is legally sufficient to include only a link to the text of the OFL on https://openfontlicense.org/, but we strongly recommend against this. Most modern font formats include metadata fields that will accept the full OFL text, and full inclusion increases the likelihood that users will understand and properly apply the license.

I can't find the license text within the font file (at least using fc-query), and this use case (hosted on GitHub) is not "bundled within a program" (even though it will be once the program is built). Even if Bevy has decided it's OK for them, if we're distributing this file, I strongly want us to make it clear that it's under the OFL.

So we should either just remove this file and only use the capability from that PR, or add the OFL.

I always planned on switching with the data from the PR, however I am not opposed to adding the OFL sooner until we can do that. I'd gladly accept a PR to add the OFL.

github-merge-queue bot pushed a commit to bevyengine/bevy that referenced this pull request Jul 22, 2024
@sebasjham @nuzzles @BD103
feat: expose the default font bytes (#14406)
d88be59 
# Objective

- Enables use cases where third-party crates would want to use the
default font as well [see linebender's
use](linebender/bevy_vello#66)

## Solution

- Uses `include_bytes` macro and make it `pub`

---------

Co-authored-by: Spencer C. Imbleau <spencer@imbleau.com>
Co-authored-by: BD103 <59022059+BD103@users.noreply.github.com>
@DJMcNab DJMcNab mentioned this pull request Aug 5, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@DJMcNab DJMcNab DJMcNab left review comments

@sebasjham sebasjham sebasjham approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@nuzzles @DJMcNab @sebasjham