mooman219 / fontdue

The fastest font renderer in the world, written in pure rust.
Apache License 2.0
1.44k stars 72 forks source link

Font::from_bytes creates NonZeroU16s containing zero #86

Closed caelunshun closed 3 years ago

caelunshun commented 3 years ago

https://github.com/mooman219/fontdue/blob/9483ae1ec3674ab1a786b0ad12e6b987c2023714/src/font.rs#L242-L244

According to the std docs, a NonZeroU16 containing 0 is always undefined behavior. It's unclear what the comment is trying to say with "the result is desirable," but undefined behavior should never be a desired result.

I think the solution here is to switch to normal u16s, though I'm not sure what the motivation was for using NonZeroU16 to start with.

mooman219 commented 3 years ago

https://github.com/mooman219/fontdue/blob/9483ae1ec3674ab1a786b0ad12e6b987c2023714/src/font.rs#L563-L567

The code is safe, I'm taking advantage of a guaranteed layout optimization. The end result makes the above lookup work with fewer branches. Option<NonZeroU16> is defined to reuse 0 as the None identifier. Specifically this invariant is defined:

assert_eq!(size_of::<Option<core::num::NonZeroU16>>(), size_of::<u16>());

...
#[rustc_layout_scalar_valid_range_start(1)]
#[rustc_nonnull_optimization_guaranteed]
pub struct NonZeroU16(u16);

The cases I'm accounting for:

In this setup all of these cases return a valid u16 that can be used as the lookup. Using unwrap_or(0) should usually get optimized to the same asm that this gets compiled into, but I had an issue in the past and this is safe in my context.