opentype.js icon indicating copy to clipboard operation
opentype.js copied to clipboard

Font is treated as a required argument in Glyph.draw()

Open opengraphica opened this issue 1 year ago • 9 comments

Description

This makes it so font is not treated as a required argument in Glyph.draw.

Motivation and Context

This looks like a simple regression bug.

Checklist:

  • [x] I did npm run test and all tests passed green (including code styling checks).
  • [x] I have added tests to cover my changes.
  • [ ] My change requires a change to the documentation.
  • [ ] I have updated the README accordingly.
  • [x] I have read the CONTRIBUTING document.

opengraphica avatar Jun 09 '24 04:06 opengraphica

The Font.defaultRenderOptions property is needed for variable font rendering, making this optional breaks variable fonts.

ILOVEPIE avatar Jun 11 '24 20:06 ILOVEPIE

Did 1.3.4 not support variable fonts?

To be honest, I do not like this API. The user always retrieves a glyph from a font object, so logically the glyph should already know about the font it is associated with. A function where there is literally only one value you should be passing into an argument should be inferred by the library.

I would leave this optional because for the majority use case it is not necessary. It doesn't break anything, since the documentation states to pass in the font for variable font rendering.

opengraphica avatar Jun 14 '24 04:06 opengraphica

Additionally, font is considered optional in getPath(), which is the function that retrieves the variation in the first place, no? The API is inconsistent.

https://github.com/opentypejs/opentype.js/blob/ab95cea6560b9a8ba20f6290603e7e6e8f4bd02b/src/glyph.mjs#L148

https://github.com/opentypejs/opentype.js/blob/ab95cea6560b9a8ba20f6290603e7e6e8f4bd02b/src/glyph.mjs#L157

opengraphica avatar Jun 14 '24 05:06 opengraphica

I added this to getPath() as the function header says it was needed for fontHinting. Yes I also don't like the API, it was a quick fix to not change more on the core libreary than strictly necessary. The draw function back then didn't have the argument at all. (which I admit was also inconsistent, honestly likely because I didn't care about it)

PS: Genereally instead of "a && a.b" .. i'd just do "a?.b"

axkibe avatar Jun 14 '24 12:06 axkibe

Glyph objects do not have a reference back to the font. You can copy a glyph from one font to another by adding the Glyph object to multiple font objects' GlyphSet.

Regarding a && b: this was previously necessary because we couldn't use the optional chaining operator for backwards compatibility. Indeed, with the new build process, we can make use of modern syntax.

Connum avatar Jun 14 '24 13:06 Connum

PS: Genereally instead of "a && a.b" .. i'd just do "a?.b"

I agree, just following the existing code convention.

Glyph objects do not have a reference back to the font. You can copy a glyph from one font to another by adding the Glyph object to multiple font objects' GlyphSet.

I see these as two separate use cases:

  1. I want to load an existing font into memory in a browser, in order to draw it.
  2. I want to create a custom font using this library, then save it to an .otf file.

I think the 2nd case is what you are referring to, which generally is not associated with the draw function being used. This scenario seems to be a weird mix where a use case not really designed for drawing messes with the draw function. Ideally, when a glyph is generated from a font, it would have a reference to all the information necessary to draw it.

opengraphica avatar Jun 15 '24 03:06 opengraphica

If you build a font editor and want to render a preview of both fonts, you'll want to use the draw function.

Connum avatar Jun 15 '24 06:06 Connum

Ok, but at the same time the library doesn't currently support writing to the hvar table anyways. Even if two fonts share the same glyph, can't make use of that "font" argument in the draw call. https://github.com/opentypejs/opentype.js/blob/master/src/tables/hvar.mjs#L41

Either way this discussion is a tangent to the PR. Since a glyph contains its own path data and can be drawn on its own, "font" shouldn't be a required argument in the draw call.

opengraphica avatar Jun 15 '24 15:06 opengraphica

And it would be nice in the future if a glyph that was retrieved from a font can have a reference to the original font it came from as a "default" setting for drawing, which can be overwritten with the "font" argument.

I think most people are using this library simply to draw a font on a canvas, or generate a SVG shape from a font, or generate a 3D mesh from a font. Would be good to optimize the API for this use case.

opengraphica avatar Jun 15 '24 15:06 opengraphica