Menu

#1578 lilypond export fails

None
closed
1
2020-05-03
2020-04-24
No

Since the change to aveverum.rg the lilypond export test fails. Not suprising really as any significant change in the rg file leads to a failure. But looking at the new ly file there were some artifacts which were new. I checked the rg file - one of the linked segments had a key and clef change - deleted those. But still there were some odd rests coming from the linked segments. I have tried modifying the lilypond exporter to ignore segments with no notes (I hope this is always OK). The result is a ly file with ony one (insignificant difference to the baseline. Here is a patch with the rg , baseline ly and code changes.

1 Attachments

Discussion

1 2 > >> (Page 1 of 2)
  • Philip Leishman

    Philip Leishman - 2020-04-24

    OK - just seen that the code change is not so good - exercise_notation.ly ends up being empty. Other files in the lilypond test are different. This still needs some thought !!

     
  • Philip Leishman

    Philip Leishman - 2020-04-24

    Here is another try. This time we ignore segments containing just rests and control events. Two mor files need to be updated:
    bogus-surf-jam.ly - the new version looks OK to me
    stormy-riders.ly - this causes a coredump from my version of lilypond !
    Maybe someone who knows the lilypond export stuff (seems pretty complicated to me) can say if this is OK !!

     
  • Philip Leishman

    Philip Leishman - 2020-04-24

    Here is the patch

     
  • Yves Guillemot

    Yves Guillemot - 2020-04-24

    Thanks for the patch and for reminding me there is some old issues in the lilypond export I still have to fix.
    I will have a look on it tomorrow.

     
  • Yves Guillemot

    Yves Guillemot - 2020-04-24

    I don't know exactly what your patch is supposed to do, but the fix is very easy without any patch.
    The Lilypond export test create a .ly file from each .rg example files and compare them to reference files stored in the test/lilypond/baseline directory.
    The aveverum.rg example file has been changed in rev 15772, but the old aveverum.ly is still in the baseline.
    To fix it, you only have to run the test, which will fail but leave an aveverum.ly file in the BUILD/test/lilypond directory. Then move this aveverum.ly file in test/lilypond/baseline where it will overwrite the original outdated .ly file.
    Now if the test is run again, it should pass.
    The baseline should now be fixed in rev 15789.

    The real problem I was thinking of is elsewhere : if you try to process aveverum.ly with lilypond, it will fail. But it will be correctly processed if convert-ly if called before lilypond.
    The print-preview menu calls convert-ly before calling lilypond, so it works fine.
    But exporting directly to lilypond frequently fails.
    I presume that everybody is using print-preview, which is very handy. That's why this bug is still here .

    I'll open a new ticket about this problem in the nex days.

     
  • Philip Leishman

    Philip Leishman - 2020-04-25

    Hi Yves,
    Yes I realized that and my first reaction was just to copy the .ly file to baseline. But then I looked at the generated pdf !
    The old version looks great but the new version had some artifacts. One of the problems was a change of key and time signature in one of the “Expression ramp” segments – I removed them – that is the new version of aveverum.rg.
    Unfortunately there were still some problems. The linked segments were being treated as an additional voice and the rests were being rendered in lilypond.
    Hence the code change. Basically I tried to get the LilpondExporter to ignore the segments (now containing only rests and control events). This worked OK but led to two different ly outputs – I put these in the patch too.
    I didn’t know about the convert-ly program. I could use lilypond on aveverum.ly directly. I tried using convert on the file with the artifacts – it makes no difference there. I am using lilypond 2.18.2 – not the newest version.

     
  • Ted Felix

    Ted Felix - 2020-04-25

    Interesting. Sounds like we need a separate aveverum.rg for this test. One that doesn't try to do tricky things like expression ramps in separate linked segments. Maybe call it aveverum-lilypond.rg? Then have the test case use that.

     
  • Ted Felix

    Ted Felix - 2020-04-25
    • labels: --> lilypond, test
     
  • Yves Guillemot

    Yves Guillemot - 2020-04-25

    I totally missed what is new in aveverum.rg.
    This is a very ingenious and handy way to add expression and other controllers to scores.

    It should not be a problem for the Lilypond export test : it is a non regression test. Its goal is to show that exporting a given .rg file always give the same .ly file. Garbages in the Lilypond output don't matter provided that these garbages are always the same.

    To print a clean document with lilypond is easy : the controllers-only segment have just not to be exported. Which needs using the "export selected segments" option.
    That could be tedious if these segments are many.

    That drives me to two possible feature requests :

    • A button to unselect all these special segments. This is only possible if these segments are marked as "special".
    • Far better : Given that these segments are marked, it should be easy to never export their content to Lilypond and to notation editor (except the adapted rulers, of course).

    About the Lilypond errors I observe : they are related to lyrics. When lyrics are not exported, there is no error.

     
    • Ted Felix

      Ted Felix - 2020-04-25

      Given that these segments are marked, it should be easy to never export their content to Lilypond

      That does sound like a handy feature. Mark the Segment(s) as "not for lilypond" or something similar.

       
      • D. Michael McIntyre

        On 4/25/20 10:28 AM, Ted Felix wrote:

        Given that these segments are marked, it should be easy to never
        export their content to Lilypond
        

        That does sound like a handy feature. Mark the Segment(s) as "not for
        lilypond" or something similar.

        Agreed.

        --
        D. Michael McIntyre

         
  • Philip Leishman

    Philip Leishman - 2020-04-25

    The goal of my patch was exactly that these segments are not exported to lilypond. I used the criteria that the segment only contaions rests and control events. This is a bit artificial. A way of labelling segments "not for lilypond" seems like a good idea to me.

     
  • Yves Guillemot

    Yves Guillemot - 2020-04-25

    It should not be "not for lilypond" but rather "not for notation".
    MusicXML exporter and notation editor should be able to use it, at least.

     
  • Philip Leishman

    Philip Leishman - 2020-04-26

    OK here is a first attempt at adding an additional attribute (forNotation) to segments.
    I am still very much a beginner with rosegarden coding so constructive criticism is welcome !
    The aveverum.rg is not changed in the patch but it seems to work if I set the forNotation flags to false for the appropriate segments.

     
    • Ted Felix

      Ted Felix - 2020-04-26

      Quick skim of the patch looks good. I didn't see any changes to SegmentParameterBox to add a checkbox for this. Feel free to add that as well. Should be able to follow the example of m_repeat in there.

       
  • Yves Guillemot

    Yves Guillemot - 2020-04-26

    I tried your last patch and I don't have any remark about the coding.

    The patch works fine here :
    - Set/unset a segment as "forNotation" : OK
    - Memorisation of the segment mode when the file is saved : OK
    - Midi controllers are still sent to the synth : OK
    - Export to Lilypond without garbage rests : OK
    - Edition in notation editor without garbage rests : OK

    The main problem I see is that it's no more possible to edit tne controllers segment in the notation editor.
    It was great being able to see in the same editor the notation on the staff and the controllers on the ruler.

    Two small enhancements could be added :
    - Another set/unset "forNotation" in the segment the menu (I had some trouble finding it the first time)
    - Some indicator showing a segment is "for notation". Maybe some character added to the segment name as "L{}" is used with the linked segments.

     
    • Ted Felix

      Ted Felix - 2020-04-26

      Some indicator showing a segment is "for notation". Maybe some character added to the segment name as "L{}" is used with the linked segments.

      I think a checkbox in the SegmentParameterBox would be less intrusive for the moment. Especially since the vast majority of Segments will be "for notation".

      Some sort of "NOT for notation" indicator on the Segment might work better. An eighth note with a line through it would be great, but modifying the displayed label should work too. E.g. add "(xn)" to the end.

       
  • Philip Leishman

    Philip Leishman - 2020-04-27

    Thanks all for your feedback and suggestions. Here is a new patch.

    1. It uses the latest commit.
    2. I added the flag to the SegmentParameterBox . It is still in the context menu. Do we need/want both ?
    3. I modified the label to indicate not for notation (xn).
    4. As it can be useful to open segments like those in aveverum in the notation editor I reverted the RosegardenMainViewWidget changes. It is up to the user which segments to select for editing !
    5. I have included a modified aveverum.rg and aveverum.ly in the patch
     
    • Ted Felix

      Ted Felix - 2020-04-27

      I tried to apply the patch with "svn patch" and it all applied fine except for the aveverum.rg file. gzip recognizes it, but it appears to be truncated.

      gzip: aveverum.rg.xml.gz: unexpected end of file
      

      Has anyone seen this issue? Did I do something wrong with svn patch?

      Can you upload just your latest aveverum.rg for me? Thanks.

       
    • Ted Felix

      Ted Felix - 2020-04-27

      Actually, I was able to create the proper aveverum.rg on my own by just disabling "For Notation" for the four expression ramp segments. The test passed.

      So, it's working great. Some minor comments:

      • SegmentForNotationCommand. The name string in the .cpp is currently "Change Segment Notation flag". It should be "Toggle For Notation".
      • SegmentParameterBox. Need a space between "For" and "Notation" in the label. "For Notation".
      • CompositionModelImpl. The "xn" is a bit too close. Maybe add three spaces and parens:
      label += QString("   (xn)");
      
      • The context menu. I would say to remove "Use for notation" from the context menu (segmenttool.rc and SegmentTool). Since "Toggle Repeat" isn't in the context menu, "Use for notation" probably doesn't need to be there either.
       
  • Philip Leishman

    Philip Leishman - 2020-04-28

    Hmm… I had difficulties with patch containing aveverum.rg too. I made the patch with the –force flag. Is there another way of putting a binary file into a patch ?

    Anyway this patch does not have the aveverum.rg file. As you commented the only change is setting forNotation false for the 4 expression segments.

    I agree with all your suggested changes. Here is the newest patch and my aveverum.rg

    Thanks all

     
  • Philip Leishman

    Philip Leishman - 2020-04-28

    The patch

     
  • Ted Felix

    Ted Felix - 2020-04-29
    • status: open --> feedback
    • assigned_to: Ted Felix
     
  • Ted Felix

    Ted Felix - 2020-04-29

    Pushed as [r15805]. Please test.

     

    Related

    Commit: [r15805]

  • Philip Leishman

    Philip Leishman - 2020-04-29

    Works for me ! All tests working and a clean print preview of aveverum. Oh - not quite - there are some crazy rests at the end. If you want to get rid of them - here is a new version.

     
1 2 > >> (Page 1 of 2)

Log in to post a comment.