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

add shared font data variant (e.g. Arc) besides &'static [u8] and owned Vec<u8> #2047

Closed
nerditation opened this issue Sep 15, 2022 · 4 comments
Labels
feature New feature or request

Comments

@nerditation
Copy link

currently egui's support for cjk fonts is quite good, and I can even use .ttc files installed on my system directly by setting the FontData::index field. but there's a small problem still. if I want to use several font variants inside a single ttc file, I have two options:

  1. statically link the data (e.g. include_bytes!("cjk.ttc")) and use FontData::from_static()
  2. load the font file at runtime and use FontData::from_owned()

the problem is, cjk fonts are typically very big, the ttc collections are even bigger. I don't want my program binary to be over 100MB, so option 1 is out. but option 2 isn't optimal either, because the current API requires the ownership must be transfered to the FontData type, so I am forced to load many copies of the same font data, or I have to extract the ttc collection file into individual ttf files.

there's two possible solutions I can think of, either:

  1. we replace the Cow<'static, [u8]> with a custom type, something like:
enum SliceArcBox {
  Static(&'static [u8]),
  Shared(Arc<[u8]>),
  Owned(Box<[u8]>),
}
  1. we move the index field out of the FontData type and into higher level types (say Font) and let this type share the same FontData, (I'm not familiar with egui internal implementation so I am just saying a random type)

relevant issues: #64 and #853

additional context: for cjk fonts, it actually makes a lot of sense to prefer ttc container if available, over individual ttf files, it can save a lot of space by sharing large amount of glyph data, for instance, typically Chinese characters are all in the same size and a mono variant of a font family could only have different latin characters (or half-width characters etc) from their proportional variant. what's more, many glyphs can be shared among Chinese, Korean and Japanese.

@nerditation nerditation added the feature New feature or request label Sep 15, 2022
@emilk
Copy link
Owner

emilk commented Sep 15, 2022

Sounds good!

I think Arc makes sense, at that also allows you to share the same font data between different egui instances (for whatever reason), and is also the smaller change.

Replacing std::borrow::Cow<'static, [u8]> with something like this:

enum SharedOrStatic {
  Static(&'static [u8]),
  Shared(Arc<[u8]>),
}

should be enough - no need for Owned, as that can always be transformed into Shared (we don't need mutable access to it).

Do you want to make a PR?

@nerditation
Copy link
Author

I think Arc makes sense, at that also allows you to share the same font data between different egui instances (for whatever reason), and is also the smaller change.

exactly! I was thinking the same thing today, as I was trying to create a program with multiple windows and render with egui_glow.

Replacing std::borrow::Cow<'static, [u8]> with something like this:

enum SharedOrStatic {
  Static(&'static [u8]),
  Shared(Arc<[u8]>),
}

should be enough - no need for Owned, as that can always be transformed into Shared (we don't need mutable access to it).

Do you want to make a PR?

sure thing. working on it.

as a side note, to implement multiple windows, I associated each window with its own egui instance and opengl context. I just need to remember to dispatch the window events to the correct egui instance and remember to make the opengl as current before rendering. so far it seems works pretty well, besides the fact that I have to load multiple copies of font data, and that I need to remember to set certain settings (like dark/light theme) for every egui instance once they changed. I wonder if we can share more resources between egui instances, like textures, persistent states, etc?

@nerditation
Copy link
Author

in short

on further investigation, we cannot improve this on our own, it's a upstream dependency restriction.

currently I am stuck with this workaround, the idea is to declare a static variable, but initialize it at runtime:

fn main() {
	//let ctx = ...; let mut font_definitions = ...;

	// unsafe here because mutable static variables require it; otherwise, this
	// code is safe: there is no race condition here because we are inside the
	// main function, and the variable is not modified once intialized.
	// crates like `lazy_static` can be helpful too
	let mut font = unsafe {
		static mut TTC: Option<Vec<u8>> = None;
		TTC.replace(std::fs::read("C:/Windows/Fonts/MyAwesomeFont.ttc")?);
		egui::FontData::from_static(TTC.as_ref().unwrap())
	};

	// proportional font
	font.index = 12;
	font_definitions.font_data.insert("Gothic".to_owned(), font.clone());

	// monospace font
	font.index = 16;
	font_definitions.font_data.insert("Term".to_owned(), font.clone());

	//font_definitions.families.get_mut(&FontFamily::Proportional)...
}

explanation

firstly, as long FontData is created from owned Vec<u8>, it is always copied when creating the underlying ab_glyph::FontVec:

ab_glyph::FontVec::try_from_vec_and_index(bytes.clone(), data.index)

although there is a ab_glyph::FontArc type, it is not usable here: it is just a type-erased dynamically dispatched wrapper type (i.e. trait object) on top of FontVec and FontRef<'static>

since ab_glyph::FontVec is just a wrapper of owned_ttf_parser::PreParsedSubtables<'font, OwnedFace>: link

pub struct FontVec(ttfp::PreParsedSubtables<'static, ttfp::OwnedFace>);

and owned_ttf_parser::OwnedFace is implemented as Pin<Box<...>>: link

pub struct OwnedFace(Pin<Box<SelfRefVecFace>>);

so, until ttf_parser changes their implementation of OwnedFace using Pin<Arc<...>>, and ab_glyph updates to use new API, only then we can implement in our codebase.

for now, I think this isssue is going nowhere.

@nerditation
Copy link
Author

I'm closing this issue for now. this is relative rare use case, and this is not an actual limitation of egui itself.

besides, the workaround using mutable static variable (or crates such as lazy_static, once_cell, etc) would work just fine.

@nerditation nerditation closed this as not planned Won't fix, can't repro, duplicate, stale Oct 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants