[on hold] generate kerning and allow sparseness in a dangerous and incorrect manner #345

rsheeter · 2023-06-26T21:19:40Z

Current naive output akin to:

@public.kern2.w = [w wacute wcircumflex wdieresis wgrave];
@public.kern1.l = [f_f_l fl l lacute lcaron lcommaaccent];
@public.kern1.dze-cy = [dze-cy];
...etc...

feature kern {
  pos @public.kern1.dze-cy @public.kern2.Ustrait-cy (wght=-1n:-60 wght=0n:-58 wght=1n:-56);
  pos @public.kern1.n @public.kern2.W (wght=-1n:-10 wght=0n:-13 wght=1n:-18);
  pos dze-cy Te-cy (wght=-1n:-69 wght=0n:-61 wght=1n:-49);
  ...etc...

NOTE TO SELF: make sure we have a test where the locations in the conditionset are not identical.

anthrotype · 2023-07-05T11:12:30Z

fontir/src/ir.rs

-    /// is an error.
-    pub variation_model: VariationModel,
+    /// is an error. This model enforces the no delta at the default location constraint
+    /// used in things like gvar.


I'm confused. The "no delta at the default location" is not just "for things like gvar", the entire OpenType Variations model works like that. Why do you need to define an alternative to ModelConstraints::ZeroAtDefault? What are you trying to do?

This stems from a misunderstanding on my part (as we discussed on monday) where I thought that in some cases variable metrics were missing values at the default location.

In reading the comments I think perhaps we are conflating source and binary output? - I was imagining we might allow kerning to be specified w/o a value at the default. The binary would still get a value at default, it merely wouldn't be required in source.

As a concrete example, imagine if you will:

A font with weight 100..900, default 400

Hand-written fea for (some pair) that says kern +10 at weight 100, +20 at weight 900

We don't get a direct value for the value at default so we interpolate it. The binary is still written out based on deltas from the default, it's merely the sources that lose the limitation.

To me this feels like the logical outcome of approaching things in a sparse, variable-first, manner. I would extend it to glyphs as well, no need to provide an actual outline for default in source as long as you provide definitions that span the default so we can interpolate it.

anthrotype · 2023-07-05T11:20:18Z

fontir/src/variations.rs

@@ -1247,4 +1303,29 @@ mod tests {
            model.deltas(&point_seqs).unwrap()
        );
    }
+
+    #[test]
+    fn model_with_no_default_location() {


The base master's location is at 0.0 (in normalized locations) by definition, otherwise it is simply not the base/default/neutral master. A VariationModel must have a base master and this has to be at 0.0, without one it's just invalid/incomplete. What's this and what does it have to do with variable kerning FEA?

anthrotype · 2023-07-05T11:28:03Z

fontbe/src/features.rs

+    /// locations we observe. We also use different `ModelConstraints` than glyf: we don't require
+    /// the effect at the default location to be zero.


We also use different ModelConstraints than glyf: we don't require the effect at the default location to be zero.

sorry but that's wrong. The effect at the default location is always 0 throughout the OT Variations, it's not about glyf or GPOS. The variable scalars in FEA must always specify a value for the default master's location, there's no exception here.

fontbe/src/features.rs

anthrotype · 2023-07-05T11:39:16Z

fontbe/src/features.rs

+    }
+
+    // For any kern that is incompletely specified fill in the missing values using UFO kerning lookup
+    // Not 100% sure if this is correct for .glyphs but lets start there


I would say yes, not least because fontmake has compiled .glyphs files by turning them into UFOs first for long time now and if Glyphs used a different kerning lookup algorithm than UFOs we would have known by now

anthrotype · 2023-07-05T11:50:36Z

fontbe/src/features.rs

+    // For any kern that is incompletely specified fill in the missing values using UFO kerning lookup
+    // Not 100% sure if this is correct for .glyphs but lets start there
+    // Generate variable format kerning per https://github.com/adobe-type-tools/afdko/pull/1350
+    // Use design values per discussion on https://github.com/harfbuzz/boring-expansion-spec/issues/94


I understand that a consensus is building on using design coordinates but at least the current feaLib implementation expects user coordinates for variable scalars in FEA, and there might be code/fonts out there relying on this

https://github.com/fonttools/fonttools/blob/3235454fcd720f270e18ddf00b058df83da63547/Lib/fontTools/feaLib/variableScalar.py#L89-L99

I see below that normalized coordinates are being used (with the proposed n suffix), so maybe the comment referring to "design" coordinates is stale and needs to be updated.

Correct, IIRC this code was written in the interval where I wasn't aware suffixes were coming.

anthrotype · 2023-07-05T12:01:56Z

fontbe/src/features.rs

+                    .join(",")
+            });
+
+            // TODO can we skip some values by dropping where value == interpolated value?


you'd have to construct a new temporary model with a given master omitted, interpolate at that location and compare original with interpolated value, might become expensive to do for many kerns * masters. We don't currently do any of that in the python pipeline, source masters are always retained even if potentially redundant.

If we let people hand-write variable value records I think we have to support building new variation models in case they write a rule that references a point that isn't aligned with any master. To be fair, we could kick that out for now as I don't believe current compilation or sources allow this.

anthrotype · 2023-07-05T12:16:00Z

fontbe/src/features.rs

+        let static_metadata = weight_variable_static_metadata(300.0, 400.0, 700.0);
+        let var_info = FeaVariationInfo::new(&static_metadata);
+
+        // Find me the default value despite that not being one of the input locations


this example won't compile in fonttools because, as I said, a value for the baster master (at default location) is always required. You'll probably get "base master not found". What you seem to be doing here is like a naive version of the L4 instancer (moving the default). May seem to make sense for this simple case with one axis and two masters, but will blow up as soon as you make it more complicated. I'll repeat, the general rule in OT variations is that the default is never sparse, always densely contains all the values, it defines the skeleton of the VF, what's left when you take away all the variation tables, it's the reference point for all deltas. The default master cannot be left undefined for it influences all the regions of the designspace (whose deltas are defined in terms of it), therefore the font developer must know exactly and take responsibility to specify where/what that looks like, it should not be left to the compiler to guess.

I understand that in the binary it must be dense at 0n. If we differ in opinion it is wrt whether it follows that the source needs to be dense at default. IMHO it doesn't, it should be fine to have a source with say weight 300-700, default 400, with source defining only two outlines, at 300 and 700, for a glyph.

Ultimately IMHO in source you just need to provide a definition that spans default. That is, provide at least one of:

A definition at default

if there are no other definitions the glyph won't vary

Definition at 2 or more locations that define a region that encompasses 0n

compiler will figure out what the value at 0n should be and the deltas to give the expected results

behdad · 2023-07-07T16:21:11Z

I'm with Cosimo here. What you are doing, resolving value at default location and reinserting it into the set of masters, breaks the modeling. As Cosimo points out, to do it correctly you need the full L4 instancing logic.

Here's an example. Imagine we have two axes A and B both running from 0 to 1. We have three kerns: at 0,0=10; 0,1=20; and 1,0=30. In this model, if the first master is picked to be the base master for modeling purposes, then the interpolated kern at 1,1 will come out as 40. That's desirable.

Now if you interpolate the kern at .5,.5 you get 25. If you insert that as your new default location / value, then suddenly the kern you get at 1,1 will drop down to 25. Ie. the rest of the masters are inactive at 1,1 because they are in different quadrants. That's just how the OT model works.

anthrotype · 2023-07-07T16:36:13Z

to do it correctly you need the full L4 instancing logic.

even then, you still need to have or select some default to begin with before you can move it to some place else, with the consequences in terms of additional masters that this entails

behdad · 2023-07-07T16:39:24Z

even then, you still need to have or select some default to begin with before you can move it to some place else, with the consequences in terms of additional masters that this entails

Correct. You'd get different interpolation results based on which master you pick as base.

rsheeter · 2023-07-19T13:44:44Z

the rest of the masters are inactive at 1,1 because they are in different quadrants. That's just how the OT model works.

IIUC this is exclusively due to the zero influence at 0n rule. OK, fine, make a non-OT model (doesn't zero at zero) and use that to figure out an OT model that gives the desired results. I wouldn't claim the WIP does that correctly, it's just exploratoyr hackery, but I think there might be a path to some interesting capabilities.

rsheeter added this to the Compile Oswald and compare to fontmake milestone Jun 26, 2023

rsheeter force-pushed the kern branch 5 times, most recently from 6ecdc8d to cb22607 Compare June 28, 2023 04:45

This was referenced Jun 29, 2023

Support unit suffixes cmyr/fea-rs#174

Closed

Sketch out API for getting deltas from the compiler cmyr/fea-rs#178

Merged

rsheeter force-pushed the kern branch 2 times, most recently from 1d8e324 to 3c1be3d Compare July 1, 2023 19:15

wip

cc52964

rsheeter force-pushed the kern branch from 3c1be3d to cc52964 Compare July 1, 2023 21:58

anthrotype reviewed Jul 5, 2023

View reviewed changes

fontbe/src/features.rs Show resolved Hide resolved

anthrotype reviewed Jul 5, 2023

View reviewed changes

rsheeter mentioned this pull request Jul 19, 2023

Generate kern fea and pass to fea-rs #373

Merged

anthrotype mentioned this pull request Jul 26, 2023

Allow moving default instance of a varfont with avar2 harfbuzz/boring-expansion-spec#89

Open

rsheeter changed the title ~~[WIP] Generate variable fea for kerning and send to fea-rs~~ [on hold] generate kerning and allow sparseness in a dangerous and incorrect manner Aug 15, 2023

rsheeter closed this Oct 27, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[on hold] generate kerning and allow sparseness in a dangerous and incorrect manner #345

[on hold] generate kerning and allow sparseness in a dangerous and incorrect manner #345

rsheeter commented Jun 26, 2023 •

edited

Loading

anthrotype Jul 5, 2023

cmyr Jul 5, 2023

rsheeter Jul 7, 2023

anthrotype Jul 5, 2023

anthrotype Jul 5, 2023

anthrotype Jul 5, 2023

anthrotype Jul 5, 2023

anthrotype Jul 5, 2023

rsheeter Jul 7, 2023

anthrotype Jul 5, 2023

rsheeter Jul 19, 2023

anthrotype Jul 5, 2023

rsheeter Jul 19, 2023

behdad commented Jul 7, 2023

anthrotype commented Jul 7, 2023

behdad commented Jul 7, 2023

rsheeter commented Jul 19, 2023

		/// locations we observe. We also use different `ModelConstraints` than glyf: we don't require
		/// the effect at the default location to be zero.

[on hold] generate kerning and allow sparseness in a dangerous and incorrect manner #345

[on hold] generate kerning and allow sparseness in a dangerous and incorrect manner #345

Conversation

rsheeter commented Jun 26, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

behdad commented Jul 7, 2023

anthrotype commented Jul 7, 2023

behdad commented Jul 7, 2023

rsheeter commented Jul 19, 2023

rsheeter commented Jun 26, 2023 •

edited

Loading