Skip to content

Commit

Permalink
Impl bytemuck traits for GenericFamily (#213)
Browse files Browse the repository at this point in the history
This can be used in the future as part of FFI work, but for now, the
`Contiguous` trait is used to provide a `MAX_VALUE` used by
`GenericFamilyMap`.

This is a backport of part of #209 where the separation between
`GenericFamily` and `GenericFamilyMap` has grown to span crates.

The testing here borrows from (copies) what is done for similar testing
of `bytemuck` traits in the `color` crate.

These crates already depend on `bytemuck`, so this isn't a new
dependency. A decision is made here though to not use the `derive`
feature as when we move this type to a vocabulary crate, we will want to
avoid the `derive` feature there to avoid a dependency on proc macros.
  • Loading branch information
waywardmonkeys authored Dec 9, 2024
1 parent ffea1e6 commit 1d9d344
Show file tree
Hide file tree
Showing 4 changed files with 107 additions and 3 deletions.
5 changes: 3 additions & 2 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ clippy.wildcard_dependencies = "warn"

[workspace.dependencies]
accesskit = "0.17"
bytemuck = { version = "1.20.0", default-features = false }
fontique = { version = "0.2.0", default-features = false, path = "fontique" }
hashbrown = "0.15.2"
parley = { version = "0.2.0", default-features = false, path = "parley" }
Expand Down
1 change: 1 addition & 0 deletions fontique/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ system = [
]

[dependencies]
bytemuck = { workspace = true }
read-fonts = { workspace = true }
peniko = { workspace = true }
smallvec = "1.13.2"
Expand Down
103 changes: 102 additions & 1 deletion fontique/src/generic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
//! Generic font families.
use super::FamilyId;
use bytemuck::{checked::CheckedBitPattern, Contiguous, NoUninit, Zeroable};
use core::fmt;
use smallvec::SmallVec;

Expand Down Expand Up @@ -50,6 +51,7 @@ pub enum GenericFamily {
/// Song and cursive-style Kai forms. This style is often used for
/// government documents.
FangSong = 12,
// NOTICE: If a new value is added, be sure to add modify `MAX_VALUE` in the bytemuck impl.
}

impl GenericFamily {
Expand Down Expand Up @@ -122,7 +124,31 @@ impl fmt::Display for GenericFamily {
}
}

const COUNT: usize = GenericFamily::FangSong as usize + 1;
// Safety: The enum is `repr(u8)` and has only fieldless variants.
unsafe impl NoUninit for GenericFamily {}

// Safety: The enum is `repr(u8)` and `0` is a valid value.
unsafe impl Zeroable for GenericFamily {}

// Safety: The enum is `repr(u8)`.
unsafe impl CheckedBitPattern for GenericFamily {
type Bits = u8;

fn is_valid_bit_pattern(bits: &u8) -> bool {
// Don't need to compare against MIN_VALUE as this is u8 and 0 is the MIN_VALUE.
*bits <= Self::MAX_VALUE
}
}

// Safety: The enum is `repr(u8)`. All values are `u8` and fall within
// the min and max values.
unsafe impl Contiguous for GenericFamily {
type Int = u8;
const MIN_VALUE: u8 = Self::Serif as u8;
const MAX_VALUE: u8 = Self::FangSong as u8;
}

const COUNT: usize = GenericFamily::MAX_VALUE as usize + 1;

/// Maps generic families to family identifiers.
#[derive(Clone, Default, Debug)]
Expand All @@ -148,3 +174,78 @@ impl GenericFamilyMap {
self.map[generic as usize].extend(families);
}
}

#[cfg(test)]
mod tests {
use crate::GenericFamily;
use bytemuck::{checked::try_from_bytes, Contiguous, Zeroable};
use core::ptr;

#[test]
fn checked_bit_pattern() {
let valid = bytemuck::bytes_of(&2_u8);
let invalid = bytemuck::bytes_of(&200_u8);

assert_eq!(
Ok(&GenericFamily::Monospace),
try_from_bytes::<GenericFamily>(valid)
);

assert!(try_from_bytes::<GenericFamily>(invalid).is_err());
}

#[test]
fn contiguous() {
let hd1 = GenericFamily::SansSerif;
let hd2 = GenericFamily::from_integer(hd1.into_integer());
assert_eq!(Some(hd1), hd2);

assert_eq!(None, GenericFamily::from_integer(255));
}

#[test]
fn zeroable() {
let hd = GenericFamily::zeroed();
assert_eq!(hd, GenericFamily::Serif);
}

/// Tests that the [`Contiguous`] impl for [`GenericFamily`] is not trivially incorrect.
const _: () = {
let mut value = 0;
while value <= GenericFamily::MAX_VALUE {
// Safety: In a const context, therefore if this makes an invalid GenericFamily, that will be detected.
// When updating the MSRV to 1.82 or later, this can use `&raw const value` instead of the addr_of!
let it: GenericFamily = unsafe { ptr::read((core::ptr::addr_of!(value)).cast()) };
// Evaluate the enum value to ensure it actually has a valid tag
if it as u8 != value {
unreachable!();
}
value += 1;
}
};
}

#[cfg(doctest)]
/// Doctests aren't collected under `cfg(test)`; we can use `cfg(doctest)` instead
mod doctests {
/// Validates that any new variants in `GenericFamily` has led to a change in the `Contiguous` impl.
/// Note that to test this robustly, we'd need 256 tests, which is impractical.
/// We make the assumption that all new variants will maintain contiguousness.
///
/// ```compile_fail,E0080
/// use bytemuck::Contiguous;
/// use fontique::GenericFamily;
/// const {
/// let value = GenericFamily::MAX_VALUE + 1;
/// // Safety: In a const context, therefore if this makes an invalid GenericFamily, that will be detected.
/// // (Indeed, we rely upon that)
/// // When updating the MSRV to 1.82 or later, this can use `&raw const value` instead of the addr_of!
/// let it: GenericFamily = unsafe { core::ptr::read((core::ptr::addr_of!(value)).cast()) };
/// // Evaluate the enum value to ensure it actually has an invalid tag
/// if it as u8 != value {
/// unreachable!();
/// }
/// }
/// ```
const _GENERIC_FAMILY: () = {};
}

0 comments on commit 1d9d344

Please sign in to comment.