Support setting SplineChar width from importOutlines. #5005

frank-trampe · 2022-05-05T14:50:46Z

This supersedes #5004.

This set of changes allows setting SplineChar width from an imported discrete SVG file by setting the new "dimensions" flag in the Python routine SplineChar.importOutlines. Default behavior remains ignoring the SVG dimensions.

Test as follows.

frank@Luther-1:~/Projects_1/2022/Weather_Typeface_1/Samples_1$ /var/tmp/ff10/fontforge/build/bin/fontforge -lang=py -script
Copyright (c) 2000-2019. See AUTHORS for Contributors.
 License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
 with many parts BSD <http://fontforge.org/license.html>. Please read LICENSE.
 Version: 20220308
 Based on sources from 17:58 UTC 23-Jul-2019-ML-D-GDK3.
 Based on source from git with hash: 4f83172d44db72544b1bacf11b79bddf1d236647
PythonUI_Init()
copyUIMethodsToBaseTable()
Program root: /var/tmp/ff10/fontforge/build
Python 3.7.5 (default, Dec  9 2021, 17:04:37)
[GCC 8.4.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> font0 = fontforge.font()
>>> font0.ascent = 55
>>> font0.descent = 0
>>> char0 = font0.createChar(257)
>>> char0.importOutlines(filename="WeatherSymbol_WMO_CloudHigh_CH_1.svg",scale=False,dimensions=True)
<fontforge.glyph at 0x0x7f27eb78e4b0 U+0101 "glyph0">
>>> font0.save("font0_5.sfd")
<fontforge.font at 0x0x7f27eb78e430 "Untitled1">
>>> quit()

…he Python bindings that sets the width of the glyph from an imported SVG.

ctrlcctrlv · 2022-05-05T17:55:53Z

fontforge/svg.c

@@ -2777,7 +2777,36 @@ return( NULL );
 return( eret );
 }

-static Entity *SVGParseSVG(xmlNodePtr svg,int em_size,int ascent,bool scale) {
+void SCDimensionFromSVG(xmlNodePtr svg, SplineChar *sc, bool vert) {


No viewBox handling needed in this change, Frank?

I don't know how we would map the viewBox values to glyph state.

I think @ctrlcctrlv was thinking that given that the viewbox is x, y, width, height, one could consider the third and fourth values as alternate sources of width and height parameters.

Thank you, that's right.

Is there any case in which the width/height attribute would be missing or inapplicable (due to non-absolute units) and the viewBox would be useful? For what it is worth, the specification seems to suggest here that width and height are at least always present.

I think values of "100%" are common, and I think I've seen elsewhere that "100%" is the implicit default for the svg element, but that could be wrong.

But yes, if the width element is a percentage or a length measure and the viewBox is present then the latter is a good source of a width dimension. That's basically what another client is going to do: treat the boundaries specified by the viewBox as the abstract dimensions of the SVG and then scale in one or both dimensions according to the length and width percentages or lengths.

I looked at an Inkscape file, since those are our most likely import candidates. Inkscape uses the user-specified page unit in the width and height fields ("8.5in" for example), with the viewBox in the user-specified drawing unit without a suffix. The patch as it is thus only works for a narrow range of inputs. But doing this right is not trivial, and I want to get what we have merged for the benefit of the weather symbols project.

Using the viewBox seems like a good fallback, but I do wonder whether we need to devote any attention to the case of one of the first two values being non-zero.

I believe the viewbox is x, y (starting point), width, height, so the first two values should be non-relevant for these purposes.

skef · 2022-05-06T09:39:12Z

fontforge/svg.c

+    double width, height;
+    num = (char *) xmlGetProp(svg,(xmlChar *) "width");
+    if ( num!=NULL ) {
+	width = strtod(num,NULL);


I'm not necessarily against this PR as-is because particular workflows will use particular conventions and we can put the onus on the user. However, we should probably discuss this a bit.

As I understand it any of these are valid SVG width/height entries:

640

80%

5cm

Taking the value from one of the latter cases seems like a mistake.

Taking @ctrlcctrlv's note into account as well, one could imagine using something like the following algorithm:

If "width" attribute is present width = strtod(num, &endptr); if endptr points to the end of the string (or whitespace only?) found = true if not found check viewBox for width if it has one, grab it and found = True if not found: Emit "dimension" flag-related warning

Agreed. Will patch.

… before applying it to the glyph.

ronaldtse · 2022-05-10T07:37:53Z

Thank you @frank-trampe for this -- look forward to using it in metanorma/WorldWeatherSymbols#1 !

skef

Looks good at the functional and Python level. It would be ideal if this were also added to _ImportParamsDlg in fontforgeexe/cvimportdlg.c . Since it's another boolean it would be pretty easy to follow the model of correct_direction (copy and edit the UI group, boost some of the array lengths above it, copy and edit the GGadgetIsChecked line).

If you'd rather skip that part, though, consider this approved.

frank-trampe · 2022-05-10T16:32:29Z

I added support to the graphical interface for the new option. I would like to add support to native scripting also, but the flag situation there is a bit of a mess.

skef · 2022-05-10T17:58:52Z

fontforgeexe/cvimportdlg.c

    label[k].text = (unichar_t *) _("Default Join Limit (PS/EPS/SVG):");
    label[k].text_is_1byte = true;
    label[k].text_in_resource = true;
    gcd[k].gd.label = &label[k];
    gcd[k].gd.flags = gg_visible | gg_enabled;
    gcd[k++].creator = GLabelCreate;
-    hvarray[6][0] = &gcd[k-1];
+    hvarray[7][0] = &gcd[k-1];


I guess I didn't give these a ++ variable. Sorry!

skef

This looks good. Any ViewBox stuff seems strictly optional, and I'm happy to review again.

ctrlcctrlv · 2022-05-10T18:01:29Z

Just confirming it's all OK w/me too, not that my consent's needed :) only offering it as I brought up the viewBox thing.

frank-trampe · 2022-05-11T21:21:02Z

I merged what we have. Discussion of the viewBox fallback continues.

Add support for a "dimensions" flag in SplineChar.importOutlines in the Python bindings and in the import dialog box that sets the width of the glyph from an imported SVG. This works only for files with no units and absolute dimensions in the "width" and "height" attributes of the SVG.

Add support for a "dimensions" flag in SplineChar.importOutlines in t…

de7d01b

…he Python bindings that sets the width of the glyph from an imported SVG.

frank-trampe requested a review from skef May 5, 2022 14:50

frank-trampe mentioned this pull request May 5, 2022

Support setting SplineChar width from importOutlines. #5004

Closed

ctrlcctrlv reviewed May 5, 2022

View reviewed changes

skef reviewed May 6, 2022

View reviewed changes

Ensure that the width or vertical width in the SVG header is absolute…

16767fe

… before applying it to the glyph.

skef reviewed May 10, 2022

View reviewed changes

Handle the glyph import dimension option in the import dialog box.

19394f1

skef reviewed May 10, 2022

View reviewed changes

skef approved these changes May 10, 2022

View reviewed changes

frank-trampe merged commit 566b261 into master May 11, 2022

ronaldtse mentioned this pull request Jun 9, 2022

Add font generation metanorma/WorldWeatherSymbols#1

Open

Support setting SplineChar width from importOutlines. #5005

Support setting SplineChar width from importOutlines. #5005

Uh oh!

Conversation

frank-trampe commented May 5, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ronaldtse commented May 10, 2022

Uh oh!

skef left a comment

Choose a reason for hiding this comment

Uh oh!

frank-trampe commented May 10, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

skef left a comment

Choose a reason for hiding this comment

Uh oh!

ctrlcctrlv commented May 10, 2022

Uh oh!

frank-trampe commented May 11, 2022

Uh oh!

Uh oh!

frank-trampe commented May 5, 2022 •

edited

Loading