Skip to content

Conversation

@PascalPixel
Copy link

@PascalPixel PascalPixel commented Feb 5, 2023

@felipesanches made initial headway into this, but never opened a pull request.

Is any of this usable / salvageable?

This relates to #95 - I create a pixel font from scratch using opentype.js ❤️ and would love to add kerning

…rs in the code so that it at least passes all pre-existing regression tests.
…test case for the parser. We'll later add similar testcases for the encoder.
…type#2 encoding is not fully implemented yet.
…dded a new round-trip decode/encode/decode test to check whether GPOS table fields are properly preserved.
…messages. Next step is to increase the scope of the test that verifies if all fields are properly preserved in a round-trip.
…ode fails to pass that and needs to be fixed)
@Connum Connum added the Spec Related to the implementation of the Opentype specification label Feb 5, 2023
@ILOVEPIE
Copy link
Contributor

ILOVEPIE commented Feb 5, 2023

I could have sworn we already have GPOS implemented.

@ILOVEPIE
Copy link
Contributor

ILOVEPIE commented Feb 5, 2023

We do have it implemented, but we need more formats added to it see #491

@Connum
Copy link
Contributor

Connum commented Feb 5, 2023

We do have it implemented, but we need more formats added to it see #491

As far as I can see, #491 adds read support, but not write support.

It's quite unfortunate that this PR consists of so many commits and most are tagged with WIP in the commit message, which makes it very hard to see whether this is ready or not. Maybe @felipesanches can tell us more, even though its been 84 7 years?

@ILOVEPIE
Copy link
Contributor

ILOVEPIE commented Feb 5, 2023

I'd say this probably needs re-implementation, I doubt we can merge this code. We definitely can use it for reference though.

@Connum Connum marked this pull request as draft February 5, 2023 22:26
@felipesanches
Copy link

I will try to review my old code and let you know what I think. Please give me a few days to go through the content of those commits.

@felipesanches
Copy link

I am now rebasing everything into a single commit and fixing the conflicts with current master branch. I also found a couple more left over commits on my local working copy that included some more refactoring. I cannot remember what I wrote 7 years ago, but I will include those as well.

@PascalPixel
Copy link
Author

PascalPixel commented Feb 6, 2023

Thanks @felipesanches! I'm currently using fontforge, directed from node, with a very weird long command string to get the kernings in 😄

import { exec as oldExec } from "child_process";
import { promisify } from "util";

const exec = promisify(oldExec);

class FontForgeError extends Error {
  constructor(message: string) {
    super(message);
    this.message = message;
    this.name = "FontForgeError";
  }
}

class FontForgeCommandError extends Error {
  constructor(e: Error, cmd: string) {
    const msg =
      `FontForge command failed: ${e.toString()}\n` + `From command: ${cmd}`;
    super(msg);
    this.message = msg;
    this.name = "FontForgeError";
  }
}

function trim(s: string) {
  if (s.length === 0) return s;
  if (s[s.length - 1] === "\n") return s.slice(0, s.length - 1);
  return s;
}

export async function getName(src = "") {
  if (!src) throw new FontForgeError("fontforge.getName(): src not provided");
  const cmd = [
    "fontforge",
    "-lang=ff",
    "-c",
    "'Open($1); Print($fontname);'",
    `'${src}'`,
    "2>",
    "/dev/null",
  ].join(" ");
  let result = "";
  try {
    result = trim((await exec(cmd)).stdout);
  } catch (e) {
    throw new FontForgeCommandError(e as Error, cmd);
  }
  return result;
}

export async function addKerning(
  src: string,
  kerningPairs: {
    left: string;
    right: string;
    gap: number;
  }[]
) {
  if (!src)
    throw new FontForgeError("fontforge.addKerning(): src not provided");
  if (!kerningPairs)
    throw new FontForgeError(
      "fontforge.addKerning(): kerningPairs not provided"
    );

  // Add the kerning pairs
  const cmd = [
    `fontforge`,
    `-lang=ff`,
    `-c`,
    `'`,
    `Open($1);`,
    `AddLookup("kern","gpos_pair",0,[["kern",[["DFTL",["dflt"]]]]]);`,
    `AddLookupSubtable("kern","kern-1");`,
    kerningPairs
      .map((kerningPair) => {
        const { left, right, gap } = kerningPair;
        const leftCharUnicodeHex = `U+${left
          .charCodeAt(0)
          .toString(16)
          .padStart(4, "0")
          .toUpperCase()}`;
        const rightCharUnicodeHex = `U+${right
          .charCodeAt(0)
          .toString(16)
          .padStart(4, "0")
          .toUpperCase()}`;
        return [
          `Select("${rightCharUnicodeHex}");`,
          `second = GlyphInfo("Name");`,
          `Select("${leftCharUnicodeHex}");`,
          `AddPosSub("kern-1",second,0,0,${gap},0,0,0,0,0);`,
        ].join("");
      })
      .join(""),
    `Generate($1);`,
    `'`,
    `'${src}'`,
    `2>`,
    `/dev/null`,
  ].join(" ");

  let result = "";
  try {
    const out = (await exec(cmd)).stdout;
    result = trim(out);
  } catch (e) {
    throw new FontForgeCommandError(e as Error, cmd);
  }
  return result;
}

@mattlag
Copy link
Contributor

mattlag commented Jun 21, 2023

Hello, Everyone! Commenting in hopes of bumping this back up - it would be great to get read/write support for kern data! @felipesanches can you comment as to the status of this?

@ILOVEPIE
Copy link
Contributor

I believe that there was another PR on this that either got merged or was being looked at, but ill take a look and let you know. I've been busy with work.

@quentin-f451
Copy link

This is indeed a much needed feature! Do you think it will come with a future release?

@Tenchi2xh
Copy link

Also writing a font from scratch and would love to be able to set simple kerning pairs 🙏🏼

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Spec Related to the implementation of the Opentype specification

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants