Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Variable anchor positioning in sparse masters #640

Closed
simoncozens opened this issue Jul 27, 2022 · 10 comments
Closed

Variable anchor positioning in sparse masters #640

simoncozens opened this issue Jul 27, 2022 · 10 comments

Comments

@simoncozens
Copy link
Contributor

This came up while I was testing #635. Consider a test which compiles this designspace file:

ufo2ft/tests/conftest.py

Lines 54 to 93 in 7d0a330

@pytest.fixture
def designspace(layertestrgufo, layertestbdufo):
ds = designspaceLib.DesignSpaceDocument()
a1 = designspaceLib.AxisDescriptor()
a1.tag = "wght"
a1.name = "Weight"
a1.default = a1.minimum = 350
a1.maximum = 625
ds.addAxis(a1)
s1 = designspaceLib.SourceDescriptor()
s1.name = "Layer Font Regular"
s1.familyName = "Layer Font"
s1.styleName = "Regular"
s1.filename = "LayerFont-Regular.ufo"
s1.location = {"Weight": 350}
s1.font = layertestrgufo
ds.addSource(s1)
s2 = designspaceLib.SourceDescriptor()
s2.name = "Layer Font Medium"
s2.familyName = "Layer Font"
s2.styleName = "Medium"
s2.filename = "LayerFont-Regular.ufo"
s2.layerName = "Medium"
s2.location = {"Weight": 450}
s2.font = layertestrgufo
ds.addSource(s2)
s3 = designspaceLib.SourceDescriptor()
s3.name = "Layer Font Bold"
s3.familyName = "Layer Font"
s3.styleName = "Bold"
s3.filename = "LayerFont-Bold.ufo"
s3.location = {"Weight": 625}
s3.font = layertestbdufo
ds.addSource(s3)
return ds

So we have regular at 350, regular again at 450 and bold at 625. Fine. My question is, how does the positioning of the e glyph's top anchor vary over the designspace?

At 350 it is regular, so:

<anchor x="314" y="556" name="top"/>

At 450 it is still regular, so the same.

At 625, it is bold, so:

<anchor x="315.32105263157894" y="644.0078947368421" name="top"/>

So the X coordinate of the anchor goes 314 at wght=350, 314 at wght=450, and 315 at wght=650.
The Y coordinate goes 556 at wght=350, 556 at wght=450, and 644 at wght=625.

Except that what we test for, the output TTX file, is:

<VarRegionList>
<!-- RegionAxisCount=1 -->
<!-- RegionCount=1 -->
<Region index="0">
<VarRegionAxis index="0">
<StartCoord value="0.0"/>
<PeakCoord value="1.0"/>
<EndCoord value="1.0"/>
</VarRegionAxis>
</Region>
</VarRegionList>
<!-- VarDataCount=1 -->
<VarData index="0">
<!-- ItemCount=2 -->
<NumShorts value="0"/>
<!-- VarRegionCount=1 -->
<VarRegionIndex index="0" value="0"/>
<Item index="0" value="[1]"/>
<Item index="1" value="[88]"/>
</VarData>

That's a linear progression from x=314, y=556 at wght=350 through to x=315, y=644 at wght=650. The medium stop has vanished. In fact, if you ask it to produce a debug feature file, the medium isn't there!

### LayerFont-Regular ###
feature liga {
    sub a e s s by s;
} liga;

markClass dotabovecomb <anchor -2 465> @MC_top;

feature mark {
    lookup mark2base {
        pos base e
            <anchor 314 556> mark @MC_top;
    } mark2base;

} mark;

### LayerFont-Bold ###
feature liga {
    sub a e s s by s;
} liga;

markClass dotabovecomb <anchor -2 465> @MC_top;

feature mark {
    lookup mark2base {
        pos base e
            <anchor 315 644> mark @MC_top;
    } mark2base;

} mark;

I think this is because the medium is the regular, and since we compiled that once, we don't compile it again. And so we end up with the wrong answer. With my PR, we get a debug feature file of:

feature liga {
    sub a e s s by s;
} liga;

markClass dotabovecomb <anchor -2 465> @MC_top;

feature mark {
    lookup mark2base {
        pos base e
            <anchor (wght=350:314 wght=450:314 wght=625:315) (wght=350:556 wght=450:556 wght=625:644)> mark @MC_top;
    } mark2base;

} mark;

which clearly looks better, and compiles to:

        <!-- RegionCount=2 -->
         <Region index="0">
           <VarRegionAxis index="0">
             <StartCoord value="0.0"/>
            <PeakCoord value="0.36365"/>
            <EndCoord value="1.0"/>
          </VarRegionAxis>
        </Region>
        <Region index="1">
          <VarRegionAxis index="0">
            <StartCoord value="0.36365"/>
             <PeakCoord value="1.0"/>
             <EndCoord value="1.0"/>
           </VarRegionAxis>
        </Region> 
       </VarRegionList> 
       <!-- VarDataCount=1 --> 
       <VarData index="0"> 
         <!-- ItemCount=2 --> 
         <NumShorts value="0"/> 
         <!-- VarRegionCount=1 --> 
         <VarRegionIndex index="0" value="1"/> 
         <Item index="0" value="[1]"/>
         <Item index="1" value="[88]"/>
       </VarData>

which I believe to be correct - but which doesn't pass tests.

@anthrotype
Copy link
Member

we have regular at 350, regular again at 450

no, in the conftest.py::designspace fixture, the second source is not the same as the first, the filename is the same "LayerFont-Regular.ufo", but note how the second has the layerName = "Medium" attribute which the first omits. The first source is a normal one, the second is a "sparse layer". I'm not sure what's going on in the rest of your test, just wanted to clarify that.

@simoncozens
Copy link
Contributor Author

Ohhhh. So the "medium" isn't a second master, and shouldn't affect layout?

@anthrotype
Copy link
Member

hm.. Well, currently we don't compile features for sparse layers since they can't have features.fea of their own, we only use them for the outlines..

@simoncozens
Copy link
Contributor Author

Right, but what about anchors...?

@anthrotype
Copy link
Member

don't know, what do you expect to happen?

@anthrotype
Copy link
Member

maybe with your variable features.fea you can take those into account, the anchors are still in the glyph, just need to get them from the named layer

@simoncozens
Copy link
Contributor Author

The issue is that the variable feature build is taking into account the anchor positions of anchors in sparse master layers, and the current HEAD does not take into account the anchor positions of anchors in sparse master layers, and I don't know which is the right behaviour.

@anthrotype
Copy link
Member

I would say taking them into account would be the right behavior, so yours is the correct one. We currently don't because sparse layer masters have no kerning.plist or features.fea of their own, they are just containers of glyph shapes (and yes anchors, so it happens), so we can't/don't write/compile OT features for the master TTFs that are produced off those

@anthrotype
Copy link
Member

anthrotype commented Jul 27, 2022

does your code handle the possibility that given anchors are missing from a sparse layer master and treat them as non-participating in the interpolation? that would allow a font developer to control whether they want or not the glyph anchors in sparse layers to contribute to variable mark/mkmk features. Right now they seem to be ignored.

@simoncozens simoncozens changed the title Using the same UFO object twice messes up creation of per-master variation? Variable anchor positioning in sparse masters Jul 27, 2022
@khaledhosny
Copy link
Collaborator

This should be fixed now.

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

No branches or pull requests

3 participants