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

Simplify and Clarify Font Modification APIs (add_font and set_fonts) #5576

Open
lalala-233 opened this issue Jan 3, 2025 · 0 comments
Open

Comments

@lalala-233
Copy link

Description

Currently, egui provides two APIs for modifying fonts:

  1. add_font: Adds a new font to the existing font definitions.
  2. set_fonts: Replaces the entire font definitions with a new set.

However, these APIs have a high degree of code overlap, and their usage is not clearly documented. Additionally, set_fonts has a performance overhead due to an expensive comparison check, and its behavior is not fully intuitive.

Problems

  1. API Overlap:

    • Both add_font and set_fonts can be used to modify fonts, but their use cases and differences are not clearly explained in the documentation.
    • This can lead to confusion for users who are unsure which API to use.
  2. Performance Overhead:

self.read(|ctx| {
if let Some(current_fonts) = ctx.fonts.get(&pixels_per_point.into()) {
// NOTE: this comparison is expensive since it checks TTF data for equality
if current_fonts.lock().fonts.definitions() == &font_definitions {
update_fonts = false; // no need to update

  1. Documentation Issues:
    • The examples in the documentation do not clearly explain the differences between add_font and set_fonts.
    • Here are some confusing statements:

// Demonstrates how to replace all fonts.
fn replace_fonts(ctx: &egui::Context) {
// Start with the default fonts (we will be adding to them rather than replacing them).
let mut fonts = egui::FontDefinitions::default();

  • The comment for set_fonts could be clearer

/// This will overwrite the existing fonts.
pub fn set_fonts(&self, font_definitions: FontDefinitions) {

  • For example, state that it will replace the existing font definitions with the provided ones.
  1. Code Overlap:
    • Both add_font and set_fonts share significant code overlap, particularly in font data insertion, font family definitions, and update logic. This duplication increases maintenance complexity and raises the risk of inconsistencies or subtle bugs.

pub fn add_font(&self, new_font: FontInsert) {
profiling::function_scope!();
let pixels_per_point = self.pixels_per_point();
let mut update_fonts = true;
self.read(|ctx| {
if let Some(current_fonts) = ctx.fonts.get(&pixels_per_point.into()) {
if current_fonts
.lock()
.fonts
.definitions()
.font_data
.contains_key(&new_font.name)
{
update_fonts = false; // no need to update
}
}
});

pub fn set_fonts(&self, font_definitions: FontDefinitions) {
profiling::function_scope!();
let pixels_per_point = self.pixels_per_point();
let mut update_fonts = true;
self.read(|ctx| {
if let Some(current_fonts) = ctx.fonts.get(&pixels_per_point.into()) {
// NOTE: this comparison is expensive since it checks TTF data for equality
if current_fonts.lock().fonts.definitions() == &font_definitions {
update_fonts = false; // no need to update
}
}
});


This issue was translated by AI.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant