Skip to content

Don't change to custom encoding on setting altuni#4598

Draft
skef wants to merge 1 commit intofontforge:masterfrom
skef:altunienc
Draft

Don't change to custom encoding on setting altuni#4598
skef wants to merge 1 commit intofontforge:masterfrom
skef:altunienc

Conversation

@skef
Copy link
Copy Markdown
Contributor

@skef skef commented Jan 30, 2021

... when the encoding is already unicode compatible.

PyFF_Glyph_set_altuni() currently ends with:

    for ( fvs=self->sc->parent->fv; fvs!=NULL; fvs=fvs->nextsame ) {
        fvs->map->enc = &custom;
        FVSetTitle(fvs);
    }

This is always executed no matter what the existing encoding. As far as I can see:

  1. There's no reason to ever do this when the current encoding is UnicodeFull
  2. There's no reason to do this when the current encoding is UnicodeBMP and none of the altuni values for the glyph are outside of the BMP range.

Anyway this is super-annoying when working with a script because (for example) createMappedChar() stops working right after you add an altuni unless you set the encoding back to UnicodeBMP (for example) after every addition.

when the encoding is already unicode compatible
@skef
Copy link
Copy Markdown
Contributor Author

skef commented Jan 30, 2021

At the time of filing this is a partial fix, because while it does eliminate the problem it doesn't cause the character in question to be displayed at the altuni-corresponding place(s) in the FontView. You can see this if you save the file and reopen it.

I played around with various combinations of code to make that work and didn't have much luck. @frank-trampe any ideas?

Comment thread fontforge/python.c
if ( !IsUnicodeEncoding(fvs->map->enc, bmp_compat) ) {
fvs->map->enc = &custom;
FVSetTitle(fvs);
} // XXX else rebuild map?
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is this this?

encoded[j] = i;

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