Minor fixes

[laurmaedje]

No description provided.

[LaurenzV]

LGTM. The thing I was worried about about the different maxp versions is already handled here:

subsetter/src/maxp.rs

Lines 18 to 49 in 816fcb9

	let version = match ctx.flavor {
	FontFlavor::TrueType => POST_TRUETYPE_VERSION,
	FontFlavor::Cff => POST_CFF_VERSION,
	// Since we convert to TrueType.
	FontFlavor::Cff2 => POST_TRUETYPE_VERSION,
	};
	let mut sub_maxp = Writer::new();
	sub_maxp.write::<u32>(version);
	sub_maxp.write::<u16>(ctx.mapper.num_gids());
	if version == POST_TRUETYPE_VERSION {
	if let Some(custom_data) = &ctx.custom_maxp_data {
	sub_maxp.write::<u16>(custom_data.max_points);
	sub_maxp.write::<u16>(custom_data.max_contours);
	sub_maxp.write::<u16>(custom_data.max_composite_points);
	sub_maxp.write::<u16>(custom_data.max_composite_contours);
	sub_maxp.write::<u16>(custom_data.max_zones);
	sub_maxp.write::<u16>(custom_data.max_twilight_points);
	sub_maxp.write::<u16>(custom_data.max_storage);
	sub_maxp.write::<u16>(custom_data.max_function_defs);
	sub_maxp.write::<u16>(custom_data.max_instruction_defs);
	sub_maxp.write::<u16>(custom_data.max_stack_elements);
	sub_maxp.write::<u16>(custom_data.max_size_of_instructions);
	sub_maxp.write::<u16>(custom_data.max_component_elements);
	sub_maxp.write::<u16>(custom_data.max_component_depth);
	} else {
	sub_maxp.extend(r.tail().ok_or(MalformedFont)?);
	}
	}
	ctx.push(Tag::MAXP, sub_maxp.finish());

So that shouldn't be a problem:

Another small thing I noticed:

subsetter/src/lib.rs

Lines 224 to 226 in 816fcb9

	ctx.process(Tag::CVT)?; // won't be subsetted.
	ctx.process(Tag::FPGM)?; // won't be subsetted.
	ctx.process(Tag::PREP)?; // won't be subsetted.

We probably shouldn't be adding those tables for variable fonts since skrifa will discard any hinting information, and since we zero out hinting data in maxp I'm not sure if this could cause problems. However, I am fairly certain that variable fonts with hinting are not really a thing, so it probably won't cause problems. But at the very least might be worth adding a TODO.

[LaurenzV]

Hmm well it does seem to be a thing at least: https://github.com/googlefonts/how-to-hint-variable-fonts

Not sure how common it is in practice though.

[laurmaedje]

What would the fix be? Just skipping those tables if we use skrifa interjection?

[LaurenzV]

Yep. If you do make a change maybe let's add a TODO to look into adding a test for this as well, but dont need to block on this I think.

[laurmaedje]

Done. I first added a helper and reshuffled the commits, but the diff since you LGTM'd is https://github.com/typst/subsetter/compare/c53fc9c34e22664582246f6aa507062763e1193f..6174cf8e0a6c9d1836b0b7c50744982dd28c9b8a.

[laurmaedje]

If this looks good, I'll merge and cut a release. Looking at the changes, I would probably make a patch release rather than a minor release. The MSRV bump is the one thing standing out, but it's not broadly considered a breaking change and when we added it in the first place I also just released a new patch version.

[LaurenzV]

Looks good!