Add Points Of Inflection / Balance / Harmonize #4900

linusromer · 2022-01-30T17:58:25Z

As intended I have implemented entries in the "Element Menu" named "Add Points Of Inflection", "Balance" and "Harmonize" for the font view, the char view and the metrics view. Corresponding scripting functions are included as well.

"Add Points Of Inflection" relies on the computation that is already used to display the points of inflection but then additionally splits the spline there. Hotkey: Ctrl+Shift+Y (homophone Ynflection)
"Balance" makes the line between the control points parallel to the chord such that the area is preserved. This is an improved version of the algorithm known as "tunnify". Hotkey: Ctrl+Shift+P (homophone Palance)
"Harmonize" moves the smooth on-curve points between its adjacent control points such that the adjacent curvatures become equal (except for sign at points of inflection). Hotkey: Ctrl+Shift+Z (harmoniZe)

The translations to German are included. However, the translations to other languages will have to be done by others.

This makes my plug-in https://github.com/linusromer/curvatura more or less useless (because the "Harmonize handles" of curvature is probably not used by anybody).

Type of change

New feature

…t (untested)

…neat

…e it later

…lations

ghost · 2022-02-01T00:23:27Z

Awesome. FTR, Here are binary artifacts for this pull already:

https://github.com/fontforge/fontforge/actions/runs/1769425722#artifacts

ctrlcctrlv · 2022-02-01T03:55:42Z

Fantastic, excellent work. Finally my decision to make MFEK's point type FontForge-similar (with next and prev points) (a, b), as well as having PrevNext trait on Contour impl's is going to be bear some fruit, as many of your algorithms here I can literally copy and paste and run a few sed's on to bring them into MFEK. :-)

Thanks a lot also for using BPAdd and BPCross so often as it really helps me understand how the code works.

ctrlcctrlv · 2022-02-01T04:06:15Z

Oh, and by the way, some code review as far as this project goes—

① Why did you add the options to the MetricsView? Seems to clutter menus and not make much sense, can you rationalize it? https://github.com/fontforge/fontforge/pull/4900/files#diff-7dcb402a6783e5b71a16f0e5edf1387845b40574aaeea9faa90c172ee7832c05
② Regarding adding new hotkeys, is this allowable @skef? I think we said it might not be.
③ Please remove the font-level Python API functions. It's my opinion they're unnecessary clutter that can be had the same by iteration, but are very unlikely to lead to pleasing results when applied to whole font. I understand having the functions in the FontView, as it only is applied to selected glyphs, but not in Python API.
④ For the Python docs, please either— ⓐ group your new functions together in each section (layer, contour, &c) with a link to your Curvatura docs on top, (breaking alphabetization is IMO ok, but if we think it's not then) ⓑ make a new docs section for these functions with images and link to that doc before the short blurb of each.
⑤ Also for the Python docs, only write the short blurb once, for the others, write please see contour.whatever. contour is likely to be the most often used API.

linusromer · 2022-02-01T08:34:09Z

@ctrlcctrlv
① Well, that is a actually a question I have asked in this discussion. I reasoned that if "Add Extrema" has an entry in the metrics view then "my" three functions should probably have entries as well. But I will gladly remove them from the metrics view, because for me personally there is no reason for them.
② I think it's a pity when Harmonize and Balance do not get default hotkeys (however, I could live happily without a hotkey for Add Points Of Inflection). There are not many hotkeys I am using for my font designs but Harmonize and Balance are among them (besides Add Extrema, Simplify, Merge, Round, Save, Autohint, Open Metrics View). One reason for doing this pull request was that a new user can use these functions out of the box without adding additional files (like curvatura.py oder a personal hotkeys file).

I will implement your suggestions 1,3,4,5 as soon as I have an answer to your question 2.

skef · 2022-02-01T09:18:43Z

I think there's an argument to be made that Add Points of Inflection should appear everywhere that Add Extrema appears, in contrast with Harmonize and Balance. The former are similar in that they're more about normalization/avoiding potential tool problems than about output geometry. So I would recommend leaving Add Points of Inflection in those places even if/when the other two are removed.

I'm not aware of any guidelines against adding hotkeys to new "core" menu items. User-chosen hotkeys override core hotkeys (I believe), so people using them for something else shouldn't be affected.

ghost · 2022-02-01T09:33:52Z

I'm not aware of any guidelines against adding hotkeys to new "core" menu items. User-chosen hotkeys override core hotkeys (I believe), so people using them for something else shouldn't be affected.

JFTR, Is there a way to fully customize FontForge hotkeys?

skef · 2022-02-01T09:40:16Z

@Symbian9 Yes, this should work: https://fontforge.org/docs/ui/misc/HotKeys.html#hotkeys-hotkeyassign

…thods, restructured the corresponding doc

linusromer · 2022-02-01T17:08:12Z

@ctrlcctrlv
After skef's comment I have implemented the following things regarding your five points:
① I have removed Balance and Harmonize from MetricsView but not Add Points Of Inflection.
② My shortcuts are still there.
③ I have removed the font-level Python API functions for Balance and Harmonize but not for Add Points Of Inflection.
④ I have chosen your option ⓐ with .. seealso:: to link to the Curvatura documentation.
⑤ Done.

skef · 2022-02-03T09:45:36Z

I'm gonna wait to review this until #4789 is in and this branch is rebased.

skef · 2022-02-13T11:38:56Z

@linusromer Now that #4789 is merged can you rebase this off the current master. I think there are a bunch of common commits and doing that will make reviewing easier.

skef · 2022-02-14T12:00:30Z

I've started reviewing this and so far the new code looks good, however, when reviewing with all commits it looks like some of #4789 was reverted. Could you take a look and either figure out how I or github is confused or fix the issue?

…forge#4763

…c cases are marked with comments

…aks between '}' and 'else', added better behaviour for conversion from tangent to smooth

…r behaviour for conversion from hvcurve to curve

linusromer · 2022-02-15T10:26:26Z

@skef Github is confused because of me. I think I have somehow messed up the commits of my "master" and "inflections" branches. But now I think I have solved the issue by cherry-picking the corresponding commits of my master branch. (In retrospective I probably should have done a rebase first to make things clearer.)
I have tested everything again and looked at the "files changed" and now I am quite sure that nothing from #4789 is reverted.

skef · 2022-02-16T03:49:56Z

fontforge/fontviewbase.c

@@ -929,6 +929,108 @@ void FVAddExtrema(FontViewBase *fv, int force_adding ) {
    ff_progress_end_indicator();
 }

+void FVAddInflections(FontViewBase *fv, int anysel) { 


I diffed these and they're identical except for the (reused) strings and the function call. The code base has that sort of thing but in this case I think its worth adding the string and function pointer to the signature of one of these and having the others be one-liners.

Your suggested generalized solution is surely preferable. I will implement it as soon as I have a little bit more time. This affects FVHarmonize and FVBalance as well.

@skef I am a little stuck here. I have tried your suggestion with a generalized function
void _FVElementAction(FontViewBase *fv, int anysel, void (*func)(SplineChar*, SplineSet*, enum ae_type, int), const char* progressmsg) that calls func(sc, sc->layers[layer].splines, anysel, emsize). However my three functions SplineCharAddInflections(), SplineCharBalance() and SplineCharHarmonize() only take 3 parameters, whereas SplineCharAddExtrema() takes one additional parameter emsize. I think there is no elegant solution of optional parameters in C. There is of course a cheap solution: void _FVElementAction(FontViewBase *fv, int anysel, const char* functionname) that contains a switch (functionname). Is there something I am obviously missing?

I was just looking at the three added functions. Don't worry about the more ambitious project of including SplineCharAddExtrema().What makes these worth doing is the identical signature of the inner function.

skef

Given the lack of refigures I'm wondering how extensively this PR was tested in the UI vs just confirming the math works.

skef · 2022-02-16T03:50:39Z

fontforge/python.c

@@ -3468,6 +3468,51 @@ return( NULL );
 Py_RETURN( self );
 }

+static PyObject *PyFFContour_AddInflections(PyFF_Contour *self) {


Same with these.

skef · 2022-02-16T03:50:56Z

fontforge/python.c

@@ -4424,6 +4475,51 @@ return( NULL );
 Py_RETURN( self );
 }

+static PyObject *PyFFLayer_AddInflections(PyFF_Layer *self) {


skef · 2022-02-16T03:51:43Z

fontforge/python.c

@@ -9395,6 +9497,30 @@ return( NULL );
 Py_RETURN( self );
 }

+static PyObject *PyFFGlyph_AddInflections(PyFF_Glyph *self) {


These are short so ambivalent here.

skef · 2022-02-16T04:07:57Z

fontforge/splineutil2.c

+
+/* Make the line between the control points parallel to the chord */
+/* such that the area is preserved (kind of an improved "tunnify") */
+Spline *SplineBalance(Spline *s) { 


You're (obviously -- this is the point) modifying SplinePoint values in this function but I don't see calls to SplineRefigure() anywhere in the path. (Above that's taken care of by SplineSplit().)

Indeed, SplineRefigure() is not called ( I just have checked it) although it probably should be called. I HAVE tested my PR and I did not run into problems so far. But my tests were not more systematic than testing some problematic cases and run the functions on some huge fonts. I did not implement a separate torture test for the UI functions. Is there an existing torture test that I have missed (like trip and trap by Donald E. Knuth)?
Anyway, I will add SplineRefigure() calls as soon as I have a little bit more time.

There is absolutely no good systematic method of UI testing, unfortunately. When one is adding or making significant modifications to UI-tied functionality it's best to just play around with it in the UI to see what happens. Making real-time changes in a CharView is probably the best way as its most likely to turn up updating problems (such as those caused by a lack of SplineRefigure().

skef · 2022-02-16T04:08:18Z

fontforge/splineutil2.c

+
+/* Make the splines adjacent to the SplinePoint sp G2-continuous */
+/* by moving sp between its adjacent control points */
+void SplinePointHarmonize(SplinePoint *sp) {


Same here, no SplineRefigure()s to be seen.

…functions, added SplineRefigure() to the base functions of Balance and Harmonize

linusromer · 2022-02-19T09:12:11Z

@skef I have abstracted all occuring AddInflections, Balance, Harmonize in python.c and in the FontView and CharView (the python font method for AddInflections excluded). And I have added SplineRefigure() as desired. My tests for this change were not very deep. I will test it more in short time.

lgtm-com · 2022-02-19T09:36:54Z

This pull request introduces 9 alerts when merging 9b9f21b into dca27e2 - view on LGTM.com

new alerts:

9 for Missing return statement

linusromer · 2022-02-19T15:34:39Z

@skef After implementing the asked changes I have now tested it more extensively (all Python functions, FontView, CharView, MetricsView) with different fonts (also with quadratic layers). I think it should work now.

iterumllc

Looks good to me!

jtanx · 2022-02-21T08:39:38Z

po/de.po


 #, c-format
 msgid "Include filename too long on line %d of %s"
-msgstr "Include-Dateiname zu lang in Zeile %d in %s"
+msgstr "Name der einzufügenden Datei zu lang in Zeile %d in %s"


This actually needs to be updated in crowdin, I'll find a way of getting it there

I'll fix this once merged

fontforge/fontviewbase.c

linusromer · 2023-04-10T05:59:22Z

Are you talking about the "Edit tunni lines" feature of Fontlab 8? While I probably could implement this feature as a raw algorithm to change a curve, I see no easy way to implement a user interface to it. Am Fr., 7. Apr. 2023 um 18:46 Uhr schrieb Sergej Lebedev < ***@***.***>:

…

@linusromer <https://github.com/linusromer> I think "Add Points Of Inflection / Balance / Harmonize" are very good functions. But it could be extended by allowing you to control the curvature of a curve by entering numbers. For example, you could make an arc in all glyphs, which can be precisely defined with tunni lines. So one could get much more control over the shapes. Like in Fontlab 8, you can define the curvature very precisely by seeing numbers and entering them. Can you please programme this? — Reply to this email directly, view it on GitHub <#4900 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ACVRWCUPV7LPRKSJOW7BCQLXABAF3ANCNFSM5NEQ6JYA> . You are receiving this because you were mentioned.Message ID: ***@***.***>

Typedesigners · 2023-04-10T07:08:40Z

@linusromer In FontLab, the tunni lines are given numbers. For example, for an arc shape 60 or 50. This allows you to determine how much an arc is curved. Whether it is flatter or rounder. This way you can create exactly the same round shapes for all letters. At the moment, FontForge does not have an exact way to easily control how much an arc is curved.

https://help.fontlab.com/fontlab-vi/Tunni-Lines/

Typedesigners · 2023-04-10T08:33:56Z

@linusromer Smart Corners - It might be possible to implement a similar function in FontForge.
https://help.fontlab.com/fontlab-vi/Using-Smart-Corners/

linusromer · 2023-04-10T11:42:29Z

The things you wish are clearly a matter of user interface. I am the wrong person to implement this or other things that have to be displayed on the canvas. Someone else might be the right person... Sergej Lebedev ***@***.***> schrieb am Mo., 10. Apr. 2023, 10:34:

…

@linusromer <https://github.com/linusromer> Smart Corners - It might be possible to implement a similar function in FontForge. https://help.fontlab.com/fontlab-vi/Using-Smart-Corners/ — Reply to this email directly, view it on GitHub <#4900 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ACVRWCTCZEARTCQBLQBA7JTXAPAX7ANCNFSM5NEQ6JYA> . You are receiving this because you were mentioned.Message ID: ***@***.***>

Typedesigners · 2023-04-10T11:46:03Z

@linusromer Maybe as a plugin?
It would also be nice to have a function in FontForge that allows you to centre elements horizontally and vertically. Such alignment functions are a matter of course in any graphics programme such as Illustrator or Affinity Designer.

linusromer · 2023-04-18T12:28:36Z

Horizontal and vertical alignment could be solved by a plugin similar to github.com/linusromer/curvatura. However, I will not work on it (I am not part of the FontForge team). You could write the plugin on your own. Before programming you should think of the interface: Should only complete contours be aligned or only selected nodes? If only complete contours should be aligned how would the selection work? To which place should it be aligned? What are the menu entries ("align horizontally" and "align vertically" ?). Am Mo., 10. Apr. 2023 um 13:46 Uhr schrieb Sergej Lebedev < ***@***.***>:

…

@linusromer <https://github.com/linusromer> Maybe as a plugin? It would also be nice to have a function in FontForge that allows you to centre elements horizontally and vertically. Such alignment functions are a matter of course in any graphics programme such as Illustrator or Affinity Designer. — Reply to this email directly, view it on GitHub <#4900 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ACVRWCXRU3U76YW5ZQOUYELXAPXINANCNFSM5NEQ6JYA> . You are receiving this because you were mentioned.Message ID: ***@***.***>

Typedesigners · 2023-04-18T12:36:26Z

I am a type designer, not a programmer, so I cannot program a plug-in.

linusromer added 18 commits January 7, 2022 13:55

menu entries for add points of inflection

f809dbb

added translations for PoI as far as possible, added scripting suppor…

2118e2f

…t (untested)

corrected two errors in language files

12a8dc6

added 2012 pixmap for inflections, mentioned it for cmake

27dc31f

removed the typo 'inflectionS'

6729e09

added fontview support for balance (area preserving tunnify)

6565620

added metricsview and charview for balance (tunnify), made code more …

ada1899

…neat

added the base function for Harmonize

26dd12b

many options for harmonize-handles algorithms but I fear I will remov…

6bcaffc

…e it later

removed the harmonize-handles algorithm

cf988c2

added harmonize for metricsview and charview

7074fdc

added scripting and python-scripting support for balance and harmonize

bd2504a

added menu shortcuts and menu hidings

635a105

changed menu shortcuts, changed German translation, reset other trans…

a5ce73c

…lations

fixed python layer commands, changed hotkeys again

1f0e75b

changed some errors concerning 'inflections' instead of 'inflection'

c9accb0

added information for balance and harmonize to the docs

0072961

element menu entry fixed

7382407

jtanx mentioned this pull request Feb 1, 2022

2021–2022 release #4840

Closed

jtanx added this to the 2022 q1 milestone Feb 1, 2022

removed balance and harmonize from metrics view and as python font me…

63897c0

…thods, restructured the corresponding doc

Merge branch 'fontforge:master' into inflections

f3bef43

linusromer added 5 commits February 15, 2022 10:53

solved issue Unsatisfying Point Type Change From Corner To Curve font…

bf13f81

…forge#4763

corrected one wrong splinerefigure and made all handles zero length

f5b4c93

fixed the behaviour of quadratic splines (only partially), problemati…

5147acf

…c cases are marked with comments

fixed the behaviour of quadratic splines (finished), removed line bre…

13e0b22

…aks between '}' and 'else', added better behaviour for conversion from tangent to smooth

solved curve updating issues after conversion to tangent; added bette…

eb29877

…r behaviour for conversion from hvcurve to curve

skef reviewed Feb 16, 2022

View reviewed changes

added more abstraction to nearly all PoI, Balance, Harmonize related …

9b9f21b

…functions, added SplineRefigure() to the base functions of Balance and Harmonize

added 'return' to python-functions

28294d6

iterumllc approved these changes Feb 21, 2022

View reviewed changes

jtanx reviewed Feb 21, 2022

View reviewed changes

fontforge/fontviewbase.c Outdated Show resolved Hide resolved

Fix gettext usage

0b4f6ae

jtanx merged commit 6d63889 into fontforge:master Feb 21, 2022

linusromer deleted the inflections branch February 22, 2022 17:49

Add Points Of Inflection / Balance / Harmonize #4900

Add Points Of Inflection / Balance / Harmonize #4900

Conversation

linusromer commented Jan 30, 2022

Type of change

ghost commented Feb 1, 2022

ctrlcctrlv commented Feb 1, 2022

ctrlcctrlv commented Feb 1, 2022

linusromer commented Feb 1, 2022

skef commented Feb 1, 2022

ghost commented Feb 1, 2022

skef commented Feb 1, 2022

linusromer commented Feb 1, 2022

skef commented Feb 3, 2022

skef commented Feb 13, 2022

skef commented Feb 14, 2022

linusromer commented Feb 15, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

skef left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

linusromer commented Feb 19, 2022

lgtm-com bot commented Feb 19, 2022

linusromer commented Feb 19, 2022

iterumllc left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

linusromer commented Apr 10, 2023 via email

Typedesigners commented Apr 10, 2023 • edited

Typedesigners commented Apr 10, 2023

linusromer commented Apr 10, 2023 via email

Typedesigners commented Apr 10, 2023

linusromer commented Apr 18, 2023 via email

Typedesigners commented Apr 18, 2023

Typedesigners commented Apr 10, 2023 •

edited