Skip to content

Don't use skrifa interjector if provided variation coordinates map to default coordinates#28

Merged
laurmaedje merged 1 commit into
mainfrom
inter
Jun 4, 2026
Merged

Don't use skrifa interjector if provided variation coordinates map to default coordinates#28
laurmaedje merged 1 commit into
mainfrom
inter

Conversation

@LaurenzV

@LaurenzV LaurenzV commented Jun 4, 2026

Copy link
Copy Markdown
Collaborator

In general, we want to avoid using the interjector since it requires completely rewriting all font outlines. Right now, we decide whether to use the skrifa interjector based on whether the variation coordinates are empty or not. However, we can take this a step further by instead just checking whether the variation coordinates map to the default. So if the user for example provides the axis wght=400, we can still use the dummy interjector if this is the default axis of the font anyway.

Not super urgent to have in a new release because we can just do the same detection on the krilla side, but I think it's pretty important to have it in some place at least. And in the long term, it's definitely better to have it in subsetter than krilla.

@LaurenzV LaurenzV requested a review from laurmaedje June 4, 2026 06:04
Comment thread src/lib.rs
Comment on lines +183 to +188
let interjector = interjector::skrifa::SkrifaInterjector::new(
data,
index,
variation_coordinates,
)
.ok_or(MalformedFont)?;

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A bit unfortunate to always construct it and then not use it, but this is very cheap anyway, just requires the construction of a FontRef.

@LaurenzV

LaurenzV commented Jun 4, 2026

Copy link
Copy Markdown
Collaborator Author

Perhaps we should also check whether the font even is a variable font? 🤔 Just to be safe.

@laurmaedje

Copy link
Copy Markdown
Member

We could also filter out unsupported axes for the purposes of the check. Or does LocationRef::from(&self.location).is_default() already do that?

@laurmaedje

laurmaedje commented Jun 4, 2026

Copy link
Copy Markdown
Member

I'm not sure whether the is_default check is correct. It checks that all axes map to zero, but the actual default could also be non-zero (e.g. 400 for wght) I think.

@LaurenzV

LaurenzV commented Jun 4, 2026

Copy link
Copy Markdown
Collaborator Author

It checks that the normalized coordinate is zero, which should be the case if it matches the default no? Otherwise, it also wouldn’t make sense that the noto test was changed.

@LaurenzV

LaurenzV commented Jun 4, 2026

Copy link
Copy Markdown
Collaborator Author

We are deriving the locaiton here:

let location = font_ref
.axes()
.location(location.iter().map(|i| (skrifa::Tag::new(i.0.get()), i.1)));

Basically, skrifa takes the tags, maps them against the font in some way and then returns the Location which contain the normalized coorindates for each axis. So if they end up having only 0.0, it means that the passed variation coordinates were the default. That's my understanding.

It's also explicitly documented that garbage tags are ignored, so that doesn't need special treatment I think: https://docs.rs/skrifa/latest/skrifa/struct.AxisCollection.html#method.location

I guess this means we might not even need the explicit check whether the font is variable, and what is implemented in this PR is enough.

@laurmaedje

Copy link
Copy Markdown
Member

Makes sense!

@laurmaedje laurmaedje merged commit b2df27a into main Jun 4, 2026
6 checks passed
@laurmaedje laurmaedje deleted the inter branch June 4, 2026 11:16
@laurmaedje

Copy link
Copy Markdown
Member

Thanks!

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

Successfully merging this pull request may close these issues.

2 participants