-
Notifications
You must be signed in to change notification settings - Fork 33
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
m(breaking): Simplify internal storage and lookup mechanisms, take 2 #46
Conversation
@@ -88,15 +88,15 @@ pub use ttf_parser::Width as Stretch; | |||
/// as different strings, but does not make any guarantees about format or | |||
/// content of the strings. | |||
#[derive(Clone, Copy, Eq, Hash, Ord, PartialEq, PartialOrd, Debug)] | |||
pub struct ID(u32); | |||
pub struct ID(usize); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can keep it as u32
. No one will have more that 4M font faces.
@@ -450,12 +445,11 @@ impl Database { | |||
/// after loading a large directory with fonts. | |||
/// Or a specific face from a font. | |||
pub fn remove_face(&mut self, id: ID) -> bool { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we should return bool
anymore.
And we should rename it to disable_face
.
@@ -714,6 +708,9 @@ pub struct FaceInfo { | |||
|
|||
/// Indicates that the font face is monospaced. | |||
pub monospaced: bool, | |||
|
|||
/// Whether or not this font face has been disabled. | |||
pub disabled: bool, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A disabled face should be ignored during querying.
While a simple Will have to think about it. I'm trying to think about a use case when someone would be constantly adding and removing fonts to a database. And I can think of one in case of SVG with embedded fonts, in which case removal of a font is necessary.... We need a 0(1) indexing, strong/unique indices and an ability to remove a face... Looks like a generational arena to me. It's better than In the end, |
We could use |
Yes, it seems like |
Superseded by #47 |
Rather than using
slab
like in #45, this PR just uses the ID as an index. This leaks memory when fonts are removed.