LilyIssues / Issues / #5247 add dynamic n for “niente” to feta

summary: (hopefully) let git-cl create sf issue after 500 server error --> add dynamic n for “niente” to feta
Description has changed:

Diff:

--- old
+++ new
@@ -1,3 +1,17 @@
-(hopefully) let git-cl create sf issue after 500 server error
+add dynamic n for “niente” to feta
+
+For a discussion see the following blog posts and their comment
+sections:
+http://lilypondblog.org/2014/02/feta-and-bravura/
+http://lilypondblog.org/2014/01/smufl-fonts-in-lilypond/
+http://lilypondblog.org/2014/04/using-special-characters-from-smufl-fonts/
+
+The first post mentions also italic numbers for fingerings/tuplets which
+Feta doesn’t contain yet (issue 5074).
+
+The “niente” n is a copied and modified version of the “mezzo” m code.
+
+Contains also:
+add dynamic script \n command for “niente”

 http://codereview.appspot.com/339090043

Needs: -->

Malte Meyn - 2017-12-27

Attachments has changed:

Diff:

--- old +++ new @@ -0,0 +1,2 @@ +document.png (27.1 kB; image/png) +document2.png (6.3 kB; image/png)
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Malte Meyn - 2017-12-27

add niente to existing regression test

http://codereview.appspot.com/339090043

If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Anonymous - 2017-12-27

Needs: -->

Patch: new --> needs_work

Type: --> Enhancement
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Fails make check

/home/james/lilypond-git/input/regression/dynamics-glyphs.ly'
Interpreting music...[8]
Preprocessing graphical objects...
Calculating line breaks... 
Drawing systems... 
/home/james/lilypond-git/input/regression/dynamics-glyphs.ly:16:12: warning: crescendo too small
  c \ppppp 
           \<
Writing header field `texidoc' to `./6e/lily-0c212671.texidoc'...

Malte Meyn - 2017-12-28

This warning also happens with the original (current master) test if I’m not mistaken. Should I add something like

\override Hairpin.to-barline = ##f

to fix it? It would be enough to get rid of that warning in current master and in my patched version of the regtest.
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Malte Meyn - 2017-12-28

make n a bit narrower, get rid of too small crescendo warning

http://codereview.appspot.com/339090043

If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Anonymous - 2017-12-29

Needs: -->

Patch: new --> needs_work

Type: --> Enhancement
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

OK this time I am getting a Ghostscript error (I did verify that the issue is not local to me by doing a full build of everything on current master which passed).

Looks like it doesn't 'understand' the /n switch

Error: /undefined in n
Operand stack:
   66.0826   -8.332
Execution stack:
   %interp_exit   .runexec2   --nostringval--   --nostringval--   --nostringval--   2   %stopped_push   --nostringval--   --nostringval--   --nostringval--   false   1   %stopped_push   1983   1   3   %oparray_pop   1982   1   3   %oparray_pop   --nostringval--   1966   1   3   %oparray_pop   1852   1   3   %oparray_pop   --nostringval--   %errorexec_pop   .runexec2   --nostringval--   --nostringval--   --nostringval--   2   %stopped_push   --nostringval--
Dictionary stack:
   --dict:1211/1684(ro)(G)--   --dict:0/20(G)--   --dict:1271/1518(L)--
Current allocation mode is local
Last OS error: No such file or directory
Current file position is 74174
GPL Ghostscript 9.21: Unrecoverable error, exit code 1
Traceback (most recent call last):
  File "/home/james/lilypond-git/build/scripts/build/out/output-distance", line 1349, in <module>
    main ()
  File "/home/james/lilypond-git/build/scripts/build/out/output-distance", line 1346, in main
    compare_tree_pairs (zip (args[0::2], args[1::2]), out, options.threshold)
  File "/home/james/lilypond-git/build/scripts/build/out/output-distance", line 1061, in compare_tree_pairs
    data.create_html_result_page (dest_dir, threshold)
  File "/home/james/lilypond-git/build/scripts/build/out/output-distance", line 1043, in create_html_result_page
    link.link_files_for_html (dest_dir)
  File "/home/james/lilypond-git/build/scripts/build/out/output-distance", line 660, in link_files_for_html
    to_compare = self.create_images (dest_dir)
  File "/home/james/lilypond-git/build/scripts/build/out/output-distance", line 650, in create_images
    system (cmd)
  File "/home/james/lilypond-git/build/scripts/build/out/output-distance", line 1091, in system
    assert stat == 0
AssertionError
rm -rf /tmp/tmpB1VjBP
/home/james/lilypond-git/GNUmakefile.in:333: recipe for target 'local-check' failed
make: *** [local-check] Error 1

Malte Meyn - 2017-12-29

Cannot reproduce (edit: yes, I can, see below) but I get other misbehaviour. However, I’ve got the impression that this is not the fault of this patch but that partial recompilation works fine with all C++ stuff but not in all cases with the fonts. I tried the following (using gs 9.22):

git checkout master rm -rf build mkdir build cd build ../configure make -j3 CPU_COUNT=3 make -j3 CPU_COUNT=3 test-baseline git checkout dev/malte/dynamicniente make -j3 CPU_COUNT=3 make -j3 CPU_COUNT=3 check

The regtest diffs show a big sans-serif “Q” instead of the feta/dynamic “n”; if i compile it by hand

cd ../input/regression ../../build/out/bin/lilypond dynamics-glyphs.ly

a big sans-serif “n” is shown.

Ok. After writing the text above I thought I should try a full recompilation. I recompiled both lilypond and the tests after a clean

cd ../../build make clean make -j3 CPU_COUNT=3 make test-clean make -j3 CPU_COUNT=3 check

The make check fails but compiling dynamics-glyphs.ly by hand shows a correct output.

(I didn’t check whether/in which cases setting the CPU_COUNT is needed but at least it doesn’t harm …)

Last edit: Malte Meyn 2017-12-29
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Anonymous - 2017-12-29

the CPU_COUNT can be used for any of the make commands.

Note that what you did and what I do to test the patch are not the same here.

Here's my work flow from a clean $LILYPOND_GIT; in case it helps you.

mkdir build
./autogen --noconfigure
cd build
../configure --disable-optimising
make -j6 CPU_COUNT=6
make -j6 CPU_COUNT=6 test-baseline
apply patch (however you do that)
make -j6 CPU_COUNT=6 clean
make -j6 CPU_COUNT=6
make -j6 CPU_COUNT=6 check

(then I do a make doc after that).

If any of the makes fails after applying the patch, then I try to report what I get but sometimes it is hard because of the multi-jobs I run when testing.

If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
- Malte Meyn - 2017-12-30
  
  Can reproduce. But I’m now almost 100% sure that this is a problem of the build/test system: I did the same things with the two branches swapped (new build dir, configure, checkout dev/malte/dynamicniente, make, make test-baseline, checkout master (“apply reverse patch”), make clean, make, make check) and gs and thus make check failed on master.
  
  So I would suggest to check this regtest by hand and allow it to be reviewed and pushed even if it seems to break make check. Of course only if someone who knows more about the build system than I do approves.
  
  If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
  - Malte Meyn - 2017-12-30
    
    Hm … The last time I can see there was a new character added to Feta was the z-shaped quarter rest three years ago [#4211]. There were no problems with make check.
    
    Related
    
    Issues: ~~#4211~~
    
    If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
  - Anonymous - 2017-12-30
    
    Staging only gets merged into master if it passes all the make steps.
    
    We need to understand why this doesn't pass make check.
    
    Malte, can you give the full output of your 'configure' step run please?
    
    If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
    - Malte Meyn - 2017-12-31
      
      Staging only gets merged into master if it passes all the make steps.
      
      We need to understand why this doesn't pass make check.
      
      Yes, that makes sense.
      
      mkdir build cd build ../configure &> configure.log
      
      See attachment.
      
      configure.log
      
      If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Werner LEMBERG - 2017-12-31

Would it be possible to get the failing PS file? Maybe it helps if we can inspect it.

If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
- Malte Meyn - 2017-12-31
  
  It contains the “word” /m at two places: l. 705 and l. 884, similar for /f, /p, /r, /s, /z. The “word” /n cannot be found.
  
  dynamics-glyphs.eps
  
  If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Werner LEMBERG - 2017-12-31

It seems that you have to add /n to the file ps/encodingdefs.ps.

If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
- Malte Meyn - 2017-12-31
  
  Hm … That of course was not done for [#4211] (https://codereview.appspot.com/177640043) because that file was added later (commit cd5b559ab01, [#4251]). I’ll try it next year ;)
  
  Related
  
  Issues: ~~#4211~~
  Issues: ~~#4251~~
  
  If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Malte Meyn - 2018-01-01

add /n to encodingdefs.ps

http://codereview.appspot.com/339090043

If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Anonymous - 2018-01-02

Needs: -->

Patch: new --> needs_work

Type: --> Enhancement
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Anonymous - 2018-01-02

Not quite there yet - still fails make check

...
fatal error: Unlisted glyphs in Documentation/included/font-table.ly: (n)
...

If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
- Malte Meyn - 2018-01-02
  
  Oh f…, I’m sorry. I’ll try my best to make it work with the next patch set. I’m not sure what I’m doing wrong because I thought I always make check before submitting a new patch set. But maybe I should also make doc.
  
  If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
  - Werner LEMBERG - 2018-01-02
    
    I've just updated mf/README accordingly :-)
    
    If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Malte Meyn - 2018-01-04

fix encoding vector lengths, add "n" to font-table.ly, add \n to NR

http://codereview.appspot.com/339090043

If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

add dynamic n for “niente” to feta

Issue Tracker for LilyPond

Searches

Help

#5247 add dynamic n for “niente” to feta

Discussion

Related

Related