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

Combine "Build Accented Character" and "Build Composite Character"? #3725

Open
skef opened this issue Jun 5, 2019 · 9 comments
Open

Combine "Build Accented Character" and "Build Composite Character"? #3725

skef opened this issue Jun 5, 2019 · 9 comments

Comments

@skef
Copy link
Contributor

skef commented Jun 5, 2019

@ctrlcctrlv has done a bunch of work to allow combining characters to be customized. That functionality can also be used for ligatures. (The "source" characters need to be mapped, although that can be in the PUA area. The "target" characters don't need to be, as far as I know, so that works for most ligatures.) I'm looking into adding a hook with priority between @ctrlcctrlv's functionality and FontForge's default functionality.

One problem with that is these two distinct menu options under "Element -> Build". I take it the distinction is supposed to be that "Build Accented Character" works off Unicode combining classes and "Build Composite Character" has glyph-name based heuristics. The two commands then let the user know which type of combination is available, through menu item deactivation. In my view, however, customization has already muddied these waters. And if we add a hook it could work on whatever basis the programmer sees fit, including unicode mapping and/or glyph names.

So: Would it be acceptable to merge these into a single "Build Composite Character" menu item, leaving the Ctrl-Shift-A default hotkey?

@skef
Copy link
Contributor Author

skef commented Jun 6, 2019

The instructions say:

You can use the Element->Build->Build Accented Glyph command to build up accented glyphs. Or you can use Element->Build->Build Composite and build slightly more general composite glyphs (ligatures and so forth). The accented version will not create ligatures and will not replace Alpha with A. The composite version will do both these things. My assumption is that ligatures (like the "fi" ligature) usually need a bit of work on the user's part to get them to look good, in the case of "fi" the dot on the "i" needs to be fused into the hook of the "f", and if you are careless with the command you might destroy your work inadvertently.

If this really is what the difference amounts to, and Build Composite will already happily build an accent glyph when desired, that strikes me as a pretty weak reason to have to have both options. I've tested that Build Composite works with Undo, so it's not like there isn't recourse if you don't like the result.

@ctrlcctrlv
Copy link
Member

Just so you know I made it so if there's any User Decomposition, both are possible regardless of the characters in the decomposition. This allows "o" to be used as the ring for "a" in å

@skef
Copy link
Contributor Author

skef commented Jun 11, 2019

Looking more closely at the code, the difference corresponds to a boolean parameter of SCBuildComposite rather than separate implementations. Which makes the "why?" question more acute: what was the intent of having the two interfaces?

Reading more of the docs, I think the intent of "Build Accented Glyph" was to have a command that could be run on large blocks of characters to be selected in a FontView window. Then only accented characters would be built and the others would be left alone. I'll try to confirm that suspicion as I'm poking around.

@ctrlcctrlv
Copy link
Member

But I added that parameter, before there wasn't one. You might be right either way though

@ctrlcctrlv
Copy link
Member

Yeah exactly the same function was run for both composites and accents before. I needed to differentiate them due to the ao case I already mentioned.

@skef
Copy link
Contributor Author

skef commented Jun 12, 2019

Sure enough. As of 2012 or so the top-level (in charview.c) was:

static void CVMenuBuildAccent(GWindow gw,struct gmenuitem *mi,GEvent *e) {
    CharView *cv = (CharView *) GDrawGetUserData(gw);
    extern int onlycopydisplayed;
    int layer = CVLayer((CharViewBase *) cv);

    if ( SFIsRotatable(cv->b.fv->sf,cv->b.sc,layer))
      /* It's ok */;
    else if ( !SFIsSomethingBuildable(cv->b.fv->sf,cv->b.sc,layer,true) )
return;
    SCBuildComposit(cv->b.fv->sf,cv->b.sc,layer,NULL,onlycopydisplayed);
}

static void CVMenuBuildComposite(GWindow gw,struct gmenuitem *mi,GEvent *e) {
    CharView *cv = (CharView *) GDrawGetUserData(gw);
    extern int onlycopydisplayed;
    int layer = CVLayer((CharViewBase *) cv);

    if ( SFIsRotatable(cv->b.fv->sf,cv->b.sc,layer))
      /* It's ok */;
    else if ( !SFIsCompositBuildable(cv->b.fv->sf,cv->b.sc->unicodeenc,cv->b.sc,layer) )
return;
    SCBuildComposit(cv->b.fv->sf,cv->b.sc,layer,NULL,onlycopydisplayed);
}

So ... what the hell?

@ctrlcctrlv
Copy link
Member

The only thing that's different even before was IsCompositBuildable (maybe not exact function name, writing this on cell phone)

@ctrlcctrlv
Copy link
Member

I think you're right that it was just to stop Ctrl-A Build Accented Character from building the ligatures ffi/ffl etc.

@skef
Copy link
Contributor Author

skef commented Jun 12, 2019

Oh, OK, I wasn't even noticing that. I was thinking the difference I cited earlier would be implemented in SCBuildComposite, but as you're pointing out it can just as easily be in SFIsCompositBuildable().

The problem this poses for customization is whether we really want customizers to have to think about the difference. I'm tempted to say that Build Accented Glyph is a minor advantage that could be discarded. It's more or less useless in the CharView and users can just highlight the slots they want to build in the FontView.

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

2 participants