Menu ▾ ▴

#4975 [Windows] Grace notes cause staff to protrude into the margin

Started
Phil Holmes
None
Defect
2020-02-06
2016-09-18
Simon Albrecht
No

http://lilypond.1069038.n5.nabble.com/Grace-notes-and-staff-miss-alignment-tp181529p188186.html
As reported by Karol and confirmed by Pierre, this code

\version "2.19.48"

\transpose c c' {
  \repeat unfold 16 { e'8 a }
  R1*8
}

\transpose c c' {
  \repeat unfold 16 { \grace dis'8 e' \grace gis8 a }
  R1*8
}

under Windows (at least 7 and 8) causes lots of programming error: mis-predicted force and has the last staff flow into the margin.
Output at http://lilypond.1069038.n5.nabble.com/file/n188186/ttttttt.pdf

The first bad version is 2.19.19.

Discussion

  • Phil Holmes

    Phil Holmes - 2016-09-28
    • status: Accepted --> Started
    • assigned_to: Phil Holmes
     

  • Phil Holmes

    Phil Holmes - 2016-09-28

    OK - I've spent quite a lot of the last week tracking this down. Short summary: commit e0af94bb8939bc6f4998db6294010baa77139092 (Replace C++ (in)equality checks with proper SCM syntax) is definitely the culprit. I've proven this by taking Gub back to its 2.19.18 state (with a few bugfix cherry picks to make it work on my system) and using 2.19.18 source on dev/philh. This produces a good output. Moving forward to 2.19.19 produces bad output. Moving back to commit "Issue 4343: quoteDuring music should be sent to midi file" produces good output. Moving forward one commit to the one identifed above produces bad output. I am therefore in no doubt that this is the issue. I've no idea why it only affect Windows, and only apparently with this issue. I think we have 2 options: revert the commit, or revert its use in simple-spacer.cc. I've not checked that the latter would work, but it seems likely, and if this looks the way forward I can burn some more CPU time and check with a further build that it works.

     

  • David Kastrup

    David Kastrup - 2016-09-28

    Please really check that reverting only the change in lily/simple-spacer.cc is sufficient for removing the problem. Because if it is, we have a far larger problem either involving a fundamental Guile bug or a GCC compiler bug: the change in simple-spacer.cc is just

    diff --git a/lily/simple-spacer.cc b/lily/simple-spacer.cc
index d17fa1e..ada1659 100644
--- a/lily/simple-spacer.cc
+++ b/lily/simple-spacer.cc
@@ -528,7 +528,7 @@ get_line_configuration (vector<Grob *> const &columns,
   for (vsize i = 1; i + 1 < ret.cols_.size (); i++)
     {
         SCM p = ret.cols_[i]->get_property ("line-break-permission");
-      if (p == ly_symbol2scm ("force"))
+      if (scm_is_eq (p, ly_symbol2scm ("force")))
         ret.satisfies_constraints_ = false;
     }

    And the respective definitions in Guile 1.8 are:
    #elif (SCM_DEBUG_TYPING_STRICTNESS == 1)
    / This is the default, which provides an intermediate level of compile time
    * type checking while still resulting in very efficient code.
    /
    typedef struct scm_unused_struct * SCM;

    /*
  The 0?: constructions makes sure that the code is never executed,
  and that there is no performance hit.  However, the alternative is
  compiled, and does generate a warning when used with the wrong
  pointer type.

  The Tru64 and ia64-hp-hpux11.23 compilers fail on `case (0?0=0:x)`
  statements, so for them type-checking is disabled.
*/
#if defined __DECC || defined __HP_cc
#   define SCM_UNPACK(x) ((scm_t_bits) (x))
#else
#   define SCM_UNPACK(x) ((scm_t_bits) (0? (*(SCM*)0=(x)): x))
#endif

    followed by

    #define scm_is_eq(x, y) (SCM_UNPACK (x) == SCM_UNPACK (y))

    and also

    /* For dealing with the bit level representation of scheme objects we define
 * scm_t_bits:
 */
/* On Solaris 7 and 8, /usr/include/sys/int_limits.h defines
   INTPTR_MAX and UINTPTR_MAX to empty, INTPTR_MIN is not defined.
   To avoid uintptr_t and intptr_t in this case we require
   UINTPTR_MAX-0 != 0 etc.  */
#if SCM_SIZEOF_INTPTR_T != 0 && defined(INTPTR_MAX) && defined(INTPTR_MIN) \
  && INTPTR_MAX-0 != 0 && INTPTR_MIN-0 != 0 \
  && SCM_SIZEOF_UINTPTR_T != 0 && defined(UINTPTR_MAX) && UINTPTR_MAX-0 != 0

typedef intptr_t scm_t_signed_bits;
#define SCM_T_SIGNED_BITS_MAX INTPTR_MAX
#define SCM_T_SIGNED_BITS_MIN INTPTR_MIN
typedef uintptr_t scm_t_bits;
#define SIZEOF_SCM_T_BITS SCM_SIZEOF_UINTPTR_T
#define SCM_T_BITS_MAX UINTPTR_MAX

#else

typedef signed long scm_t_signed_bits;
#define SCM_T_SIGNED_BITS_MAX LONG_MAX
#define SCM_T_SIGNED_BITS_MIN LONG_MIN
typedef unsigned long scm_t_bits;
#define SIZEOF_SCM_T_BITS SCM_SIZEOF_UNSIGNED_LONG
#define SCM_T_BITS_MAX ULONG_MAX

#endif

    Either scm_t_bits is defined as an unsigned long (of length 32 bit) unable to actually hold a pointer value, in which case Guile will be broken. Or we have a compiler error during code generation. Either will affect a lot more code than just this.

    So we better make quite sure that this change is responsible. And if it is, I'll need to look at the output of autoconf on the cross compilation of Guile, and then I'll likely need to check the generated assembly code.

     

  • Phil Holmes

    Phil Holmes - 2016-09-29

    OK - I've compiled a windows binary with just the single line "if (p == ly_symbol2scm ("force"))" restored (see the git branch dev/philh) and this does not fix the grace note/line length bug. So there is something deeper at work with that commit.

     

    • David Kastrup

      David Kastrup - 2016-09-29

      Well, I am sort-of relieved. I'll review the whole commit now for something more suspicious. But platform-specific bugs tend to be tricky to find.

       

  • David Kastrup

    David Kastrup - 2016-09-29

    Ok, so I reviewed all of the commit. There are few changes which are not strictly equivalent (but should be functionally equivalent), and If I remember correctly, most of them were likely proposed by me during review. Cough cough.

    However, pretty much all of them should not be in code triggered by the example input. So the only other suggestion I can come up with is trying to revert the changes in lily/include/lily-guile-macros.hh and see whether that makes a difference.

    I also have found a bug in stencil-traverse in stencil-integral.cc (comparing to an empty string with scm_is_eq) but that bug again should be independent from the commit. So while it warrants fixing, it should not be related to this issue here.

    And I'd like the config.log (and probably config.hh) from the Windows cross compilation.

    Thanks for all the work you put in here.

     

  • Phil Holmes

    Phil Holmes - 2016-09-30

    The bug still exists with lily-guile-macros.hh manually reverted to its previous state. The config.log and config.hh are attached - please let me know if this doesn't work for downloading them and I'll email them over.

     
    config.tar.gz

    • David Kastrup

      David Kastrup - 2016-09-30

      Ugh. This is pure 32bit, with pointers and long being 32bit types. So not likely a typing problem. I'll dial back my compilation environment to 32bit and see whether I have better luck at reproducing the problem.

       

      • David Kastrup

        David Kastrup - 2016-09-30

        Nope. But my compiler is 6.2.0, the one in the compilation is 4.9.2 (and a few Windows-specific API options but I don't think they are responsible).

        Is that what we have in Gub also for Linux? Or should we try updating the Windows version?

         

  • Masamichi Hosoda

    Masamichi Hosoda - 2016-10-01

    If I understand correctly, it is not a compiler problem.

    First, I've tried some other environment on Windows 10 64bit.

    lilypond-2.19.46-1.mingw.exe (from lilypond.org):
    Reproduced.

    Cygwin 64 bit + lilypond-2.19.42 (from cygwin package):
    No problem.

    Cygwin 32 bit + lilypond-2.19.42 (from cygwin package):
    No problem.

    lilypond-2.19.47 (built by my 64 bit GUB environment):
    Reproduced.

    Then, I've tried to upgrade GUB's gcc to gcc-5.4.0.
    I've failed to build mingw installer.
    But, I've succeeded to build Windows exe files.

    I've replaced three files (lilypond.exe, lilypond-windows.exe, libstdc++-6.dll) in installed lilypond folder.
    Unfortunately, reproduced even if gcc-5.4.0 is used.

     

  • Karol Majewski

    Karol Majewski - 2016-10-22

    From what I understand, we do know which commit is the culprit but we don't know which files from this commit are responsible for the issue (i.e. reverting which files fixes the issue). There are 140 files changed in this commit, so reverting them one by one would be a time consuming process. But maybe you (Phil) could start with reverting a bunch of files that are least likely to be the cause and then you could check whether the bug still exists. If so, you could revert another bunch of files and so on...

     

  • Simon Albrecht

    Simon Albrecht - 2016-11-19

    Another example reported by Ralph:

    \version "2.19.40"
\include "english.ly"

tune =
\relative c'' {
  \clef treble
  \key a \minor
  \time 6/8

  e8 a a a4

  \key a \major
  e8 |
  \acciaccatura b'8
  a8 gs a a, cs e |
}

\score {
  \tune
}
     

  • Jean Abou Samra

    Jean Abou Samra - 2019-07-14

    Also reported on Mac OS (at least 10.12.6 Sierra [me] and 10.13.6 High Sierra [Michael Hendry]) with LilyPond 2.21.0, but not LilyPond 2.19.83, quite surprisingly as the bug appeared with 2.19.19 on Windows. Both above examples are reproduced, and I can add a third one:

    \version "2.21.0"
\relative {
c'1
\acciaccatura { bes8 } c1
}

    Full outputs and logs on the three examples using 2.21.0, 2.19.83 and 2.18.2 are available in the attached archive.
    Two additional remarks:
    On the first example, "mis-predicted force" programming errors show up, and yet the output is perfectly normal, the staff doesn't protrude.
     On the last example, the bug is triggered as soon as the grace note is altered, no matter the alteration and no matter wether \acciaccatura or \grace is used.
    Hope that helps!

     
    lilypond-issue-4975.zip

  • Peter Toye

    Peter Toye - 2019-11-16

    Here's another MWE. This gives mis-directed force messages without the staff going into the margins. As with Jean's example, it makes no difference if \grace, \appoggiatura or \acciaccatura are used.

    But it does make a difference if the staff size is changed. This example gives different numbers of errors if the staff size is 14, 18, 21 or 25, but no errors with other values in tis range. Hope this helps.

    Using LP 2.19.83 and WIndows 7.

    Peter

    \version "2.19.80"

\language "english"

#(set-global-staff-size 25)

\score {
  \new Staff  {
    \clef "treble"
    \time 2/4
    {
      \repeat unfold 64 { \acciaccatura b'8 c''16  }
    }
  }
}
     

    Last edit: Peter Toye 2019-11-16

  • Jean Abou Samra

    Jean Abou Samra - 2019-11-17

    Interesting that the bug occurs on all major platforms except GNU/Linux. Actually it's a chance that it is present on Mac OS because in theory you can compile LilyPond natively on a Mac. Unfortunately I'm no longer using a Mac and anyway my compilation attempts were all unsucessful at the time as my environment was quite messed up. But if we could find some Mac OS expert around, then that person could run GDB and find out exactly what the problem is…

     

  • Masamichi Hosoda

    Masamichi Hosoda - 2020-01-29

    The cause of this issue and Issue 4943 seems to be the same.
    I've tried x87mant53.dll showed in the german forum.
    https://lilypondforum.de/index.php/topic,609.msg3443.html#msg3443
    It solved not only Issue 4943 but also Issue 4975.

    I guess that these issues do not only occur on Windows, but on all x86 32 bit platforms except Linux.
    Therefore, I think that defined (__MINGW32__) is not necessary.
    https://lilypondforum.de/index.php/topic,609.msg3463.html#msg3463